Unify the node removal code in ContainerNode and expand the coverage of NoEventDispat...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Oct 2017 20:20:48 +0000 (20:20 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Oct 2017 20:20:48 +0000 (20:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178568

Reviewed by Antti Koivisto.

Consolidated the code to remove a child node in ContainerNode into removeAllChildrenWithScriptAssertion
and removeNodeWithScriptAssertion to share code and make the semantics of when it becomes unsafe to run scripts.

Also renamed getChildNodes to collectChildNodes, and made it return NodeVector instead of taking an out argument.

No new tests since there should be no behavioral changes.

* dom/ContainerNode.cpp:
(WebCore::ContainerNode::removeAllChildrenWithScriptAssertion): Added.
(WebCore::ContainerNode::removeNodeWithScriptAssertion): Added.
(WebCore::collectChildrenAndRemoveFromOldParent):
(WebCore::ContainerNode::takeAllChildrenFrom): Deployed removeAllChildrenWithScriptAssertion.
(WebCore::ContainerNode::notifyChildRemoved): Deleted. Merged into removeNodeWithScriptAssertion.
(WebCore::willRemoveChild): Deleted. Ditto.
(WebCore::willRemoveChildren): Deleted. Merged into removeAllChildrenWithScriptAssertion.
(WebCore::ContainerNode::removeChild): Deployed removeNodeWithScriptAssertion.
(WebCore::ContainerNode::parserRemoveChild): Ditto.
(WebCore::ContainerNode::replaceAllChildren): Deployed removeAllChildrenWithScriptAssertion. Now removes the node
outside executeNodeInsertionWithScriptAssertion but that's okay since executeNodeInsertionWithScriptAssertion
doesn't execute any code with a side effect before invoking the callback.
(WebCore::ContainerNode::removeChildren):
(WebCore::dispatchChildRemovalEvents): Refactored to take Ref<Node>&.
* dom/ContainerNode.h:
(WebCore::collectChildNodes): Renamed from getChildNodes. Also removed the useless comment about NodeVector's
initial size and instead prefer to webkit.org/b/80706 where the number 11 was picked.
* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode):
* editing/ReplaceNodeWithSpanCommand.cpp:
(WebCore::swapInNodePreservingAttributesAndChildren):
* svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::clearShadowTree): Added an assertion exception while tearing down the UA shadow tree.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223788 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/ContainerNode.cpp
Source/WebCore/dom/ContainerNode.h
Source/WebCore/editing/ApplyStyleCommand.cpp
Source/WebCore/editing/ReplaceNodeWithSpanCommand.cpp
Source/WebCore/html/track/VTTCue.cpp
Source/WebCore/svg/SVGUseElement.cpp

index b8a8f5b..d3e624c 100644 (file)
@@ -1,3 +1,42 @@
+2017-10-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Unify the node removal code in ContainerNode and expand the coverage of NoEventDispatchAssertion
+        https://bugs.webkit.org/show_bug.cgi?id=178568
+
+        Reviewed by Antti Koivisto.
+
+        Consolidated the code to remove a child node in ContainerNode into removeAllChildrenWithScriptAssertion
+        and removeNodeWithScriptAssertion to share code and make the semantics of when it becomes unsafe to run scripts.
+
+        Also renamed getChildNodes to collectChildNodes, and made it return NodeVector instead of taking an out argument.
+
+        No new tests since there should be no behavioral changes.
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::removeAllChildrenWithScriptAssertion): Added.
+        (WebCore::ContainerNode::removeNodeWithScriptAssertion): Added.
+        (WebCore::collectChildrenAndRemoveFromOldParent):
+        (WebCore::ContainerNode::takeAllChildrenFrom): Deployed removeAllChildrenWithScriptAssertion.
+        (WebCore::ContainerNode::notifyChildRemoved): Deleted. Merged into removeNodeWithScriptAssertion.
+        (WebCore::willRemoveChild): Deleted. Ditto.
+        (WebCore::willRemoveChildren): Deleted. Merged into removeAllChildrenWithScriptAssertion.
+        (WebCore::ContainerNode::removeChild): Deployed removeNodeWithScriptAssertion.
+        (WebCore::ContainerNode::parserRemoveChild): Ditto.
+        (WebCore::ContainerNode::replaceAllChildren): Deployed removeAllChildrenWithScriptAssertion. Now removes the node
+        outside executeNodeInsertionWithScriptAssertion but that's okay since executeNodeInsertionWithScriptAssertion
+        doesn't execute any code with a side effect before invoking the callback.
+        (WebCore::ContainerNode::removeChildren):
+        (WebCore::dispatchChildRemovalEvents): Refactored to take Ref<Node>&.
+        * dom/ContainerNode.h:
+        (WebCore::collectChildNodes): Renamed from getChildNodes. Also removed the useless comment about NodeVector's
+        initial size and instead prefer to webkit.org/b/80706 where the number 11 was picked.
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode):
+        * editing/ReplaceNodeWithSpanCommand.cpp:
+        (WebCore::swapInNodePreservingAttributesAndChildren):
+        * svg/SVGUseElement.cpp:
+        (WebCore::SVGUseElement::clearShadowTree): Added an assertion exception while tearing down the UA shadow tree.
+
 2017-10-20  Youenn Fablet  <youenn@apple.com>
 
         ResourceResponse should have a ServiceWorker source
index 18b52cc..4f133ae 100644 (file)
@@ -66,7 +66,7 @@
 namespace WebCore {
 
 static void dispatchChildInsertionEvents(Node&);
-static void dispatchChildRemovalEvents(Node&);
+static void dispatchChildRemovalEvents(Ref<Node>&);
 
 ChildNodesLazySnapshot* ChildNodesLazySnapshot::latestSnapshot;
 
@@ -76,6 +76,96 @@ unsigned NoEventDispatchAssertion::DisableAssertionsInScope::s_existingCount = 0
 NoEventDispatchAssertion::EventAllowedScope* NoEventDispatchAssertion::EventAllowedScope::s_currentScope = nullptr;
 #endif
 
+ALWAYS_INLINE NodeVector ContainerNode::removeAllChildrenWithScriptAssertion(ChildChangeSource source, DeferChildrenChanged deferChildrenChanged)
+{
+    auto children = collectChildNodes(*this);
+
+    if (source == ContainerNode::ChildChangeSource::API) {
+        ChildListMutationScope mutation(*this);
+        for (auto& child : children) {
+            mutation.willRemoveChild(child.get());
+            child->notifyMutationObserversNodeWillDetach();
+            dispatchChildRemovalEvents(child);
+        }
+    } else {
+        ASSERT(source == ContainerNode::ChildChangeSource::Parser);
+        NoEventDispatchAssertion assertNoEventDispatch;
+        if (UNLIKELY(document().hasMutationObserversOfType(MutationObserver::ChildList))) {
+            ChildListMutationScope mutation(*this);
+            for (auto& child : children)
+                mutation.willRemoveChild(child.get());
+        }
+    }
+
+    disconnectSubframesIfNeeded(*this, DescendantsOnly);
+
+    WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+    NoEventDispatchAssertion assertNoEventDispatch;
+
+    document().nodeChildrenWillBeRemoved(*this);
+
+    while (RefPtr<Node> child = m_firstChild) {
+        removeBetween(nullptr, child->nextSibling(), *child);
+        notifyChildNodeRemoved(*this, *child);
+    }
+
+    if (deferChildrenChanged == DeferChildrenChanged::No)
+        childrenChanged(ContainerNode::ChildChange { ContainerNode::AllChildrenRemoved, nullptr, nullptr, source });
+
+    return children;
+}
+
+ALWAYS_INLINE bool ContainerNode::removeNodeWithScriptAssertion(Node& childToRemove, ChildChangeSource source)
+{
+    Ref<Node> protectedChildToRemove(childToRemove);
+    ASSERT_WITH_SECURITY_IMPLICATION(childToRemove.parentNode() == this);
+    {
+        NoEventDispatchAssertion assertNoEventDispatch;
+        ChildListMutationScope(*this).willRemoveChild(childToRemove);
+    }
+
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(childToRemove));
+    if (source == ContainerNode::ChildChangeSource::API) {
+        childToRemove.notifyMutationObserversNodeWillDetach();
+        dispatchChildRemovalEvents(protectedChildToRemove);
+        if (childToRemove.parentNode() != this)
+            return false;
+    }
+
+    if (source == ContainerNode::ChildChangeSource::Parser) {
+        // FIXME: Merge these two code paths. It's a bug in the parser not to update connectedSubframeCount in time.
+        disconnectSubframesIfNeeded(*this, DescendantsOnly);
+    } else {
+        if (is<ContainerNode>(childToRemove))
+            disconnectSubframesIfNeeded(downcast<ContainerNode>(childToRemove), RootAndDescendants);
+    }
+
+    if (childToRemove.parentNode() != this)
+        return false;
+
+    WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+    NoEventDispatchAssertion assertNoEventDispatch;
+
+    document().nodeWillBeRemoved(childToRemove);
+
+    ASSERT_WITH_SECURITY_IMPLICATION(childToRemove.parentNode() == this);
+    ASSERT(!childToRemove.isDocumentFragment());
+
+    RefPtr<Node> previousSibling = childToRemove.previousSibling();
+    RefPtr<Node> nextSibling = childToRemove.nextSibling();
+    removeBetween(previousSibling.get(), nextSibling.get(), childToRemove);
+    notifyChildNodeRemoved(*this, childToRemove);
+
+    ChildChange change;
+    change.type = is<Element>(childToRemove) ? ElementRemoved : (is<Text>(childToRemove) ? TextRemoved : NonContentsChildRemoved);
+    change.previousSiblingElement = (!previousSibling || is<Element>(*previousSibling)) ? downcast<Element>(previousSibling.get()) : ElementTraversal::previousSibling(*previousSibling);
+    change.nextSiblingElement = (!nextSibling || is<Element>(*nextSibling)) ? downcast<Element>(nextSibling.get()) : ElementTraversal::nextSibling(*nextSibling);
+    change.source = source;
+    childrenChanged(change);
+
+    return true;
+}
+
 enum class ReplacedAllChildren { No, Yes };
 
 template<typename DOMInsertionWork>
@@ -120,7 +210,7 @@ static ExceptionOr<void> collectChildrenAndRemoveFromOldParent(Node& node, NodeV
         return oldParent->removeChild(node);
     }
 
-    getChildNodes(node, nodes);
+    nodes = collectChildNodes(node);
     downcast<DocumentFragment>(node).removeChildren();
     return { };
 }
@@ -154,29 +244,7 @@ void ContainerNode::takeAllChildrenFrom(ContainerNode* oldParent)
 {
     ASSERT(oldParent);
 
-    NodeVector children;
-    getChildNodes(*oldParent, children);
-
-    if (oldParent->document().hasMutationObserversOfType(MutationObserver::ChildList)) {
-        ChildListMutationScope mutation(*oldParent);
-        for (auto& child : children)
-            mutation.willRemoveChild(child);
-    }
-
-    disconnectSubframesIfNeeded(*oldParent, DescendantsOnly);
-    {
-        NoEventDispatchAssertion assertNoEventDispatch;
-
-        oldParent->document().nodeChildrenWillBeRemoved(*oldParent);
-
-        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
-        while (RefPtr<Node> child = oldParent->m_firstChild) {
-            oldParent->removeBetween(nullptr, child->nextSibling(), *child);
-            notifyChildNodeRemoved(*oldParent, *child);
-        }
-        ChildChange change = { AllChildrenRemoved, nullptr, nullptr, ChildChangeSource::Parser };
-        childrenChanged(change);
-    }
+    auto children = oldParent->removeAllChildrenWithScriptAssertion(ChildChangeSource::Parser);
 
     // FIXME: assert that we don't dispatch events here since this container node is still disconnected.
     for (auto& child : children) {
@@ -369,20 +437,6 @@ void ContainerNode::appendChildCommon(Node& child)
     m_lastChild = &child;
 }
 
-void ContainerNode::notifyChildRemoved(Node& child, Node* previousSibling, Node* nextSibling, ChildChangeSource source)
-{
-    NoEventDispatchAssertion assertNoEventDispatch;
-    notifyChildNodeRemoved(*this, child);
-
-    ChildChange change;
-    change.type = is<Element>(child) ? ElementRemoved : is<Text>(child) ? TextRemoved : NonContentsChildRemoved;
-    change.previousSiblingElement = (!previousSibling || is<Element>(*previousSibling)) ? downcast<Element>(previousSibling) : ElementTraversal::previousSibling(*previousSibling);
-    change.nextSiblingElement = (!nextSibling || is<Element>(*nextSibling)) ? downcast<Element>(nextSibling) : ElementTraversal::nextSibling(*nextSibling);
-    change.source = source;
-
-    childrenChanged(change);
-}
-
 void ContainerNode::parserInsertBefore(Node& newChild, Node& nextChild)
 {
     ASSERT(nextChild.parentNode() == this);
@@ -479,38 +533,6 @@ ExceptionOr<void> ContainerNode::replaceChild(Node& newChild, Node& oldChild)
     return { };
 }
 
-static void willRemoveChild(ContainerNode& container, Node& child)
-{
-    ASSERT(child.parentNode());
-
-    ChildListMutationScope(*child.parentNode()).willRemoveChild(child);
-    child.notifyMutationObserversNodeWillDetach();
-    dispatchChildRemovalEvents(child);
-
-    if (child.parentNode() != &container)
-        return;
-
-    if (is<ContainerNode>(child))
-        disconnectSubframesIfNeeded(downcast<ContainerNode>(child), RootAndDescendants);
-}
-
-static void willRemoveChildren(ContainerNode& container)
-{
-    NodeVector children;
-    getChildNodes(container, children);
-
-    ChildListMutationScope mutation(container);
-    for (auto& child : children) {
-        mutation.willRemoveChild(child.get());
-        child->notifyMutationObserversNodeWillDetach();
-
-        // fire removed from document mutation events.
-        dispatchChildRemovalEvents(child.get());
-    }
-
-    disconnectSubframesIfNeeded(container, DescendantsOnly);
-}
-
 void ContainerNode::disconnectDescendantFrames()
 {
     disconnectSubframesIfNeeded(*this, RootAndDescendants);
@@ -528,27 +550,9 @@ ExceptionOr<void> ContainerNode::removeChild(Node& oldChild)
     if (oldChild.parentNode() != this)
         return Exception { NotFoundError };
 
-    Ref<Node> child(oldChild);
-
-    willRemoveChild(*this, child);
-
-    // Mutation events in willRemoveChild might have moved this child into a different parent.
-    if (child->parentNode() != this)
+    if (!removeNodeWithScriptAssertion(oldChild, ChildChangeSource::API))
         return Exception { NotFoundError };
 
-    {
-        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
-        NoEventDispatchAssertion assertNoEventDispatch;
-
-        document().nodeWillBeRemoved(child);
-
-        Node* prev = child->previousSibling();
-        Node* next = child->nextSibling();
-        removeBetween(prev, next, child);
-
-        notifyChildRemoved(child, prev, next, ChildChangeSource::API);
-    }
-
     rebuildSVGExtensionsElementsIfNecessary();
     dispatchSubtreeModifiedEvent();
 
@@ -591,28 +595,7 @@ void ContainerNode::removeBetween(Node* previousChild, Node* nextChild, Node& ol
 
 void ContainerNode::parserRemoveChild(Node& oldChild)
 {
-    disconnectSubframesIfNeeded(*this, DescendantsOnly);
-    if (oldChild.parentNode() != this)
-        return;
-
-    {
-        NoEventDispatchAssertion assertNoEventDispatch;
-
-        document().nodeChildrenWillBeRemoved(*this);
-
-        ASSERT(oldChild.parentNode() == this);
-        ASSERT(!oldChild.isDocumentFragment());
-
-        Node* prev = oldChild.previousSibling();
-        Node* next = oldChild.nextSibling();
-
-        ChildListMutationScope(*this).willRemoveChild(oldChild);
-        oldChild.notifyMutationObserversNodeWillDetach();
-
-        removeBetween(prev, next, oldChild);
-
-        notifyChildRemoved(oldChild, prev, next, ChildChangeSource::Parser);
-    }
+    removeNodeWithScriptAssertion(oldChild, ChildChangeSource::Parser);
 }
 
 // https://dom.spec.whatwg.org/#concept-node-replace-all
@@ -638,28 +621,14 @@ void ContainerNode::replaceAllChildren(Ref<Node>&& node)
 
     Ref<ContainerNode> protectedThis(*this);
     ChildListMutationScope mutation(*this);
+    removeAllChildrenWithScriptAssertion(ChildChangeSource::API, DeferChildrenChanged::Yes);
 
-    willRemoveChildren(*this);
-
-    node->setTreeScopeRecursively(treeScope());
-
-    {
-        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
-        executeNodeInsertionWithScriptAssertion(*this, node.get(), ChildChangeSource::API, ReplacedAllChildren::Yes, [&] {
-            document().nodeChildrenWillBeRemoved(*this);
-
-            while (RefPtr<Node> child = m_firstChild) {
-                removeBetween(nullptr, child->nextSibling(), *child);
-                notifyChildNodeRemoved(*this, *child);
-            }
-
-            // If node is not null, insert node into parent before null.
-            ASSERT(!ensurePreInsertionValidity(node, nullptr).hasException());
-            InspectorInstrumentation::willInsertDOMNode(document(), *this);
-
-            appendChildCommon(node);
-        });
-    }
+    executeNodeInsertionWithScriptAssertion(*this, node.get(), ChildChangeSource::API, ReplacedAllChildren::Yes, [&] {
+        ASSERT(!ensurePreInsertionValidity(node, nullptr).hasException());
+        InspectorInstrumentation::willInsertDOMNode(document(), *this);
+        node->setTreeScopeRecursively(treeScope());
+        appendChildCommon(node);
+    });
 
     rebuildSVGExtensionsElementsIfNecessary();
     dispatchSubtreeModifiedEvent();
@@ -678,26 +647,8 @@ void ContainerNode::removeChildren()
     if (!m_firstChild)
         return;
 
-    // The container node can be removed from event handlers.
     Ref<ContainerNode> protectedThis(*this);
-
-    // Do any prep work needed before actually starting to detach
-    // and remove... e.g. stop loading frames, fire unload events.
-    willRemoveChildren(*this);
-
-    {
-        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
-        NoEventDispatchAssertion assertNoEventDispatch;
-
-        document().nodeChildrenWillBeRemoved(*this);
-
-        while (RefPtr<Node> child = m_firstChild) {
-            removeBetween(0, child->nextSibling(), *child);
-            notifyChildNodeRemoved(*this, *child);
-        }
-
-        childrenChanged(ChildChange { AllChildrenRemoved, nullptr, nullptr, ChildChangeSource::API });
-    }
+    removeAllChildrenWithScriptAssertion(ChildChangeSource::API);
 
     rebuildSVGExtensionsElementsIfNecessary();
     dispatchSubtreeModifiedEvent();
@@ -826,29 +777,28 @@ static void dispatchChildInsertionEvents(Node& child)
     }
 }
 
-static void dispatchChildRemovalEvents(Node& child)
+static void dispatchChildRemovalEvents(Ref<Node>& child)
 {
-    if (child.isInShadowTree()) {
-        InspectorInstrumentation::willRemoveDOMNode(child.document(), child);
-        return;
-    }
-
     ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(child));
+    InspectorInstrumentation::willRemoveDOMNode(child->document(), child.get());
 
-    willCreatePossiblyOrphanedTreeByRemoval(&child);
-    InspectorInstrumentation::willRemoveDOMNode(child.document(), child);
+    if (child->isInShadowTree())
+        return;
 
-    RefPtr<Node> c = &child;
-    Ref<Document> document(child.document());
+    // FIXME: This doesn't belong in dispatchChildRemovalEvents.
+    // FIXME: Nodes removed from a shadow tree should also be kept alive.
+    willCreatePossiblyOrphanedTreeByRemoval(child.ptr());
+
+    Ref<Document> document = child->document();
 
     // dispatch pre-removal mutation events
-    if (c->parentNode() && document->hasListenerType(Document::DOMNODEREMOVED_LISTENER))
-        c->dispatchScopedEvent(MutationEvent::create(eventNames().DOMNodeRemovedEvent, true, c->parentNode()));
+    if (child->parentNode() && document->hasListenerType(Document::DOMNODEREMOVED_LISTENER))
+        child->dispatchScopedEvent(MutationEvent::create(eventNames().DOMNodeRemovedEvent, true, child->parentNode()));
 
     // dispatch the DOMNodeRemovedFromDocument event to all descendants
-    if (c->isConnected() && document->hasListenerType(Document::DOMNODEREMOVEDFROMDOCUMENT_LISTENER)) {
-        for (; c; c = NodeTraversal::next(*c, &child))
-            c->dispatchScopedEvent(MutationEvent::create(eventNames().DOMNodeRemovedFromDocumentEvent, false));
+    if (child->isConnected() && document->hasListenerType(Document::DOMNODEREMOVEDFROMDOCUMENT_LISTENER)) {
+        for (RefPtr<Node> currentNode = child.copyRef(); currentNode; currentNode = NodeTraversal::next(*currentNode, child.ptr()))
+            currentNode->dispatchScopedEvent(MutationEvent::create(eventNames().DOMNodeRemovedFromDocumentEvent, false));
     }
 }
 
index 644a48f..f1644ac 100644 (file)
@@ -32,6 +32,9 @@ class HTMLCollection;
 class RadioNodeList;
 class RenderElement;
 
+const int initialNodeVectorSize = 11; // Covers 99.5%. See webkit.org/b/80706
+typedef Vector<Ref<Node>, initialNodeVectorSize> NodeVector;
+
 class ContainerNode : public Node {
 public:
     virtual ~ContainerNode();
@@ -137,13 +140,16 @@ protected:
     HTMLCollection* cachedHTMLCollection(CollectionType);
 
 private:
+    void executePreparedChildrenRemoval();
+    enum class DeferChildrenChanged { Yes, No };
+    NodeVector removeAllChildrenWithScriptAssertion(ChildChangeSource, DeferChildrenChanged = DeferChildrenChanged::No);
+    bool removeNodeWithScriptAssertion(Node&, ChildChangeSource);
+
     void removeBetween(Node* previousChild, Node* nextChild, Node& oldChild);
     ExceptionOr<void> appendChildWithoutPreInsertionValidityCheck(Node&);
     void insertBeforeCommon(Node& nextChild, Node& oldChild);
     void appendChildCommon(Node&);
 
-    void notifyChildRemoved(Node& child, Node* previousSibling, Node* nextSibling, ChildChangeSource);
-
     void rebuildSVGExtensionsElementsIfNecessary();
 
     bool isContainerNode() const = delete;
@@ -197,17 +203,12 @@ inline Node& Node::rootNode() const
     return traverseToRootNode();
 }
 
-// This constant controls how much buffer is initially allocated
-// for a Node Vector that is used to store child Nodes of a given Node.
-// FIXME: Optimize the value.
-const int initialNodeVectorSize = 11;
-typedef Vector<Ref<Node>, initialNodeVectorSize> NodeVector;
-
-inline void getChildNodes(Node& node, NodeVector& nodes)
+inline NodeVector collectChildNodes(Node& node)
 {
-    ASSERT(nodes.isEmpty());
+    NodeVector children;
     for (Node* child = node.firstChild(); child; child = child->nextSibling())
-        nodes.append(*child);
+        children.append(*child);
+    return children;
 }
 
 class ChildNodesLazySnapshot {
index bc63aab..e492657 100644 (file)
@@ -1045,8 +1045,7 @@ void ApplyStyleCommand::pushDownInlineStyleAroundNode(EditingStyle& style, Node*
     // Each child of the removed element, exclusing ancestors of targetNode, is then wrapped by clones of elements in elementsToPushDown.
     Vector<Ref<Element>> elementsToPushDown;
     while (current && current != targetNode && current->contains(targetNode)) {
-        NodeVector currentChildren;
-        getChildNodes(*current.get(), currentChildren);
+        auto currentChildren = collectChildNodes(*current);
 
         RefPtr<StyledElement> styledElement;
         if (is<StyledElement>(*current) && isStyledInlineElementToRemove(downcast<Element>(current.get()))) {
index 720f2cd..734e03c 100644 (file)
@@ -49,8 +49,7 @@ static void swapInNodePreservingAttributesAndChildren(HTMLElement& newNode, HTML
 
     // FIXME: Fix this to send the proper MutationRecords when MutationObservers are present.
     newNode.cloneDataFromElement(nodeToReplace);
-    NodeVector children;
-    getChildNodes(nodeToReplace, children);
+    auto children = collectChildNodes(nodeToReplace);
     for (auto& child : children)
         newNode.appendChild(child);
 
index 8a100b3..21ea664 100644 (file)
@@ -859,6 +859,9 @@ void VTTCue::removeDisplayTree()
 
     if (!hasDisplayTree())
         return;
+
+    // The display tree is never exposed to author scripts so it's safe to dispatch events here.
+    NoEventDispatchAssertion::EventAllowedScope allowedScope(displayTreeInternal());
     displayTreeInternal().remove();
 }
 
index a191483..21ecd00 100644 (file)
@@ -31,6 +31,7 @@
 #include "ElementIterator.h"
 #include "Event.h"
 #include "EventNames.h"
+#include "NoEventDispatchAssertion.h"
 #include "RenderSVGResource.h"
 #include "RenderSVGTransformableContainer.h"
 #include "SVGDocumentExtensions.h"
@@ -214,8 +215,10 @@ static inline bool isDisallowedElement(const Element& element)
 
 void SVGUseElement::clearShadowTree()
 {
-    if (auto root = userAgentShadowRoot())
+    if (auto root = userAgentShadowRoot()) {
+        NoEventDispatchAssertion::EventAllowedScope scope(*root);
         root->removeChildren();
+    }
 }
 
 void SVGUseElement::buildPendingResource()