WebKit briefly shows wrong webpage after swiping back (gigaom.com, or any site on...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Mar 2015 20:53:32 +0000 (20:53 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Mar 2015 20:53:32 +0000 (20:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143256
<rdar://problem/19458648>

Reviewed by Dan Bernstein.

Instead of allowing a flash of the previous page when the swipe snapshot
timeouts fire (removing the snapshot before the new page is loaded), show
the snapshotted background color.

This fixes the problem on iOS, where UI-side compositing makes it easy to fix,
but not yet on OS X.

* UIProcess/API/Cocoa/WKWebView.mm:
(baseScrollViewBackgroundColor):
(scrollViewBackgroundColor):
If the ViewGestureController returns a valid background color, use that
instead of the page's background color.

* UIProcess/API/Cocoa/WKWebViewInternal.h:
Expose _updateScrollViewBackground.

* UIProcess/PageClient.h:
* UIProcess/ios/PageClientImplIOS.h:
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::didChangeBackgroundColor):
* UIProcess/mac/PageClientImpl.h:
* UIProcess/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::didChangeBackgroundColor):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didChangeBackgroundColor):
* UIProcess/WebPageProxy.h:
Add and plumb didChangeBackgroundColor, which calls
_updateScrollViewBackground on iOS.

* UIProcess/ios/ViewGestureControllerIOS.mm:
(WebKit::ViewGestureController::endSwipeGesture):
Hide the content of the drawing area until the next commit. This way,
even if the snapshot is removed (say, because a timeout fired), we won't
ever show the old page content (but we will show background color).

Store the background color associated with the current snapshot.

Let WKWebView know that it needs to recompute the background color.

(WebKit::ViewGestureController::removeSwipeSnapshot):
Clear the background color so that the next time we commit, WKWebView
will get an invalid color from ViewGestureController and fall back
to the page's extended background color instead.

* UIProcess/mac/ViewGestureController.h:
(WebKit::ViewGestureController::backgroundColorForCurrentSnapshot):
* UIProcess/mac/ViewGestureControllerMac.mm:
(WebKit::ViewGestureController::beginSwipeGesture):
(WebKit::ViewGestureController::removeSwipeSnapshot):
Keep backgroundColorForCurrentSnapshot up to date on Mac too, even
though we don't use it yet.

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

16 files changed:
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit2/UIProcess/API/Cocoa/WKWebViewInternal.h
Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp
Source/WebKit2/UIProcess/API/gtk/PageClientImpl.h
Source/WebKit2/UIProcess/CoordinatedGraphics/WebView.h
Source/WebKit2/UIProcess/PageClient.h
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm
Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm
Source/WebKit2/UIProcess/mac/PageClientImpl.h
Source/WebKit2/UIProcess/mac/PageClientImpl.mm
Source/WebKit2/UIProcess/mac/ViewGestureController.h
Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm

index e8125c15e1ff11fbb13443357353b778f3b0e93c..c4ca6f7835fee7d35dec84f48fb89434177093b1 100644 (file)
@@ -1,3 +1,63 @@
+2015-03-31  Timothy Horton  <timothy_horton@apple.com>
+
+        WebKit briefly shows wrong webpage after swiping back (gigaom.com, or any site on a slow network)
+        https://bugs.webkit.org/show_bug.cgi?id=143256
+        <rdar://problem/19458648>
+
+        Reviewed by Dan Bernstein.
+
+        Instead of allowing a flash of the previous page when the swipe snapshot
+        timeouts fire (removing the snapshot before the new page is loaded), show
+        the snapshotted background color.
+
+        This fixes the problem on iOS, where UI-side compositing makes it easy to fix,
+        but not yet on OS X.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (baseScrollViewBackgroundColor):
+        (scrollViewBackgroundColor):
+        If the ViewGestureController returns a valid background color, use that
+        instead of the page's background color.
+
+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
+        Expose _updateScrollViewBackground.
+
+        * UIProcess/PageClient.h:
+        * UIProcess/ios/PageClientImplIOS.h:
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::didChangeBackgroundColor):
+        * UIProcess/mac/PageClientImpl.h:
+        * UIProcess/mac/PageClientImpl.mm:
+        (WebKit::PageClientImpl::didChangeBackgroundColor):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didChangeBackgroundColor):
+        * UIProcess/WebPageProxy.h:
+        Add and plumb didChangeBackgroundColor, which calls
+        _updateScrollViewBackground on iOS.
+        
+        * UIProcess/ios/ViewGestureControllerIOS.mm:
+        (WebKit::ViewGestureController::endSwipeGesture):
+        Hide the content of the drawing area until the next commit. This way,
+        even if the snapshot is removed (say, because a timeout fired), we won't
+        ever show the old page content (but we will show background color).
+
+        Store the background color associated with the current snapshot.
+
+        Let WKWebView know that it needs to recompute the background color.
+
+        (WebKit::ViewGestureController::removeSwipeSnapshot):
+        Clear the background color so that the next time we commit, WKWebView
+        will get an invalid color from ViewGestureController and fall back
+        to the page's extended background color instead.
+
+        * UIProcess/mac/ViewGestureController.h:
+        (WebKit::ViewGestureController::backgroundColorForCurrentSnapshot):
+        * UIProcess/mac/ViewGestureControllerMac.mm:
+        (WebKit::ViewGestureController::beginSwipeGesture):
+        (WebKit::ViewGestureController::removeSwipeSnapshot):
+        Keep backgroundColorForCurrentSnapshot up to date on Mac too, even
+        though we don't use it yet.
+
 2015-03-31  Timothy Horton  <timothy_horton@apple.com>
 
         [iOS] Rotating PDF in Safari scrolls to the wrong position
index 3d6174d0b57f0ed2a541a87815e503a495c7ff27..ab84a9738ef3a3aeab73abe54fbc2dc069e941c5 100644 (file)
@@ -740,17 +740,26 @@ static CGFloat contentZoomScale(WKWebView *webView)
     return scale;
 }
 
+static WebCore::Color baseScrollViewBackgroundColor(WKWebView *webView)
+{
+    if (webView->_customContentView)
+        return [webView->_customContentView backgroundColor].CGColor;
+
+    if (webView->_gestureController) {
+        WebCore::Color color = webView->_gestureController->backgroundColorForCurrentSnapshot();
+        if (color.isValid())
+            return color;
+    }
+
+    return webView->_page->pageExtendedBackgroundColor();
+}
+
 static WebCore::Color scrollViewBackgroundColor(WKWebView *webView)
 {
     if (!webView.opaque)
         return WebCore::Color::transparent;
 
-    WebCore::Color color;
-
-    if (webView->_customContentView)
-        color = [webView->_customContentView backgroundColor].CGColor;
-    else
-        color = webView->_page->pageExtendedBackgroundColor();
+    WebCore::Color color = baseScrollViewBackgroundColor(webView);
 
     if (!color.isValid())
         color = WebCore::Color::white;
index 509e8eb250f0fca1765968d04fc447aafbd0a7c0..d4f64db7d6037a86e4f666bbaa837e1b4715ea60 100644 (file)
@@ -99,6 +99,8 @@ struct PrintInfo;
 - (BOOL)_isShowingVideoOptimized;
 - (BOOL)_mayAutomaticallyShowVideoOptimized;
 
+- (void)_updateScrollViewBackground;
+
 @property (nonatomic, readonly) UIEdgeInsets _computedContentInset;
 #else
 @property (nonatomic, setter=_setIgnoresNonWheelEvents:) BOOL _ignoresNonWheelEvents;
index b83beeacf3dae844d9ffe5ba4c9e82bf0b456f82..3ded923929ffb144072ebd55f5eb55d88cd3d3ca 100644 (file)
@@ -430,4 +430,8 @@ void PageClientImpl::didSameDocumentNavigationForMainFrame(SameDocumentNavigatio
 {
 }
 
+void PageClientImpl::didChangeBackgroundColor()
+{
+}
+
 } // namespace WebKit
index d16245e8cce79a34e408a87eaefda18308e1707f..450af07eab7a673ba620676798c65fd3e5ad2718 100644 (file)
@@ -132,6 +132,8 @@ private:
 
     virtual void doneWithTouchEvent(const NativeWebTouchEvent&, bool wasEventHandled) override;
 
+    virtual void didChangeBackgroundColor() override;
+
     // Members of PageClientImpl class
     GtkWidget* m_viewWidget;
     DefaultUndoController m_undoController;
index 4b8c884b5d9aabb7ba1f17cbcb0bd8062db7c4f4..4d89d8e12284c26d351b2b75d6cdeac9277c9b20 100644 (file)
@@ -204,6 +204,8 @@ protected:
     virtual void navigationGestureDidEnd(bool, WebBackForwardListItem&) override { };
     virtual void willRecordNavigationSnapshot(WebBackForwardListItem&) override { };
 
+    virtual void didChangeBackgroundColor() override { }
+
     WebViewClient m_client;
     RefPtr<WebPageProxy> m_page;
     DefaultUndoController m_undoController;
index 5063520e5679d928ebadb0982b85a6f8aef40643..8606816cfa5df1c15031627d2dad485248b27cbe 100644 (file)
@@ -319,6 +319,8 @@ public:
     virtual void didFinishLoadForMainFrame() = 0;
     virtual void didSameDocumentNavigationForMainFrame(SameDocumentNavigationType) = 0;
 
+    virtual void didChangeBackgroundColor() = 0;
+
 #if PLATFORM(MAC)
     virtual void didPerformActionMenuHitTest(const ActionMenuHitTestResult&, bool forImmediateAction, API::Object*) = 0;
 #endif
index 10a469385ed22d0b4c7844700ece4006dde60d4e..43de138ad4b22049208e344ba54290aa3e1bb0a3 100644 (file)
@@ -5671,4 +5671,9 @@ void WebPageProxy::externalOutputDeviceAvailableDidChange(bool available)
 
 #endif
 
+void WebPageProxy::didChangeBackgroundColor()
+{
+    m_pageClient.didChangeBackgroundColor();
+}
+
 } // namespace WebKit
index 0322ada7d551b1aa2ba294ea6c8e9c7fd48fdd21..fca59803776f71d2533592d07f1670cf3d145ad2 100644 (file)
@@ -1025,6 +1025,8 @@ public:
     virtual void externalOutputDeviceAvailableDidChange(bool) override;
 #endif
 
+    void didChangeBackgroundColor();
+
 private:
     WebPageProxy(PageClient&, WebProcessProxy&, uint64_t pageID, const WebPageConfiguration&);
     void platformInitialize();
index 0851f394354f99d672444769c94b548b14cc109d..debaeff53b3d2d6a07e21e10d6c650993535ee11 100644 (file)
@@ -180,6 +180,8 @@ private:
     virtual void didFinishLoadForMainFrame() override;
     virtual void didSameDocumentNavigationForMainFrame(SameDocumentNavigationType) override;
 
+    virtual void didChangeBackgroundColor() override;
+
     WKContentView *m_contentView;
     WKWebView *m_webView;
     WKView *m_wkView;
index 28091792742abf9b98d8af47136d529f655df9ff..bc858a5bd456f1fe6711939caea7e071fc81c000 100644 (file)
@@ -713,6 +713,11 @@ void PageClientImpl::didSameDocumentNavigationForMainFrame(SameDocumentNavigatio
     [m_webView _didSameDocumentNavigationForMainFrame:navigationType];
 }
 
+void PageClientImpl::didChangeBackgroundColor()
+{
+    [m_webView _updateScrollViewBackground];
+}
+
 } // namespace WebKit
 
 #endif // PLATFORM(IOS)
index 0dee931b28b25fe5285a4706e5d44ccb55bd2cbc..6ad90467349fe0036b01846a3738566410d4035c 100644 (file)
@@ -294,6 +294,7 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
             if (gestureControllerIter != viewGestureControllersForAllPages().end() && gestureControllerIter->value->m_gesturePendingSnapshotRemoval == gesturePendingSnapshotRemoval)
                 gestureControllerIter->value->willCommitPostSwipeTransitionLayerTree(error == CallbackBase::Error::None);
         });
+        drawingArea->hideContentUntilNextUpdate();
     } else {
         removeSwipeSnapshot();
         return;
@@ -306,6 +307,11 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
     m_swipeWaitingForScrollPositionRestoration = true;
 
     m_swipeWatchdogTimer.startOneShot(swipeSnapshotRemovalWatchdogDuration.count());
+
+    if (ViewSnapshot* snapshot = targetItem->snapshot()) {
+        m_backgroundColorForCurrentSnapshot = snapshot->backgroundColor();
+        m_webPageProxy.didChangeBackgroundColor();
+    }
 }
 
 void ViewGestureController::willCommitPostSwipeTransitionLayerTree(bool successful)
@@ -432,6 +438,8 @@ void ViewGestureController::removeSwipeSnapshot()
     m_webPageProxyForBackForwardListForCurrentSwipe = nullptr;
 
     m_swipeTransitionContext = nullptr;
+
+    m_backgroundColorForCurrentSnapshot = Color();
 }
 
 } // namespace WebKit
index b059318539ec670e6e2edb31849cf05fa725700e..632b56d589d23b4468cd74e50c752dfc3a1a6e8b 100644 (file)
@@ -198,6 +198,8 @@ private:
     virtual void didPerformActionMenuHitTest(const ActionMenuHitTestResult&, bool forImmediateAction, API::Object*) override;
     virtual void showPlatformContextMenu(NSMenu *, WebCore::IntPoint) override;
 
+    virtual void didChangeBackgroundColor() override;
+
     WKView *m_wkView;
     WKWebView *m_webView;
     RetainPtr<WKEditorUndoTargetObjC> m_undoTarget;
index 3bdf5b551d1c9e4b7fecf0d693020533b6da4a0a..e11b3bc555b761c1a164db48e1beec5a6f6ec41b 100644 (file)
@@ -779,6 +779,11 @@ void PageClientImpl::removeNavigationGestureSnapshot()
     [m_wkView _removeNavigationGestureSnapshot];
 }
 
+void PageClientImpl::didChangeBackgroundColor()
+{
+    notImplemented();
+}
+
 CGRect PageClientImpl::boundsOfLayerInLayerBackedWindowCoordinates(CALayer *layer) const
 {
     CALayer *windowContentLayer = static_cast<NSView *>(m_wkView.window.contentView).layer;
index f484862874cc56cd77da7893ad0d6d1b487c02c6..17b94996d12f7741db2b06811718624508af35be 100644 (file)
@@ -29,6 +29,7 @@
 #include "MessageReceiver.h"
 #include "SameDocumentNavigationType.h"
 #include "WeakObjCPtr.h"
+#include <WebCore/Color.h>
 #include <WebCore/FloatRect.h>
 #include <wtf/RetainPtr.h>
 #include <wtf/RunLoop.h>
@@ -124,6 +125,8 @@ public:
     void removeSwipeSnapshot();
     void didSameDocumentNavigationForMainFrame(SameDocumentNavigationType);
 
+    WebCore::Color backgroundColorForCurrentSnapshot() const { return m_backgroundColorForCurrentSnapshot; }
+
 private:
     // IPC::MessageReceiver.
     virtual void didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&) override;
@@ -163,6 +166,8 @@ private:
     RunLoop::Timer<ViewGestureController> m_swipeWatchdogTimer;
     RunLoop::Timer<ViewGestureController> m_swipeActiveLoadMonitoringTimer;
 
+    WebCore::Color m_backgroundColorForCurrentSnapshot;
+
 #if PLATFORM(MAC)
     RefPtr<ViewSnapshot> m_currentSwipeSnapshot;
 
index b5945ee8cdc5a32203d4a16bbe06de7c64977d80..fc4305146142107367f344df058583d9a3ac3717 100644 (file)
@@ -661,7 +661,12 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
     m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
     m_webPageProxy.goToBackForwardItem(targetItem);
 
+    // FIXME: Like on iOS, we should ensure that even if one of the timeouts fires,
+    // we never show the old page content, instead showing white (or the snapshot background color).
     m_swipeWatchdogTimer.startOneShot(swipeSnapshotRemovalWatchdogDuration.count());
+
+    if (ViewSnapshot* snapshot = targetItem->snapshot())
+        m_backgroundColorForCurrentSnapshot = snapshot->backgroundColor();
 }
 
 void ViewGestureController::didHitRenderTreeSizeThreshold()
@@ -778,6 +783,8 @@ void ViewGestureController::removeSwipeSnapshot()
     m_activeGestureType = ViewGestureType::None;
 
     m_webPageProxy.navigationGestureSnapshotWasRemoved();
+
+    m_backgroundColorForCurrentSnapshot = Color();
 }
 
 void ViewGestureController::endActiveGesture()