Title: [142504] trunk/Source/WebCore
Revision
142504
Author
simon.fra...@apple.com
Date
2013-02-11 13:28:12 -0800 (Mon, 11 Feb 2013)

Log Message

Move m_stateNodeMap from ScrollingCoordinatorMac to ScrollingStateTree
https://bugs.webkit.org/show_bug.cgi?id=109361

Reviewed by Sam Weinig.

The map of scrolling node IDs to ScollingStateNodes was maintained by
ScrollingCoordinatorMac, rather than ScrollingStateTree. This is different
from the ScrollingTree (which owns its node map), and added some amount
of to-and-fro between ScrollingStateTree and ScrollingCoordinatorMac.

Having ScrollingCoordinatorMac maintain the map of IDs to state nodes
simplifies things.

No behavior change.

* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::attachNode):
(WebCore::ScrollingStateTree::detachNode):
(WebCore::ScrollingStateTree::clear):
(WebCore::ScrollingStateTree::removeNode):
(WebCore::ScrollingStateTree::stateNodeForID):
* page/scrolling/ScrollingStateTree.h:
(ScrollingStateTree): Remove some stale comments.
(WebCore::ScrollingStateTree::removedNodes):
* page/scrolling/mac/ScrollingCoordinatorMac.h:
(ScrollingCoordinatorMac):
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
(WebCore::ScrollingCoordinatorMac::frameViewLayoutUpdated):
(WebCore::ScrollingCoordinatorMac::recomputeWheelEventHandlerCountForFrameView):
(WebCore::ScrollingCoordinatorMac::frameViewRootLayerDidChange):
(WebCore::ScrollingCoordinatorMac::requestScrollPositionUpdate):
(WebCore::ScrollingCoordinatorMac::attachToStateTree):
(WebCore::ScrollingCoordinatorMac::detachFromStateTree):
(WebCore::ScrollingCoordinatorMac::clearStateTree):
(WebCore::ScrollingCoordinatorMac::updateScrollingNode):
(WebCore::ScrollingCoordinatorMac::updateViewportConstrainedNode):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (142503 => 142504)


--- trunk/Source/WebCore/ChangeLog	2013-02-11 21:26:48 UTC (rev 142503)
+++ trunk/Source/WebCore/ChangeLog	2013-02-11 21:28:12 UTC (rev 142504)
@@ -1,3 +1,42 @@
+2013-02-11  Simon Fraser  <simon.fra...@apple.com>
+
+        Move m_stateNodeMap from ScrollingCoordinatorMac to ScrollingStateTree
+        https://bugs.webkit.org/show_bug.cgi?id=109361
+
+        Reviewed by Sam Weinig.
+        
+        The map of scrolling node IDs to ScollingStateNodes was maintained by
+        ScrollingCoordinatorMac, rather than ScrollingStateTree. This is different
+        from the ScrollingTree (which owns its node map), and added some amount
+        of to-and-fro between ScrollingStateTree and ScrollingCoordinatorMac.
+        
+        Having ScrollingCoordinatorMac maintain the map of IDs to state nodes
+        simplifies things.
+
+        No behavior change.
+
+        * page/scrolling/ScrollingStateTree.cpp:
+        (WebCore::ScrollingStateTree::attachNode):
+        (WebCore::ScrollingStateTree::detachNode):
+        (WebCore::ScrollingStateTree::clear):
+        (WebCore::ScrollingStateTree::removeNode):
+        (WebCore::ScrollingStateTree::stateNodeForID):
+        * page/scrolling/ScrollingStateTree.h:
+        (ScrollingStateTree): Remove some stale comments.
+        (WebCore::ScrollingStateTree::removedNodes):
+        * page/scrolling/mac/ScrollingCoordinatorMac.h:
+        (ScrollingCoordinatorMac):
+        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+        (WebCore::ScrollingCoordinatorMac::frameViewLayoutUpdated):
+        (WebCore::ScrollingCoordinatorMac::recomputeWheelEventHandlerCountForFrameView):
+        (WebCore::ScrollingCoordinatorMac::frameViewRootLayerDidChange):
+        (WebCore::ScrollingCoordinatorMac::requestScrollPositionUpdate):
+        (WebCore::ScrollingCoordinatorMac::attachToStateTree):
+        (WebCore::ScrollingCoordinatorMac::detachFromStateTree):
+        (WebCore::ScrollingCoordinatorMac::clearStateTree):
+        (WebCore::ScrollingCoordinatorMac::updateScrollingNode):
+        (WebCore::ScrollingCoordinatorMac::updateViewportConstrainedNode):
+
 2013-02-11  Mark Rowe  <mr...@apple.com>
 
         Build fix.

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (142503 => 142504)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp	2013-02-11 21:26:48 UTC (rev 142503)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp	2013-02-11 21:28:12 UTC (rev 142504)
@@ -24,10 +24,15 @@
  */
  
 #include "config.h"
-#include "ScrollingStateTree.h"
 
 #if ENABLE(THREADED_SCROLLING)
 
+#include "ScrollingStateTree.h"
+
+#include "ScrollingStateFixedNode.h"
+#include "ScrollingStateScrollingNode.h"
+#include "ScrollingStateStickyNode.h"
+
 namespace WebCore {
 
 PassOwnPtr<ScrollingStateTree> ScrollingStateTree::create()
@@ -45,6 +50,78 @@
 {
 }
 
+ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID)
+{
+    ASSERT(newNodeID);
+
+    if (ScrollingStateNode* node = stateNodeForID(newNodeID)) {
+        ScrollingStateNode* parent = stateNodeForID(parentID);
+        if (!parent)
+            return newNodeID;
+        if (node->parent() == parent)
+            return newNodeID;
+
+        // The node is being re-parented. To do that, we'll remove it, and then re-create a new node.
+        removeNode(node);
+    }
+
+    ScrollingStateNode* newNode;
+    if (!parentID) {
+        // If we're resetting the root node, we should clear the HashMap and destroy the current children.
+        clear();
+
+        rootStateNode()->setScrollingNodeID(newNodeID);
+        newNode = rootStateNode();
+    } else {
+        ScrollingStateNode* parent = stateNodeForID(parentID);
+        switch (nodeType) {
+        case FixedNode: {
+            OwnPtr<ScrollingStateFixedNode> fixedNode = ScrollingStateFixedNode::create(this, newNodeID);
+            newNode = fixedNode.get();
+            parent->appendChild(fixedNode.release());
+            break;
+        }
+        case StickyNode: {
+            OwnPtr<ScrollingStateStickyNode> stickyNode = ScrollingStateStickyNode::create(this, newNodeID);
+            newNode = stickyNode.get();
+            parent->appendChild(stickyNode.release());
+            break;
+        }
+        case ScrollingNode: {
+            // FIXME: We currently only support child nodes that are fixed.
+            ASSERT_NOT_REACHED();
+            OwnPtr<ScrollingStateScrollingNode> scrollingNode = ScrollingStateScrollingNode::create(this, newNodeID);
+            newNode = scrollingNode.get();
+            parent->appendChild(scrollingNode.release());
+            break;
+        }
+        default:
+            ASSERT_NOT_REACHED();
+        }
+    }
+
+    m_stateNodeMap.set(newNodeID, newNode);
+    return newNodeID;
+}
+
+void ScrollingStateTree::detachNode(ScrollingNodeID nodeID)
+{
+    if (!nodeID)
+        return;
+
+    // The node may not be found if clearStateTree() was recently called.
+    ScrollingStateNode* node = m_stateNodeMap.take(nodeID);
+    if (!node)
+        return;
+
+    removeNode(node);
+}
+
+void ScrollingStateTree::clear()
+{
+    removeNode(rootStateNode());
+}
+
 PassOwnPtr<ScrollingStateTree> ScrollingStateTree::commit()
 {
     // This function clones and resets the current state tree, but leaves the tree structure intact. 
@@ -65,6 +142,12 @@
 {
     ASSERT(m_rootStateNode);
     m_rootStateNode->removeChild(node);
+
+    // ScrollingStateTree::removeNode() will destroy children, so we have to make sure we remove those children
+    // from the HashMap.
+    size_t size = m_nodesRemovedSinceLastCommit.size();
+    for (size_t i = 0; i < size; ++i)
+        m_stateNodeMap.remove(m_nodesRemovedSinceLastCommit[i]);
 }
 
 void ScrollingStateTree::didRemoveNode(ScrollingNodeID nodeID)
@@ -73,6 +156,18 @@
     m_hasChangedProperties = true;
 }
 
+ScrollingStateNode* ScrollingStateTree::stateNodeForID(ScrollingNodeID scrollLayerID)
+{
+    if (!scrollLayerID)
+        return 0;
+
+    HashMap<ScrollingNodeID, ScrollingStateNode*>::const_iterator it = m_stateNodeMap.find(scrollLayerID);
+    if (it == m_stateNodeMap.end())
+        return 0;
+
+    return it->value;
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(THREADED_SCROLLING)

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h (142503 => 142504)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h	2013-02-11 21:26:48 UTC (rev 142503)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h	2013-02-11 21:28:12 UTC (rev 142504)
@@ -40,29 +40,25 @@
 // will be informed and will schedule a timer that will clone the new state tree and send it over to
 // the scrolling thread, avoiding locking. 
 
-// FIXME: Right now the scrolling thread only ever looks at the root node. In the future, it should
-// look at the whole tree and build a ScrollingTree that mimics the ScrollingStateTree.
-
-// FIXME: Right now there is only one type of ScrollingStateNode, which is the ScrollingStateScrollingNode.
-// It is currently used for the main frame, but in the future it should be able to be used for subframes
-// and overflow areas. In the future, more classes will inherit from ScrollingStateNode, such as
-// ScrollingStateFixedNode and ScrollingStateStickyNode for fixed position objects and sticky positoned
-// objects, respectively.
-
 class ScrollingStateTree {
+    friend class ScrollingStateNode;
 public:
+    
     static PassOwnPtr<ScrollingStateTree> create();
     ~ScrollingStateTree();
 
     ScrollingStateScrollingNode* rootStateNode() const { return m_rootStateNode.get(); }
+    ScrollingStateNode* stateNodeForID(ScrollingNodeID);
 
+    ScrollingNodeID attachNode(ScrollingNodeType, ScrollingNodeID, ScrollingNodeID parentID);
+    void detachNode(ScrollingNodeID);
+    void clear();
+    
+    const Vector<ScrollingNodeID>& removedNodes() const { return m_nodesRemovedSinceLastCommit; }
+
     // Copies the current tree state and clears the changed properties mask in the original.
     PassOwnPtr<ScrollingStateTree> commit();
 
-    void removeNode(ScrollingStateNode*);
-    void didRemoveNode(ScrollingNodeID);
-    const Vector<ScrollingNodeID>& removedNodes() const { return m_nodesRemovedSinceLastCommit; }
-
     void setHasChangedProperties(bool changedProperties) { m_hasChangedProperties = changedProperties; }
     bool hasChangedProperties() const { return m_hasChangedProperties; }
 
@@ -70,9 +66,12 @@
     ScrollingStateTree();
 
     void setRootStateNode(PassOwnPtr<ScrollingStateScrollingNode> rootStateNode) { m_rootStateNode = rootStateNode; }
+    void removeNode(ScrollingStateNode*);
+    void didRemoveNode(ScrollingNodeID);
 
     PassOwnPtr<ScrollingStateTree> clone();
 
+    HashMap<ScrollingNodeID, ScrollingStateNode*> m_stateNodeMap;
     OwnPtr<ScrollingStateScrollingNode> m_rootStateNode;
     Vector<ScrollingNodeID> m_nodesRemovedSinceLastCommit;
     bool m_hasChangedProperties;

Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h (142503 => 142504)


--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h	2013-02-11 21:26:48 UTC (rev 142503)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h	2013-02-11 21:28:12 UTC (rev 142504)
@@ -96,7 +96,6 @@
     virtual bool hasVisibleSlowRepaintViewportConstrainedObjects(FrameView*) const { return false; }
 
     void ensureRootStateNodeForFrameView(FrameView*);
-    ScrollingStateNode* stateNodeForID(ScrollingNodeID);
 
     struct ScrollParameters {
         ScrollElasticity horizontalScrollElasticity;
@@ -129,13 +128,9 @@
     void scrollingStateTreeCommitterTimerFired(Timer<ScrollingCoordinatorMac>*);
     void commitTreeState();
 
-    void removeNode(ScrollingStateNode*);
-
     OwnPtr<ScrollingStateTree> m_scrollingStateTree;
     RefPtr<ScrollingTree> m_scrollingTree;
     Timer<ScrollingCoordinatorMac> m_scrollingStateTreeCommitterTimer;
-
-    HashMap<ScrollingNodeID, ScrollingStateNode*> m_stateNodeMap;
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (142503 => 142504)


--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm	2013-02-11 21:26:48 UTC (rev 142503)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm	2013-02-11 21:28:12 UTC (rev 142504)
@@ -119,7 +119,7 @@
     if (!coordinatesScrollingForFrameView(frameView))
         return;
 
-    ScrollingStateScrollingNode* node = toScrollingStateScrollingNode(stateNodeForID(frameView->scrollLayerID()));
+    ScrollingStateScrollingNode* node = toScrollingStateScrollingNode(m_scrollingStateTree->stateNodeForID(frameView->scrollLayerID()));
     if (!node)
         return;
 
@@ -141,7 +141,7 @@
 
 void ScrollingCoordinatorMac::recomputeWheelEventHandlerCountForFrameView(FrameView* frameView)
 {
-    ScrollingStateScrollingNode* node = toScrollingStateScrollingNode(stateNodeForID(frameView->scrollLayerID()));
+    ScrollingStateScrollingNode* node = toScrollingStateScrollingNode(m_scrollingStateTree->stateNodeForID(frameView->scrollLayerID()));
     if (!node)
         return;
     setWheelEventHandlerCountForNode(computeCurrentWheelEventHandlerCount(), node);
@@ -161,7 +161,7 @@
 
     ScrollingCoordinator::frameViewRootLayerDidChange(frameView);
 
-    ScrollingStateScrollingNode* node = toScrollingStateScrollingNode(stateNodeForID(frameView->scrollLayerID()));
+    ScrollingStateScrollingNode* node = toScrollingStateScrollingNode(m_scrollingStateTree->stateNodeForID(frameView->scrollLayerID()));
     setScrollLayerForNode(scrollLayerForFrameView(frameView), node);
     setCounterScrollingLayerForNode(counterScrollingLayerForFrameView(frameView), node);
 }
@@ -204,7 +204,7 @@
     if (frameView->frame()->document()->inPageCache())
         return true;
 
-    ScrollingStateScrollingNode* stateNode = toScrollingStateScrollingNode(stateNodeForID(frameView->scrollLayerID()));
+    ScrollingStateScrollingNode* stateNode = toScrollingStateScrollingNode(m_scrollingStateTree->stateNodeForID(frameView->scrollLayerID()));
     if (!stateNode)
         return false;
 
@@ -228,102 +228,19 @@
 
 ScrollingNodeID ScrollingCoordinatorMac::attachToStateTree(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID)
 {
-    ASSERT(newNodeID);
-
-    if (ScrollingStateNode* node = stateNodeForID(newNodeID)) {
-        ScrollingStateNode* parent = stateNodeForID(parentID);
-        if (!parent)
-            return newNodeID;
-        if (node->parent() == parent)
-            return newNodeID;
-
-        // The node is being re-parented. To do that, we'll remove it, and then re-create a new node.
-        removeNode(node);
-    }
-
-    ScrollingStateNode* newNode;
-    if (!parentID) {
-        // If we're resetting the root node, we should clear the HashMap and destroy the current children.
-        clearStateTree();
-
-        m_scrollingStateTree->rootStateNode()->setScrollingNodeID(newNodeID);
-        newNode = m_scrollingStateTree->rootStateNode();
-    } else {
-        ScrollingStateNode* parent = stateNodeForID(parentID);
-        switch (nodeType) {
-        case FixedNode: {
-            ASSERT(supportsFixedPositionLayers());
-            OwnPtr<ScrollingStateFixedNode> fixedNode = ScrollingStateFixedNode::create(m_scrollingStateTree.get(), newNodeID);
-            newNode = fixedNode.get();
-            parent->appendChild(fixedNode.release());
-            break;
-        }
-        case StickyNode: {
-            ASSERT(supportsFixedPositionLayers());
-            OwnPtr<ScrollingStateStickyNode> stickyNode = ScrollingStateStickyNode::create(m_scrollingStateTree.get(), newNodeID);
-            newNode = stickyNode.get();
-            parent->appendChild(stickyNode.release());
-            break;
-        }
-        case ScrollingNode: {
-            // FIXME: We currently only support child nodes that are fixed.
-            ASSERT_NOT_REACHED();
-            OwnPtr<ScrollingStateScrollingNode> scrollingNode = ScrollingStateScrollingNode::create(m_scrollingStateTree.get(), newNodeID);
-            newNode = scrollingNode.get();
-            parent->appendChild(scrollingNode.release());
-            break;
-        }
-        default:
-            ASSERT_NOT_REACHED();
-        }
-    }
-
-    m_stateNodeMap.set(newNodeID, newNode);
-    return newNodeID;
+    return m_scrollingStateTree->attachNode(nodeType, newNodeID, parentID);
 }
 
-void ScrollingCoordinatorMac::removeNode(ScrollingStateNode* node)
+void ScrollingCoordinatorMac::detachFromStateTree(ScrollingNodeID nodeID)
 {
-    m_scrollingStateTree->removeNode(node);
-
-    // ScrollingStateTree::removeNode() will destroy children, so we have to make sure we remove those children
-    // from the HashMap.
-    const Vector<ScrollingNodeID>& removedNodes = m_scrollingStateTree->removedNodes();
-    size_t size = removedNodes.size();
-    for (size_t i = 0; i < size; ++i)
-        m_stateNodeMap.remove(removedNodes[i]);
+    m_scrollingStateTree->detachNode(nodeID);
 }
 
-void ScrollingCoordinatorMac::detachFromStateTree(ScrollingNodeID scrollLayerID)
-{
-    if (!scrollLayerID)
-        return;
-
-    // The node may not be found if clearStateTree() was recently called.
-    ScrollingStateNode* node = m_stateNodeMap.take(scrollLayerID);
-    if (!node)
-        return;
-
-    removeNode(node);
-}
-
 void ScrollingCoordinatorMac::clearStateTree()
 {
-    removeNode(m_scrollingStateTree->rootStateNode());
+    m_scrollingStateTree->clear();
 }
 
-ScrollingStateNode* ScrollingCoordinatorMac::stateNodeForID(ScrollingNodeID scrollLayerID)
-{
-    if (!scrollLayerID)
-        return 0;
-
-    HashMap<ScrollingNodeID, ScrollingStateNode*>::const_iterator it = m_stateNodeMap.find(scrollLayerID);
-    if (it == m_stateNodeMap.end())
-        return 0;
-
-    return it->value;
-}
-
 void ScrollingCoordinatorMac::ensureRootStateNodeForFrameView(FrameView* frameView)
 {
     attachToStateTree(ScrollingNode, frameView->scrollLayerID(), 0);
@@ -412,7 +329,7 @@
 
 void ScrollingCoordinatorMac::updateScrollingNode(ScrollingNodeID nodeID, GraphicsLayer* scrollLayer, GraphicsLayer* counterScrollingLayer)
 {
-    ScrollingStateScrollingNode* node = toScrollingStateScrollingNode(stateNodeForID(nodeID));
+    ScrollingStateScrollingNode* node = toScrollingStateScrollingNode(m_scrollingStateTree->stateNodeForID(nodeID));
     ASSERT(node);
     if (!node)
         return;
@@ -428,13 +345,13 @@
 
     switch (constraints.constraintType()) {
     case ViewportConstraints::FixedPositionConstaint: {
-        ScrollingStateFixedNode* node = toScrollingStateFixedNode(stateNodeForID(nodeID));
+        ScrollingStateFixedNode* node = toScrollingStateFixedNode(m_scrollingStateTree->stateNodeForID(nodeID));
         setScrollLayerForNode(graphicsLayer, node);
         node->updateConstraints((const FixedPositionViewportConstraints&)constraints);
         break;
     }
     case ViewportConstraints::StickyPositionConstraint: {
-        ScrollingStateStickyNode* node = toScrollingStateStickyNode(stateNodeForID(nodeID));
+        ScrollingStateStickyNode* node = toScrollingStateStickyNode(m_scrollingStateTree->stateNodeForID(nodeID));
         setScrollLayerForNode(graphicsLayer, node);
         node->updateConstraints((const StickyPositionViewportConstraints&)constraints);
         break;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to