Repro crash when swiping back from a NY Times article @ WebPageProxy::navigationGestu...

https://bugs.webkit.org/show_bug.cgi?id=146083
<rdar://problem/20974232>

Reviewed by Darin Adler.

* UIProcess/ios/ViewGestureControllerIOS.mm:
(WebKit::ViewGestureController::didSameDocumentNavigationForMainFrame):
(WebKit::ViewGestureController::activeLoadMonitoringTimerFired):
It is possible to get didSameDocumentNavigationForMainFrame *before*
endSwipeGesture, while the user is still interactively swiping. We
cannot remove the snapshot in this case, nor should we start the active
load monitoring timer; all of these things should happen only after the
swipe is completed and we've performed the navigation.

This was particularly bad (a crash instead of just a disappearing snapshot)
because removing the snapshot also causes m_webPageProxyForBackForwardListForCurrentSwipe
to be nulled out, but then it is dereferenced during endSwipeGesture.

Make sure that we never call removeSwipeSnapshotIfReady unless we were actually
waiting to remove the swipe snapshot (because the gesture had completed).
Most callers already did ensure this, but these two did not.

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

diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
index 6c906c7..2aff7c0 100644 (file)
--- a/Source/WebKit2/ChangeLog
+++ b/Source/WebKit2/ChangeLog
@@ -1,3 +1,28 @@
+2015-06-17  Tim Horton  <timothy_horton@apple.com>
+
+        Repro crash when swiping back from a NY Times article @ WebPageProxy::navigationGestureDidEnd
+        https://bugs.webkit.org/show_bug.cgi?id=146083
+        <rdar://problem/20974232>
+
+        Reviewed by Darin Adler.
+
+        * UIProcess/ios/ViewGestureControllerIOS.mm:
+        (WebKit::ViewGestureController::didSameDocumentNavigationForMainFrame):
+        (WebKit::ViewGestureController::activeLoadMonitoringTimerFired):
+        It is possible to get didSameDocumentNavigationForMainFrame *before*
+        endSwipeGesture, while the user is still interactively swiping. We
+        cannot remove the snapshot in this case, nor should we start the active
+        load monitoring timer; all of these things should happen only after the
+        swipe is completed and we've performed the navigation.
+
+        This was particularly bad (a crash instead of just a disappearing snapshot)
+        because removing the snapshot also causes m_webPageProxyForBackForwardListForCurrentSwipe
+        to be nulled out, but then it is dereferenced during endSwipeGesture.
+
+        Make sure that we never call removeSwipeSnapshotIfReady unless we were actually
+        waiting to remove the swipe snapshot (because the gesture had completed).
+        Most callers already did ensure this, but these two did not.
+
 2015-06-17  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Improve Full Screen support for Web Inspector windows

diff --git a/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm b/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm
index 592a600..1a4b310 100644 (file)
--- a/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm
+++ b/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm
@@ -391,6 +391,9 @@ void ViewGestureController::didSameDocumentNavigationForMainFrame(SameDocumentNa
         return;
 
     // This is nearly equivalent to didFinishLoad in the same document navigation case.
+    if (!m_swipeWaitingForDidFinishLoad)
+        return;
+
     m_swipeWaitingForDidFinishLoad = false;
 
     if (type != SameDocumentNavigationSessionStateReplace && type != SameDocumentNavigationSessionStatePop)
@@ -404,6 +407,9 @@ void ViewGestureController::activeLoadMonitoringTimerFired()
     if (m_webPageProxy.pageLoadState().isLoading())
         return;
 
+    if (!m_swipeWaitingForSubresourceLoads)
+        return;
+
     m_swipeWaitingForSubresourceLoads = false;
     removeSwipeSnapshotIfReady();
 }