Changeset 172635 in webkit


Ignore:
Timestamp:
Aug 15, 2014 11:42:24 AM (10 years ago)
Author:
timothy_horton@apple.com
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):

Location:
trunk/Source/WebKit2
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit2/ChangeLog

    r172634 r172635  
     12014-08-15  Tim Horton  <timothy_horton@apple.com>
     2
     3        REGRESSION (WebKit2 Gestures): White flash when swiping back to cnn.com's homepage from an article
     4        https://bugs.webkit.org/show_bug.cgi?id=135951
     5        <rdar://problem/18006149>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Wait for (the first visually non-empty layout AND the render tree size threshold to be hit),
     10        OR didFinishLoadForFrame, whichever comes first. Once we've done the first visually non-empty layout,
     11        we'll start the watchdog and tear down the snapshot in three seconds no matter what.
     12        Also, force a repaint so we can asynchronously wait for the Web Process to paint and return to us
     13        before removing the snapshot, which improves our chances that something is actually on the screen.
     14
     15        * UIProcess/API/mac/WKView.mm:
     16        (-[WKView _didFirstVisuallyNonEmptyLayoutForMainFrame]):
     17        (-[WKView _didFinishLoadForMainFrame]):
     18        (-[WKView _removeNavigationGestureSnapshot]):
     19        * UIProcess/API/mac/WKViewInternal.h:
     20        * UIProcess/PageClient.h:
     21        * UIProcess/WebPageProxy.cpp:
     22        (WebKit::WebPageProxy::didFinishLoadForFrame):
     23        (WebKit::WebPageProxy::didFirstVisuallyNonEmptyLayoutForFrame):
     24        (WebKit::WebPageProxy::removeNavigationGestureSnapshot):
     25        * UIProcess/WebPageProxy.h:
     26        * UIProcess/ios/PageClientImplIOS.h:
     27        * UIProcess/ios/PageClientImplIOS.mm:
     28        (WebKit::PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame):
     29        (WebKit::PageClientImpl::didFinishLoadForMainFrame):
     30        * UIProcess/mac/PageClientImpl.h:
     31        * UIProcess/mac/PageClientImpl.mm:
     32        (WebKit::PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame):
     33        (WebKit::PageClientImpl::didFinishLoadForMainFrame):
     34        (WebKit::PageClientImpl::removeNavigationGestureSnapshot):
     35        Plumb didFirstVisuallyNonEmptyLayoutForMainFrame and didFinishLoadForMainFrame
     36        through to ViewGestureController from WebPageProxy via the PageClient, etc.
     37
     38        Ditto for removeNavigationGestureSnapshot, though it is called from a
     39        VoidCallback in ViewGestureController instead of from WebFrameLoaderClient and friends.
     40
     41        * UIProcess/mac/ViewGestureController.h:
     42        * UIProcess/mac/ViewGestureControllerMac.mm:
     43        (WebKit::ViewGestureController::ViewGestureController):
     44        (WebKit::ViewGestureController::endSwipeGesture):
     45        When finishing a swipe, we want to wait for both the first visually non-empty layout
     46        and the render tree size threshold being hit.
     47
     48        (WebKit::ViewGestureController::didHitRenderTreeSizeThreshold):
     49        (WebKit::ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame):
     50        When both of these things have happened, remove the swipe snapshot (after forcing a repaint).
     51        For didFirstVisuallyNonEmptyLayoutForMainFrame, we will also start a watchdog
     52        ensuring that we remove the snapshot in three seconds.
     53
     54        (WebKit::ViewGestureController::didFinishLoadForMainFrame):
     55        When didFinishLoadForMainFrame happens, remove the swipe snapshot (after forcing a repaint).
     56
     57        (WebKit::ViewGestureController::swipeSnapshotWatchdogTimerFired):
     58        If the watchdog timer fires, remove the swipe snapshot (after forcing a repaint).
     59
     60        (WebKit::ViewGestureController::removeSwipeSnapshotAfterRepaint):
     61        Force a repaint and wait for the async callback before removing the snapshot.
     62        It is safe to hold on to the WebPageProxy here because it will always
     63        call all of its callbacks before it is destroyed.
     64        Avoid enqueuing multiple force-repaints.
     65
     66        (WebKit::ViewGestureController::removeSwipeSnapshot):
     67
    1682014-08-15  Gavin Barraclough  <barraclough@apple.com>
    269

  • trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm

    r172104 r172635  
    35993599#endif
    36003600
     3601- (void)_didFirstVisuallyNonEmptyLayoutForMainFrame
     3602{
     3603    if (_data->_gestureController)
     3604        _data->_gestureController->didFirstVisuallyNonEmptyLayoutForMainFrame();
     3605}
     3606
     3607- (void)_didFinishLoadForMainFrame
     3608{
     3609    if (_data->_gestureController)
     3610        _data->_gestureController->didFinishLoadForMainFrame();
     3611}
     3612
     3613- (void)_removeNavigationGestureSnapshot
     3614{
     3615    if (_data->_gestureController)
     3616        _data->_gestureController->removeSwipeSnapshot();
     3617}
     3618
    36013619@end
    36023620

  • trunk/Source/WebKit2/UIProcess/API/mac/WKViewInternal.h

    r170974 r172635  
    108108- (BOOL)_suppressVisibilityUpdates;
    109109
     110- (void)_didFirstVisuallyNonEmptyLayoutForMainFrame;
     111- (void)_didFinishLoadForMainFrame;
     112- (void)_removeNavigationGestureSnapshot;
     113
    110114#if WK_API_ENABLED
    111115@property (nonatomic, setter=_setThumbnailView:) _WKThumbnailView *_thumbnailView;

  • trunk/Source/WebKit2/UIProcess/PageClient.h

    r172307 r172635  
    230230    virtual void recordAutocorrectionResponse(WebCore::AutocorrectionResponseType, const String& replacedString, const String& replacementString) = 0;
    231231    virtual void recommendedScrollbarStyleDidChange(int32_t newStyle) = 0;
     232    virtual void removeNavigationGestureSnapshot() = 0;
    232233
    233234    virtual ColorSpaceData colorSpace() = 0;
    … …  
    302303    virtual void navigationGestureDidEnd(bool willNavigate, WebBackForwardListItem&) = 0;
    303304    virtual void willRecordNavigationSnapshot(WebBackForwardListItem&) = 0;
     305
     306    virtual void didFirstVisuallyNonEmptyLayoutForMainFrame() = 0;
     307    virtual void didFinishLoadForMainFrame() = 0;
    304308};
    305309

  • trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp

    r172592 r172635  
    26552655    auto transaction = m_pageLoadState.transaction();
    26562656
    2657     if (frame->isMainFrame())
     2657    bool isMainFrame = frame->isMainFrame();
     2658    if (isMainFrame)
    26582659        m_pageLoadState.didFinishLoad(transaction);
    26592660
    … …  
    26622663    m_pageLoadState.commitChanges();
    26632664    m_loaderClient->didFinishLoadForFrame(this, frame, navigationID, userData.get());
     2665
     2666    if (isMainFrame)
     2667        m_pageClient.didFinishLoadForMainFrame();
    26642668}
    26652669
    … …  
    27552759
    27562760    m_loaderClient->didFirstVisuallyNonEmptyLayoutForFrame(this, frame, userData.get());
     2761
     2762    if (frame->isMainFrame())
     2763        m_pageClient.didFirstVisuallyNonEmptyLayoutForMainFrame();
    27572764}
    27582765
    … …  
    51755182}
    51765183
     5184void WebPageProxy::removeNavigationGestureSnapshot()
     5185{
     5186    m_pageClient.removeNavigationGestureSnapshot();
     5187}
     5188
    51775189} // namespace WebKit

  • trunk/Source/WebKit2/UIProcess/WebPageProxy.h

    r172104 r172635  
    911911
    912912    bool isShowingNavigationGestureSnapshot() const { return m_isShowingNavigationGestureSnapshot; }
     913    void removeNavigationGestureSnapshot();
    913914
    914915private:

  • trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.h

    r172307 r172635  
    176176    virtual void willRecordNavigationSnapshot(WebBackForwardListItem&) override;
    177177
     178    virtual void didFirstVisuallyNonEmptyLayoutForMainFrame() override;
     179    virtual void didFinishLoadForMainFrame() override;
     180
    178181    WKContentView *m_contentView;
    179182    WKWebView *m_webView;

  • trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm

    r172307 r172635  
    683683}
    684684
     685void PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame()
     686{
     687}
     688
     689void PageClientImpl::didFinishLoadForMainFrame()
     690{
     691}
     692
    685693} // namespace WebKit
    686694

  • trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.h

    r172307 r172635  
    180180    NSView *activeView() const;
    181181
     182    virtual void didFirstVisuallyNonEmptyLayoutForMainFrame() override;
     183    virtual void didFinishLoadForMainFrame() override;
     184    virtual void removeNavigationGestureSnapshot() override;
     185
    182186    WKView *m_wkView;
    183187    WKWebView *m_webView;

  • trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm

    r172307 r172635  
    723723}
    724724
     725void PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame()
     726{
     727    [m_wkView _didFirstVisuallyNonEmptyLayoutForMainFrame];
     728}
     729
     730void PageClientImpl::didFinishLoadForMainFrame()
     731{
     732    [m_wkView _didFinishLoadForMainFrame];
     733}
     734
     735void PageClientImpl::removeNavigationGestureSnapshot()
     736{
     737    [m_wkView _removeNavigationGestureSnapshot];
     738}
     739
    725740} // namespace WebKit
    726741

  • trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h

    r171317 r172635  
    105105    bool shouldIgnorePinnedState() { return m_shouldIgnorePinnedState; }
    106106    void setShouldIgnorePinnedState(bool ignore) { m_shouldIgnorePinnedState = ignore; }
     107
     108    void didFirstVisuallyNonEmptyLayoutForMainFrame();
     109    void didFinishLoadForMainFrame();
    107110#else
    108111    void installSwipeHandler(UIView *gestureRecognizerView, UIView *swipingView);
    … …  
    115118#endif
    116119
     120    void removeSwipeSnapshot();
     121
    117122private:
    118123    // IPC::MessageReceiver.
    119124    virtual void didReceiveMessage(IPC::Connection*, IPC::MessageDecoder&) override;
    120    
    121     void removeSwipeSnapshot();
     125
    122126    void swipeSnapshotWatchdogTimerFired();
    123127
    … …  
    127131    void didCollectGeometryForSmartMagnificationGesture(WebCore::FloatPoint origin, WebCore::FloatRect renderRect, WebCore::FloatRect visibleContentBounds, bool isReplacedElement, double viewportMinimumScale, double viewportMaximumScale);
    128132    void didHitRenderTreeSizeThreshold();
     133    void removeSwipeSnapshotAfterRepaint();
    129134
    130135    void endMagnificationGesture();
    … …  
    146151    WebPageProxy& m_webPageProxy;
    147152    ViewGestureType m_activeGestureType;
    148    
     153
    149154    RunLoop::Timer<ViewGestureController> m_swipeWatchdogTimer;
    150155
    … …  
    182187
    183188    bool m_shouldIgnorePinnedState;
     189
     190    bool m_swipeWaitingForVisuallyNonEmptyLayout;
     191    bool m_swipeWaitingForRenderTreeSizeThreshold;
     192    bool m_swipeWaitingForRepaint;
    184193#else   
    185194    UIView *m_liveSwipeView;

  • trunk/Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm

    r171317 r172635  
    106106    , m_pendingSwipeReason(PendingSwipeReason::None)
    107107    , m_shouldIgnorePinnedState(false)
     108    , m_swipeWaitingForVisuallyNonEmptyLayout(false)
     109    , m_swipeWaitingForRenderTreeSizeThreshold(false)
     110    , m_swipeWaitingForRepaint(false)
    108111{
    109112    m_webPageProxy.process().addMessageReceiver(Messages::ViewGestureController::messageReceiverName(), m_webPageProxy.pageID(), *this);
    … …  
    642645    m_webPageProxy.process().send(Messages::ViewGestureGeometryCollector::SetRenderTreeSizeNotificationThreshold(renderTreeSize * swipeSnapshotRemovalRenderTreeSizeTargetFraction), m_webPageProxy.pageID());
    643646
     647    m_swipeWaitingForVisuallyNonEmptyLayout = true;
     648    m_swipeWaitingForRenderTreeSizeThreshold = true;
     649
    644650    m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
    645651    m_webPageProxy.goToBackForwardItem(targetItem);
    646 
    647     if (!renderTreeSize) {
    648         removeSwipeSnapshot();
    649         return;
    650     }
    651 
    652     m_swipeWatchdogTimer.startOneShot(swipeSnapshotRemovalWatchdogDuration.count());
    653652}
    654653
    655654void ViewGestureController::didHitRenderTreeSizeThreshold()
    656655{
    657     removeSwipeSnapshot();
     656    if (m_activeGestureType != ViewGestureType::Swipe)
     657        return;
     658
     659    m_swipeWaitingForRenderTreeSizeThreshold = false;
     660
     661    if (!m_swipeWaitingForVisuallyNonEmptyLayout)
     662        removeSwipeSnapshotAfterRepaint();
     663}
     664
     665void ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame()
     666{
     667    if (m_activeGestureType != ViewGestureType::Swipe)
     668        return;
     669
     670    m_swipeWaitingForVisuallyNonEmptyLayout = false;
     671
     672    if (!m_swipeWaitingForRenderTreeSizeThreshold)
     673        removeSwipeSnapshotAfterRepaint();
     674    else
     675        m_swipeWatchdogTimer.startOneShot(swipeSnapshotRemovalWatchdogDuration.count());
     676}
     677
     678void ViewGestureController::didFinishLoadForMainFrame()
     679{
     680    if (m_activeGestureType != ViewGestureType::Swipe)
     681        return;
     682
     683    removeSwipeSnapshotAfterRepaint();
    658684}
    659685
    660686void ViewGestureController::swipeSnapshotWatchdogTimerFired()
    661687{
    662     removeSwipeSnapshot();
     688    removeSwipeSnapshotAfterRepaint();
     689}
     690
     691void ViewGestureController::removeSwipeSnapshotAfterRepaint()
     692{
     693    if (m_activeGestureType != ViewGestureType::Swipe)
     694        return;
     695
     696    if (m_swipeWaitingForRepaint)
     697        return;
     698
     699    m_swipeWaitingForRepaint = true;
     700
     701    WebPageProxy* webPageProxy = &m_webPageProxy;
     702    m_webPageProxy.forceRepaint(VoidCallback::create([webPageProxy] (CallbackBase::Error error) {
     703        webPageProxy->removeNavigationGestureSnapshot();
     704    }));
    663705}
    664706
    665707void ViewGestureController::removeSwipeSnapshot()
    666708{
     709    m_swipeWaitingForRepaint = false;
     710
    667711    m_swipeWatchdogTimer.stop();
    668712
Note: See TracChangeset for help on using the changeset viewer.