- Revision
- 170035
- Author
- [email protected]
- Date
- 2014-06-16 16:45:27 -0700 (Mon, 16 Jun 2014)
Log Message
[iOS][wk2] Swiping back briefly shows the previous page before loading the new one
https://bugs.webkit.org/show_bug.cgi?id=133885
Reviewed by Simon Fraser.
Remove a race between the UI and Web processes when removing the swipe snapshot.
Previously, it was possible to get a commit from the Web process with layer content
(and render tree size) from the previous page *after* sending the navigation request
to the page, because of the asynchronicity of layer tree commits. This could cause
the snapshot to be removed early (if the previous fully-loaded page had a sufficiently
large render tree size), revealing the old tiles underneath the snapshot.
* Shared/mac/RemoteLayerTreeTransaction.h:
(WebKit::RemoteLayerTreeTransaction::transactionID):
(WebKit::RemoteLayerTreeTransaction::setTransactionID):
* Shared/mac/RemoteLayerTreeTransaction.mm:
(WebKit::RemoteLayerTreeTransaction::encode):
(WebKit::RemoteLayerTreeTransaction::decode):
* UIProcess/DrawingAreaProxy.h:
(WebKit::DrawingAreaProxy::lastVisibleTransactionID):
* UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:
* UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxy::RemoteLayerTreeDrawingAreaProxy):
(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
(WebKit::RemoteLayerTreeDrawingAreaProxy::coreAnimationDidCommitLayers):
* WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h:
* WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::RemoteLayerTreeDrawingArea):
(WebKit::RemoteLayerTreeDrawingArea::flushLayers):
(WebKit::RemoteLayerTreeDrawingArea::didUpdate):
Keep track of an ever-increasing transaction ID in RemoteLayerTreeDrawingArea(Proxy).
It increments in the UI process at didUpdate time, because the Web process cannot
have started on a new layer tree commit until didUpdate is sent.
It increments in the Web process at commit time.
* UIProcess/ios/ViewGestureControllerIOS.mm:
(WebKit::ViewGestureController::endSwipeGesture):
(WebKit::ViewGestureController::setRenderTreeSize):
* UIProcess/mac/ViewGestureController.h:
Adopt transaction IDs; don't remove the snapshot until the commit
that includes the navigation arrives.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (170034 => 170035)
--- trunk/Source/WebKit2/ChangeLog 2014-06-16 23:19:29 UTC (rev 170034)
+++ trunk/Source/WebKit2/ChangeLog 2014-06-16 23:45:27 UTC (rev 170035)
@@ -1,3 +1,47 @@
+2014-06-16 Timothy Horton <[email protected]>
+
+ [iOS][wk2] Swiping back briefly shows the previous page before loading the new one
+ https://bugs.webkit.org/show_bug.cgi?id=133885
+
+ Reviewed by Simon Fraser.
+
+ Remove a race between the UI and Web processes when removing the swipe snapshot.
+ Previously, it was possible to get a commit from the Web process with layer content
+ (and render tree size) from the previous page *after* sending the navigation request
+ to the page, because of the asynchronicity of layer tree commits. This could cause
+ the snapshot to be removed early (if the previous fully-loaded page had a sufficiently
+ large render tree size), revealing the old tiles underneath the snapshot.
+
+ * Shared/mac/RemoteLayerTreeTransaction.h:
+ (WebKit::RemoteLayerTreeTransaction::transactionID):
+ (WebKit::RemoteLayerTreeTransaction::setTransactionID):
+ * Shared/mac/RemoteLayerTreeTransaction.mm:
+ (WebKit::RemoteLayerTreeTransaction::encode):
+ (WebKit::RemoteLayerTreeTransaction::decode):
+ * UIProcess/DrawingAreaProxy.h:
+ (WebKit::DrawingAreaProxy::lastVisibleTransactionID):
+ * UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:
+ * UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:
+ (WebKit::RemoteLayerTreeDrawingAreaProxy::RemoteLayerTreeDrawingAreaProxy):
+ (WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
+ (WebKit::RemoteLayerTreeDrawingAreaProxy::coreAnimationDidCommitLayers):
+ * WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h:
+ * WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:
+ (WebKit::RemoteLayerTreeDrawingArea::RemoteLayerTreeDrawingArea):
+ (WebKit::RemoteLayerTreeDrawingArea::flushLayers):
+ (WebKit::RemoteLayerTreeDrawingArea::didUpdate):
+ Keep track of an ever-increasing transaction ID in RemoteLayerTreeDrawingArea(Proxy).
+ It increments in the UI process at didUpdate time, because the Web process cannot
+ have started on a new layer tree commit until didUpdate is sent.
+ It increments in the Web process at commit time.
+
+ * UIProcess/ios/ViewGestureControllerIOS.mm:
+ (WebKit::ViewGestureController::endSwipeGesture):
+ (WebKit::ViewGestureController::setRenderTreeSize):
+ * UIProcess/mac/ViewGestureController.h:
+ Adopt transaction IDs; don't remove the snapshot until the commit
+ that includes the navigation arrives.
+
2014-06-16 Dan Bernstein <[email protected]>
<rdar://problem/17327707> [Cocoa] Expose WebPreferences::storageBlockingPolicy
Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h (170034 => 170035)
--- trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h 2014-06-16 23:19:29 UTC (rev 170034)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h 2014-06-16 23:45:27 UTC (rev 170035)
@@ -204,6 +204,9 @@
bool allowsUserScaling() const { return m_allowsUserScaling; }
void setAllowsUserScaling(bool allowsUserScaling) { m_allowsUserScaling = allowsUserScaling; }
+
+ uint64_t transactionID() const { return m_transactionID; }
+ void setTransactionID(uint64_t transactionID) { m_transactionID = transactionID; }
private:
WebCore::GraphicsLayer::PlatformLayerID m_rootLayerID;
@@ -221,6 +224,7 @@
double m_minimumScaleFactor;
double m_maximumScaleFactor;
uint64_t m_renderTreeSize;
+ uint64_t m_transactionID;
bool m_scaleWasSetByUIProcess;
bool m_allowsUserScaling;
};
Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm (170034 => 170035)
--- trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm 2014-06-16 23:19:29 UTC (rev 170034)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm 2014-06-16 23:45:27 UTC (rev 170035)
@@ -461,6 +461,7 @@
encoder << m_maximumScaleFactor;
encoder << m_renderTreeSize;
+ encoder << m_transactionID;
encoder << m_scaleWasSetByUIProcess;
encoder << m_allowsUserScaling;
@@ -529,6 +530,9 @@
if (!decoder.decode(result.m_renderTreeSize))
return false;
+ if (!decoder.decode(result.m_transactionID))
+ return false;
+
if (!decoder.decode(result.m_scaleWasSetByUIProcess))
return false;
Modified: trunk/Source/WebKit2/UIProcess/DrawingAreaProxy.h (170034 => 170035)
--- trunk/Source/WebKit2/UIProcess/DrawingAreaProxy.h 2014-06-16 23:19:29 UTC (rev 170034)
+++ trunk/Source/WebKit2/UIProcess/DrawingAreaProxy.h 2014-06-16 23:45:27 UTC (rev 170035)
@@ -85,6 +85,8 @@
virtual void showDebugIndicator(bool) { }
virtual bool isShowingDebugIndicator() const { return false; }
+ virtual uint64_t lastVisibleTransactionID() const { ASSERT_NOT_REACHED(); return 0; }
+
protected:
explicit DrawingAreaProxy(DrawingAreaType, WebPageProxy*);
Modified: trunk/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm (170034 => 170035)
--- trunk/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm 2014-06-16 23:19:29 UTC (rev 170034)
+++ trunk/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm 2014-06-16 23:45:27 UTC (rev 170035)
@@ -127,7 +127,7 @@
: m_webPageProxy(webPageProxy)
, m_activeGestureType(ViewGestureType::None)
, m_swipeWatchdogTimer(this, &ViewGestureController::swipeSnapshotWatchdogTimerFired)
- , m_targetRenderTreeSize(0)
+ , m_snapshotRemovalTargetRenderTreeSize(0)
{
}
@@ -225,9 +225,10 @@
}
ViewSnapshot snapshot;
- m_targetRenderTreeSize = 0;
+ m_snapshotRemovalTargetRenderTreeSize = 0;
if (ViewSnapshotStore::shared().getSnapshot(targetItem, snapshot))
- m_targetRenderTreeSize = snapshot.renderTreeSize * swipeSnapshotRemovalRenderTreeSizeTargetFraction;
+ m_snapshotRemovalTargetRenderTreeSize = snapshot.renderTreeSize * swipeSnapshotRemovalRenderTreeSizeTargetFraction;
+ m_snapshotRemovalTargetTransactionID = m_webPageProxy.drawingArea()->lastVisibleTransactionID() + 1;
// We don't want to replace the current back-forward item's snapshot
// like we normally would when going back or forward, because we are
@@ -236,7 +237,7 @@
m_webPageProxy.goToBackForwardItem(targetItem);
ViewSnapshotStore::shared().enableSnapshotting();
- if (!m_targetRenderTreeSize) {
+ if (!m_snapshotRemovalTargetRenderTreeSize) {
removeSwipeSnapshot();
return;
}
@@ -249,7 +250,10 @@
if (m_activeGestureType != ViewGestureType::Swipe)
return;
- if (m_targetRenderTreeSize && renderTreeSize > m_targetRenderTreeSize)
+ // Don't remove the swipe snapshot until we get a drawing area transaction more recent than the navigation,
+ // and we hit the render tree size threshold. This avoids potentially removing the snapshot early,
+ // when receiving commits from the previous (pre-navigation) page.
+ if (m_snapshotRemovalTargetRenderTreeSize && renderTreeSize > m_snapshotRemovalTargetRenderTreeSize && m_webPageProxy.drawingArea()->lastVisibleTransactionID() >= m_snapshotRemovalTargetTransactionID)
removeSwipeSnapshot();
}
@@ -274,7 +278,7 @@
[m_snapshotView removeFromSuperview];
m_snapshotView = nullptr;
- m_targetRenderTreeSize = 0;
+ m_snapshotRemovalTargetRenderTreeSize = 0;
m_activeGestureType = ViewGestureType::None;
}
Modified: trunk/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h (170034 => 170035)
--- trunk/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h 2014-06-16 23:19:29 UTC (rev 170034)
+++ trunk/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h 2014-06-16 23:45:27 UTC (rev 170035)
@@ -76,6 +76,8 @@
void sendUpdateGeometry();
+ virtual uint64_t lastVisibleTransactionID() const override { return m_lastVisibleTransactionID; }
+
RemoteLayerTreeHost m_remoteLayerTreeHost;
bool m_isWaitingForDidUpdateGeometry;
@@ -87,6 +89,9 @@
RetainPtr<CALayer> m_exposedRectIndicatorLayer;
std::unique_ptr<WebCore::RunLoopObserver> m_layerCommitObserver;
+
+ uint64_t m_lastVisibleTransactionID;
+ uint64_t m_transactionIDForPendingCACommit;
};
DRAWING_AREA_PROXY_TYPE_CASTS(RemoteLayerTreeDrawingAreaProxy, type() == DrawingAreaTypeRemoteLayerTree);
Modified: trunk/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm (170034 => 170035)
--- trunk/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm 2014-06-16 23:19:29 UTC (rev 170034)
+++ trunk/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm 2014-06-16 23:45:27 UTC (rev 170035)
@@ -47,6 +47,7 @@
: DrawingAreaProxy(DrawingAreaTypeRemoteLayerTree, webPageProxy)
, m_remoteLayerTreeHost(*this)
, m_isWaitingForDidUpdateGeometry(false)
+ , m_lastVisibleTransactionID(0)
{
#if USE(IOSURFACE)
// We don't want to pool surfaces in the UI process.
@@ -119,6 +120,9 @@
LOG(RemoteLayerTree, "%s", layerTreeTransaction.description().data());
LOG(RemoteLayerTree, "%s", scrollingTreeTransaction.description().data());
+ ASSERT(layerTreeTransaction.transactionID() == m_lastVisibleTransactionID + 1);
+ m_transactionIDForPendingCACommit = layerTreeTransaction.transactionID();
+
if (m_remoteLayerTreeHost.updateLayerTree(layerTreeTransaction))
m_webPageProxy->setAcceleratedCompositingRootLayer(m_remoteLayerTreeHost.rootLayer());
@@ -305,6 +309,8 @@
// using our backing store. We can improve this by waiting for the render server to commit
// if we find API to do so, but for now we will make extra buffers if need be.
m_webPageProxy->process().send(Messages::DrawingArea::DidUpdate(), m_webPageProxy->pageID());
+
+ m_lastVisibleTransactionID = m_transactionIDForPendingCACommit;
}
} // namespace WebKit
Modified: trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h (170034 => 170035)
--- trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h 2014-06-16 23:19:29 UTC (rev 170034)
+++ trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h 2014-06-16 23:45:27 UTC (rev 170035)
@@ -183,7 +183,8 @@
RetainPtr<UIView> m_snapshotView;
RetainPtr<UIView> m_transitionContainerView;
RetainPtr<WKSwipeTransitionController> m_swipeInteractiveTransitionDelegate;
- uint64_t m_targetRenderTreeSize;
+ uint64_t m_snapshotRemovalTargetRenderTreeSize;
+ uint64_t m_snapshotRemovalTargetTransactionID;
#endif
};
Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h (170034 => 170035)
--- trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h 2014-06-16 23:19:29 UTC (rev 170034)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h 2014-06-16 23:45:27 UTC (rev 170035)
@@ -107,6 +107,8 @@
WebCore::TiledBacking* mainFrameTiledBacking() const;
+ uint64_t takeNextTransactionID() { return ++m_currentTransactionID; }
+
class BackingStoreFlusher : public ThreadSafeRefCounted<BackingStoreFlusher> {
public:
static PassRefPtr<BackingStoreFlusher> create(IPC::Connection*, std::unique_ptr<IPC::MessageEncoder>, Vector<RetainPtr<CGContextRef>>);
@@ -144,6 +146,8 @@
HashSet<RemoteLayerTreeDisplayRefreshMonitor*> m_displayRefreshMonitors;
HashSet<RemoteLayerTreeDisplayRefreshMonitor*>* m_displayRefreshMonitorsToNotify;
+
+ uint64_t m_currentTransactionID;
};
DRAWING_AREA_TYPE_CASTS(RemoteLayerTreeDrawingArea, type() == DrawingAreaTypeRemoteLayerTree);
Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm (170034 => 170035)
--- trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm 2014-06-16 23:19:29 UTC (rev 170034)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm 2014-06-16 23:45:27 UTC (rev 170035)
@@ -62,6 +62,7 @@
, m_waitingForBackingStoreSwap(false)
, m_hadFlushDeferredWhileWaitingForBackingStoreSwap(false)
, m_displayRefreshMonitorsToNotify(nullptr)
+ , m_currentTransactionID(0)
{
webPage->corePage()->settings().setForceCompositingMode(true);
#if PLATFORM(IOS)
@@ -275,6 +276,7 @@
// FIXME: Minimize these transactions if nothing changed.
RemoteLayerTreeTransaction layerTransaction;
+ layerTransaction.setTransactionID(takeNextTransactionID());
m_remoteLayerTreeContext->buildTransaction(layerTransaction, *toGraphicsLayerCARemote(m_rootLayer.get())->platformCALayer());
backingStoreCollection.willCommitLayerTree(layerTransaction);
m_webPage->willCommitLayerTree(layerTransaction);
