Changeset 185673 in webkit

Timestamp:

Jun 17, 2015, 3:47:56 PM (11 years ago)

Author:

timothy_horton@apple.com

Message:

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.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/ios/ViewGestureControllerIOS.mm (modified) (2 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm

@@ -392 +392 @@
 
     // This is nearly equivalent to didFinishLoad in the same document navigation case.
+    if (!m_swipeWaitingForDidFinishLoad)
+        return;
+
     m_swipeWaitingForDidFinishLoad = false;
 
@@ -403 +406 @@
 {
     if (m_webPageProxy.pageLoadState().isLoading())
+        return;
+
+    if (!m_swipeWaitingForSubresourceLoads)
         return;