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 e8125c1..c4ca6f7 100644 (file)
@@ -1,5 +1,65 @@
 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
         https://bugs.webkit.org/show_bug.cgi?id=143259
         <rdar://problem/19872693>
index 3d6174d..ab84a97 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 509e8eb..d4f64db 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 b83beea..3ded923 100644 (file)
@@ -430,4 +430,8 @@ void PageClientImpl::didSameDocumentNavigationForMainFrame(SameDocumentNavigatio
 {
 }
 
+void PageClientImpl::didChangeBackgroundColor()
+{
+}
+
 } // namespace WebKit
index d16245e..450af07 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 4b8c884..4d89d8e 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 5063520..8606816 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 10a4693..43de138 100644 (file)
@@ -5671,4 +5671,9 @@ void WebPageProxy::externalOutputDeviceAvailableDidChange(bool available)
 
 #endif
 
+void WebPageProxy::didChangeBackgroundColor()
+{
+    m_pageClient.didChangeBackgroundColor();
+}
+
 } // namespace WebKit
index 0322ada..fca5980 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 0851f39..debaeff 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 2809179..bc858a5 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 0dee931..6ad9046 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 b059318..632b56d 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 3bdf5b5..e11b3bc 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 f484862..17b9499 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 b5945ee..fc43051 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()