Changeset 179884 in webkit

Timestamp:

Feb 10, 2015, 2:40:00 PM (12 years ago)

Author:

Lucas Forschler

Message:

Merged r176899. rdar://problem/19739097

Location:

branches/safari-600.5-branch

Files:

• ChangeLog (modified) (1 diff)
• ManualTests/programmatic-scroll-flicker.html (copied) (copied from trunk/ManualTests/programmatic-scroll-flicker.html )
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/FrameView.cpp (modified) (3 diffs)
• Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (modified) (3 diffs)

branches/safari-600.5-branch/ChangeLog

@@ +1 @@
+2015-02-10  Lucas Forschler  <lforschler@apple.com>
+
+        Merge r176899
+
+    2014-12-05  Simon Fraser  <simon.fraser@apple.com>
+
+            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.
+
 2015-01-21  Babak Shafiei  <bshafiei@apple.com>
 

branches/safari-600.5-branch/Source/WebCore/ChangeLog

@@ +1 @@
+2015-02-10  Lucas Forschler  <lforschler@apple.com>
+
+        Merge r176899
+
+    2014-12-05  Simon Fraser  <simon.fraser@apple.com>
+
+            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.
+
+            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.
+
 2015-02-06  Babak Shafiei  <bshafiei@apple.com>
 

branches/safari-600.5-branch/Source/WebCore/page/FrameView.cpp

@@ -271 +271 @@
     m_isVisuallyNonEmpty = false;
     m_firstVisuallyNonEmptyLayoutCallbackPending = true;
-    m_maintainScrollPositionAnchor = 0;
+    m_maintainScrollPositionAnchor = nullptr;
 }
 
@@ -2005 +2005 @@
 {
     TemporaryChange<bool> changeInProgrammaticScroll(m_inProgrammaticScroll, true);
-    m_maintainScrollPositionAnchor = 0;
+    m_maintainScrollPositionAnchor = nullptr;
     ScrollView::setScrollPosition(scrollPoint);
 }
@@ -3600 +3600 @@
     if (m_inProgrammaticScroll)
         return;
-    m_maintainScrollPositionAnchor = 0;
+    m_maintainScrollPositionAnchor = nullptr;
     if (m_wasScrolledByUser == wasScrolledByUser)
         return;

branches/safari-600.5-branch/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp

@@ -154 +154 @@
         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
@@ -166 +167 @@
         return false;
 
-    stateNode->setRequestedScrollPosition(scrollPosition, frameView->inProgrammaticScroll());
+    stateNode->setRequestedScrollPosition(scrollPosition, isProgrammaticScroll);
     return true;
 }
@@ -174 +175 @@
     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)) {