Attachment 360806 Details for Bug 194126 – Patch

[patch] Patch

bug-194126-20190131165434.patch (text/plain), 15.16 KB, created by Simon Fraser (smfr) on 2019-01-31 16:54:35 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-01-31 16:54:35 PST

Size: 15.16 KB

patch

obsolete

>Subversion Revision: 240826
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 33881fd5291e617f7d197bd277bdb9d33fe31369..a6998390a61df2766ecb6acef482634f405e3608 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,39 @@
>+2019-01-31  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Use ScrollingNodeID in more places, and improve the name of a ScrollableArea function that returns a ScrollingNodeID
>+        https://bugs.webkit.org/show_bug.cgi?id=194126
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Change uint64_t ScrollableArea::scrollLayerID() to ScrollingNodeID ScrollableArea::scrollingNodeID()
>+        and fix callers.
>+
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::scrollingNodeID const):
>+        (WebCore::FrameView::scrollLayerID const): Deleted.
>+        * page/FrameView.h:
>+        * page/scrolling/AsyncScrollingCoordinator.cpp:
>+        (WebCore::AsyncScrollingCoordinator::frameViewLayoutUpdated):
>+        (WebCore::AsyncScrollingCoordinator::updateExpectsWheelEventTestTriggerWithFrameView):
>+        (WebCore::AsyncScrollingCoordinator::frameViewRootLayerDidChange):
>+        (WebCore::AsyncScrollingCoordinator::requestScrollPositionUpdate):
>+        (WebCore::AsyncScrollingCoordinator::frameViewForScrollingNode const):
>+        (WebCore::AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll):
>+        (WebCore::AsyncScrollingCoordinator::reconcileScrollingState):
>+        (WebCore::AsyncScrollingCoordinator::scrollableAreaScrollbarLayerDidChange):
>+        (WebCore::AsyncScrollingCoordinator::ensureRootStateNodeForFrameView):
>+        (WebCore::AsyncScrollingCoordinator::setSynchronousScrollingReasons):
>+        (WebCore::AsyncScrollingCoordinator::setActiveScrollSnapIndices):
>+        (WebCore::AsyncScrollingCoordinator::updateScrollSnapPropertiesWithFrameView):
>+        * page/scrolling/ScrollingCoordinatorTypes.h:
>+        * platform/ScrollTypes.h:
>+        * platform/ScrollableArea.h:
>+        (WebCore::ScrollableArea::scrollingNodeID const):
>+        (WebCore::ScrollableArea::scrollLayerID const): Deleted.
>+        * rendering/RenderLayerCompositor.cpp:
>+        (WebCore::RenderLayerCompositor::parentFrameContentLayers):
>+        (WebCore::RenderLayerCompositor::detachRootLayer):
>+
> 2019-01-31  Simon Fraser  <simon.fraser@apple.com>
> 
>         Basic scrollability for async overflow scrolling on macOS
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index d5c8897fe12ce6a4c81c75944870548fac1e9e83..b56b4eb96f8ec7ca12f35049382dcfbaced2699f 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -893,7 +893,7 @@ TiledBacking* FrameView::tiledBacking() const
>     return backing->tiledBacking();
> }
> 
>-uint64_t FrameView::scrollLayerID() const
>+ScrollingNodeID FrameView::scrollingNodeID() const
> {
>     RenderView* renderView = this->renderView();
>     if (!renderView)
>diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
>index 6a97102976af4d6693ad100cf51ba1f28fd82bdf..349346ca521f1b27cc73d1771c915b76641a621c 100644
>--- a/Source/WebCore/page/FrameView.h
>+++ b/Source/WebCore/page/FrameView.h
>@@ -152,7 +152,7 @@ public:
> 
>     WEBCORE_EXPORT TiledBacking* tiledBacking() const final;
> 
>-    uint64_t scrollLayerID() const override;
>+    ScrollingNodeID scrollingNodeID() const override;
>     ScrollableArea* scrollableAreaForScrollLayerID(uint64_t) const;
>     bool usesAsyncScrolling() const final;
> 
>diff --git a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
>index a43d5a0e817fa4fbe968875703db29fb4d3f0bdf..f3f13a18fbd8d017b56cd84612d1449243ad779e 100644
>--- a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
>+++ b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
>@@ -130,7 +130,7 @@ void AsyncScrollingCoordinator::frameViewLayoutUpdated(FrameView& frameView)
>     if (!coordinatesScrollingForFrameView(frameView))
>         return;
> 
>-    auto* node = m_scrollingStateTree->stateNodeForID(frameView.scrollLayerID());
>+    auto* node = m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID());
>     if (!node || !is<ScrollingStateFrameScrollingNode>(*node))
>         return;
> 
>@@ -190,7 +190,7 @@ void AsyncScrollingCoordinator::updateExpectsWheelEventTestTriggerWithFrameView(
>     if (!page)
>         return;
> 
>-    auto* node = downcast<ScrollingStateFrameScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollLayerID()));
>+    auto* node = downcast<ScrollingStateFrameScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID()));
>     if (!node)
>         return;
> 
>@@ -216,16 +216,16 @@ void AsyncScrollingCoordinator::frameViewRootLayerDidChange(FrameView& frameView
>     
>     // FIXME: In some navigation scenarios, the FrameView has no RenderView or that RenderView has not been composited.
>     // This needs cleaning up: https://bugs.webkit.org/show_bug.cgi?id=132724
>-    if (!frameView.scrollLayerID())
>+    if (!frameView.scrollingNodeID())
>         return;
>     
>     // If the root layer does not have a ScrollingStateNode, then we should create one.
>     ensureRootStateNodeForFrameView(frameView);
>-    ASSERT(m_scrollingStateTree->stateNodeForID(frameView.scrollLayerID()));
>+    ASSERT(m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID()));
> 
>     ScrollingCoordinator::frameViewRootLayerDidChange(frameView);
> 
>-    auto* node = downcast<ScrollingStateFrameScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollLayerID()));
>+    auto* node = downcast<ScrollingStateFrameScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID()));
>     node->setLayer(scrollLayerForFrameView(frameView));
>     node->setRootContentsLayer(rootContentsLayerForFrameView(frameView));
>     node->setCounterScrollingLayer(counterScrollingLayerForFrameView(frameView));
>@@ -248,14 +248,14 @@ bool AsyncScrollingCoordinator::requestScrollPositionUpdate(FrameView& frameView
> 
>     bool isProgrammaticScroll = frameView.inProgrammaticScroll();
>     if (isProgrammaticScroll || frameView.frame().document()->pageCacheState() != Document::NotInPageCache)
>-        updateScrollPositionAfterAsyncScroll(frameView.scrollLayerID(), scrollPosition, WTF::nullopt, isProgrammaticScroll, ScrollingLayerPositionAction::Set);
>+        updateScrollPositionAfterAsyncScroll(frameView.scrollingNodeID(), scrollPosition, WTF::nullopt, isProgrammaticScroll, ScrollingLayerPositionAction::Set);
> 
>     // If this frame view's document is being put into the page cache, we don't want to update our
>     // main frame scroll position. Just let the FrameView think that we did.
>     if (frameView.frame().document()->pageCacheState() != Document::NotInPageCache)
>         return true;
> 
>-    auto* stateNode = downcast<ScrollingStateScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollLayerID()));
>+    auto* stateNode = downcast<ScrollingStateScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID()));
>     if (!stateNode)
>         return false;
> 
>@@ -318,7 +318,7 @@ FrameView* AsyncScrollingCoordinator::frameViewForScrollingNode(ScrollingNodeID
>     // from ScrollingTreeStateNodes.
>     for (Frame* frame = &m_page->mainFrame(); frame; frame = frame->tree().traverseNext()) {
>         if (auto* view = frame->view()) {
>-            if (view->scrollLayerID() == parentNode->scrollingNodeID())
>+            if (view->scrollingNodeID() == parentNode->scrollingNodeID())
>                 return view;
>         }
>     }
>@@ -341,7 +341,7 @@ void AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll(ScrollingNo
> 
>     auto& frameView = *frameViewPtr;
> 
>-    if (scrollingNodeID == frameView.scrollLayerID()) {
>+    if (scrollingNodeID == frameView.scrollingNodeID()) {
>         reconcileScrollingState(frameView, scrollPosition, layoutViewportOrigin, programmaticScroll, ViewportRectStability::Stable, scrollingLayerPositionAction);
> 
> #if PLATFORM(COCOA)
>@@ -406,7 +406,7 @@ void AsyncScrollingCoordinator::reconcileScrollingState(FrameView& frameView, co
>     frameView.setInProgrammaticScroll(oldProgrammaticScroll);
> 
>     if (!programmaticScroll && scrollingLayerPositionAction != ScrollingLayerPositionAction::Set) {
>-        auto scrollingNodeID = frameView.scrollLayerID();
>+        auto scrollingNodeID = frameView.scrollingNodeID();
>         if (viewportRectStability == ViewportRectStability::Stable)
>             reconcileViewportConstrainedLayerPositions(scrollingNodeID, frameView.rectForFixedPositionLayout(), scrollingLayerPositionAction);
>         else if (layoutViewportRect)
>@@ -473,7 +473,7 @@ void AsyncScrollingCoordinator::scrollableAreaScrollbarLayerDidChange(Scrollable
>     ASSERT(isMainThread());
>     ASSERT(m_page);
> 
>-    auto* node = m_scrollingStateTree->stateNodeForID(scrollableArea.scrollLayerID());
>+    auto* node = m_scrollingStateTree->stateNodeForID(scrollableArea.scrollingNodeID());
>     if (is<ScrollingStateFrameScrollingNode>(node)) {
>         auto& scrollingNode = downcast<ScrollingStateFrameScrollingNode>(*node);
>         if (orientation == VerticalScrollbar)
>@@ -563,14 +563,14 @@ void AsyncScrollingCoordinator::reconcileViewportConstrainedLayerPositions(Scrol
> 
> void AsyncScrollingCoordinator::ensureRootStateNodeForFrameView(FrameView& frameView)
> {
>-    ASSERT(frameView.scrollLayerID());
>-    if (m_scrollingStateTree->stateNodeForID(frameView.scrollLayerID()))
>+    ASSERT(frameView.scrollingNodeID());
>+    if (m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID()))
>         return;
> 
>     // For non-main frames, it is only possible to arrive in this function from
>     // RenderLayerCompositor::updateBacking where the node has already been created.
>     ASSERT(frameView.frame().isMainFrame());
>-    insertNode(ScrollingNodeType::MainFrame, frameView.scrollLayerID(), 0, 0);
>+    insertNode(ScrollingNodeType::MainFrame, frameView.scrollingNodeID(), 0, 0);
> }
> 
> void AsyncScrollingCoordinator::setNodeLayers(ScrollingNodeID nodeID, GraphicsLayer* layer, GraphicsLayer* scrolledContentsLayer, GraphicsLayer* counterScrollingLayer, GraphicsLayer* insetClipLayer, GraphicsLayer* rootContentsLayer)
>@@ -651,7 +651,7 @@ void AsyncScrollingCoordinator::setViewportConstraintedNodeGeometry(ScrollingNod
> 
> void AsyncScrollingCoordinator::setSynchronousScrollingReasons(FrameView& frameView, SynchronousScrollingReasons reasons)
> {
>-    auto* scrollingStateNode = static_cast<ScrollingStateFrameScrollingNode*>(m_scrollingStateTree->stateNodeForID(frameView.scrollLayerID()));
>+    auto* scrollingStateNode = static_cast<ScrollingStateFrameScrollingNode*>(m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID()));
>     if (!scrollingStateNode)
>         return;
> 
>@@ -714,7 +714,7 @@ void AsyncScrollingCoordinator::setActiveScrollSnapIndices(ScrollingNodeID scrol
>     if (!frameView)
>         return;
>     
>-    if (scrollingNodeID == frameView->scrollLayerID()) {
>+    if (scrollingNodeID == frameView->scrollingNodeID()) {
>         frameView->setCurrentHorizontalSnapPointIndex(horizontalIndex);
>         frameView->setCurrentVerticalSnapPointIndex(verticalIndex);
>         return;
>@@ -760,7 +760,7 @@ bool AsyncScrollingCoordinator::isScrollSnapInProgress() const
> 
> void AsyncScrollingCoordinator::updateScrollSnapPropertiesWithFrameView(const FrameView& frameView)
> {
>-    if (auto node = downcast<ScrollingStateFrameScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollLayerID()))) {
>+    if (auto node = downcast<ScrollingStateFrameScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID()))) {
>         setStateScrollingNodeSnapOffsetsAsFloat(*node, ScrollEventAxis::Horizontal, frameView.horizontalSnapOffsets(), frameView.horizontalSnapOffsetRanges(), m_page->deviceScaleFactor());
>         setStateScrollingNodeSnapOffsetsAsFloat(*node, ScrollEventAxis::Vertical, frameView.verticalSnapOffsets(), frameView.verticalSnapOffsetRanges(), m_page->deviceScaleFactor());
>         node->setCurrentHorizontalSnapPointIndex(frameView.currentHorizontalSnapPointIndex());
>diff --git a/Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h b/Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h
>index 5d6e2009b0aa95dfdf0366b9c1369ae0655f5b87..1486dee6f3daf5e841d14973ba65b9f50cbb3916 100644
>--- a/Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h
>+++ b/Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h
>@@ -30,7 +30,6 @@
> namespace WebCore {
> 
> typedef unsigned SynchronousScrollingReasons;
>-typedef uint64_t ScrollingNodeID;
> 
> enum class ScrollingNodeType : uint8_t {
>     MainFrame,
>diff --git a/Source/WebCore/platform/ScrollTypes.h b/Source/WebCore/platform/ScrollTypes.h
>index 33f38c31168b0bc3bab4c51b91459f130d9afe4c..b97917a22f36693ef246652e2a2890b738b5f6bb 100644
>--- a/Source/WebCore/platform/ScrollTypes.h
>+++ b/Source/WebCore/platform/ScrollTypes.h
>@@ -193,6 +193,8 @@ enum class ScrollbarButtonPressAction {
>     Scroll
> };
> 
>+using ScrollingNodeID = uint64_t;
>+
> }
> 
> #endif
>diff --git a/Source/WebCore/platform/ScrollableArea.h b/Source/WebCore/platform/ScrollableArea.h
>index f2c08f94ed55aefa27c0dae1c0a18d0415057b10..e311ae90c69eb2e703067ad1fcd2a07e04302612 100644
>--- a/Source/WebCore/platform/ScrollableArea.h
>+++ b/Source/WebCore/platform/ScrollableArea.h
>@@ -143,7 +143,7 @@ public:
>     ScrollbarOverlayStyle scrollbarOverlayStyle() const { return static_cast<ScrollbarOverlayStyle>(m_scrollbarOverlayStyle); }
>     bool useDarkAppearanceForScrollbars() const;
> 
>-    virtual uint64_t scrollLayerID() const { return 0; }
>+    virtual ScrollingNodeID scrollingNodeID() const { return 0; }
> 
>     // This getter will create a ScrollAnimator if it doesn't already exist.
>     WEBCORE_EXPORT ScrollAnimator& scrollAnimator() const;
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>index d902dec300811433396d7675a20675bd41549de3..190f2df98af9f13ad9012cb1343b44d43e2ce3d9 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>@@ -1966,7 +1966,7 @@ bool RenderLayerCompositor::parentFrameContentLayers(RenderWidget& renderer)
> 
>     if (auto frameHostingNodeID = backing->scrollingNodeIDForRole(ScrollCoordinationRole::FrameHosting)) {
>         auto* contentsRenderView = frameContentsRenderView(renderer);
>-        if (auto frameRootScrollingNodeID = contentsRenderView->frameView().scrollLayerID()) {
>+        if (auto frameRootScrollingNodeID = contentsRenderView->frameView().scrollingNodeID()) {
>             if (auto* scrollingCoordinator = this->scrollingCoordinator())
>                 scrollingCoordinator->insertNode(ScrollingNodeType::Subframe, frameRootScrollingNodeID, frameHostingNodeID, 0);
>         }
>@@ -3579,7 +3579,7 @@ void RenderLayerCompositor::detachRootLayer()
>         if (auto* ownerElement = m_renderView.document().ownerElement())
>             ownerElement->scheduleInvalidateStyleAndLayerComposition();
> 
>-        if (auto frameRootScrollingNodeID = m_renderView.frameView().scrollLayerID()) {
>+        if (auto frameRootScrollingNodeID = m_renderView.frameView().scrollingNodeID()) {
>             if (auto* scrollingCoordinator = this->scrollingCoordinator())
>                 scrollingCoordinator->unparentNode(frameRootScrollingNodeID);
>         }

Actions: View | Formatted Diff | Diff

Attachments on bug 194126: 360806