Title: [172635] trunk/Source/WebKit2
Revision
172635
Author
[email protected]
Date
2014-08-15 11:42:24 -0700 (Fri, 15 Aug 2014)

Log Message

REGRESSION (WebKit2 Gestures): White flash when swiping back to cnn.com's homepage from an article
https://bugs.webkit.org/show_bug.cgi?id=135951
<rdar://problem/18006149>

Reviewed by Simon Fraser.

Wait for (the first visually non-empty layout AND the render tree size threshold to be hit),
OR didFinishLoadForFrame, whichever comes first. Once we've done the first visually non-empty layout,
we'll start the watchdog and tear down the snapshot in three seconds no matter what.
Also, force a repaint so we can asynchronously wait for the Web Process to paint and return to us
before removing the snapshot, which improves our chances that something is actually on the screen.

* UIProcess/API/mac/WKView.mm:
(-[WKView _didFirstVisuallyNonEmptyLayoutForMainFrame]):
(-[WKView _didFinishLoadForMainFrame]):
(-[WKView _removeNavigationGestureSnapshot]):
* UIProcess/API/mac/WKViewInternal.h:
* UIProcess/PageClient.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didFinishLoadForFrame):
(WebKit::WebPageProxy::didFirstVisuallyNonEmptyLayoutForFrame):
(WebKit::WebPageProxy::removeNavigationGestureSnapshot):
* UIProcess/WebPageProxy.h:
* UIProcess/ios/PageClientImplIOS.h:
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame):
(WebKit::PageClientImpl::didFinishLoadForMainFrame):
* UIProcess/mac/PageClientImpl.h:
* UIProcess/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame):
(WebKit::PageClientImpl::didFinishLoadForMainFrame):
(WebKit::PageClientImpl::removeNavigationGestureSnapshot):
Plumb didFirstVisuallyNonEmptyLayoutForMainFrame and didFinishLoadForMainFrame
through to ViewGestureController from WebPageProxy via the PageClient, etc.

Ditto for removeNavigationGestureSnapshot, though it is called from a
VoidCallback in ViewGestureController instead of from WebFrameLoaderClient and friends.

* UIProcess/mac/ViewGestureController.h:
* UIProcess/mac/ViewGestureControllerMac.mm:
(WebKit::ViewGestureController::ViewGestureController):
(WebKit::ViewGestureController::endSwipeGesture):
When finishing a swipe, we want to wait for both the first visually non-empty layout
and the render tree size threshold being hit.

(WebKit::ViewGestureController::didHitRenderTreeSizeThreshold):
(WebKit::ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame):
When both of these things have happened, remove the swipe snapshot (after forcing a repaint).
For didFirstVisuallyNonEmptyLayoutForMainFrame, we will also start a watchdog
ensuring that we remove the snapshot in three seconds.

(WebKit::ViewGestureController::didFinishLoadForMainFrame):
When didFinishLoadForMainFrame happens, remove the swipe snapshot (after forcing a repaint).

(WebKit::ViewGestureController::swipeSnapshotWatchdogTimerFired):
If the watchdog timer fires, remove the swipe snapshot (after forcing a repaint).

(WebKit::ViewGestureController::removeSwipeSnapshotAfterRepaint):
Force a repaint and wait for the async callback before removing the snapshot.
It is safe to hold on to the WebPageProxy here because it will always
call all of its callbacks before it is destroyed.
Avoid enqueuing multiple force-repaints.

(WebKit::ViewGestureController::removeSwipeSnapshot):

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (172634 => 172635)


--- trunk/Source/WebKit2/ChangeLog	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/ChangeLog	2014-08-15 18:42:24 UTC (rev 172635)
@@ -1,3 +1,70 @@
+2014-08-15  Tim Horton  <[email protected]>
+
+        REGRESSION (WebKit2 Gestures): White flash when swiping back to cnn.com's homepage from an article
+        https://bugs.webkit.org/show_bug.cgi?id=135951
+        <rdar://problem/18006149>
+
+        Reviewed by Simon Fraser.
+
+        Wait for (the first visually non-empty layout AND the render tree size threshold to be hit),
+        OR didFinishLoadForFrame, whichever comes first. Once we've done the first visually non-empty layout,
+        we'll start the watchdog and tear down the snapshot in three seconds no matter what.
+        Also, force a repaint so we can asynchronously wait for the Web Process to paint and return to us
+        before removing the snapshot, which improves our chances that something is actually on the screen.
+
+        * UIProcess/API/mac/WKView.mm:
+        (-[WKView _didFirstVisuallyNonEmptyLayoutForMainFrame]):
+        (-[WKView _didFinishLoadForMainFrame]):
+        (-[WKView _removeNavigationGestureSnapshot]):
+        * UIProcess/API/mac/WKViewInternal.h:
+        * UIProcess/PageClient.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didFinishLoadForFrame):
+        (WebKit::WebPageProxy::didFirstVisuallyNonEmptyLayoutForFrame):
+        (WebKit::WebPageProxy::removeNavigationGestureSnapshot):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/PageClientImplIOS.h:
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame):
+        (WebKit::PageClientImpl::didFinishLoadForMainFrame):
+        * UIProcess/mac/PageClientImpl.h:
+        * UIProcess/mac/PageClientImpl.mm:
+        (WebKit::PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame):
+        (WebKit::PageClientImpl::didFinishLoadForMainFrame):
+        (WebKit::PageClientImpl::removeNavigationGestureSnapshot):
+        Plumb didFirstVisuallyNonEmptyLayoutForMainFrame and didFinishLoadForMainFrame
+        through to ViewGestureController from WebPageProxy via the PageClient, etc.
+
+        Ditto for removeNavigationGestureSnapshot, though it is called from a
+        VoidCallback in ViewGestureController instead of from WebFrameLoaderClient and friends.
+
+        * UIProcess/mac/ViewGestureController.h:
+        * UIProcess/mac/ViewGestureControllerMac.mm:
+        (WebKit::ViewGestureController::ViewGestureController):
+        (WebKit::ViewGestureController::endSwipeGesture):
+        When finishing a swipe, we want to wait for both the first visually non-empty layout
+        and the render tree size threshold being hit.
+
+        (WebKit::ViewGestureController::didHitRenderTreeSizeThreshold):
+        (WebKit::ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame):
+        When both of these things have happened, remove the swipe snapshot (after forcing a repaint).
+        For didFirstVisuallyNonEmptyLayoutForMainFrame, we will also start a watchdog
+        ensuring that we remove the snapshot in three seconds.
+
+        (WebKit::ViewGestureController::didFinishLoadForMainFrame):
+        When didFinishLoadForMainFrame happens, remove the swipe snapshot (after forcing a repaint).
+
+        (WebKit::ViewGestureController::swipeSnapshotWatchdogTimerFired):
+        If the watchdog timer fires, remove the swipe snapshot (after forcing a repaint).
+
+        (WebKit::ViewGestureController::removeSwipeSnapshotAfterRepaint):
+        Force a repaint and wait for the async callback before removing the snapshot.
+        It is safe to hold on to the WebPageProxy here because it will always
+        call all of its callbacks before it is destroyed.
+        Avoid enqueuing multiple force-repaints.
+
+        (WebKit::ViewGestureController::removeSwipeSnapshot):
+
 2014-08-15  Gavin Barraclough  <[email protected]>
 
         Fix plugin visibility initialization

Modified: trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm	2014-08-15 18:42:24 UTC (rev 172635)
@@ -3598,6 +3598,24 @@
 
 #endif
 
+- (void)_didFirstVisuallyNonEmptyLayoutForMainFrame
+{
+    if (_data->_gestureController)
+        _data->_gestureController->didFirstVisuallyNonEmptyLayoutForMainFrame();
+}
+
+- (void)_didFinishLoadForMainFrame
+{
+    if (_data->_gestureController)
+        _data->_gestureController->didFinishLoadForMainFrame();
+}
+
+- (void)_removeNavigationGestureSnapshot
+{
+    if (_data->_gestureController)
+        _data->_gestureController->removeSwipeSnapshot();
+}
+
 @end
 
 @implementation WKView (Private)

Modified: trunk/Source/WebKit2/UIProcess/API/mac/WKViewInternal.h (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/API/mac/WKViewInternal.h	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/API/mac/WKViewInternal.h	2014-08-15 18:42:24 UTC (rev 172635)
@@ -107,6 +107,10 @@
 - (void)_setSuppressVisibilityUpdates:(BOOL)suppressVisibilityUpdates;
 - (BOOL)_suppressVisibilityUpdates;
 
+- (void)_didFirstVisuallyNonEmptyLayoutForMainFrame;
+- (void)_didFinishLoadForMainFrame;
+- (void)_removeNavigationGestureSnapshot;
+
 #if WK_API_ENABLED
 @property (nonatomic, setter=_setThumbnailView:) _WKThumbnailView *_thumbnailView;
 - (void)_reparentLayerTreeInThumbnailView;

Modified: trunk/Source/WebKit2/UIProcess/PageClient.h (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/PageClient.h	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/PageClient.h	2014-08-15 18:42:24 UTC (rev 172635)
@@ -229,6 +229,7 @@
     virtual String dismissCorrectionPanelSoon(WebCore::ReasonForDismissingAlternativeText) = 0;
     virtual void recordAutocorrectionResponse(WebCore::AutocorrectionResponseType, const String& replacedString, const String& replacementString) = 0;
     virtual void recommendedScrollbarStyleDidChange(int32_t newStyle) = 0;
+    virtual void removeNavigationGestureSnapshot() = 0;
 
     virtual ColorSpaceData colorSpace() = 0;
 
@@ -301,6 +302,9 @@
     virtual void navigationGestureWillEnd(bool willNavigate, WebBackForwardListItem&) = 0;
     virtual void navigationGestureDidEnd(bool willNavigate, WebBackForwardListItem&) = 0;
     virtual void willRecordNavigationSnapshot(WebBackForwardListItem&) = 0;
+
+    virtual void didFirstVisuallyNonEmptyLayoutForMainFrame() = 0;
+    virtual void didFinishLoadForMainFrame() = 0;
 };
 
 } // namespace WebKit

Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp	2014-08-15 18:42:24 UTC (rev 172635)
@@ -2654,13 +2654,17 @@
 
     auto transaction = m_pageLoadState.transaction();
 
-    if (frame->isMainFrame())
+    bool isMainFrame = frame->isMainFrame();
+    if (isMainFrame)
         m_pageLoadState.didFinishLoad(transaction);
 
     frame->didFinishLoad();
 
     m_pageLoadState.commitChanges();
     m_loaderClient->didFinishLoadForFrame(this, frame, navigationID, userData.get());
+
+    if (isMainFrame)
+        m_pageClient.didFinishLoadForMainFrame();
 }
 
 void WebPageProxy::didFailLoadForFrame(uint64_t frameID, uint64_t navigationID, const ResourceError& error, IPC::MessageDecoder& decoder)
@@ -2754,6 +2758,9 @@
     MESSAGE_CHECK(frame);
 
     m_loaderClient->didFirstVisuallyNonEmptyLayoutForFrame(this, frame, userData.get());
+
+    if (frame->isMainFrame())
+        m_pageClient.didFirstVisuallyNonEmptyLayoutForMainFrame();
 }
 
 void WebPageProxy::didLayout(uint32_t layoutMilestones, IPC::MessageDecoder& decoder)
@@ -5174,4 +5181,9 @@
     recordNavigationSnapshot();
 }
 
+void WebPageProxy::removeNavigationGestureSnapshot()
+{
+    m_pageClient.removeNavigationGestureSnapshot();
+}
+
 } // namespace WebKit

Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.h (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/WebPageProxy.h	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.h	2014-08-15 18:42:24 UTC (rev 172635)
@@ -910,6 +910,7 @@
     void willRecordNavigationSnapshot(WebBackForwardListItem&);
 
     bool isShowingNavigationGestureSnapshot() const { return m_isShowingNavigationGestureSnapshot; }
+    void removeNavigationGestureSnapshot();
 
 private:
     WebPageProxy(PageClient&, WebProcessProxy&, uint64_t pageID, const WebPageConfiguration&);

Modified: trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.h (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.h	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.h	2014-08-15 18:42:24 UTC (rev 172635)
@@ -175,6 +175,9 @@
     virtual void navigationGestureDidEnd(bool willNavigate, WebBackForwardListItem&) override;
     virtual void willRecordNavigationSnapshot(WebBackForwardListItem&) override;
 
+    virtual void didFirstVisuallyNonEmptyLayoutForMainFrame() override;
+    virtual void didFinishLoadForMainFrame() override;
+
     WKContentView *m_contentView;
     WKWebView *m_webView;
     RetainPtr<WKEditorUndoTargetObjC> m_undoTarget;

Modified: trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm	2014-08-15 18:42:24 UTC (rev 172635)
@@ -682,6 +682,14 @@
     NavigationState::fromWebPage(*m_webView->_page).willRecordNavigationSnapshot(item);
 }
 
+void PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame()
+{
+}
+
+void PageClientImpl::didFinishLoadForMainFrame()
+{
+}
+
 } // namespace WebKit
 
 #endif // PLATFORM(IOS)

Modified: trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.h (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.h	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.h	2014-08-15 18:42:24 UTC (rev 172635)
@@ -179,6 +179,10 @@
 
     NSView *activeView() const;
 
+    virtual void didFirstVisuallyNonEmptyLayoutForMainFrame() override;
+    virtual void didFinishLoadForMainFrame() override;
+    virtual void removeNavigationGestureSnapshot() override;
+
     WKView *m_wkView;
     WKWebView *m_webView;
     RetainPtr<WKEditorUndoTargetObjC> m_undoTarget;

Modified: trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm	2014-08-15 18:42:24 UTC (rev 172635)
@@ -722,6 +722,21 @@
 #endif
 }
 
+void PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame()
+{
+    [m_wkView _didFirstVisuallyNonEmptyLayoutForMainFrame];
+}
+
+void PageClientImpl::didFinishLoadForMainFrame()
+{
+    [m_wkView _didFinishLoadForMainFrame];
+}
+
+void PageClientImpl::removeNavigationGestureSnapshot()
+{
+    [m_wkView _removeNavigationGestureSnapshot];
+}
+
 } // namespace WebKit
 
 #endif // PLATFORM(MAC)

Modified: trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h	2014-08-15 18:42:24 UTC (rev 172635)
@@ -104,6 +104,9 @@
 
     bool shouldIgnorePinnedState() { return m_shouldIgnorePinnedState; }
     void setShouldIgnorePinnedState(bool ignore) { m_shouldIgnorePinnedState = ignore; }
+
+    void didFirstVisuallyNonEmptyLayoutForMainFrame();
+    void didFinishLoadForMainFrame();
 #else
     void installSwipeHandler(UIView *gestureRecognizerView, UIView *swipingView);
     void setAlternateBackForwardListSourceView(WKWebView *);
@@ -114,11 +117,12 @@
     void setRenderTreeSize(uint64_t);
 #endif
 
+    void removeSwipeSnapshot();
+
 private:
     // IPC::MessageReceiver.
     virtual void didReceiveMessage(IPC::Connection*, IPC::MessageDecoder&) override;
-    
-    void removeSwipeSnapshot();
+
     void swipeSnapshotWatchdogTimerFired();
 
 #if PLATFORM(MAC)
@@ -126,6 +130,7 @@
     void didCollectGeometryForMagnificationGesture(WebCore::FloatRect visibleContentBounds, bool frameHandlesMagnificationGesture);
     void didCollectGeometryForSmartMagnificationGesture(WebCore::FloatPoint origin, WebCore::FloatRect renderRect, WebCore::FloatRect visibleContentBounds, bool isReplacedElement, double viewportMinimumScale, double viewportMaximumScale);
     void didHitRenderTreeSizeThreshold();
+    void removeSwipeSnapshotAfterRepaint();
 
     void endMagnificationGesture();
     WebCore::FloatPoint scaledMagnificationOrigin(WebCore::FloatPoint origin, double scale);
@@ -145,7 +150,7 @@
 
     WebPageProxy& m_webPageProxy;
     ViewGestureType m_activeGestureType;
-    
+
     RunLoop::Timer<ViewGestureController> m_swipeWatchdogTimer;
 
 #if USE(IOSURFACE)
@@ -181,6 +186,10 @@
     WebCore::FloatSize m_cumulativeDeltaForPendingSwipe;
 
     bool m_shouldIgnorePinnedState;
+
+    bool m_swipeWaitingForVisuallyNonEmptyLayout;
+    bool m_swipeWaitingForRenderTreeSizeThreshold;
+    bool m_swipeWaitingForRepaint;
 #else    
     UIView *m_liveSwipeView;
     RetainPtr<UIView> m_liveSwipeViewClippingView;

Modified: trunk/Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm (172634 => 172635)


--- trunk/Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm	2014-08-15 18:26:34 UTC (rev 172634)
+++ trunk/Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm	2014-08-15 18:42:24 UTC (rev 172635)
@@ -105,6 +105,9 @@
     , m_customSwipeViewsTopContentInset(0)
     , m_pendingSwipeReason(PendingSwipeReason::None)
     , m_shouldIgnorePinnedState(false)
+    , m_swipeWaitingForVisuallyNonEmptyLayout(false)
+    , m_swipeWaitingForRenderTreeSizeThreshold(false)
+    , m_swipeWaitingForRepaint(false)
 {
     m_webPageProxy.process().addMessageReceiver(Messages::ViewGestureController::messageReceiverName(), m_webPageProxy.pageID(), *this);
 }
@@ -641,29 +644,70 @@
 
     m_webPageProxy.process().send(Messages::ViewGestureGeometryCollector::SetRenderTreeSizeNotificationThreshold(renderTreeSize * swipeSnapshotRemovalRenderTreeSizeTargetFraction), m_webPageProxy.pageID());
 
+    m_swipeWaitingForVisuallyNonEmptyLayout = true;
+    m_swipeWaitingForRenderTreeSizeThreshold = true;
+
     m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
     m_webPageProxy.goToBackForwardItem(targetItem);
+}
 
-    if (!renderTreeSize) {
-        removeSwipeSnapshot();
+void ViewGestureController::didHitRenderTreeSizeThreshold()
+{
+    if (m_activeGestureType != ViewGestureType::Swipe)
         return;
-    }
 
-    m_swipeWatchdogTimer.startOneShot(swipeSnapshotRemovalWatchdogDuration.count());
+    m_swipeWaitingForRenderTreeSizeThreshold = false;
+
+    if (!m_swipeWaitingForVisuallyNonEmptyLayout)
+        removeSwipeSnapshotAfterRepaint();
 }
 
-void ViewGestureController::didHitRenderTreeSizeThreshold()
+void ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame()
 {
-    removeSwipeSnapshot();
+    if (m_activeGestureType != ViewGestureType::Swipe)
+        return;
+
+    m_swipeWaitingForVisuallyNonEmptyLayout = false;
+
+    if (!m_swipeWaitingForRenderTreeSizeThreshold)
+        removeSwipeSnapshotAfterRepaint();
+    else
+        m_swipeWatchdogTimer.startOneShot(swipeSnapshotRemovalWatchdogDuration.count());
 }
 
+void ViewGestureController::didFinishLoadForMainFrame()
+{
+    if (m_activeGestureType != ViewGestureType::Swipe)
+        return;
+
+    removeSwipeSnapshotAfterRepaint();
+}
+
 void ViewGestureController::swipeSnapshotWatchdogTimerFired()
 {
-    removeSwipeSnapshot();
+    removeSwipeSnapshotAfterRepaint();
 }
 
+void ViewGestureController::removeSwipeSnapshotAfterRepaint()
+{
+    if (m_activeGestureType != ViewGestureType::Swipe)
+        return;
+
+    if (m_swipeWaitingForRepaint)
+        return;
+
+    m_swipeWaitingForRepaint = true;
+
+    WebPageProxy* webPageProxy = &m_webPageProxy;
+    m_webPageProxy.forceRepaint(VoidCallback::create([webPageProxy] (CallbackBase::Error error) {
+        webPageProxy->removeNavigationGestureSnapshot();
+    }));
+}
+
 void ViewGestureController::removeSwipeSnapshot()
 {
+    m_swipeWaitingForRepaint = false;
+
     m_swipeWatchdogTimer.stop();
 
     if (m_activeGestureType != ViewGestureType::Swipe)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to