Flash of page scrolled to wrong origin before restoring scroll position after swiping...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Sep 2014 20:59:20 +0000 (20:59 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Sep 2014 20:59:20 +0000 (20:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136788
<rdar://problem/18314597>

Reviewed by Sam Weinig.

* UIProcess/mac/ViewGestureControllerMac.mm:
(WebKit::ViewGestureController::didHitRenderTreeSizeThreshold):
(WebKit::ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame):
Always wait until didFinishLoadForMainFrame or didSameDocumentNavigationForMainFrame
before removing the snapshot, because otherwise we don't know if the scroll
position has been restored yet.

We should revisit this at some point, because it should be possible to
determine if the scroll position has been restored appropriately, but for
now it is safest to restore the antique behavior.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@173573 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm

index 76e679d..4c574c1 100644 (file)
@@ -1,3 +1,22 @@
+2014-09-12  Tim Horton  <timothy_horton@apple.com>
+
+        Flash of page scrolled to wrong origin before restoring scroll position after swiping back to CNN front page from an article
+        https://bugs.webkit.org/show_bug.cgi?id=136788
+        <rdar://problem/18314597>
+
+        Reviewed by Sam Weinig.
+
+        * UIProcess/mac/ViewGestureControllerMac.mm:
+        (WebKit::ViewGestureController::didHitRenderTreeSizeThreshold):
+        (WebKit::ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame):
+        Always wait until didFinishLoadForMainFrame or didSameDocumentNavigationForMainFrame
+        before removing the snapshot, because otherwise we don't know if the scroll
+        position has been restored yet.
+
+        We should revisit this at some point, because it should be possible to
+        determine if the scroll position has been restored appropriately, but for
+        now it is safest to restore the antique behavior.
+
 2014-09-12  Dan Bernstein  <mitz@apple.com>
 
         Build fix.
index ef27a2d..b47dcf9 100644 (file)
@@ -693,8 +693,10 @@ void ViewGestureController::didHitRenderTreeSizeThreshold()
 
     m_swipeWaitingForRenderTreeSizeThreshold = false;
 
-    if (!m_swipeWaitingForVisuallyNonEmptyLayout)
-        removeSwipeSnapshotAfterRepaint();
+    if (!m_swipeWaitingForVisuallyNonEmptyLayout) {
+        // FIXME: Ideally we would call removeSwipeSnapshotAfterRepaint() here, but sometimes
+        // scroll position isn't done restoring until didFinishLoadForFrame, so we flash the wrong content.
+    }
 }
 
 void ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame()
@@ -704,9 +706,10 @@ void ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame()
 
     m_swipeWaitingForVisuallyNonEmptyLayout = false;
 
-    if (!m_swipeWaitingForRenderTreeSizeThreshold)
-        removeSwipeSnapshotAfterRepaint();
-    else {
+    if (!m_swipeWaitingForRenderTreeSizeThreshold) {
+        // FIXME: Ideally we would call removeSwipeSnapshotAfterRepaint() here, but sometimes
+        // scroll position isn't done restoring until didFinishLoadForFrame, so we flash the wrong content.
+    } else {
         m_swipeWatchdogAfterFirstVisuallyNonEmptyLayoutTimer.startOneShot(swipeSnapshotRemovalWatchdogAfterFirstVisuallyNonEmptyLayoutDuration.count());
         m_swipeWatchdogTimer.stop();
     }