Title: [170131] trunk/Source/WebCore
Revision
170131
Author
simon.fra...@apple.com
Date
2014-06-18 16:49:26 -0700 (Wed, 18 Jun 2014)

Log Message

Move the removeNode() tree walking from ScrollingStateNote into ScrollingStateTree
https://bugs.webkit.org/show_bug.cgi?id=134043

Reviewed by Beth Dakin.

It's cleaner if ScrollingStateTree does the descendant walk when removing nodes.
We can simply start the "willBeRemoved" walk at the node in question.

Have willRemoveNode() just remove the node from the m_stateNodeMap directly, rather
than this happening in a separate walk of m_nodesRemovedSinceLastCommit.

* page/scrolling/ScrollingStateNode.cpp:
(WebCore::ScrollingStateNode::removeDescendant): Deleted.
(WebCore::ScrollingStateNode::willBeRemovedFromStateTree): Deleted.
* page/scrolling/ScrollingStateNode.h:
* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::attachNode):
(WebCore::ScrollingStateTree::detachNode):
(WebCore::ScrollingStateTree::clear):
(WebCore::ScrollingStateTree::removeNodeAndAllDescendants):
(WebCore::ScrollingStateTree::recursiveNodeWillBeRemoved):
(WebCore::ScrollingStateTree::willRemoveNode):
(WebCore::ScrollingStateTree::removeNode): Deleted.
* page/scrolling/ScrollingStateTree.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (170130 => 170131)


--- trunk/Source/WebCore/ChangeLog	2014-06-18 23:42:57 UTC (rev 170130)
+++ trunk/Source/WebCore/ChangeLog	2014-06-18 23:49:26 UTC (rev 170131)
@@ -1,3 +1,30 @@
+2014-06-18  Simon Fraser  <simon.fra...@apple.com>
+
+        Move the removeNode() tree walking from ScrollingStateNote into ScrollingStateTree
+        https://bugs.webkit.org/show_bug.cgi?id=134043
+
+        Reviewed by Beth Dakin.
+
+        It's cleaner if ScrollingStateTree does the descendant walk when removing nodes.
+        We can simply start the "willBeRemoved" walk at the node in question.
+        
+        Have willRemoveNode() just remove the node from the m_stateNodeMap directly, rather
+        than this happening in a separate walk of m_nodesRemovedSinceLastCommit.
+
+        * page/scrolling/ScrollingStateNode.cpp:
+        (WebCore::ScrollingStateNode::removeDescendant): Deleted.
+        (WebCore::ScrollingStateNode::willBeRemovedFromStateTree): Deleted.
+        * page/scrolling/ScrollingStateNode.h:
+        * page/scrolling/ScrollingStateTree.cpp:
+        (WebCore::ScrollingStateTree::attachNode):
+        (WebCore::ScrollingStateTree::detachNode):
+        (WebCore::ScrollingStateTree::clear):
+        (WebCore::ScrollingStateTree::removeNodeAndAllDescendants):
+        (WebCore::ScrollingStateTree::recursiveNodeWillBeRemoved):
+        (WebCore::ScrollingStateTree::willRemoveNode):
+        (WebCore::ScrollingStateTree::removeNode): Deleted.
+        * page/scrolling/ScrollingStateTree.h:
+
 2014-06-18  Alex Christensen  <achristen...@webkit.org>
 
         [iOS WebGL] Fixed WEBGL_compressed_texture_pvrtc.

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp (170130 => 170131)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp	2014-06-18 23:42:57 UTC (rev 170130)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp	2014-06-18 23:49:26 UTC (rev 170131)
@@ -102,32 +102,6 @@
     m_children->append(childNode);
 }
 
-void ScrollingStateNode::removeDescendant(ScrollingStateNode* node)
-{
-    if (!m_children)
-        return;
-
-    size_t index = m_children->find(node);
-    if (index != notFound) {
-        node->willBeRemovedFromStateTree();
-        m_children->remove(index);
-        return;
-    }
-
-    for (auto& child : *m_children)
-        child->removeDescendant(node);
-}
-
-void ScrollingStateNode::willBeRemovedFromStateTree()
-{
-    scrollingStateTree().willRemoveNode(this);
-    if (!m_children)
-        return;
-
-    for (auto& child : *m_children)
-        child->willBeRemovedFromStateTree();
-}
-
 void ScrollingStateNode::setLayer(const LayerRepresentation& layerRepresentation)
 {
     if (layerRepresentation == m_layer)

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h (170130 => 170131)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h	2014-06-18 23:42:57 UTC (rev 170130)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h	2014-06-18 23:49:26 UTC (rev 170131)
@@ -198,7 +198,6 @@
     Vector<RefPtr<ScrollingStateNode>>* children() const { return m_children.get(); }
 
     void appendChild(PassRefPtr<ScrollingStateNode>);
-    void removeDescendant(ScrollingStateNode*);
 
     String scrollingStateTreeAsText() const;
 
@@ -209,7 +208,6 @@
     void dump(TextStream&, int indent) const;
 
     virtual void dumpProperties(TextStream&, int indent) const = 0;
-    void willBeRemovedFromStateTree();
 
     const ScrollingNodeType m_nodeType;
     ScrollingNodeID m_nodeID;

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (170130 => 170131)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp	2014-06-18 23:42:57 UTC (rev 170130)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp	2014-06-18 23:49:26 UTC (rev 170131)
@@ -83,7 +83,7 @@
             return newNodeID;
 
         // The node is being re-parented. To do that, we'll remove it, and then re-create a new node.
-        removeNode(node);
+        removeNodeAndAllDescendants(node);
     }
 
     ScrollingStateNode* newNode = nullptr;
@@ -141,12 +141,13 @@
     if (!node)
         return;
 
-    removeNode(node);
+    removeNodeAndAllDescendants(node);
 }
 
 void ScrollingStateTree::clear()
 {
-    removeNode(rootStateNode());
+    removeNodeAndAllDescendants(rootStateNode());
+    ASSERT(m_stateNodeMap.isEmpty());
     m_stateNodeMap.clear();
 }
 
@@ -177,29 +178,39 @@
     m_stateNodeMap.add(node->scrollingNodeID(), node);
 }
 
-void ScrollingStateTree::removeNode(ScrollingStateNode* node)
+void ScrollingStateTree::removeNodeAndAllDescendants(ScrollingStateNode* node)
 {
     if (!node)
         return;
 
-    if (node == m_rootStateNode) {
-        willRemoveNode(node);
+    recursiveNodeWillBeRemoved(node);
+
+    if (node == m_rootStateNode)
         m_rootStateNode = nullptr;
-        return;
+    else if (ScrollingStateNode* parent = node->parent()) {
+        ASSERT(parent->children() && parent->children()->find(node) != notFound);
+        if (auto children = parent->children()) {
+            size_t index = children->find(node);
+            if (index != notFound)
+                children->remove(index);
+        }
     }
+}
 
-    ASSERT(m_rootStateNode);
-    m_rootStateNode->removeDescendant(node);
+void ScrollingStateTree::recursiveNodeWillBeRemoved(ScrollingStateNode* currNode)
+{
+    willRemoveNode(currNode);
 
-    // ScrollingStateTree::removeDescendant() will destroy children, so we have to make sure we remove those children
-    // from the HashMap.
-    for (auto removedNodeID : m_nodesRemovedSinceLastCommit)
-        m_stateNodeMap.remove(removedNodeID);
+    if (auto children = currNode->children()) {
+        for (auto& child : *children)
+            recursiveNodeWillBeRemoved(child.get());
+    }
 }
 
 void ScrollingStateTree::willRemoveNode(ScrollingStateNode* node)
 {
     m_nodesRemovedSinceLastCommit.append(node->scrollingNodeID());
+    m_stateNodeMap.remove(node->scrollingNodeID());
     setHasChangedProperties();
 }
 

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h (170130 => 170131)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h	2014-06-18 23:42:57 UTC (rev 170130)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h	2014-06-18 23:49:26 UTC (rev 170131)
@@ -81,7 +81,9 @@
 
     void setRootStateNode(PassRefPtr<ScrollingStateFrameScrollingNode> rootStateNode) { m_rootStateNode = rootStateNode; }
     void addNode(ScrollingStateNode*);
-    void removeNode(ScrollingStateNode*);
+    void removeNodeAndAllDescendants(ScrollingStateNode*);
+
+    void recursiveNodeWillBeRemoved(ScrollingStateNode* currNode);
     void willRemoveNode(ScrollingStateNode*);
 
     AsyncScrollingCoordinator* m_scrollingCoordinator;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to