REGRESSION (r253394): After swiping back during a navigation, WKWebView gets stuck...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Jan 2020 02:03:37 +0000 (02:03 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Jan 2020 02:03:37 +0000 (02:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=206268
<rdar://problem/58536702>

Reviewed by Simon Fraser.

* UIProcess/ViewGestureController.cpp:
(WebKit::ViewGestureController::endSwipeGesture):
* UIProcess/ViewGestureController.h:
* UIProcess/ios/ViewGestureControllerIOS.mm:
(WebKit::ViewGestureController::beginSwipeGesture):
(WebKit::ViewGestureController::endSwipeGesture):
(WebKit::ViewGestureController::removeSwipeSnapshot):
* UIProcess/mac/ViewGestureControllerMac.mm:
(WebKit::ViewGestureController::beginSwipeGesture):
(WebKit::ViewGestureController::removeSwipeSnapshot):
After r253394, we start loading the destination page in willEndSwipeGesture,
when we know the gesture will complete, instead of in endSwipeGesture,
when it is actually done.

This means that if we decide that we should tear down the snapshot immediately,
this can now happen in the window between willEndSwipeGesture and endSwipeGesture.

However, removeSwipeSnapshot has numerous dependencies on endSwipeGesture
(especially on iOS, where there are /also/ dependencies in the other direction -
endSwipeGesture will never be called after removeSwipeSnapshot because
of the gestureID mismatch).

Regardless, it does not make sense to remove the snapshot while the animation
is still running. So, if something causes removeSwipeSnapshot to be called
before endSwipeGesture, we just set a bit and call it inside endSwipeGesture instead.

This ends up putting the snapshot removal ordering back as it was before r253394.

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ViewGestureController.cpp
Source/WebKit/UIProcess/ViewGestureController.h
Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm
Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm

index e559952..6983641 100644 (file)
@@ -1,3 +1,39 @@
+2020-01-14  Tim Horton  <timothy_horton@apple.com>
+
+        REGRESSION (r253394): After swiping back during a navigation, WKWebView gets stuck with the forward content, stops repainting
+        https://bugs.webkit.org/show_bug.cgi?id=206268
+        <rdar://problem/58536702>
+
+        Reviewed by Simon Fraser.
+
+        * UIProcess/ViewGestureController.cpp:
+        (WebKit::ViewGestureController::endSwipeGesture):
+        * UIProcess/ViewGestureController.h:
+        * UIProcess/ios/ViewGestureControllerIOS.mm:
+        (WebKit::ViewGestureController::beginSwipeGesture):
+        (WebKit::ViewGestureController::endSwipeGesture):
+        (WebKit::ViewGestureController::removeSwipeSnapshot):
+        * UIProcess/mac/ViewGestureControllerMac.mm:
+        (WebKit::ViewGestureController::beginSwipeGesture):
+        (WebKit::ViewGestureController::removeSwipeSnapshot):
+        After r253394, we start loading the destination page in willEndSwipeGesture,
+        when we know the gesture will complete, instead of in endSwipeGesture,
+        when it is actually done.
+
+        This means that if we decide that we should tear down the snapshot immediately,
+        this can now happen in the window between willEndSwipeGesture and endSwipeGesture.
+
+        However, removeSwipeSnapshot has numerous dependencies on endSwipeGesture
+        (especially on iOS, where there are /also/ dependencies in the other direction - 
+        endSwipeGesture will never be called after removeSwipeSnapshot because
+        of the gestureID mismatch).
+
+        Regardless, it does not make sense to remove the snapshot while the animation
+        is still running. So, if something causes removeSwipeSnapshot to be called
+        before endSwipeGesture, we just set a bit and call it inside endSwipeGesture instead.
+
+        This ends up putting the snapshot removal ordering back as it was before r253394.
+
 2020-01-14  Per Arne Vollan  <pvollan@apple.com>
 
         REGRESSION(iOS 13): createMediaElementSource not working
index 8a173bf..4b08c67 100644 (file)
@@ -605,6 +605,8 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
     m_swipeCancellationTracker = nullptr;
 #endif
 
+    m_didCallEndSwipeGesture = true;
+
     if (cancelled) {
         removeSwipeSnapshot();
         m_webPageProxy.navigationGestureDidEnd(false, *targetItem);
@@ -614,6 +616,13 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
     m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
 
     m_snapshotRemovalTracker.eventOccurred(SnapshotRemovalTracker::SwipeAnimationEnd, SnapshotRemovalTracker::ShouldIgnoreEventIfPaused::No);
+
+    // removeSwipeSnapshot() was called between willEndSwipeGesture() and endSwipeGesture().
+    // We couldn't remove it then, because the animation was still running, but now we can!
+    if (m_removeSnapshotImmediatelyWhenGestureEnds) {
+        removeSwipeSnapshot();
+        return;
+    }
 }
 
 void ViewGestureController::requestRenderTreeSizeNotificationIfNeeded()
index 43f3d1d..63231d9 100644 (file)
@@ -435,6 +435,9 @@ private:
     bool m_isConnectedToProcess { false };
     bool m_didStartProvisionalLoad { false };
 
+    bool m_didCallEndSwipeGesture { false };
+    bool m_removeSnapshotImmediatelyWhenGestureEnds { false };
+
     SnapshotRemovalTracker m_snapshotRemovalTracker;
     WTF::Function<void()> m_loadCallback;
 };
index 906f7c5..404969c 100644 (file)
@@ -258,6 +258,8 @@ void ViewGestureController::beginSwipeGesture(_UINavigationInteractiveTransition
     [m_swipeTransitionContext _setInteractor:transition];
     [m_swipeTransitionContext _setTransitionIsInFlight:YES];
     m_didCallWillEndSwipeGesture = false;
+    m_didCallEndSwipeGesture = false;
+    m_removeSnapshotImmediatelyWhenGestureEnds = false;
     [m_swipeTransitionContext _setInteractiveUpdateHandler:^(BOOL finish, CGFloat percent, BOOL transitionCompleted, _UIViewControllerTransitionContext *) {
         if (finish)
             willEndSwipeGesture(*targetItem, !transitionCompleted);
@@ -316,6 +318,8 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
     if (!m_didCallWillEndSwipeGesture)
         willEndSwipeGesture(*targetItem, cancelled);
 
+    m_didCallEndSwipeGesture = true;
+
     [context _setTransitionIsInFlight:NO];
     [context _setInteractor:nil];
     [context _setAnimator:nil];
@@ -346,6 +350,13 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
         return;
     }
 
+    // removeSwipeSnapshot() was called between willEndSwipeGesture() and endSwipeGesture().
+    // We couldn't remove it then, because the animation was still running, but now we can!
+    if (m_removeSnapshotImmediatelyWhenGestureEnds) {
+        removeSwipeSnapshot();
+        return;
+    }
+
     auto pageID = m_webPageProxy.identifier();
     GestureID gestureID = m_currentGestureID;
 
@@ -398,6 +409,11 @@ void ViewGestureController::removeSwipeSnapshot()
     if (m_activeGestureType != ViewGestureType::Swipe)
         return;
 
+    if (!m_didCallEndSwipeGesture) {
+        m_removeSnapshotImmediatelyWhenGestureEnds = true;
+        return;
+    }
+
     [m_snapshotView removeFromSuperview];
     m_snapshotView = nullptr;
     
index 423a06a..210548c 100644 (file)
@@ -467,6 +467,9 @@ void ViewGestureController::beginSwipeGesture(WebBackForwardListItem* targetItem
     if (m_webPageProxy.preferences().viewGestureDebuggingEnabled())
         applyDebuggingPropertiesToSwipeViews();
 
+    m_didCallEndSwipeGesture = false;
+    m_removeSnapshotImmediatelyWhenGestureEnds = false;
+
     CALayer *layerAdjacentToSnapshot = determineLayerAdjacentToSnapshotForParent(direction, snapshotLayerParent);
     BOOL swipingLeft = isPhysicallySwipingLeft(direction);
     if (swipingLeft)
@@ -599,6 +602,11 @@ void ViewGestureController::removeSwipeSnapshot()
     if (m_activeGestureType != ViewGestureType::Swipe)
         return;
 
+    if (!m_didCallEndSwipeGesture) {
+        m_removeSnapshotImmediatelyWhenGestureEnds = true;
+        return;
+    }
+
     if (m_currentSwipeSnapshot)
         m_currentSwipeSnapshot->setVolatile(true);
     m_currentSwipeSnapshot = nullptr;