WebKit Bugzilla
Attachment 356598 Details for
Bug 192398
: Minor refactoring of the scrolling code
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[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:
Patch
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)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 192398
: 356598