[iOS][wk2] Swiping back briefly shows the previous page before loading the new one
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Jun 2014 23:45:27 +0000 (23:45 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Jun 2014 23:45:27 +0000 (23:45 +0000)
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.

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

Source/WebKit2/ChangeLog
Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h
Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm
Source/WebKit2/UIProcess/DrawingAreaProxy.h
Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm
Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h
Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm
Source/WebKit2/UIProcess/mac/ViewGestureController.h
Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h
Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm

index ceb20d5fc1fa4a50d96590e026bd47ec7ed95b24..85142c90cedcff2bc1c978018b137ef38724909c 100644 (file)
@@ -1,3 +1,47 @@
+2014-06-16  Timothy Horton  <timothy_horton@apple.com>
+
+        [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  <mitz@apple.com>
 
         <rdar://problem/17327707> [Cocoa] Expose WebPreferences::storageBlockingPolicy
index ee0fca87a1d9a9abe4eec4890004e7983c6eaa3d..0d2e0d191637005af13ac2219a4c485cb7494368 100644 (file)
@@ -204,6 +204,9 @@ public:
 
     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 @@ private:
     double m_minimumScaleFactor;
     double m_maximumScaleFactor;
     uint64_t m_renderTreeSize;
+    uint64_t m_transactionID;
     bool m_scaleWasSetByUIProcess;
     bool m_allowsUserScaling;
 };
index 95c68add3cec55e3722c3417a4c30643864ee6b2..519e6b0025847710edf403fbb733340751f2e55d 100644 (file)
@@ -461,6 +461,7 @@ void RemoteLayerTreeTransaction::encode(IPC::ArgumentEncoder& encoder) const
     encoder << m_maximumScaleFactor;
 
     encoder << m_renderTreeSize;
+    encoder << m_transactionID;
 
     encoder << m_scaleWasSetByUIProcess;
     encoder << m_allowsUserScaling;
@@ -529,6 +530,9 @@ bool RemoteLayerTreeTransaction::decode(IPC::ArgumentDecoder& decoder, RemoteLay
     if (!decoder.decode(result.m_renderTreeSize))
         return false;
 
+    if (!decoder.decode(result.m_transactionID))
+        return false;
+
     if (!decoder.decode(result.m_scaleWasSetByUIProcess))
         return false;
 
index 48c67ae77a9fcb46b9cc8953cbf7cbd39e875ac6..7a5b7ac6fe3ea84da9b8ba5c6db492c5645f4b44 100644 (file)
@@ -85,6 +85,8 @@ public:
     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*);
 
index 6dc9673e24e932a619e3e3ab7157d5401d1e4796..bedb5a24e270458cb87bd42336e62b52ffba41d2 100644 (file)
@@ -127,7 +127,7 @@ ViewGestureController::ViewGestureController(WebPageProxy& webPageProxy)
     : m_webPageProxy(webPageProxy)
     , m_activeGestureType(ViewGestureType::None)
     , m_swipeWatchdogTimer(this, &ViewGestureController::swipeSnapshotWatchdogTimerFired)
-    , m_targetRenderTreeSize(0)
+    , m_snapshotRemovalTargetRenderTreeSize(0)
 {
 }
 
@@ -225,9 +225,10 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
     }
 
     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 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
     m_webPageProxy.goToBackForwardItem(targetItem);
     ViewSnapshotStore::shared().enableSnapshotting();
     
-    if (!m_targetRenderTreeSize) {
+    if (!m_snapshotRemovalTargetRenderTreeSize) {
         removeSwipeSnapshot();
         return;
     }
@@ -249,7 +250,10 @@ void ViewGestureController::setRenderTreeSize(uint64_t renderTreeSize)
     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 @@ void ViewGestureController::removeSwipeSnapshot()
     [m_snapshotView removeFromSuperview];
     m_snapshotView = nullptr;
     
-    m_targetRenderTreeSize = 0;
+    m_snapshotRemovalTargetRenderTreeSize = 0;
     m_activeGestureType = ViewGestureType::None;
 }
 
index a81bf301db21c15e6d53510c602f2dfb0a6e435d..2f2f87eef8dbde3e1af6683239e9213eefab734f 100644 (file)
@@ -76,6 +76,8 @@ private:
     
     void sendUpdateGeometry();
 
+    virtual uint64_t lastVisibleTransactionID() const override { return m_lastVisibleTransactionID; }
+
     RemoteLayerTreeHost m_remoteLayerTreeHost;
     bool m_isWaitingForDidUpdateGeometry;
 
@@ -87,6 +89,9 @@ private:
     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);
index 2cc7fe92c8fe7111c9a42651950a4442bdb72608..b394c28874d4a54a7f7d1612158cf4e9c634fa55 100644 (file)
@@ -47,6 +47,7 @@ RemoteLayerTreeDrawingAreaProxy::RemoteLayerTreeDrawingAreaProxy(WebPageProxy* w
     : 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 @@ void RemoteLayerTreeDrawingAreaProxy::commitLayerTree(const RemoteLayerTreeTrans
     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 @@ void RemoteLayerTreeDrawingAreaProxy::coreAnimationDidCommitLayers()
     // 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
index dcb055b09e89cc4a8111cd21f40be9e029938c20..8fb3e7e651f4891b1aa26d49ed3b8a1b1bd0cae8 100644 (file)
@@ -183,7 +183,8 @@ private:
     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
 };
 
index 1f687155db65f78a26271b4444b18ed714752206..2069797a571d53ba0aa5224697c7d2383bea7794 100644 (file)
@@ -107,6 +107,8 @@ private:
 
     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 @@ private:
 
     HashSet<RemoteLayerTreeDisplayRefreshMonitor*> m_displayRefreshMonitors;
     HashSet<RemoteLayerTreeDisplayRefreshMonitor*>* m_displayRefreshMonitorsToNotify;
+
+    uint64_t m_currentTransactionID;
 };
 
 DRAWING_AREA_TYPE_CASTS(RemoteLayerTreeDrawingArea, type() == DrawingAreaTypeRemoteLayerTree);
index 706bf1ce131e62897e66f233803f846e33d03808..de453fd2a85e1d0ce86f5dd5afcb6a2f7e1f7ae4 100644 (file)
@@ -62,6 +62,7 @@ RemoteLayerTreeDrawingArea::RemoteLayerTreeDrawingArea(WebPage* webPage, const W
     , m_waitingForBackingStoreSwap(false)
     , m_hadFlushDeferredWhileWaitingForBackingStoreSwap(false)
     , m_displayRefreshMonitorsToNotify(nullptr)
+    , m_currentTransactionID(0)
 {
     webPage->corePage()->settings().setForceCompositingMode(true);
 #if PLATFORM(IOS)
@@ -275,6 +276,7 @@ void RemoteLayerTreeDrawingArea::flushLayers()
 
     // 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);