[webkit-reviews] review granted: [Bug 103868] Change ChildNodeInsertionNotifier::m_postInsertionsNotificationTargets from a Vector to an OwnPtr : [Attachment 177209] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Kentaro Hara <haraken at chromium.org>'s
request for review:
Bug 103868: Change
ChildNodeInsertionNotifier::m_postInsertionsNotificationTargets from a Vector
to an OwnPtr
https://bugs.webkit.org/show_bug.cgi?id=103868

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=177209&action=review


> Source/WebCore/dom/ContainerNodeAlgorithms.h:50
> +    OwnPtr<Vector< RefPtr<Node> > > m_postInsertionNotificationTargets;

The space in Vector< RefPtr is not the correct style. It should be
Vector<RefPtr.

> Source/WebCore/dom/ContainerNodeAlgorithms.h:197
> +    if (UNLIKELY(Node::InsertionShouldCallDidNotifySubtreeInsertions ==
node->insertedInto(m_insertionPoint))) {

Are you sure this improved performance? Normally we only use LIKELY and
UNLIKELY when we have concrete evidence it’s helpful.

> Source/WebCore/dom/ContainerNodeAlgorithms.h:199
> +	       m_postInsertionNotificationTargets = adoptPtr(new
Vector<RefPtr<Node> >());

The () here is not needed.

> Source/WebCore/dom/ContainerNodeAlgorithms.h:213
> +	       m_postInsertionNotificationTargets = adoptPtr(new
Vector<RefPtr<Node> >());

The () here is not needed.