⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 236966 in webkit


Ignore:
Timestamp:
Oct 9, 2018, 10:18:58 AM (8 years ago)
Author:
timothy_horton@apple.com
Message:

REGRESSION (r232416): Can not scroll after swiping back on quoteunquoteapps.com
​https://bugs.webkit.org/show_bug.cgi?id=190377
<rdar://problem/45108222>

Reviewed by Andy Estes.

Introduce the notion of 'pausing' to SnapshotRemovalTracker.
Reimplement r232416 in terms of this: the SnapshotRemovalTracker
starts out paused (not accepting events), and un-pauses when
we get a provisional load or same-document navigation.
This way, we don't lose the watchdog timer in cases where we get
no provisional load, same-document navigation, or main frame load
(which is the separate root cause for this bug -- this just papers
over it with a timeout).

  • UIProcess/Cocoa/ViewGestureController.cpp:

(WebKit::ViewGestureController::didStartProvisionalLoadForMainFrame):
(WebKit::ViewGestureController::didSameDocumentNavigationForMainFrame):
Resume the snapshot removal tracker.

(WebKit::ViewGestureController::didReachMainFrameLoadTerminalState):
If we didn't see a provisional load or same document navigation,
but somehow got to the terminal load state, immediately remove the snapshot.

(WebKit::ViewGestureController::SnapshotRemovalTracker::resume):
(WebKit::ViewGestureController::SnapshotRemovalTracker::start):
Start the SnapshotRemovalTracker out in the paused state; it will be
resumed in the same places we previously would call the
provisionalOrSameDocumentLoadCallback.

(WebKit::ViewGestureController::SnapshotRemovalTracker::stopWaitingForEvent):
Ignore (but debug log) incoming events while paused.

  • UIProcess/Cocoa/ViewGestureController.h:

(WebKit::ViewGestureController::SnapshotRemovalTracker::pause):
(WebKit::ViewGestureController::SnapshotRemovalTracker::isPaused const):
Add the pausing bit.

  • UIProcess/ios/ViewGestureControllerIOS.mm:

(WebKit::ViewGestureController::endSwipeGesture):

  • UIProcess/mac/ViewGestureControllerMac.mm:

(WebKit::ViewGestureController::endSwipeGesture):
Remove m_provisionalOrSameDocumentLoadCallback.

Location:
trunk/Source/WebKit
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r236959 r236966  
     12018-10-09  Tim Horton  <timothy_horton@apple.com>
     2
     3        REGRESSION (r232416): Can not scroll after swiping back on quoteunquoteapps.com
     4        https://bugs.webkit.org/show_bug.cgi?id=190377
     5        <rdar://problem/45108222>
     6
     7        Reviewed by Andy Estes.
     8
     9        Introduce the notion of 'pausing' to SnapshotRemovalTracker.
     10        Reimplement r232416 in terms of this: the SnapshotRemovalTracker
     11        starts out paused (not accepting events), and un-pauses when
     12        we get a provisional load or same-document navigation.
     13        This way, we don't lose the watchdog timer in cases where we get
     14        no provisional load, same-document navigation, or main frame load
     15        (which is the separate root cause for this bug -- this just papers
     16        over it with a timeout).
     17
     18        * UIProcess/Cocoa/ViewGestureController.cpp:
     19        (WebKit::ViewGestureController::didStartProvisionalLoadForMainFrame):
     20        (WebKit::ViewGestureController::didSameDocumentNavigationForMainFrame):
     21        Resume the snapshot removal tracker.
     22
     23        (WebKit::ViewGestureController::didReachMainFrameLoadTerminalState):
     24        If we didn't see a provisional load or same document navigation,
     25        but somehow got to the terminal load state, immediately remove the snapshot.
     26       
     27        (WebKit::ViewGestureController::SnapshotRemovalTracker::resume):
     28        (WebKit::ViewGestureController::SnapshotRemovalTracker::start):
     29        Start the SnapshotRemovalTracker out in the paused state; it will be
     30        resumed in the same places we previously would call the
     31        provisionalOrSameDocumentLoadCallback.
     32
     33        (WebKit::ViewGestureController::SnapshotRemovalTracker::stopWaitingForEvent):
     34        Ignore (but debug log) incoming events while paused.
     35
     36        * UIProcess/Cocoa/ViewGestureController.h:
     37        (WebKit::ViewGestureController::SnapshotRemovalTracker::pause):
     38        (WebKit::ViewGestureController::SnapshotRemovalTracker::isPaused const):
     39        Add the pausing bit.
     40
     41        * UIProcess/ios/ViewGestureControllerIOS.mm:
     42        (WebKit::ViewGestureController::endSwipeGesture):
     43        * UIProcess/mac/ViewGestureControllerMac.mm:
     44        (WebKit::ViewGestureController::endSwipeGesture):
     45        Remove m_provisionalOrSameDocumentLoadCallback.
     46
    1472018-10-09  Antti Koivisto  <antti@apple.com>
    248

  • trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp

    r236086 r236966  
    130130void ViewGestureController::didStartProvisionalLoadForMainFrame()
    131131{
    132     if (auto provisionalOrSameDocumentLoadCallback = WTFMove(m_provisionalOrSameDocumentLoadCallback))
    133         provisionalOrSameDocumentLoadCallback();
    134 }
    135 
     132    m_snapshotRemovalTracker.resume();
     133}
    136134
    137135void ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame()
    … …  
    162160void ViewGestureController::didReachMainFrameLoadTerminalState()
    163161{
    164     if (m_provisionalOrSameDocumentLoadCallback) {
    165         m_provisionalOrSameDocumentLoadCallback = nullptr;
     162    if (m_snapshotRemovalTracker.isPaused()) {
    166163        removeSwipeSnapshot();
    167164        return;
    … …  
    192189void ViewGestureController::didSameDocumentNavigationForMainFrame(SameDocumentNavigationType type)
    193190{
    194 
    195     if (auto provisionalOrSameDocumentLoadCallback = WTFMove(m_provisionalOrSameDocumentLoadCallback))
    196         provisionalOrSameDocumentLoadCallback();
     191    m_snapshotRemovalTracker.resume();
    197192
    198193    bool cancelledOutstandingEvent = false;
    … …  
    261256    LOG(ViewGestures, "Swipe Snapshot Removal (%0.2f ms) - %s", sinceStart.milliseconds(), log.utf8().data());
    262257}
     258   
     259void ViewGestureController::SnapshotRemovalTracker::resume()
     260{
     261    if (isPaused() && m_outstandingEvents)
     262        log("resume");
     263    m_paused = false;
     264}
    263265
    264266void ViewGestureController::SnapshotRemovalTracker::start(Events desiredEvents, WTF::Function<void()>&& removalCallback)
    … …  
    271273
    272274    startWatchdog(swipeSnapshotRemovalWatchdogDuration);
     275   
     276    // Initially start out paused; we'll resume when the load is committed.
     277    // This avoids processing callbacks from earlier loads.
     278    pause();
    273279}
    274280
    … …  
    289295        return false;
    290296
     297    if (isPaused()) {
     298        log("is paused; ignoring event: " + eventsDescription(event));
     299        return false;
     300    }
     301
    291302    log(logReason + eventsDescription(event));
    292303

  • trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h

    r236086 r236966  
    170170        void start(Events, WTF::Function<void()>&&);
    171171        void reset();
     172       
     173        void pause() { m_paused = true; }
     174        void resume();
     175        bool isPaused() const { return m_paused; }
    172176
    173177        bool eventOccurred(Events);
    … …  
    191195
    192196        RunLoop::Timer<SnapshotRemovalTracker> m_watchdogTimer;
     197       
     198        bool m_paused { true };
    193199    };
    194200
    … …  
    302308#endif
    303309
    304     WTF::Function<void()> m_provisionalOrSameDocumentLoadCallback;
    305310    SnapshotRemovalTracker m_snapshotRemovalTracker;
    306311};

  • trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm

    r235265 r236966  
    301301    }
    302302
    303     m_provisionalOrSameDocumentLoadCallback = [this] {
    304         if (auto drawingArea = m_webPageProxy.drawingArea()) {
    305             uint64_t pageID = m_webPageProxy.pageID();
    306             GestureID gestureID = m_currentGestureID;
    307             drawingArea->dispatchAfterEnsuringDrawing([pageID, gestureID] (CallbackBase::Error error) {
    308                 if (auto gestureController = controllerForGesture(pageID, gestureID))
    309                     gestureController->willCommitPostSwipeTransitionLayerTree(error == CallbackBase::Error::None);
    310             });
    311             drawingArea->hideContentUntilPendingUpdate();
    312         } else {
    313             removeSwipeSnapshot();
    314             return;
    315         }
    316 
    317         // FIXME: Should we wait for VisuallyNonEmptyLayout like we do on Mac?
    318         m_snapshotRemovalTracker.start(SnapshotRemovalTracker::RenderTreeSizeThreshold
    319             | SnapshotRemovalTracker::RepaintAfterNavigation
    320             | SnapshotRemovalTracker::MainFrameLoad
    321             | SnapshotRemovalTracker::SubresourceLoads
    322             | SnapshotRemovalTracker::ScrollPositionRestoration, [this] {
    323                 this->removeSwipeSnapshot();
     303    if (auto drawingArea = m_webPageProxy.drawingArea()) {
     304        uint64_t pageID = m_webPageProxy.pageID();
     305        GestureID gestureID = m_currentGestureID;
     306        drawingArea->dispatchAfterEnsuringDrawing([pageID, gestureID] (CallbackBase::Error error) {
     307            if (auto gestureController = controllerForGesture(pageID, gestureID))
     308                gestureController->willCommitPostSwipeTransitionLayerTree(error == CallbackBase::Error::None);
    324309        });
    325     };
     310        drawingArea->hideContentUntilPendingUpdate();
     311    } else {
     312        removeSwipeSnapshot();
     313        return;
     314    }
     315
     316    // FIXME: Should we wait for VisuallyNonEmptyLayout like we do on Mac?
     317    m_snapshotRemovalTracker.start(SnapshotRemovalTracker::RenderTreeSizeThreshold
     318        | SnapshotRemovalTracker::RepaintAfterNavigation
     319        | SnapshotRemovalTracker::MainFrameLoad
     320        | SnapshotRemovalTracker::SubresourceLoads
     321        | SnapshotRemovalTracker::ScrollPositionRestoration, [this] {
     322            this->removeSwipeSnapshot();
     323    });
    326324
    327325    if (ViewSnapshot* snapshot = targetItem->snapshot()) {

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

    r235265 r236966  
    744744    m_webPageProxy.goToBackForwardItem(*targetItem);
    745745
    746     m_provisionalOrSameDocumentLoadCallback = [this, renderTreeSize] {
    747         SnapshotRemovalTracker::Events desiredEvents = SnapshotRemovalTracker::VisuallyNonEmptyLayout
    748             | SnapshotRemovalTracker::MainFrameLoad
    749             | SnapshotRemovalTracker::SubresourceLoads
    750             | SnapshotRemovalTracker::ScrollPositionRestoration;
    751         if (renderTreeSize)
    752             desiredEvents |= SnapshotRemovalTracker::RenderTreeSizeThreshold;
    753         m_snapshotRemovalTracker.start(desiredEvents, [this] { this->forceRepaintIfNeeded(); });
    754     };
     746    SnapshotRemovalTracker::Events desiredEvents = SnapshotRemovalTracker::VisuallyNonEmptyLayout
     747        | SnapshotRemovalTracker::MainFrameLoad
     748        | SnapshotRemovalTracker::SubresourceLoads
     749        | SnapshotRemovalTracker::ScrollPositionRestoration;
     750    if (renderTreeSize)
     751        desiredEvents |= SnapshotRemovalTracker::RenderTreeSizeThreshold;
     752    m_snapshotRemovalTracker.start(desiredEvents, [this] { this->forceRepaintIfNeeded(); });
    755753
    756754    // FIXME: Like on iOS, we should ensure that even if one of the timeouts fires,
Note: See TracChangeset for help on using the changeset viewer.