Changeset 238898 in webkit

Timestamp:

Dec 5, 2018 10:20:56 AM (5 years ago)

Author:

Simon Fraser

Message:

Minor refactoring of the scrolling code
​https://bugs.webkit.org/show_bug.cgi?id=192398

Patch by Frederic Wang <​fwang@igalia.com> on 2018-12-05
Reviewed by Simon Fraser.

Based on an earlier patch by Simon Fraser.

This patch performs some minor refactoring of the scrolling code:

• Rename ScrollingCoordinator::uniqueScrollLayerID() to uniqueScrollingNodeID() since it is really a node id.
• Inline ScrollingStateTree::setRootStateNode() so we only need to forward declare ScrollingStateFrameScrollingNode in headers.
• Pass argument to ScrollingStateTree::addNode() as a reference rather than a pointer.
• Initialize ScrollingStateTree::m_changedProperties and ScrollingStateTree::m_parent in the header file.
• Remove obsolete comment about ScrollingCoordinatorMac.

No new tests, behavior unchanged.

• page/scrolling/ScrollingCoordinator.cpp:

(WebCore::ScrollingCoordinator::uniqueScrollingNodeID):
(WebCore::ScrollingCoordinator::uniqueScrollLayerID): Deleted.

• page/scrolling/ScrollingCoordinator.h:
• page/scrolling/ScrollingStateNode.cpp:

(WebCore::ScrollingStateNode::ScrollingStateNode):

• page/scrolling/ScrollingStateNode.h:
• page/scrolling/ScrollingStateTree.cpp:

(WebCore::ScrollingStateTree::attachNode):
(WebCore::ScrollingStateTree::setRootStateNode):
(WebCore::ScrollingStateTree::addNode):

• page/scrolling/ScrollingStateTree.h:

(WebCore::ScrollingStateTree::setRootStateNode): Deleted.

• page/scrolling/ScrollingTree.cpp:
• page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
• rendering/RenderLayerCompositor.cpp:

(WebCore::RenderLayerCompositor::attachScrollingNode):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/scrolling/ScrollingCoordinator.cpp (modified) (1 diff)
• page/scrolling/ScrollingCoordinator.h (modified) (2 diffs)
• page/scrolling/ScrollingStateNode.cpp (modified) (2 diffs)
• page/scrolling/ScrollingStateNode.h (modified) (1 diff)
• page/scrolling/ScrollingStateTree.cpp (modified) (2 diffs)
• page/scrolling/ScrollingStateTree.h (modified) (3 diffs)
• page/scrolling/ScrollingTree.cpp (modified) (1 diff)
• page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h (modified) (1 diff)
• rendering/RenderLayerCompositor.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-12-05  Frederic Wang  <fwang@igalia.com>
+
+        Minor refactoring of the scrolling code
+        https://bugs.webkit.org/show_bug.cgi?id=192398
+
+        Reviewed by Simon Fraser.
+
+        Based on an earlier patch by Simon Fraser.
+
+        This patch performs some minor refactoring of the scrolling code:
+        - Rename ScrollingCoordinator::uniqueScrollLayerID() to uniqueScrollingNodeID() since it
+          is really a node id.
+        - Inline ScrollingStateTree::setRootStateNode() so we only need to forward declare
+          ScrollingStateFrameScrollingNode in headers.
+        - Pass argument to ScrollingStateTree::addNode() as a reference rather than a pointer.
+        - Initialize ScrollingStateTree::m_changedProperties and ScrollingStateTree::m_parent in
+          the header file.
+        - Remove obsolete comment about ScrollingCoordinatorMac.
+
+        No new tests, behavior unchanged.
+
+        * page/scrolling/ScrollingCoordinator.cpp:
+        (WebCore::ScrollingCoordinator::uniqueScrollingNodeID):
+        (WebCore::ScrollingCoordinator::uniqueScrollLayerID): Deleted.
+        * page/scrolling/ScrollingCoordinator.h:
+        * page/scrolling/ScrollingStateNode.cpp:
+        (WebCore::ScrollingStateNode::ScrollingStateNode):
+        * page/scrolling/ScrollingStateNode.h:
+        * page/scrolling/ScrollingStateTree.cpp:
+        (WebCore::ScrollingStateTree::attachNode):
+        (WebCore::ScrollingStateTree::setRootStateNode):
+        (WebCore::ScrollingStateTree::addNode):
+        * page/scrolling/ScrollingStateTree.h:
+        (WebCore::ScrollingStateTree::setRootStateNode): Deleted.
+        * page/scrolling/ScrollingTree.cpp:
+        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::attachScrollingNode):
+
 2018-12-05  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp

@@ -362 +362 @@
 }
 
-ScrollingNodeID ScrollingCoordinator::uniqueScrollLayerID()
-{
-    static ScrollingNodeID uniqueScrollLayerID = 1;
-    return uniqueScrollLayerID++;
+ScrollingNodeID ScrollingCoordinator::uniqueScrollingNodeID()
+{
+    static ScrollingNodeID uniqueScrollingNodeID = 1;
+    return uniqueScrollingNodeID++;
 }
 

trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h

@@ -162 +162 @@
 
     // These virtual functions are currently unique to the threaded scrolling architecture. 
-    // Their meaningful implementations are in ScrollingCoordinatorMac.
     virtual void commitTreeStateIfNeeded() { }
     virtual bool requestScrollPositionUpdate(FrameView&, const IntPoint&) { return false; }
@@ -199 +198 @@
     virtual void setScrollPinningBehavior(ScrollPinningBehavior) { }
 
-    // Generated a unique id for scroll layers.
-    ScrollingNodeID uniqueScrollLayerID();
+    // Generated a unique id for scrolling nodes.
+    ScrollingNodeID uniqueScrollingNodeID();
 
     enum MainThreadScrollingReasonFlags {

trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp

@@ -40 +40 @@
     : m_nodeType(nodeType)
     , m_nodeID(nodeID)
-    , m_changedProperties(0)
     , m_scrollingStateTree(scrollingStateTree)
-    , m_parent(nullptr)
 {
 }
@@ -53 +51 @@
     , m_changedProperties(stateNode.changedProperties())
     , m_scrollingStateTree(adoptiveTree)
-    , m_parent(nullptr)
 {
     if (hasChangedProperty(ScrollLayer))
         setLayer(stateNode.layer().toRepresentation(adoptiveTree.preferredLayerRepresentation()));
-    scrollingStateTree().addNode(this);
+    scrollingStateTree().addNode(*this);
 }
 

trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h

@@ -249 +249 @@
 
     const ScrollingNodeType m_nodeType;
-    ScrollingNodeID m_nodeID;
-    ChangedProperties m_changedProperties;
+    const ScrollingNodeID m_nodeID;
+    ChangedProperties m_changedProperties { 0 };
 
     ScrollingStateTree& m_scrollingStateTree;
 
-    ScrollingStateNode* m_parent;
+    ScrollingStateNode* m_parent { nullptr };
     std::unique_ptr<Vector<RefPtr<ScrollingStateNode>>> m_children;
 

trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp

@@ -106 +106 @@
         // If the type has changed, we need to destroy and recreate the node with a new ID.
         if (nodeType != node->nodeType())
-            newNodeID = m_scrollingCoordinator->uniqueScrollLayerID();
+            newNodeID = m_scrollingCoordinator->uniqueScrollingNodeID();
 #endif
 
@@ -197 +197 @@
 }
 
-void ScrollingStateTree::addNode(ScrollingStateNode* node)
-{
-    m_stateNodeMap.add(node->scrollingNodeID(), node);
+void ScrollingStateTree::setRootStateNode(Ref<ScrollingStateFrameScrollingNode>&& rootStateNode)
+{
+    m_rootStateNode = WTFMove(rootStateNode);
+}
+
+void ScrollingStateTree::addNode(ScrollingStateNode& node)
+{
+    m_stateNodeMap.add(node.scrollingNodeID(), &node);
 }
 

trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h

@@ -28 +28 @@
 #if ENABLE(ASYNC_SCROLLING) || USE(COORDINATED_GRAPHICS)
 
-#include "ScrollingStateFrameScrollingNode.h"
+#include "ScrollingCoordinator.h"
+#include "ScrollingStateNode.h"
 #include <wtf/RefPtr.h>
  
@@ -34 +35 @@
 
 class AsyncScrollingCoordinator;
+class ScrollingStateFrameScrollingNode;
 
 // The ScrollingStateTree is a tree that managed ScrollingStateNodes. The nodes keep track of the current
@@ -75 +77 @@
 
 private:
-    void setRootStateNode(Ref<ScrollingStateFrameScrollingNode>&& rootStateNode) { m_rootStateNode = WTFMove(rootStateNode); }
-    void addNode(ScrollingStateNode*);
+    void setRootStateNode(Ref<ScrollingStateFrameScrollingNode>&&);
+    void addNode(ScrollingStateNode&);
 
     Ref<ScrollingStateNode> createNode(ScrollingNodeType, ScrollingNodeID);

trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp

@@ -32 +32 @@
 #include "Logging.h"
 #include "PlatformWheelEvent.h"
+#include "ScrollingStateFrameScrollingNode.h"
 #include "ScrollingStateTree.h"
 #include "ScrollingTreeFrameScrollingNode.h"

trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h

@@ -30 +30 @@
 #include "ScrollController.h"
 #include "ScrollbarThemeMac.h"
+#include "ScrollingStateFrameScrollingNode.h"
 #include "ScrollingTreeFrameScrollingNode.h"
 #include <wtf/RetainPtr.h>

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -3836 +3836 @@
     ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(role);
     if (!nodeID)
-        nodeID = scrollingCoordinator->uniqueScrollLayerID();
+        nodeID = scrollingCoordinator->uniqueScrollingNodeID();
 
     nodeID = scrollingCoordinator->attachToStateTree(nodeType, nodeID, parentNodeID);