- Revision
- 236966
- Author
- timothy_hor...@apple.com
- Date
- 2018-10-09 10:18:58 -0700 (Tue, 09 Oct 2018)
Log Message
REGRESSION (r232416): Can not scroll after swiping back on quoteunquoteapps.com
https://bugs.webkit.org/show_bug.cgi?id=190377
<rdar://problem/45108222>
Reviewed by Andy Estes.
Introduce the notion of 'pausing' to SnapshotRemovalTracker.
Reimplement r232416 in terms of this: the SnapshotRemovalTracker
starts out paused (not accepting events), and un-pauses when
we get a provisional load or same-document navigation.
This way, we don't lose the watchdog timer in cases where we get
no provisional load, same-document navigation, or main frame load
(which is the separate root cause for this bug -- this just papers
over it with a timeout).
* UIProcess/Cocoa/ViewGestureController.cpp:
(WebKit::ViewGestureController::didStartProvisionalLoadForMainFrame):
(WebKit::ViewGestureController::didSameDocumentNavigationForMainFrame):
Resume the snapshot removal tracker.
(WebKit::ViewGestureController::didReachMainFrameLoadTerminalState):
If we didn't see a provisional load or same document navigation,
but somehow got to the terminal load state, immediately remove the snapshot.
(WebKit::ViewGestureController::SnapshotRemovalTracker::resume):
(WebKit::ViewGestureController::SnapshotRemovalTracker::start):
Start the SnapshotRemovalTracker out in the paused state; it will be
resumed in the same places we previously would call the
provisionalOrSameDocumentLoadCallback.
(WebKit::ViewGestureController::SnapshotRemovalTracker::stopWaitingForEvent):
Ignore (but debug log) incoming events while paused.
* UIProcess/Cocoa/ViewGestureController.h:
(WebKit::ViewGestureController::SnapshotRemovalTracker::pause):
(WebKit::ViewGestureController::SnapshotRemovalTracker::isPaused const):
Add the pausing bit.
* UIProcess/ios/ViewGestureControllerIOS.mm:
(WebKit::ViewGestureController::endSwipeGesture):
* UIProcess/mac/ViewGestureControllerMac.mm:
(WebKit::ViewGestureController::endSwipeGesture):
Remove m_provisionalOrSameDocumentLoadCallback.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (236965 => 236966)
--- trunk/Source/WebKit/ChangeLog 2018-10-09 17:15:21 UTC (rev 236965)
+++ trunk/Source/WebKit/ChangeLog 2018-10-09 17:18:58 UTC (rev 236966)
@@ -1,3 +1,49 @@
+2018-10-09 Tim Horton <timothy_hor...@apple.com>
+
+ REGRESSION (r232416): Can not scroll after swiping back on quoteunquoteapps.com
+ https://bugs.webkit.org/show_bug.cgi?id=190377
+ <rdar://problem/45108222>
+
+ Reviewed by Andy Estes.
+
+ Introduce the notion of 'pausing' to SnapshotRemovalTracker.
+ Reimplement r232416 in terms of this: the SnapshotRemovalTracker
+ starts out paused (not accepting events), and un-pauses when
+ we get a provisional load or same-document navigation.
+ This way, we don't lose the watchdog timer in cases where we get
+ no provisional load, same-document navigation, or main frame load
+ (which is the separate root cause for this bug -- this just papers
+ over it with a timeout).
+
+ * UIProcess/Cocoa/ViewGestureController.cpp:
+ (WebKit::ViewGestureController::didStartProvisionalLoadForMainFrame):
+ (WebKit::ViewGestureController::didSameDocumentNavigationForMainFrame):
+ Resume the snapshot removal tracker.
+
+ (WebKit::ViewGestureController::didReachMainFrameLoadTerminalState):
+ If we didn't see a provisional load or same document navigation,
+ but somehow got to the terminal load state, immediately remove the snapshot.
+
+ (WebKit::ViewGestureController::SnapshotRemovalTracker::resume):
+ (WebKit::ViewGestureController::SnapshotRemovalTracker::start):
+ Start the SnapshotRemovalTracker out in the paused state; it will be
+ resumed in the same places we previously would call the
+ provisionalOrSameDocumentLoadCallback.
+
+ (WebKit::ViewGestureController::SnapshotRemovalTracker::stopWaitingForEvent):
+ Ignore (but debug log) incoming events while paused.
+
+ * UIProcess/Cocoa/ViewGestureController.h:
+ (WebKit::ViewGestureController::SnapshotRemovalTracker::pause):
+ (WebKit::ViewGestureController::SnapshotRemovalTracker::isPaused const):
+ Add the pausing bit.
+
+ * UIProcess/ios/ViewGestureControllerIOS.mm:
+ (WebKit::ViewGestureController::endSwipeGesture):
+ * UIProcess/mac/ViewGestureControllerMac.mm:
+ (WebKit::ViewGestureController::endSwipeGesture):
+ Remove m_provisionalOrSameDocumentLoadCallback.
+
2018-10-09 Antti Koivisto <an...@apple.com>
Prewarm FontDatabase on process swap
Modified: trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp (236965 => 236966)
--- trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp 2018-10-09 17:15:21 UTC (rev 236965)
+++ trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp 2018-10-09 17:18:58 UTC (rev 236966)
@@ -129,11 +129,9 @@
void ViewGestureController::didStartProvisionalLoadForMainFrame()
{
- if (auto provisionalOrSameDocumentLoadCallback = WTFMove(m_provisionalOrSameDocumentLoadCallback))
- provisionalOrSameDocumentLoadCallback();
+ m_snapshotRemovalTracker.resume();
}
-
void ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame()
{
if (!m_snapshotRemovalTracker.eventOccurred(SnapshotRemovalTracker::VisuallyNonEmptyLayout))
@@ -161,8 +159,7 @@
void ViewGestureController::didReachMainFrameLoadTerminalState()
{
- if (m_provisionalOrSameDocumentLoadCallback) {
- m_provisionalOrSameDocumentLoadCallback = nullptr;
+ if (m_snapshotRemovalTracker.isPaused()) {
removeSwipeSnapshot();
return;
}
@@ -191,10 +188,8 @@
void ViewGestureController::didSameDocumentNavigationForMainFrame(SameDocumentNavigationType type)
{
+ m_snapshotRemovalTracker.resume();
- if (auto provisionalOrSameDocumentLoadCallback = WTFMove(m_provisionalOrSameDocumentLoadCallback))
- provisionalOrSameDocumentLoadCallback();
-
bool cancelledOutstandingEvent = false;
// Same-document navigations don't have a main frame load or first visually non-empty layout.
@@ -260,6 +255,13 @@
#endif
LOG(ViewGestures, "Swipe Snapshot Removal (%0.2f ms) - %s", sinceStart.milliseconds(), log.utf8().data());
}
+
+void ViewGestureController::SnapshotRemovalTracker::resume()
+{
+ if (isPaused() && m_outstandingEvents)
+ log("resume");
+ m_paused = false;
+}
void ViewGestureController::SnapshotRemovalTracker::start(Events desiredEvents, WTF::Function<void()>&& removalCallback)
{
@@ -270,6 +272,10 @@
log("start");
startWatchdog(swipeSnapshotRemovalWatchdogDuration);
+
+ // Initially start out paused; we'll resume when the load is committed.
+ // This avoids processing callbacks from earlier loads.
+ pause();
}
void ViewGestureController::SnapshotRemovalTracker::reset()
@@ -288,6 +294,11 @@
if (!(m_outstandingEvents & event))
return false;
+ if (isPaused()) {
+ log("is paused; ignoring event: " + eventsDescription(event));
+ return false;
+ }
+
log(logReason + eventsDescription(event));
m_outstandingEvents &= ~event;
Modified: trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h (236965 => 236966)
--- trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h 2018-10-09 17:15:21 UTC (rev 236965)
+++ trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h 2018-10-09 17:18:58 UTC (rev 236966)
@@ -169,6 +169,10 @@
void start(Events, WTF::Function<void()>&&);
void reset();
+
+ void pause() { m_paused = true; }
+ void resume();
+ bool isPaused() const { return m_paused; }
bool eventOccurred(Events);
bool cancelOutstandingEvent(Events);
@@ -190,6 +194,8 @@
MonotonicTime m_startTime;
RunLoop::Timer<SnapshotRemovalTracker> m_watchdogTimer;
+
+ bool m_paused { true };
};
#if PLATFORM(MAC)
@@ -301,7 +307,6 @@
uint64_t m_snapshotRemovalTargetRenderTreeSize { 0 };
#endif
- WTF::Function<void()> m_provisionalOrSameDocumentLoadCallback;
SnapshotRemovalTracker m_snapshotRemovalTracker;
};
Modified: trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm (236965 => 236966)
--- trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm 2018-10-09 17:15:21 UTC (rev 236965)
+++ trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm 2018-10-09 17:18:58 UTC (rev 236966)
@@ -300,30 +300,28 @@
return;
}
- m_provisionalOrSameDocumentLoadCallback = [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 (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/ViewGestureControllerMac.mm (236965 => 236966)
--- trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm 2018-10-09 17:15:21 UTC (rev 236965)
+++ trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm 2018-10-09 17:18:58 UTC (rev 236966)
@@ -743,15 +743,13 @@
m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
m_webPageProxy.goToBackForwardItem(*targetItem);
- m_provisionalOrSameDocumentLoadCallback = [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(); });
- };
+ 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.
