Call insertedInto or removedFrom before childrenChanged

Source/core/dom/ContainerNode.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
@@ skipping 260 matching lines @@
     newChild.setNextSibling(&nextChild);
 }
 
 void ContainerNode::parserInsertBefore(PassRefPtrWillBeRawPtr<Node> newChild, Node& nextChild)
 {
     ASSERT(newChild);
     ASSERT(nextChild.parentNode() == this);
     ASSERT(!newChild->isDocumentFragment());
     ASSERT(!isHTMLTemplateElement(this));
 
-    if (nextChild.previousSibling() == newChild || &nextChild == newChild) // nothing to do
+    RefPtrWillBeRawPtr<Node> protect(this);

[abarth-chromium, 2014/06/07 01:17:51]

Why not do this after checking for early return?

+
+    if (nextChild.previousSibling() == newChild || nextChild == newChild) // nothing to do

[sof, 2014/06/07 15:41:08]

Oilpan compilation broke as a result of dropping the & from what was there
previously.

Fixed compilation via https://codereview.chromium.org/325573002/

         return;
 
     if (document() != newChild->document())
         document().adoptNode(newChild.get(), ASSERT_NO_EXCEPTION);
 
     insertBeforeCommon(nextChild, *newChild);
 
     newChild->updateAncestorConnectedSubframeCountForInsertion();
 
     ChildListMutationScope(*this).childAdded(*newChild);
 
-    childrenChanged(true, newChild->previousSibling(), &nextChild, 1);
-
-    notifyNodeInserted(*newChild);
+    notifyNodeInserted(*newChild, InsertedByParser);
 }
 
 void ContainerNode::replaceChild(PassRefPtrWillBeRawPtr<Node> newChild, Node* oldChild, ExceptionState& exceptionState)
 {
 #if !ENABLE(OILPAN)
     // Check that this node is not "floating".
     // If it is, it can be deleted as a side effect of sending mutation events.
     ASSERT(refCount() || parentOrShadowHostNode());
 #endif
 
@@ skipping 156 matching lines @@
         exceptionState.throwDOMException(NotFoundError, "The node to be removed is no longer a child of this node. Perhaps it was moved in response to a mutation?");
         return;
     }
 
     {
         HTMLFrameOwnerElement::UpdateSuspendScope suspendWidgetHierarchyUpdates;
 
         Node* prev = child->previousSibling();
         Node* next = child->nextSibling();
         removeBetween(prev, next, *child);
+        notifyNodeRemoved(*child);
         childrenChanged(false, prev, next, -1);
-        notifyNodeRemoved(*child);
     }
     dispatchSubtreeModifiedEvent();
 }
 
 void ContainerNode::removeBetween(Node* previousChild, Node* nextChild, Node& oldChild)
 {
     NoEventDispatchAssertion assertNoEventDispatch;
 
     ASSERT(oldChild.parentNode() == this);
 
@@ skipping 14 matching lines @@
     oldChild.setParentOrShadowHostNode(0);
 
     document().adoptIfNeeded(oldChild);
 }
 
 void ContainerNode::parserRemoveChild(Node& oldChild)
 {
     ASSERT(oldChild.parentNode() == this);
     ASSERT(!oldChild.isDocumentFragment());
 
+    ScriptForbiddenScope forbidScript;
+
     Node* prev = oldChild.previousSibling();
     Node* next = oldChild.nextSibling();
 
     oldChild.updateAncestorConnectedSubframeCountForRemoval();
 
     ChildListMutationScope(*this).willRemoveChild(oldChild);
     oldChild.notifyMutationObserversNodeWillDetach();
 
     removeBetween(prev, next, oldChild);
-
+    notifyNodeRemoved(oldChild);
     childrenChanged(true, prev, next, -1);
-    notifyNodeRemoved(oldChild);
 }
 
 // this differs from other remove functions because it forcibly removes all the children,
 // regardless of read-only status or event exceptions, e.g.
 void ContainerNode::removeChildren()
 {
     if (!m_firstChild)
         return;
 
     // The container node can be removed from event handlers.
@@ skipping 14 matching lines @@
         // Exclude this node when looking for removed focusedElement since only
         // children will be removed.
         // This must be later than willRemoveChildren, which might change focus
         // state of a child.
         document().removeFocusedElementOfSubtree(this, true);
 
         // Removing a node from a selection can cause widget updates.
         document().nodeChildrenWillBeRemoved(*this);
     }
 
-
-    NodeVector removedChildren;
     {
         HTMLFrameOwnerElement::UpdateSuspendScope suspendWidgetHierarchyUpdates;
-        {
+        ScriptForbiddenScope forbidScript;
+
+        // FIXME: We can't have a top level NoEventDispatchAssertion since
+        // SVGElement::invalidateInstances() is called inside ::childrenChanged
+        // and will stamp out new <use> shadow trees which mutates the DOM and
+        // hits the event dispatching assert in ::notifyNodeInserted. We should
+        // figure out how to make ths work async.
+
+        int childCount = 0;
+        while (RefPtrWillBeRawPtr<Node> child = m_firstChild) {
             NoEventDispatchAssertion assertNoEventDispatch;
-            removedChildren.reserveInitialCapacity(countChildren());
-            while (m_firstChild) {
-                removedChildren.append(m_firstChild);
-                removeBetween(0, m_firstChild->nextSibling(), *m_firstChild);
-            }
+            ++childCount;
+            removeBetween(0, child->nextSibling(), *child);
+            notifyNodeRemoved(*child);
         }
 
-        childrenChanged(false, 0, 0, -static_cast<int>(removedChildren.size()));
-
-        for (size_t i = 0; i < removedChildren.size(); ++i)
-            notifyNodeRemoved(*removedChildren[i]);
+        childrenChanged(false, 0, 0, -childCount);
     }
 
     dispatchSubtreeModifiedEvent();
 }
 
 void ContainerNode::appendChild(PassRefPtrWillBeRawPtr<Node> newChild, ExceptionState& exceptionState)
 {
     RefPtrWillBeRawPtr<ContainerNode> protect(this);
 
 #if !ENABLE(OILPAN)
@@ skipping 50 matching lines @@
     dispatchSubtreeModifiedEvent();
 }
 
 void ContainerNode::parserAppendChild(PassRefPtrWillBeRawPtr<Node> newChild)
 {
     ASSERT(newChild);
     ASSERT(!newChild->parentNode()); // Use appendChild if you need to handle reparenting (and want DOM mutation events).
     ASSERT(!newChild->isDocumentFragment());
     ASSERT(!isHTMLTemplateElement(this));
 
+    RefPtrWillBeRawPtr<Node> protect(this);
+
     if (document() != newChild->document())
         document().adoptNode(newChild.get(), ASSERT_NO_EXCEPTION);
 
-    Node* last = m_lastChild;
-
     {
         NoEventDispatchAssertion assertNoEventDispatch;
         ScriptForbiddenScope forbidScript;
 
         treeScope().adoptIfNeeded(*newChild);
         // FIXME: This method should take a PassRefPtr.
         appendChildToContainer(*newChild, *this);
         newChild->updateAncestorConnectedSubframeCountForInsertion();
         ChildListMutationScope(*this).childAdded(*newChild);
     }
 
-    childrenChanged(true, last, 0, 1);
-    notifyNodeInserted(*newChild);
+    notifyNodeInserted(*newChild, InsertedByParser);
 }
 
-void ContainerNode::notifyNodeInserted(Node& root)
+void ContainerNode::notifyNodeInserted(Node& root, InsertionSourceType source)
 {
     ASSERT(!NoEventDispatchAssertion::isEventDispatchForbidden());
 
     InspectorInstrumentation::didInsertDOMNode(&root);
 
     RefPtrWillBeRawPtr<Node> protect(this);
     RefPtrWillBeRawPtr<Node> protectNode(root);
 
     NodeVector postInsertionNotificationTargets;
     notifyNodeInsertedInternal(root, postInsertionNotificationTargets);
 
+    // ShadowRoots are not real children, we don't need to tell host that it's
+    // children changed when one is added.
+    // FIXME: We should have a separate code path for ShadowRoot since it only
+    // needs to call insertedInto and the rest of this logic is not needed.
+    if (!root.isShadowRoot())
+        childrenChanged(source == InsertedByParser, root.previousSibling(), root.nextSibling(), 1);
+
     for (size_t i = 0; i < postInsertionNotificationTargets.size(); ++i) {
         Node* targetNode = postInsertionNotificationTargets[i].get();
         if (targetNode->inDocument())
             targetNode->didNotifySubtreeInsertionsToDocument();
     }
 }
 
 void ContainerNode::notifyNodeInsertedInternal(Node& root, NodeVector& postInsertionNotificationTargets)
 {
     NoEventDispatchAssertion assertNoEventDispatch;
@@ skipping 391 matching lines @@
 
 void ContainerNode::updateTreeAfterInsertion(Node& child)
 {
 #if !ENABLE(OILPAN)
     ASSERT(refCount());
     ASSERT(child.refCount());
 #endif
 
     ChildListMutationScope(*this).childAdded(child);
 
-    childrenChanged(false, child.previousSibling(), child.nextSibling(), 1);
-
     notifyNodeInserted(child);
 
     dispatchChildInsertionEvents(child);
 }
 
 bool ContainerNode::hasRestyleFlagInternal(DynamicRestyleFlags mask) const
 {
     return rareData()->hasRestyleFlag(mask);
 }
 
@@ skipping 182 matching lines @@
         return true;
 
     if (node->isElementNode() && toElement(node)->shadow())
         return true;
 
     return false;
 }
 #endif
 
 } // namespace WebCore