[webkit-reviews] review granted: [Bug 97352] Simplify and optimize ChildListMutationScope : [Attachment 165169] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Adam Klein <adamk at chromium.org>'s
request for review:
Bug 97352: Simplify and optimize ChildListMutationScope
https://bugs.webkit.org/show_bug.cgi?id=97352

Attachment 165169: Patch
https://bugs.webkit.org/attachment.cgi?id=165169&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=165169&action=review


This looks right.

> Source/WebCore/dom/ChildListMutationScope.cpp:70
> +    if (!m_observers)
> +	   return;

I would move this check to ChildListMutationScope so that it can be inlined
into ContainerNode member functions if I were you.

> Source/WebCore/dom/ChildListMutationScope.cpp:115
>	       m_target, StaticNodeList::adopt(m_addedNodes),
StaticNodeList::adopt(m_removedNodes), m_previousSibling.release(),
m_nextSibling.release()));

Nit: Wrong indentation (I know this is an existing code but...)

> Source/WebCore/dom/ChildListMutationScope.cpp:149
> +    delete this;

I'm not a big fun of manually calling delete like this.

> Source/WebCore/dom/ChildListMutationScope.h:96
> +	   typedef HashMap<Node*, MutationAccumulator*> AccumulatorMap;

Can we use HashMap<Node*, OwnPtr<MutationAccumulator> > instead so as to avoid
manual new/delete?