Programmatic scrolling and content changes are not always synchronized
https://bugs.webkit.org/show_bug.cgi?id=139245
rdar://problem/18833612
Reviewed by Anders Carlsson.
Manual test that tries to sync layout with programmatic scrolling.
* ManualTests/programmatic-scroll-flicker.html: Added.
Source/WebCore:
For programmatic scrolls, AsyncScrollingCoordinator::requestScrollPositionUpdate()
calls updateScrollPositionAfterAsyncScroll(), then dispatches the requested
scroll position to the scrolling thread.
Once the scrolling thread commits, it calls back to the main thread via
scheduleUpdateScrollPositionAfterAsyncScroll(), which schedules a second
call to updateScrollPositionAfterAsyncScroll() on a timer. That's a problem,
because some other scroll may have happened in the meantime; when the timer
fires, it can sometimes restore a stale scroll position.
Fix by bailing early from scheduleUpdateScrollPositionAfterAsyncScroll()
for programmatic scrolls, since we know that requestScrollPositionUpdate()
already did the updateScrollPositionAfterAsyncScroll().
Test:
ManualTests/programmatic-scroll-flicker.html
* page/FrameView.cpp:
(WebCore::FrameView::reset): nullptr.
(WebCore::FrameView::setScrollPosition): Ditto.
(WebCore::FrameView::setWasScrolledByUser): Ditto.
* page/scrolling/AsyncScrollingCoordinator.cpp:
(WebCore::AsyncScrollingCoordinator::requestScrollPositionUpdate): Use a local variable for
isProgrammaticScroll just to make sure we use the same value for the duration of this function.
(WebCore::AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll): Do nothing
if this is a programmatic scroll.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@176899 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
index 1f82303..e4cb06f 100644
--- a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
+++ b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
@@ -176,8 +176,9 @@
if (!coordinatesScrollingForFrameView(frameView))
return false;
- if (frameView->inProgrammaticScroll() || frameView->frame().document()->inPageCache())
- updateScrollPositionAfterAsyncScroll(frameView->scrollLayerID(), scrollPosition, frameView->inProgrammaticScroll(), SetScrollingLayerPosition);
+ bool isProgrammaticScroll = frameView->inProgrammaticScroll();
+ if (isProgrammaticScroll || frameView->frame().document()->inPageCache())
+ updateScrollPositionAfterAsyncScroll(frameView->scrollLayerID(), scrollPosition, isProgrammaticScroll, SetScrollingLayerPosition);
// 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.
@@ -188,7 +189,7 @@
if (!stateNode)
return false;
- stateNode->setRequestedScrollPosition(scrollPosition, frameView->inProgrammaticScroll());
+ stateNode->setRequestedScrollPosition(scrollPosition, isProgrammaticScroll);
return true;
}
@@ -196,6 +197,10 @@
{
ScheduledScrollUpdate scrollUpdate(nodeID, scrollPosition, programmaticScroll, scrollingLayerPositionAction);
+ // For programmatic scrolls, requestScrollPositionUpdate() has already called updateScrollPositionAfterAsyncScroll().
+ if (programmaticScroll)
+ return;
+
if (m_updateNodeScrollPositionTimer.isActive()) {
if (m_scheduledScrollUpdate.matchesUpdateType(scrollUpdate)) {
m_scheduledScrollUpdate.scrollPosition = scrollPosition;
