[webkit-changes] [217926] trunk

Thu, 08 Jun 2017 00:31:08 -0700

Title: [217926] trunk
Revision
217926
Author
[email protected]
Date
2017-06-08 00:30:37 -0700 (Thu, 08 Jun 2017)

Log Message

IsInShadowTreeFlag does not get updated for a non-container node
https://bugs.webkit.org/show_bug.cgi?id=173084

Reviewed by Antti Koivisto.

.:

* WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme:

Source/WebCore:

insertedInto and removedFrom were only called on ContainerNode nodes when they're not connected to a document.
As a result IsInShadowTreeFlag could have gotten out-of-date when a node was inserted or removed from a shadow root
which is not connected to a document.

Fixed this inconsistency by always falling insertedInto and removedFrom on all nodes.

* dom/ContainerNodeAlgorithms.cpp:
(WebCore::notifyNodeInsertedIntoDocument): Merged notifyDescendantInsertedIntoDocument. Now takes a Node instead
of a ContainerNode.
(WebCore::notifyNodeInsertedIntoTree): Merged notifyDescendantInsertedIntoTree. Now takes a Node instead of
a ContainerNode.
(WebCore::notifyChildNodeInserted): Always call notifyNodeInsertedIntoTree on an inserted node.
(WebCore::notifyNodeRemovedFromDocument): Now takes a Node instead of a ContainerNode.
(WebCore::notifyNodeRemovedFromTree): Ditto.
(WebCore::notifyChildNodeRemoved): Always call notifyNodeRemovedFromTree on an inserted node.
(WebCore::addChildNodesToDeletionQueue): Directly call adoptIfNeeded on document() since onwerDocument() only returns
nullptr on a Document node but this function is never called on a root node and Document can only be a root node.
Also assert that a node not put into the deletion queue is no longer in a document or a shadow tree.

* dom/Node.cpp:
(WebCore::Node::insertedInto): Removed the assertion that's no longer true.
(WebCore::Node::removedFrom): Ditto.

Modified Paths

Diff

Modified: trunk/ChangeLog (217925 => 217926)


--- trunk/ChangeLog	2017-06-08 06:48:22 UTC (rev 217925)
+++ trunk/ChangeLog	2017-06-08 07:30:37 UTC (rev 217926)
@@ -1,3 +1,12 @@
+2017-06-07  Ryosuke Niwa  <[email protected]>
+
+        IsInShadowTreeFlag does not get updated for a non-container node
+        https://bugs.webkit.org/show_bug.cgi?id=173084
+
+        Reviewed by Antti Koivisto.
+
+        * WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme:
+
 2017-06-07  Carlos Garcia Campos  <[email protected]>
 
         [WPE] Enable resource usage

Modified: trunk/Source/WebCore/ChangeLog (217925 => 217926)


--- trunk/Source/WebCore/ChangeLog	2017-06-08 06:48:22 UTC (rev 217925)
+++ trunk/Source/WebCore/ChangeLog	2017-06-08 07:30:37 UTC (rev 217926)
@@ -1,3 +1,33 @@
+2017-06-07  Ryosuke Niwa  <[email protected]>
+
+        IsInShadowTreeFlag does not get updated for a non-container node
+        https://bugs.webkit.org/show_bug.cgi?id=173084
+
+        Reviewed by Antti Koivisto.
+
+        insertedInto and removedFrom were only called on ContainerNode nodes when they're not connected to a document.
+        As a result IsInShadowTreeFlag could have gotten out-of-date when a node was inserted or removed from a shadow root
+        which is not connected to a document.
+
+        Fixed this inconsistency by always falling insertedInto and removedFrom on all nodes.
+
+        * dom/ContainerNodeAlgorithms.cpp:
+        (WebCore::notifyNodeInsertedIntoDocument): Merged notifyDescendantInsertedIntoDocument. Now takes a Node instead
+        of a ContainerNode.
+        (WebCore::notifyNodeInsertedIntoTree): Merged notifyDescendantInsertedIntoTree. Now takes a Node instead of
+        a ContainerNode.
+        (WebCore::notifyChildNodeInserted): Always call notifyNodeInsertedIntoTree on an inserted node.
+        (WebCore::notifyNodeRemovedFromDocument): Now takes a Node instead of a ContainerNode.
+        (WebCore::notifyNodeRemovedFromTree): Ditto.
+        (WebCore::notifyChildNodeRemoved): Always call notifyNodeRemovedFromTree on an inserted node.
+        (WebCore::addChildNodesToDeletionQueue): Directly call adoptIfNeeded on document() since onwerDocument() only returns
+        nullptr on a Document node but this function is never called on a root node and Document can only be a root node.
+        Also assert that a node not put into the deletion queue is no longer in a document or a shadow tree.
+
+        * dom/Node.cpp:
+        (WebCore::Node::insertedInto): Removed the assertion that's no longer true.
+        (WebCore::Node::removedFrom): Ditto.
+
 2017-06-07  Carlos Garcia Campos  <[email protected]>
 
         [WPE] Enable resource usage

Modified: trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp (217925 => 217926)


--- trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp	2017-06-08 06:48:22 UTC (rev 217925)
+++ trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp	2017-06-08 07:30:37 UTC (rev 217926)
@@ -34,13 +34,15 @@
 
 namespace WebCore {
 
-static void notifyNodeInsertedIntoTree(ContainerNode& insertionPoint, ContainerNode&, NodeVector& postInsertionNotificationTargets);
-static void notifyNodeInsertedIntoDocument(ContainerNode& insertionPoint, Node&, NodeVector& postInsertionNotificationTargets);
-static void notifyNodeRemovedFromTree(ContainerNode& insertionPoint, ContainerNode&);
-static void notifyNodeRemovedFromDocument(ContainerNode& insertionPoint, Node&);
+static void notifyNodeInsertedIntoDocument(ContainerNode& insertionPoint, Node& node, NodeVector& postInsertionNotificationTargets)
+{
+    ASSERT(insertionPoint.isConnected());
+    if (node.insertedInto(insertionPoint) == Node::InsertionShouldCallFinishedInsertingSubtree)
+        postInsertionNotificationTargets.append(node);
 
-static void notifyDescendantInsertedIntoDocument(ContainerNode& insertionPoint, ContainerNode& node, NodeVector& postInsertionNotificationTargets)
-{
+    if (!is<ContainerNode>(node))
+        return;
+
     ChildNodesLazySnapshot snapshot(node);
     while (RefPtr<Node> child = snapshot.nextNode()) {
         // If we have been removed from the document during this loop, then
@@ -59,34 +61,25 @@
     }
 }
 
-static void notifyDescendantInsertedIntoTree(ContainerNode& insertionPoint, ContainerNode& node, NodeVector& postInsertionNotificationTargets)
+static void notifyNodeInsertedIntoTree(ContainerNode& insertionPoint, Node& node, NodeVector& postInsertionNotificationTargets)
 {
-    for (Node* child = node.firstChild(); child; child = child->nextSibling()) {
-        if (is<ContainerNode>(*child))
-            notifyNodeInsertedIntoTree(insertionPoint, downcast<ContainerNode>(*child), postInsertionNotificationTargets);
-    }
+    NoEventDispatchAssertion assertNoEventDispatch;
+    ASSERT(!insertionPoint.isConnected());
 
-    if (ShadowRoot* root = node.shadowRoot())
-        notifyNodeInsertedIntoTree(insertionPoint, *root, postInsertionNotificationTargets);
-}
-
-void notifyNodeInsertedIntoDocument(ContainerNode& insertionPoint, Node& node, NodeVector& postInsertionNotificationTargets)
-{
-    ASSERT(insertionPoint.isConnected());
     if (node.insertedInto(insertionPoint) == Node::InsertionShouldCallFinishedInsertingSubtree)
         postInsertionNotificationTargets.append(node);
-    if (is<ContainerNode>(node))
-        notifyDescendantInsertedIntoDocument(insertionPoint, downcast<ContainerNode>(node), postInsertionNotificationTargets);
-}
 
-void notifyNodeInsertedIntoTree(ContainerNode& insertionPoint, ContainerNode& node, NodeVector& postInsertionNotificationTargets)
-{
-    NoEventDispatchAssertion assertNoEventDispatch;
-    ASSERT(!insertionPoint.isConnected());
+    if (!is<ContainerNode>(node))
+        return;
 
-    if (node.insertedInto(insertionPoint) == Node::InsertionShouldCallFinishedInsertingSubtree)
-        postInsertionNotificationTargets.append(node);
-    notifyDescendantInsertedIntoTree(insertionPoint, node, postInsertionNotificationTargets);
+    for (auto* child = node.firstChild(); child; child = child->nextSibling())
+        notifyNodeInsertedIntoTree(insertionPoint, *child, postInsertionNotificationTargets);
+
+    if (!is<Element>(node))
+        return;
+
+    if (RefPtr<ShadowRoot> root = downcast<Element>(node).shadowRoot())
+        notifyNodeInsertedIntoTree(insertionPoint, *root, postInsertionNotificationTargets);
 }
 
 void notifyChildNodeInserted(ContainerNode& insertionPoint, Node& node, NodeVector& postInsertionNotificationTargets)
@@ -100,11 +93,11 @@
 
     if (insertionPoint.isConnected())
         notifyNodeInsertedIntoDocument(insertionPoint, node, postInsertionNotificationTargets);
-    else if (is<ContainerNode>(node))
-        notifyNodeInsertedIntoTree(insertionPoint, downcast<ContainerNode>(node), postInsertionNotificationTargets);
+    else
+        notifyNodeInsertedIntoTree(insertionPoint, node, postInsertionNotificationTargets);
 }
 
-void notifyNodeRemovedFromDocument(ContainerNode& insertionPoint, Node& node)
+static void notifyNodeRemovedFromDocument(ContainerNode& insertionPoint, Node& node)
 {
     ASSERT(insertionPoint.isConnected());
     node.removedFrom(insertionPoint);
@@ -111,6 +104,7 @@
 
     if (!is<ContainerNode>(node))
         return;
+
     ChildNodesLazySnapshot snapshot(node);
     while (RefPtr<Node> child = snapshot.nextNode()) {
         // If we have been added to the document during this loop, then we
@@ -132,7 +126,7 @@
     }
 }
 
-void notifyNodeRemovedFromTree(ContainerNode& insertionPoint, ContainerNode& node)
+static void notifyNodeRemovedFromTree(ContainerNode& insertionPoint, Node& node)
 {
     NoEventDispatchAssertion assertNoEventDispatch;
     ASSERT(!insertionPoint.isConnected());
@@ -139,26 +133,22 @@
 
     node.removedFrom(insertionPoint);
 
-    for (Node* child = node.firstChild(); child; child = child->nextSibling()) {
-        if (is<ContainerNode>(*child))
-            notifyNodeRemovedFromTree(insertionPoint, downcast<ContainerNode>(*child));
-    }
+    for (Node* child = node.firstChild(); child; child = child->nextSibling())
+        notifyNodeRemovedFromTree(insertionPoint, *child);
 
     if (!is<Element>(node))
         return;
 
     if (RefPtr<ShadowRoot> root = downcast<Element>(node).shadowRoot())
-        notifyNodeRemovedFromTree(insertionPoint, *root.get());
+        notifyNodeRemovedFromTree(insertionPoint, *root);
 }
 
 void notifyChildNodeRemoved(ContainerNode& insertionPoint, Node& child)
 {
-    if (!child.isConnected()) {
-        if (is<ContainerNode>(child))
-            notifyNodeRemovedFromTree(insertionPoint, downcast<ContainerNode>(child));
-        return;
-    }
-    notifyNodeRemovedFromDocument(insertionPoint, child);
+    if (child.isConnected())
+        notifyNodeRemovedFromDocument(insertionPoint, child);
+    else
+        notifyNodeRemovedFromTree(insertionPoint, child);
 }
 
 void addChildNodesToDeletionQueue(Node*& head, Node*& tail, ContainerNode& container)
@@ -189,10 +179,10 @@
             tail = node;
         } else {
             Ref<Node> protect(*node); // removedFromDocument may remove remove all references to this node.
-            if (Document* containerDocument = container.ownerDocument())
-                containerDocument->adoptIfNeeded(*node);
+            container.document().adoptIfNeeded(*node);
             if (node->isInTreeScope())
                 notifyChildNodeRemoved(container, *node);
+            ASSERT_WITH_SECURITY_IMPLICATION(!node->isInTreeScope());
         }
     }

Modified: trunk/Source/WebCore/dom/Node.cpp (217925 => 217926)


--- trunk/Source/WebCore/dom/Node.cpp	2017-06-08 06:48:22 UTC (rev 217925)
+++ trunk/Source/WebCore/dom/Node.cpp	2017-06-08 07:30:37 UTC (rev 217926)
@@ -1248,10 +1248,9 @@
 
 Node::InsertionNotificationRequest Node::insertedInto(ContainerNode& insertionPoint)
 {
-    ASSERT(insertionPoint.isConnected() || isContainerNode());
     if (insertionPoint.isConnected())
         setFlag(IsConnectedFlag);
-    if (parentOrShadowHostNode()->isInShadowTree())
+    if (insertionPoint.isInShadowTree())
         setFlag(IsInShadowTreeFlag);
 
     invalidateStyle(Style::Validity::SubtreeAndRenderersInvalid);
@@ -1261,7 +1260,6 @@
 
 void Node::removedFrom(ContainerNode& insertionPoint)
 {
-    ASSERT(insertionPoint.isConnected() || isContainerNode());
     if (insertionPoint.isConnected())
         clearFlag(IsConnectedFlag);
     if (isInShadowTree() && !treeScope().rootNode().isShadowRoot())

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to