Skip to content

Commit c4fe229

Browse files
smfrsmfr
committed
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
1 parent 78249d4 commit c4fe229

6 files changed

Lines changed: 40 additions & 4 deletions

File tree

‎Source/WebCore/ChangeLog‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,27 @@
1+
2021-04-09 Simon Fraser <simon.fraser@apple.com>
2+
3+
scrollingTreeNodeDidScroll() should just trigger a rendering udpate
4+
https://bugs.webkit.org/show_bug.cgi?id=224394
5+
6+
Reviewed by Tim Horton.
7+
8+
After handling wheel events on the scrolling thread,
9+
ThreadedScrollingTree::scrollingTreeNodeDidScroll() appends to a queue of pending updates
10+
and then triggers applyPendingScrollUpdates() on the main thread to process those updates.
11+
However, every rendering update also processes pending scroll updates via
12+
synchronizeStateFromScrollingTree(), so it's simpler if we just trigger a rendering update.
13+
14+
* page/scrolling/AsyncScrollingCoordinator.cpp:
15+
(WebCore::AsyncScrollingCoordinator::scrollingThreadAddedPendingUpdate):
16+
(WebCore::AsyncScrollingCoordinator::scheduleRenderingUpdate):
17+
* page/scrolling/AsyncScrollingCoordinator.h:
18+
* page/scrolling/ThreadedScrollingTree.cpp:
19+
(WebCore::ThreadedScrollingTree::scrollingTreeNodeDidScroll):
20+
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
21+
(WebCore::ScrollingCoordinatorMac::scheduleTreeStateCommit):
22+
* page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp:
23+
(WebCore::ScrollingCoordinatorNicosia::scheduleTreeStateCommit):
24+
125
2021-04-09 Jean-Yves Avenard <jya@apple.com>
226

327
Media Session action should default to the MediaElement's default when no MediaSession handler are set

‎Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,11 @@ void AsyncScrollingCoordinator::scrollingStateTreePropertiesChanged()
6969
scheduleTreeStateCommit();
7070
}
7171

72+
void AsyncScrollingCoordinator::scrollingThreadAddedPendingUpdate()
73+
{
74+
scheduleRenderingUpdate();
75+
}
76+
7277
#if PLATFORM(COCOA)
7378
void AsyncScrollingCoordinator::handleWheelEventPhase(ScrollingNodeID nodeID, PlatformWheelEventPhase phase)
7479
{
@@ -305,6 +310,11 @@ void AsyncScrollingCoordinator::applyPendingScrollUpdates()
305310
}
306311
}
307312

313+
void AsyncScrollingCoordinator::scheduleRenderingUpdate()
314+
{
315+
m_page->scheduleRenderingUpdate(RenderingUpdateStep::ScrollingTreeUpdate);
316+
}
317+
308318
FrameView* AsyncScrollingCoordinator::frameViewForScrollingNode(ScrollingNodeID scrollingNodeID) const
309319
{
310320
if (!m_scrollingStateTree->rootStateNode())

‎Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class AsyncScrollingCoordinator : public ScrollingCoordinator {
5050
ScrollingTree* scrollingTree() const { return m_scrollingTree.get(); }
5151

5252
void scrollingStateTreePropertiesChanged();
53+
void scrollingThreadAddedPendingUpdate();
5354

5455
void applyPendingScrollUpdates();
5556

@@ -87,6 +88,7 @@ class AsyncScrollingCoordinator : public ScrollingCoordinator {
8788
WEBCORE_EXPORT String scrollingTreeAsText(ScrollingStateTreeAsTextBehavior = ScrollingStateTreeAsTextBehaviorNormal) const override;
8889
WEBCORE_EXPORT void willCommitTree() override;
8990
void synchronizeStateFromScrollingTree();
91+
void scheduleRenderingUpdate();
9092

9193
bool eventTrackingRegionsDirty() const { return m_eventTrackingRegionsDirty; }
9294

‎Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,15 +210,15 @@ void ThreadedScrollingTree::scrollingTreeNodeDidScroll(ScrollingTreeScrollingNod
210210
return;
211211
}
212212

213-
LOG_WITH_STREAM(Scrolling, stream << "ThreadedScrollingTree::scrollingTreeNodeDidScroll " << node.scrollingNodeID() << " to " << scrollPosition << " bouncing to main thread");
213+
LOG_WITH_STREAM(Scrolling, stream << "ThreadedScrollingTree::scrollingTreeNodeDidScroll " << node.scrollingNodeID() << " to " << scrollPosition << " triggering main thread rendering update");
214214

215215
auto scrollUpdate = ScrollUpdate { node.scrollingNodeID(), scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction };
216216
addPendingScrollUpdate(WTFMove(scrollUpdate));
217217

218218
auto deferrer = WheelEventTestMonitorCompletionDeferrer { wheelEventTestMonitor(), reinterpret_cast<WheelEventTestMonitor::ScrollableAreaIdentifier>(node.scrollingNodeID()), WheelEventTestMonitor::ScrollingThreadSyncNeeded };
219219
RunLoop::main().dispatch([strongThis = makeRef(*this), deferrer = WTFMove(deferrer)] {
220220
if (auto* scrollingCoordinator = strongThis->m_scrollingCoordinator.get())
221-
scrollingCoordinator->applyPendingScrollUpdates();
221+
scrollingCoordinator->scrollingThreadAddedPendingUpdate();
222222
});
223223
}
224224

‎Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,7 @@
102102

103103
void ScrollingCoordinatorMac::scheduleTreeStateCommit()
104104
{
105-
// FIXME: This one needs work.
106-
m_page->scheduleRenderingUpdate(RenderingUpdateStep::ScrollingTreeUpdate);
105+
scheduleRenderingUpdate();
107106
}
108107

109108
void ScrollingCoordinatorMac::commitTreeStateIfNeeded()

‎Source/WebCore/page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ void ScrollingCoordinatorNicosia::wheelEventWasProcessedByMainThread(const Platf
8989

9090
void ScrollingCoordinatorNicosia::scheduleTreeStateCommit()
9191
{
92+
// FIXME: This should probably just call scheduleRenderingUpdate().
9293
if (!m_scrollingStateTreeCommitterTimer.isActive())
9394
m_scrollingStateTreeCommitterTimer.startOneShot(0_s);
9495
}

0 commit comments

Comments
 (0)