Title: [238885] trunk/Source/WebCore
Revision
238885
Author
commit-qu...@webkit.org
Date
2018-12-04 19:58:01 -0800 (Tue, 04 Dec 2018)

Log Message

Always pass scrollingGeometry to update*ScrollingNode functions
https://bugs.webkit.org/show_bug.cgi?id=192358

Patch by Frederic Wang <fw...@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):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (238884 => 238885)


--- trunk/Source/WebCore/ChangeLog	2018-12-05 03:23:54 UTC (rev 238884)
+++ trunk/Source/WebCore/ChangeLog	2018-12-05 03:58:01 UTC (rev 238885)
@@ -1,3 +1,31 @@
+2018-12-04  Frederic Wang  <fw...@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  <rn...@webkit.org>
 
         Crash in HTMLCollection::updateNamedElementCache

Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (238884 => 238885)


--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp	2018-12-05 03:23:54 UTC (rev 238884)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp	2018-12-05 03:58:01 UTC (rev 238885)
@@ -512,7 +512,7 @@
     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 @@
     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 @@
 
     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)

Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h (238884 => 238885)


--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h	2018-12-05 03:23:54 UTC (rev 238884)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h	2018-12-05 03:58:01 UTC (rev 238885)
@@ -104,8 +104,8 @@
     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;
 

Modified: trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h (238884 => 238885)


--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h	2018-12-05 03:23:54 UTC (rev 238884)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h	2018-12-05 03:58:01 UTC (rev 238885)
@@ -189,8 +189,8 @@
 #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; }

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (238884 => 238885)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2018-12-05 03:23:54 UTC (rev 238884)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2018-12-05 03:58:01 UTC (rev 238885)
@@ -3869,10 +3869,21 @@
 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 @@
                 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 @@
 
             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);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to