Always pass scrollingGeometry to update*ScrollingNode functions
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Dec 2018 03:58:01 +0000 (03:58 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Dec 2018 03:58:01 +0000 (03:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192358

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

Currently, the scrollingGeometry parameter of updateOverflowScrollingNode is always used
while the one of updateFrameScrollingNode is never used. Both of them are passed as possibly
null pointers. This commit makes things more consistent by making the parameter a reference
and explicitly setting the scrollingGeometry of updateFrameScrollingNode. This will help
other efforts (such as support for macOS/iOS asynchronous scrolling of overflow nodes /
subframes or for CSS overscroll-behavior) for which new data members have to be passed to the
scrolling nodes.

No new tests, no behavior changes.

* page/scrolling/AsyncScrollingCoordinator.cpp:
(WebCore::AsyncScrollingCoordinator::updateFrameScrollingNode):
(WebCore::AsyncScrollingCoordinator::updateOverflowScrollingNode):
* page/scrolling/AsyncScrollingCoordinator.h:
* page/scrolling/ScrollingCoordinator.h:
(WebCore::ScrollingCoordinator::updateFrameScrollingNode):
(WebCore::ScrollingCoordinator::updateOverflowScrollingNode):
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::updateScrollCoordinationForThisFrame):
(WebCore::RenderLayerCompositor::updateScrollCoordinatedLayer):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238885 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h
Source/WebCore/page/scrolling/ScrollingCoordinator.h
Source/WebCore/rendering/RenderLayerCompositor.cpp

index f74f8ed..438c7e7 100644 (file)
@@ -1,3 +1,31 @@
+2018-12-04  Frederic Wang  <fwang@igalia.com>
+
+        Always pass scrollingGeometry to update*ScrollingNode functions
+        https://bugs.webkit.org/show_bug.cgi?id=192358
+
+        Reviewed by Simon Fraser.
+
+        Currently, the scrollingGeometry parameter of updateOverflowScrollingNode is always used
+        while the one of updateFrameScrollingNode is never used. Both of them are passed as possibly
+        null pointers. This commit makes things more consistent by making the parameter a reference
+        and explicitly setting the scrollingGeometry of updateFrameScrollingNode. This will help
+        other efforts (such as support for macOS/iOS asynchronous scrolling of overflow nodes /
+        subframes or for CSS overscroll-behavior) for which new data members have to be passed to the
+        scrolling nodes.
+
+        No new tests, no behavior changes.
+
+        * page/scrolling/AsyncScrollingCoordinator.cpp:
+        (WebCore::AsyncScrollingCoordinator::updateFrameScrollingNode):
+        (WebCore::AsyncScrollingCoordinator::updateOverflowScrollingNode):
+        * page/scrolling/AsyncScrollingCoordinator.h:
+        * page/scrolling/ScrollingCoordinator.h:
+        (WebCore::ScrollingCoordinator::updateFrameScrollingNode):
+        (WebCore::ScrollingCoordinator::updateOverflowScrollingNode):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::updateScrollCoordinationForThisFrame):
+        (WebCore::RenderLayerCompositor::updateScrollCoordinatedLayer):
+
 2018-12-04  Ryosuke Niwa  <rniwa@webkit.org>
 
         Crash in HTMLCollection::updateNamedElementCache
index dc30a7e..2638826 100644 (file)
@@ -512,7 +512,7 @@ void AsyncScrollingCoordinator::ensureRootStateNodeForFrameView(FrameView& frame
     attachToStateTree(MainFrameScrollingNode, frameView.scrollLayerID(), 0);
 }
 
-void AsyncScrollingCoordinator::updateFrameScrollingNode(ScrollingNodeID nodeID, GraphicsLayer* layer, GraphicsLayer* scrolledContentsLayer, GraphicsLayer* counterScrollingLayer, GraphicsLayer* insetClipLayer, const ScrollingGeometry* scrollingGeometry)
+void AsyncScrollingCoordinator::updateFrameScrollingNode(ScrollingNodeID nodeID, GraphicsLayer* layer, GraphicsLayer* scrolledContentsLayer, GraphicsLayer* counterScrollingLayer, GraphicsLayer* insetClipLayer, const ScrollingGeometry& scrollingGeometry)
 {
     auto* node = downcast<ScrollingStateFrameScrollingNode>(m_scrollingStateTree->stateNodeForID(nodeID));
     ASSERT(node);
@@ -523,18 +523,15 @@ void AsyncScrollingCoordinator::updateFrameScrollingNode(ScrollingNodeID nodeID,
     node->setInsetClipLayer(insetClipLayer);
     node->setScrolledContentsLayer(scrolledContentsLayer);
     node->setCounterScrollingLayer(counterScrollingLayer);
-
-    if (scrollingGeometry) {
-        node->setParentRelativeScrollableRect(scrollingGeometry->parentRelativeScrollableRect);
-        node->setScrollOrigin(scrollingGeometry->scrollOrigin);
-        node->setScrollPosition(scrollingGeometry->scrollPosition);
-        node->setTotalContentsSize(scrollingGeometry->contentSize);
-        node->setReachableContentsSize(scrollingGeometry->reachableContentSize);
-        node->setScrollableAreaSize(scrollingGeometry->scrollableAreaSize);
-    }
+    node->setParentRelativeScrollableRect(scrollingGeometry.parentRelativeScrollableRect);
+    node->setScrollOrigin(scrollingGeometry.scrollOrigin);
+    node->setScrollPosition(scrollingGeometry.scrollPosition);
+    node->setTotalContentsSize(scrollingGeometry.contentSize);
+    node->setReachableContentsSize(scrollingGeometry.reachableContentSize);
+    node->setScrollableAreaSize(scrollingGeometry.scrollableAreaSize);
 }
     
-void AsyncScrollingCoordinator::updateOverflowScrollingNode(ScrollingNodeID nodeID, GraphicsLayer* layer, GraphicsLayer* scrolledContentsLayer, const ScrollingGeometry* scrollingGeometry)
+void AsyncScrollingCoordinator::updateOverflowScrollingNode(ScrollingNodeID nodeID, GraphicsLayer* layer, GraphicsLayer* scrolledContentsLayer, const ScrollingGeometry& scrollingGeometry)
 {
     auto* node = downcast<ScrollingStateOverflowScrollingNode>(m_scrollingStateTree->stateNodeForID(nodeID));
     ASSERT(node);
@@ -543,21 +540,18 @@ void AsyncScrollingCoordinator::updateOverflowScrollingNode(ScrollingNodeID node
 
     node->setLayer(layer);
     node->setScrolledContentsLayer(scrolledContentsLayer);
-    
-    if (scrollingGeometry) {
-        node->setParentRelativeScrollableRect(scrollingGeometry->parentRelativeScrollableRect);
-        node->setScrollOrigin(scrollingGeometry->scrollOrigin);
-        node->setScrollPosition(scrollingGeometry->scrollPosition);
-        node->setTotalContentsSize(scrollingGeometry->contentSize);
-        node->setReachableContentsSize(scrollingGeometry->reachableContentSize);
-        node->setScrollableAreaSize(scrollingGeometry->scrollableAreaSize);
+    node->setParentRelativeScrollableRect(scrollingGeometry.parentRelativeScrollableRect);
+    node->setScrollOrigin(scrollingGeometry.scrollOrigin);
+    node->setScrollPosition(scrollingGeometry.scrollPosition);
+    node->setTotalContentsSize(scrollingGeometry.contentSize);
+    node->setReachableContentsSize(scrollingGeometry.reachableContentSize);
+    node->setScrollableAreaSize(scrollingGeometry.scrollableAreaSize);
 #if ENABLE(CSS_SCROLL_SNAP)
-        setStateScrollingNodeSnapOffsetsAsFloat(*node, ScrollEventAxis::Horizontal, &scrollingGeometry->horizontalSnapOffsets, &scrollingGeometry->horizontalSnapOffsetRanges, m_page->deviceScaleFactor());
-        setStateScrollingNodeSnapOffsetsAsFloat(*node, ScrollEventAxis::Vertical, &scrollingGeometry->verticalSnapOffsets, &scrollingGeometry->verticalSnapOffsetRanges, m_page->deviceScaleFactor());
-        node->setCurrentHorizontalSnapPointIndex(scrollingGeometry->currentHorizontalSnapPointIndex);
-        node->setCurrentVerticalSnapPointIndex(scrollingGeometry->currentVerticalSnapPointIndex);
+    setStateScrollingNodeSnapOffsetsAsFloat(*node, ScrollEventAxis::Horizontal, &scrollingGeometry.horizontalSnapOffsets, &scrollingGeometry.horizontalSnapOffsetRanges, m_page->deviceScaleFactor());
+    setStateScrollingNodeSnapOffsetsAsFloat(*node, ScrollEventAxis::Vertical, &scrollingGeometry.verticalSnapOffsets, &scrollingGeometry.verticalSnapOffsetRanges, m_page->deviceScaleFactor());
+    node->setCurrentHorizontalSnapPointIndex(scrollingGeometry.currentHorizontalSnapPointIndex);
+    node->setCurrentVerticalSnapPointIndex(scrollingGeometry.currentVerticalSnapPointIndex);
 #endif
-    }
 }
 
 void AsyncScrollingCoordinator::updateNodeLayer(ScrollingNodeID nodeID, GraphicsLayer* graphicsLayer)
index 3d64327..5d678e4 100644 (file)
@@ -104,8 +104,8 @@ private:
     WEBCORE_EXPORT void updateNodeLayer(ScrollingNodeID, GraphicsLayer*) override;
     WEBCORE_EXPORT void updateNodeViewportConstraints(ScrollingNodeID, const ViewportConstraints&) override;
     
-    WEBCORE_EXPORT void updateFrameScrollingNode(ScrollingNodeID, GraphicsLayer* scrollLayer, GraphicsLayer* scrolledContentsLayer, GraphicsLayer* counterScrollingLayer, GraphicsLayer* insetClipLayer, const ScrollingGeometry* = nullptr) override;
-    WEBCORE_EXPORT void updateOverflowScrollingNode(ScrollingNodeID, GraphicsLayer* scrollLayer, GraphicsLayer* scrolledContentsLayer, const ScrollingGeometry* = nullptr) override;
+    WEBCORE_EXPORT void updateFrameScrollingNode(ScrollingNodeID, GraphicsLayer* scrollLayer, GraphicsLayer* scrolledContentsLayer, GraphicsLayer* counterScrollingLayer, GraphicsLayer* insetClipLayer, const ScrollingGeometry&) override;
+    WEBCORE_EXPORT void updateOverflowScrollingNode(ScrollingNodeID, GraphicsLayer* scrollLayer, GraphicsLayer* scrolledContentsLayer, const ScrollingGeometry&) override;
     
     WEBCORE_EXPORT void reconcileScrollingState(FrameView&, const FloatPoint&, const LayoutViewportOriginOrOverrideRect&, bool programmaticScroll, ViewportRectStability, ScrollingLayerPositionAction) override;
 
index a5033b7..f917089 100644 (file)
@@ -189,8 +189,8 @@ public:
 #endif
     };
 
-    virtual void updateFrameScrollingNode(ScrollingNodeID, GraphicsLayer* /*scrollLayer*/, GraphicsLayer* /*scrolledContentsLayer*/, GraphicsLayer* /*counterScrollingLayer*/, GraphicsLayer* /*insetClipLayer*/, const ScrollingGeometry* = nullptr) { }
-    virtual void updateOverflowScrollingNode(ScrollingNodeID, GraphicsLayer* /*scrollLayer*/, GraphicsLayer* /*scrolledContentsLayer*/, const ScrollingGeometry* = nullptr) { }
+    virtual void updateFrameScrollingNode(ScrollingNodeID, GraphicsLayer* /*scrollLayer*/, GraphicsLayer* /*scrolledContentsLayer*/, GraphicsLayer* /*counterScrollingLayer*/, GraphicsLayer* /*insetClipLayer*/, const ScrollingGeometry&) { }
+    virtual void updateOverflowScrollingNode(ScrollingNodeID, GraphicsLayer* /*scrollLayer*/, GraphicsLayer* /*scrolledContentsLayer*/, const ScrollingGeometry&) { }
     virtual void reconcileViewportConstrainedLayerPositions(ScrollingNodeID, const LayoutRect&, ScrollingLayerPositionAction) { }
     virtual String scrollingStateTreeAsText(ScrollingStateTreeAsTextBehavior = ScrollingStateTreeAsTextBehaviorNormal) const;
     virtual bool isRubberBandInProgress() const { return false; }
index 208c656..ec2f5a0 100644 (file)
@@ -3869,10 +3869,21 @@ void RenderLayerCompositor::detachScrollCoordinatedLayer(RenderLayer& layer, Opt
 void RenderLayerCompositor::updateScrollCoordinationForThisFrame(ScrollingNodeID parentNodeID)
 {
     auto* scrollingCoordinator = this->scrollingCoordinator();
-    ASSERT(scrollingCoordinator->coordinatesScrollingForFrameView(m_renderView.frameView()));
+    FrameView& frameView = m_renderView.frameView();
+    ASSERT(scrollingCoordinator->coordinatesScrollingForFrameView(frameView));
 
     ScrollingNodeID nodeID = attachScrollingNode(*m_renderView.layer(), m_renderView.frame().isMainFrame() ? MainFrameScrollingNode : SubframeScrollingNode, parentNodeID);
-    scrollingCoordinator->updateFrameScrollingNode(nodeID, m_scrollLayer.get(), m_rootContentLayer.get(), fixedRootBackgroundLayer(), clipLayer());
+    ScrollingCoordinator::ScrollingGeometry scrollingGeometry;
+    // FIXME(https://webkit.org/b/172917): Pass parentRelativeScrollableRect?
+    scrollingGeometry.scrollOrigin = frameView.scrollOrigin();
+    scrollingGeometry.scrollableAreaSize = frameView.visibleContentRect().size();
+    scrollingGeometry.contentSize = frameView.totalContentsSize();
+    scrollingGeometry.reachableContentSize = frameView.totalContentsSize();
+#if ENABLE(CSS_SCROLL_SNAP)
+    frameView.updateSnapOffsets();
+    scrollingCoordinator->updateScrollSnapPropertiesWithFrameView(frameView);
+#endif
+    scrollingCoordinator->updateFrameScrollingNode(nodeID, m_scrollLayer.get(), m_rootContentLayer.get(), fixedRootBackgroundLayer(), clipLayer(), scrollingGeometry);
 }
 
 void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, OptionSet<LayerScrollCoordinationRole> reasons, OptionSet<ScrollingNodeChangeFlags> changes)
@@ -3951,6 +3962,7 @@ void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, Opt
                 return;
 
             ScrollingCoordinator::ScrollingGeometry scrollingGeometry;
+            // FIXME(https://webkit.org/b/172917): Pass parentRelativeScrollableRect?
             scrollingGeometry.scrollOrigin = layer.scrollOrigin();
             scrollingGeometry.scrollPosition = layer.scrollPosition();
             scrollingGeometry.scrollableAreaSize = layer.visibleSize();
@@ -3971,7 +3983,7 @@ void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, Opt
 
             LOG(Compositing, "Registering Scrolling scrolling node %" PRIu64 " (layer %" PRIu64 ") as child of %" PRIu64, nodeID, backing->graphicsLayer()->primaryLayerID(), parentNodeID);
 
-            scrollingCoordinator->updateOverflowScrollingNode(nodeID, backing->scrollingLayer(), backing->scrollingContentsLayer(), &scrollingGeometry);
+            scrollingCoordinator->updateOverflowScrollingNode(nodeID, backing->scrollingLayer(), backing->scrollingContentsLayer(), scrollingGeometry);
         }
     } else
         detachScrollCoordinatedLayer(layer, Scrolling);