Attachment 356598 Details for Bug 192398 – Patch

[patch] Patch

bug-192398-20181205144407.patch (text/plain), 11.29 KB, created by Frédéric Wang (:fredw) on 2018-12-05 05:44:08 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Frédéric Wang (:fredw)

Created: 2018-12-05 05:44:08 PST

Size: 11.29 KB

patch

obsolete

>Subversion Revision: 238891
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 4b5e21bbb267406f78838cd72ebd31ec0a546540..b2cec33bc0ce2885b42a5256dff221fbd28d323e 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,42 @@
>+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 NOBODY (OOPS!).
>+
>+        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  Rob Buis  <rbuis@igalia.com>
> 
>         [Mac] HEAD requests changed to GET after 301, 302, and 303 redirections (http/tests/xmlhttprequest/head-redirection.html)
>diff --git a/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
>index f50221a51ce7ef3c9211626a1b1ead66da99fea9..2818ea94bf0e93b43031be934ae6cb708be821f8 100644
>--- a/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
>+++ b/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
>@@ -361,10 +361,10 @@ bool ScrollingCoordinator::shouldUpdateScrollLayerPositionSynchronously(const Fr
>     return true;
> }
> 
>-ScrollingNodeID ScrollingCoordinator::uniqueScrollLayerID()
>+ScrollingNodeID ScrollingCoordinator::uniqueScrollingNodeID()
> {
>-    static ScrollingNodeID uniqueScrollLayerID = 1;
>-    return uniqueScrollLayerID++;
>+    static ScrollingNodeID uniqueScrollingNodeID = 1;
>+    return uniqueScrollingNodeID++;
> }
> 
> String ScrollingCoordinator::scrollingStateTreeAsText(ScrollingStateTreeAsTextBehavior) const
>diff --git a/Source/WebCore/page/scrolling/ScrollingCoordinator.h b/Source/WebCore/page/scrolling/ScrollingCoordinator.h
>index f917089a5afe429c6631bd2f7157ff3eae1e0edb..53a6918ec2eee69896e8127a475693e0b9e0681f 100644
>--- a/Source/WebCore/page/scrolling/ScrollingCoordinator.h
>+++ b/Source/WebCore/page/scrolling/ScrollingCoordinator.h
>@@ -161,7 +161,6 @@ public:
>     WEBCORE_EXPORT void setForceSynchronousScrollLayerPositionUpdates(bool);
> 
>     // 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; }
>     virtual bool handleWheelEvent(FrameView&, const PlatformWheelEvent&) { return true; }
>@@ -198,8 +197,8 @@ public:
>     virtual void updateScrollSnapPropertiesWithFrameView(const FrameView&) { }
>     virtual void setScrollPinningBehavior(ScrollPinningBehavior) { }
> 
>-    // Generated a unique id for scroll layers.
>-    ScrollingNodeID uniqueScrollLayerID();
>+    // Generated a unique id for scrolling nodes.
>+    ScrollingNodeID uniqueScrollingNodeID();
> 
>     enum MainThreadScrollingReasonFlags {
>         ForcedOnMainThread                                          = 1 << 0,
>diff --git a/Source/WebCore/page/scrolling/ScrollingStateNode.cpp b/Source/WebCore/page/scrolling/ScrollingStateNode.cpp
>index 4c7e143fa2fc0a86e4ea677ce0da5f016a32af9d..d9bf7c5e68040c3b47865e4d767cb1513efeca7d 100644
>--- a/Source/WebCore/page/scrolling/ScrollingStateNode.cpp
>+++ b/Source/WebCore/page/scrolling/ScrollingStateNode.cpp
>@@ -39,9 +39,7 @@ namespace WebCore {
> ScrollingStateNode::ScrollingStateNode(ScrollingNodeType nodeType, ScrollingStateTree& scrollingStateTree, ScrollingNodeID nodeID)
>     : m_nodeType(nodeType)
>     , m_nodeID(nodeID)
>-    , m_changedProperties(0)
>     , m_scrollingStateTree(scrollingStateTree)
>-    , m_parent(nullptr)
> {
> }
> 
>@@ -52,11 +50,10 @@ ScrollingStateNode::ScrollingStateNode(const ScrollingStateNode& stateNode, Scro
>     , m_nodeID(stateNode.scrollingNodeID())
>     , 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);
> }
> 
> ScrollingStateNode::~ScrollingStateNode() = default;
>diff --git a/Source/WebCore/page/scrolling/ScrollingStateNode.h b/Source/WebCore/page/scrolling/ScrollingStateNode.h
>index 455dc1adfae80b7a08deaaf4e3c7458a6654bf52..f310487454f4cc41568386e07984b01378790ba9 100644
>--- a/Source/WebCore/page/scrolling/ScrollingStateNode.h
>+++ b/Source/WebCore/page/scrolling/ScrollingStateNode.h
>@@ -248,12 +248,12 @@ private:
>     void dump(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const;
> 
>     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;
> 
>     LayerRepresentation m_layer;
>diff --git a/Source/WebCore/page/scrolling/ScrollingStateTree.cpp b/Source/WebCore/page/scrolling/ScrollingStateTree.cpp
>index b33aa0cdafe497a2f8f9ca6b7b079f452fa7b076..70a95f47e68abf48841dad59f38df5058f5c93cf 100644
>--- a/Source/WebCore/page/scrolling/ScrollingStateTree.cpp
>+++ b/Source/WebCore/page/scrolling/ScrollingStateTree.cpp
>@@ -105,7 +105,7 @@ ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, Scrol
> #if ENABLE(ASYNC_SCROLLING)
>         // 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
> 
>         // The node is being re-parented. To do that, we'll remove it, and then create a new node.
>@@ -196,9 +196,14 @@ std::unique_ptr<ScrollingStateTree> ScrollingStateTree::commit(LayerRepresentati
>     return treeStateClone;
> }
> 
>-void ScrollingStateTree::addNode(ScrollingStateNode* node)
>+void ScrollingStateTree::setRootStateNode(Ref<ScrollingStateFrameScrollingNode>&& rootStateNode)
> {
>-    m_stateNodeMap.add(node->scrollingNodeID(), node);
>+    m_rootStateNode = WTFMove(rootStateNode);
>+}
>+
>+void ScrollingStateTree::addNode(ScrollingStateNode& node)
>+{
>+    m_stateNodeMap.add(node.scrollingNodeID(), &node);
> }
> 
> void ScrollingStateTree::removeNodeAndAllDescendants(ScrollingStateNode* node, SubframeNodeRemoval subframeNodeRemoval)
>diff --git a/Source/WebCore/page/scrolling/ScrollingStateTree.h b/Source/WebCore/page/scrolling/ScrollingStateTree.h
>index 3b256151ad744bcf0234d72d4fd3a0c75d436911..cc9e571abd83b9b07f86703e0a5bb48d689a2235 100644
>--- a/Source/WebCore/page/scrolling/ScrollingStateTree.h
>+++ b/Source/WebCore/page/scrolling/ScrollingStateTree.h
>@@ -27,12 +27,14 @@
> 
> #if ENABLE(ASYNC_SCROLLING) || USE(COORDINATED_GRAPHICS)
> 
>-#include "ScrollingStateFrameScrollingNode.h"
>+#include "ScrollingCoordinator.h"
>+#include "ScrollingStateNode.h"
> #include <wtf/RefPtr.h>
>  
> namespace WebCore {
> 
> class AsyncScrollingCoordinator;
>+class ScrollingStateFrameScrollingNode;
> 
> // The ScrollingStateTree is a tree that managed ScrollingStateNodes. The nodes keep track of the current
> // state of scrolling related properties. Whenever any properties change, the scrolling coordinator
>@@ -74,8 +76,8 @@ public:
>     void setPreferredLayerRepresentation(LayerRepresentation::Type representation) { m_preferredLayerRepresentation = representation; }
> 
> 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);
> 
>diff --git a/Source/WebCore/page/scrolling/ScrollingTree.cpp b/Source/WebCore/page/scrolling/ScrollingTree.cpp
>index ef4d5c44621a9c651fe44a6c64054080894f61e5..03dfbe49fc98be8703a046c7a816be600e3c993d 100644
>--- a/Source/WebCore/page/scrolling/ScrollingTree.cpp
>+++ b/Source/WebCore/page/scrolling/ScrollingTree.cpp
>@@ -31,6 +31,7 @@
> #include "EventNames.h"
> #include "Logging.h"
> #include "PlatformWheelEvent.h"
>+#include "ScrollingStateFrameScrollingNode.h"
> #include "ScrollingStateTree.h"
> #include "ScrollingTreeFrameScrollingNode.h"
> #include "ScrollingTreeNode.h"
>diff --git a/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h b/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h
>index a894233e1a05ad781c74fd2f71963ac78769d6e1..2c8190dfaf83384ed0ad139a5e7810b4f601602e 100644
>--- a/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h
>+++ b/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h
>@@ -29,6 +29,7 @@
> 
> #include "ScrollController.h"
> #include "ScrollbarThemeMac.h"
>+#include "ScrollingStateFrameScrollingNode.h"
> #include "ScrollingTreeFrameScrollingNode.h"
> #include <wtf/RetainPtr.h>
> 
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>index ec2f5a0539d077a1c63f49c94b163174635984db..77b0120fbaed32236d03f7e858dcd7819fda505e 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>@@ -3835,7 +3835,7 @@ ScrollingNodeID RenderLayerCompositor::attachScrollingNode(RenderLayer& layer, S
>     LayerScrollCoordinationRole role = scrollCoordinationRoleForNodeType(nodeType);
>     ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(role);
>     if (!nodeID)
>-        nodeID = scrollingCoordinator->uniqueScrollLayerID();
>+        nodeID = scrollingCoordinator->uniqueScrollingNodeID();
> 
>     nodeID = scrollingCoordinator->attachToStateTree(nodeType, nodeID, parentNodeID);
>     if (!nodeID)

Actions: View | Formatted Diff | Diff

Attachments on bug 192398: 356598