Changeset 254552 in webkit

Timestamp:

Jan 14, 2020, 6:03:37 PM (7 years ago)

Author:

timothy_horton@apple.com

Message:

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.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/ViewGestureController.cpp (modified) (2 diffs)
• UIProcess/ViewGestureController.h (modified) (1 diff)
• UIProcess/ios/ViewGestureControllerIOS.mm (modified) (4 diffs)
• UIProcess/mac/ViewGestureControllerMac.mm (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

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

trunk/Source/WebKit/UIProcess/ViewGestureController.cpp

@@ -606 +606 @@
 #endif
 
+    m_didCallEndSwipeGesture = true;
+
     if (cancelled) {
         removeSwipeSnapshot();
@@ -615 +617 @@
 
     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;
+    }
 }
 

trunk/Source/WebKit/UIProcess/ViewGestureController.h

@@ -436 +436 @@
     bool m_didStartProvisionalLoad { false };
 
+    bool m_didCallEndSwipeGesture { false };
+    bool m_removeSnapshotImmediatelyWhenGestureEnds { false };
+
     SnapshotRemovalTracker m_snapshotRemovalTracker;
     WTF::Function<void()> m_loadCallback;

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

@@ -259 +259 @@
     [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)
@@ -317 +319 @@
         willEndSwipeGesture(*targetItem, cancelled);
 
+    m_didCallEndSwipeGesture = true;
+
     [context _setTransitionIsInFlight:NO];
     [context _setInteractor:nil];
@@ -347 +351 @@
     }
 
+    // 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;
@@ -399 +410 @@
         return;
 
+    if (!m_didCallEndSwipeGesture) {
+        m_removeSnapshotImmediatelyWhenGestureEnds = true;
+        return;
+    }
+
     [m_snapshotView removeFromSuperview];
     m_snapshotView = nullptr;

trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm

@@ -467 +467 @@
     if (m_webPageProxy.preferences().viewGestureDebuggingEnabled())
         applyDebuggingPropertiesToSwipeViews();
+
+    m_didCallEndSwipeGesture = false;
+    m_removeSnapshotImmediatelyWhenGestureEnds = false;
 
     CALayer *layerAdjacentToSnapshot = determineLayerAdjacentToSnapshotForParent(direction, snapshotLayerParent);
@@ -600 +603 @@
         return;
 
+    if (!m_didCallEndSwipeGesture) {
+        m_removeSnapshotImmediatelyWhenGestureEnds = true;
+        return;
+    }
+
     if (m_currentSwipeSnapshot)
         m_currentSwipeSnapshot->setVolatile(true);