- Revision
- 232416
- Author
- [email protected]
- Date
- 2018-06-01 13:58:27 -0700 (Fri, 01 Jun 2018)
Log Message
Regression(r230876): Swipe navigation snapshot may get removed too early
https://bugs.webkit.org/show_bug.cgi?id=186168
<rdar://problem/39743617>
Reviewed by Tim Horton.
The swipe navigation snapshot would get removed too early when receiving a paint
event after requesting a history navigation but before the provisional load has
actually started. This is because of the asynchronous navigation policy decision
which occurs after requesting to navigate. To address the issue, we now start
listening for events only after the provisional load has started.
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _didStartProvisionalLoadForMainFrame]):
* UIProcess/API/Cocoa/WKWebViewInternal.h:
* UIProcess/Cocoa/ViewGestureController.cpp:
(WebKit::ViewGestureController::didStartProvisionalLoadForMainFrame):
(WebKit::ViewGestureController::didReachMainFrameLoadTerminalState):
(WebKit::ViewGestureController::didSameDocumentNavigationForMainFrame):
* UIProcess/Cocoa/ViewGestureController.h:
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::didStartProvisionalLoadForMainFrame):
* UIProcess/ios/ViewGestureControllerIOS.mm:
(WebKit::ViewGestureController::endSwipeGesture):
* UIProcess/mac/PageClientImplMac.h:
* UIProcess/mac/PageClientImplMac.mm:
(WebKit::PageClientImpl::didStartProvisionalLoadForMainFrame):
* UIProcess/mac/ViewGestureControllerMac.mm:
(WebKit::ViewGestureController::endSwipeGesture):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (232415 => 232416)
--- trunk/Source/WebKit/ChangeLog 2018-06-01 20:57:45 UTC (rev 232415)
+++ trunk/Source/WebKit/ChangeLog 2018-06-01 20:58:27 UTC (rev 232416)
@@ -1,3 +1,35 @@
+2018-06-01 Chris Dumez <[email protected]>
+
+ Regression(r230876): Swipe navigation snapshot may get removed too early
+ https://bugs.webkit.org/show_bug.cgi?id=186168
+ <rdar://problem/39743617>
+
+ Reviewed by Tim Horton.
+
+ The swipe navigation snapshot would get removed too early when receiving a paint
+ event after requesting a history navigation but before the provisional load has
+ actually started. This is because of the asynchronous navigation policy decision
+ which occurs after requesting to navigate. To address the issue, we now start
+ listening for events only after the provisional load has started.
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView _didStartProvisionalLoadForMainFrame]):
+ * UIProcess/API/Cocoa/WKWebViewInternal.h:
+ * UIProcess/Cocoa/ViewGestureController.cpp:
+ (WebKit::ViewGestureController::didStartProvisionalLoadForMainFrame):
+ (WebKit::ViewGestureController::didReachMainFrameLoadTerminalState):
+ (WebKit::ViewGestureController::didSameDocumentNavigationForMainFrame):
+ * UIProcess/Cocoa/ViewGestureController.h:
+ * UIProcess/ios/PageClientImplIOS.mm:
+ (WebKit::PageClientImpl::didStartProvisionalLoadForMainFrame):
+ * UIProcess/ios/ViewGestureControllerIOS.mm:
+ (WebKit::ViewGestureController::endSwipeGesture):
+ * UIProcess/mac/PageClientImplMac.h:
+ * UIProcess/mac/PageClientImplMac.mm:
+ (WebKit::PageClientImpl::didStartProvisionalLoadForMainFrame):
+ * UIProcess/mac/ViewGestureControllerMac.mm:
+ (WebKit::ViewGestureController::endSwipeGesture):
+
2018-06-01 Jiewen Tan <[email protected]>
Unreviewed, build fix for r232276.
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (232415 => 232416)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2018-06-01 20:57:45 UTC (rev 232415)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2018-06-01 20:58:27 UTC (rev 232416)
@@ -2829,6 +2829,12 @@
}
}
+- (void)_didStartProvisionalLoadForMainFrame
+{
+ if (_gestureController)
+ _gestureController->didStartProvisionalLoadForMainFrame();
+}
+
- (void)_didFinishLoadForMainFrame
{
if (_gestureController)
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h (232415 => 232416)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h 2018-06-01 20:57:45 UTC (rev 232415)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h 2018-06-01 20:58:27 UTC (rev 232416)
@@ -105,6 +105,7 @@
- (void)_scheduleVisibleContentRectUpdate;
+- (void)_didStartProvisionalLoadForMainFrame;
- (void)_didFinishLoadForMainFrame;
- (void)_didFailLoadForMainFrame;
- (void)_didSameDocumentNavigationForMainFrame:(WebKit::SameDocumentNavigationType)navigationType;
Modified: trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp (232415 => 232416)
--- trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp 2018-06-01 20:57:45 UTC (rev 232415)
+++ trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp 2018-06-01 20:58:27 UTC (rev 232416)
@@ -127,7 +127,13 @@
return !!backForwardList.forwardItem();
}
+void ViewGestureController::didStartProvisionalLoadForMainFrame()
+{
+ if (auto provisionalLoadCallback = WTFMove(m_provisionalLoadCallback))
+ provisionalLoadCallback();
+}
+
void ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame()
{
if (!m_snapshotRemovalTracker.eventOccurred(SnapshotRemovalTracker::VisuallyNonEmptyLayout))
@@ -155,6 +161,12 @@
void ViewGestureController::didReachMainFrameLoadTerminalState()
{
+ if (m_provisionalLoadCallback) {
+ m_provisionalLoadCallback = nullptr;
+ removeSwipeSnapshot();
+ return;
+ }
+
if (!m_snapshotRemovalTracker.eventOccurred(SnapshotRemovalTracker::MainFrameLoad))
return;
@@ -179,6 +191,12 @@
void ViewGestureController::didSameDocumentNavigationForMainFrame(SameDocumentNavigationType type)
{
+ if (m_provisionalLoadCallback) {
+ m_provisionalLoadCallback = nullptr;
+ removeSwipeSnapshot();
+ return;
+ }
+
bool cancelledOutstandingEvent = false;
// Same-document navigations don't have a main frame load or first visually non-empty layout.
Modified: trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h (232415 => 232416)
--- trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h 2018-06-01 20:57:45 UTC (rev 232415)
+++ trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h 2018-06-01 20:58:27 UTC (rev 232416)
@@ -122,6 +122,7 @@
WebCore::Color backgroundColorForCurrentSnapshot() const { return m_backgroundColorForCurrentSnapshot; }
+ void didStartProvisionalLoadForMainFrame();
void didFinishLoadForMainFrame() { didReachMainFrameLoadTerminalState(); }
void didFailLoadForMainFrame() { didReachMainFrameLoadTerminalState(); }
void didFirstVisuallyNonEmptyLayoutForMainFrame();
@@ -295,6 +296,7 @@
uint64_t m_snapshotRemovalTargetRenderTreeSize { 0 };
#endif
+ WTF::Function<void()> m_provisionalLoadCallback;
SnapshotRemovalTracker m_snapshotRemovalTracker;
};
Modified: trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm (232415 => 232416)
--- trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm 2018-06-01 20:57:45 UTC (rev 232415)
+++ trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm 2018-06-01 20:58:27 UTC (rev 232416)
@@ -245,6 +245,7 @@
void PageClientImpl::didStartProvisionalLoadForMainFrame()
{
+ [m_webView _didStartProvisionalLoadForMainFrame];
[m_webView _hidePasswordView];
}
Modified: trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm (232415 => 232416)
--- trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm 2018-06-01 20:57:45 UTC (rev 232415)
+++ trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm 2018-06-01 20:58:27 UTC (rev 232416)
@@ -296,28 +296,35 @@
m_webPageProxyForBackForwardListForCurrentSwipe->goToBackForwardItem(*targetItem);
- if (auto drawingArea = m_webPageProxy.drawingArea()) {
- uint64_t pageID = m_webPageProxy.pageID();
- GestureID gestureID = m_currentGestureID;
- drawingArea->dispatchAfterEnsuringDrawing([pageID, gestureID] (CallbackBase::Error error) {
- if (auto gestureController = controllerForGesture(pageID, gestureID))
- gestureController->willCommitPostSwipeTransitionLayerTree(error == CallbackBase::Error::None);
- });
- drawingArea->hideContentUntilPendingUpdate();
- } else {
+ if (!m_webPageProxy.drawingArea()) {
removeSwipeSnapshot();
return;
}
- // FIXME: Should we wait for VisuallyNonEmptyLayout like we do on Mac?
- m_snapshotRemovalTracker.start(SnapshotRemovalTracker::RenderTreeSizeThreshold
- | SnapshotRemovalTracker::RepaintAfterNavigation
- | SnapshotRemovalTracker::MainFrameLoad
- | SnapshotRemovalTracker::SubresourceLoads
- | SnapshotRemovalTracker::ScrollPositionRestoration, [this] {
- this->removeSwipeSnapshot();
- });
+ m_provisionalLoadCallback = [this] {
+ if (auto drawingArea = m_webPageProxy.drawingArea()) {
+ uint64_t pageID = m_webPageProxy.pageID();
+ GestureID gestureID = m_currentGestureID;
+ drawingArea->dispatchAfterEnsuringDrawing([pageID, gestureID] (CallbackBase::Error error) {
+ if (auto gestureController = controllerForGesture(pageID, gestureID))
+ gestureController->willCommitPostSwipeTransitionLayerTree(error == CallbackBase::Error::None);
+ });
+ drawingArea->hideContentUntilPendingUpdate();
+ } else {
+ removeSwipeSnapshot();
+ return;
+ }
+ // FIXME: Should we wait for VisuallyNonEmptyLayout like we do on Mac?
+ m_snapshotRemovalTracker.start(SnapshotRemovalTracker::RenderTreeSizeThreshold
+ | SnapshotRemovalTracker::RepaintAfterNavigation
+ | SnapshotRemovalTracker::MainFrameLoad
+ | SnapshotRemovalTracker::SubresourceLoads
+ | SnapshotRemovalTracker::ScrollPositionRestoration, [this] {
+ this->removeSwipeSnapshot();
+ });
+ };
+
if (ViewSnapshot* snapshot = targetItem->snapshot()) {
m_backgroundColorForCurrentSnapshot = snapshot->backgroundColor();
m_webPageProxy.didChangeBackgroundColor();
Modified: trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.h (232415 => 232416)
--- trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.h 2018-06-01 20:57:45 UTC (rev 232415)
+++ trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.h 2018-06-01 20:58:27 UTC (rev 232416)
@@ -205,6 +205,7 @@
NSView *activeView() const;
NSWindow *activeWindow() const;
+ void didStartProvisionalLoadForMainFrame() override;
void didFirstVisuallyNonEmptyLayoutForMainFrame() override;
void didFinishLoadForMainFrame() override;
void didFailLoadForMainFrame() override;
Modified: trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.mm (232415 => 232416)
--- trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.mm 2018-06-01 20:57:45 UTC (rev 232415)
+++ trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.mm 2018-06-01 20:58:27 UTC (rev 232416)
@@ -755,6 +755,12 @@
#endif
}
+void PageClientImpl::didStartProvisionalLoadForMainFrame()
+{
+ if (auto gestureController = m_impl->gestureController())
+ gestureController->didStartProvisionalLoadForMainFrame();
+}
+
void PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame()
{
if (auto gestureController = m_impl->gestureController())
Modified: trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm (232415 => 232416)
--- trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm 2018-06-01 20:57:45 UTC (rev 232415)
+++ trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm 2018-06-01 20:58:27 UTC (rev 232416)
@@ -743,13 +743,15 @@
m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
m_webPageProxy.goToBackForwardItem(*targetItem);
- SnapshotRemovalTracker::Events desiredEvents = SnapshotRemovalTracker::VisuallyNonEmptyLayout
- | SnapshotRemovalTracker::MainFrameLoad
- | SnapshotRemovalTracker::SubresourceLoads
- | SnapshotRemovalTracker::ScrollPositionRestoration;
- if (renderTreeSize)
- desiredEvents |= SnapshotRemovalTracker::RenderTreeSizeThreshold;
- m_snapshotRemovalTracker.start(desiredEvents, [this] { this->forceRepaintIfNeeded(); });
+ m_provisionalLoadCallback = [this, renderTreeSize] {
+ SnapshotRemovalTracker::Events desiredEvents = SnapshotRemovalTracker::VisuallyNonEmptyLayout
+ | SnapshotRemovalTracker::MainFrameLoad
+ | SnapshotRemovalTracker::SubresourceLoads
+ | SnapshotRemovalTracker::ScrollPositionRestoration;
+ if (renderTreeSize)
+ desiredEvents |= SnapshotRemovalTracker::RenderTreeSizeThreshold;
+ m_snapshotRemovalTracker.start(desiredEvents, [this] { this->forceRepaintIfNeeded(); });
+ };
// FIXME: Like on iOS, we should ensure that even if one of the timeouts fires,
// we never show the old page content, instead showing the snapshot background color.
