scrollingTreeNodeDidScroll() should just trigger a rendering udpate

https://bugs.webkit.org/show_bug.cgi?id=224394 Reviewed by Tim Horton. After handling wheel events on the scrolling thread, ThreadedScrollingTree::scrollingTreeNodeDidScroll() appends to a queue of pending updates and then triggers applyPendingScrollUpdates() on the main thread to process those updates. However, every rendering update also processes pending scroll updates via synchronizeStateFromScrollingTree(), so it's simpler if we just trigger a rendering update. * page/scrolling/AsyncScrollingCoordinator.cpp: (WebCore::AsyncScrollingCoordinator::scrollingThreadAddedPendingUpdate): (WebCore::AsyncScrollingCoordinator::scheduleRenderingUpdate): * page/scrolling/AsyncScrollingCoordinator.h: * page/scrolling/ThreadedScrollingTree.cpp: (WebCore::ThreadedScrollingTree::scrollingTreeNodeDidScroll): * page/scrolling/mac/ScrollingCoordinatorMac.mm: (WebCore::ScrollingCoordinatorMac::scheduleTreeStateCommit): * page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp: (WebCore::ScrollingCoordinatorNicosia::scheduleTreeStateCommit): Canonical link: https://commits.webkit.org/236361@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275790 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Apr 10, 2021 · c4fe229 · c4fe229
1 parent 78249d4
commit c4fe229
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 4 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2021-04-09  Simon Fraser  <simon.fraser@apple.com>
+
+        scrollingTreeNodeDidScroll() should just trigger a rendering udpate
+        https://bugs.webkit.org/show_bug.cgi?id=224394
+
+        Reviewed by Tim Horton.
+
+        After handling wheel events on the scrolling thread,
+        ThreadedScrollingTree::scrollingTreeNodeDidScroll() appends to a queue of pending updates
+        and then triggers applyPendingScrollUpdates() on the main thread to process those updates.
+        However, every rendering update also processes pending scroll updates via
+        synchronizeStateFromScrollingTree(), so it's simpler if we just trigger a rendering update.
+
+        * page/scrolling/AsyncScrollingCoordinator.cpp:
+        (WebCore::AsyncScrollingCoordinator::scrollingThreadAddedPendingUpdate):
+        (WebCore::AsyncScrollingCoordinator::scheduleRenderingUpdate):
+        * page/scrolling/AsyncScrollingCoordinator.h:
+        * page/scrolling/ThreadedScrollingTree.cpp:
+        (WebCore::ThreadedScrollingTree::scrollingTreeNodeDidScroll):
+        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+        (WebCore::ScrollingCoordinatorMac::scheduleTreeStateCommit):
+        * page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp:
+        (WebCore::ScrollingCoordinatorNicosia::scheduleTreeStateCommit):
+
 2021-04-09  Jean-Yves Avenard  <jya@apple.com>
 
         Media Session action should default to the MediaElement's default when no MediaSession handler are set

diff --git a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
@@ -69,6 +69,11 @@ void AsyncScrollingCoordinator::scrollingStateTreePropertiesChanged()
     scheduleTreeStateCommit();
 }
 
+void AsyncScrollingCoordinator::scrollingThreadAddedPendingUpdate()
+{
+    scheduleRenderingUpdate();
+}
+
 #if PLATFORM(COCOA)
 void AsyncScrollingCoordinator::handleWheelEventPhase(ScrollingNodeID nodeID, PlatformWheelEventPhase phase)
 {
@@ -305,6 +310,11 @@ void AsyncScrollingCoordinator::applyPendingScrollUpdates()
     }
 }
 
+void AsyncScrollingCoordinator::scheduleRenderingUpdate()
+{
+    m_page->scheduleRenderingUpdate(RenderingUpdateStep::ScrollingTreeUpdate);
+}
+
 FrameView* AsyncScrollingCoordinator::frameViewForScrollingNode(ScrollingNodeID scrollingNodeID) const
 {
     if (!m_scrollingStateTree->rootStateNode())

diff --git a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h
@@ -50,6 +50,7 @@ class AsyncScrollingCoordinator : public ScrollingCoordinator {
     ScrollingTree* scrollingTree() const { return m_scrollingTree.get(); }
 
     void scrollingStateTreePropertiesChanged();
+    void scrollingThreadAddedPendingUpdate();
 
     void applyPendingScrollUpdates();
 
@@ -87,6 +88,7 @@ class AsyncScrollingCoordinator : public ScrollingCoordinator {
     WEBCORE_EXPORT String scrollingTreeAsText(ScrollingStateTreeAsTextBehavior = ScrollingStateTreeAsTextBehaviorNormal) const override;
     WEBCORE_EXPORT void willCommitTree() override;
     void synchronizeStateFromScrollingTree();
+    void scheduleRenderingUpdate();
 
     bool eventTrackingRegionsDirty() const { return m_eventTrackingRegionsDirty; }
 

diff --git a/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp b/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp
@@ -210,15 +210,15 @@ void ThreadedScrollingTree::scrollingTreeNodeDidScroll(ScrollingTreeScrollingNod
         return;
     }
 
-    LOG_WITH_STREAM(Scrolling, stream << "ThreadedScrollingTree::scrollingTreeNodeDidScroll " << node.scrollingNodeID() << " to " << scrollPosition << " bouncing to main thread");
+    LOG_WITH_STREAM(Scrolling, stream << "ThreadedScrollingTree::scrollingTreeNodeDidScroll " << node.scrollingNodeID() << " to " << scrollPosition << " triggering main thread rendering update");
 
     auto scrollUpdate = ScrollUpdate { node.scrollingNodeID(), scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction };
     addPendingScrollUpdate(WTFMove(scrollUpdate));
 
     auto deferrer = WheelEventTestMonitorCompletionDeferrer { wheelEventTestMonitor(), reinterpret_cast<WheelEventTestMonitor::ScrollableAreaIdentifier>(node.scrollingNodeID()), WheelEventTestMonitor::ScrollingThreadSyncNeeded };
     RunLoop::main().dispatch([strongThis = makeRef(*this), deferrer = WTFMove(deferrer)] {
         if (auto* scrollingCoordinator = strongThis->m_scrollingCoordinator.get())
-            scrollingCoordinator->applyPendingScrollUpdates();
+            scrollingCoordinator->scrollingThreadAddedPendingUpdate();
     });
 }
 

diff --git a/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm b/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm
@@ -102,8 +102,7 @@
 
 void ScrollingCoordinatorMac::scheduleTreeStateCommit()
 {
-    // FIXME: This one needs work.
-    m_page->scheduleRenderingUpdate(RenderingUpdateStep::ScrollingTreeUpdate);
+    scheduleRenderingUpdate();
 }
 
 void ScrollingCoordinatorMac::commitTreeStateIfNeeded()

diff --git a/Source/WebCore/page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp b/Source/WebCore/page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp
@@ -89,6 +89,7 @@ void ScrollingCoordinatorNicosia::wheelEventWasProcessedByMainThread(const Platf
 
 void ScrollingCoordinatorNicosia::scheduleTreeStateCommit()
 {
+    // FIXME: This should probably just call scheduleRenderingUpdate().
     if (!m_scrollingStateTreeCommitterTimer.isActive())
         m_scrollingStateTreeCommitterTimer.startOneShot(0_s);
 }