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

Changeset 243855 in webkit


Ignore:
Timestamp:
Apr 3, 2019, 8:57:22 PM (7 years ago)
Author:
Simon Fraser
Message:

Simplify some "programmaticScroll" code paths
​https://bugs.webkit.org/show_bug.cgi?id=196589

Reviewed by Zalan Bujtas.

Source/WebCore:

AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll() just returned early if programmaticScroll
was true, so instead, just never call it. This means we can remove the "programmaticScroll" argument from
scheduleUpdateScrollPositionAfterAsyncScroll(). Also change some callers to use the ScrollType enum
instead of a bool.

Now, ThreadedScrollingTree::scrollingTreeNodeDidScroll() just returns early. Programmatic scrolls
update state on the main thread before updating the scrolling tree, so this makes sense.

  • page/scrolling/AsyncScrollingCoordinator.cpp:

(WebCore::AsyncScrollingCoordinator::requestScrollPositionUpdate):
(WebCore::AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll):
(WebCore::AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScrollTimerFired):
(WebCore::AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll):
(WebCore::AsyncScrollingCoordinator::reconcileScrollingState):

  • page/scrolling/AsyncScrollingCoordinator.h:

(WebCore::AsyncScrollingCoordinator::ScheduledScrollUpdate::ScheduledScrollUpdate):
(WebCore::AsyncScrollingCoordinator::ScheduledScrollUpdate::matchesUpdateType const):

  • page/scrolling/ScrollingCoordinator.cpp:

(WebCore::operator<<):

  • page/scrolling/ScrollingCoordinator.h:

(WebCore::ScrollingCoordinator::reconcileScrollingState):

  • page/scrolling/ThreadedScrollingTree.cpp:

(WebCore::ThreadedScrollingTree::scrollingTreeNodeDidScroll):

Source/WebKit:

  • WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm: Remove the parameter.

(WebKit::RemoteScrollingCoordinator::scrollPositionChangedForNode): Use the enum type.

  • WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::updateVisibleContentRects):

Location:
trunk/Source
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r243850 r243855  
     12019-04-03  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Simplify some "programmaticScroll" code paths
     4        https://bugs.webkit.org/show_bug.cgi?id=196589
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll() just returned early if programmaticScroll
     9        was true, so instead, just never call it. This means we can remove the "programmaticScroll" argument from
     10        scheduleUpdateScrollPositionAfterAsyncScroll(). Also change some callers to use the ScrollType enum
     11        instead of a bool.
     12
     13        Now, ThreadedScrollingTree::scrollingTreeNodeDidScroll() just returns early. Programmatic scrolls
     14        update state on the main thread before updating the scrolling tree, so this makes sense.
     15
     16        * page/scrolling/AsyncScrollingCoordinator.cpp:
     17        (WebCore::AsyncScrollingCoordinator::requestScrollPositionUpdate):
     18        (WebCore::AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll):
     19        (WebCore::AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScrollTimerFired):
     20        (WebCore::AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll):
     21        (WebCore::AsyncScrollingCoordinator::reconcileScrollingState):
     22        * page/scrolling/AsyncScrollingCoordinator.h:
     23        (WebCore::AsyncScrollingCoordinator::ScheduledScrollUpdate::ScheduledScrollUpdate):
     24        (WebCore::AsyncScrollingCoordinator::ScheduledScrollUpdate::matchesUpdateType const):
     25        * page/scrolling/ScrollingCoordinator.cpp:
     26        (WebCore::operator<<):
     27        * page/scrolling/ScrollingCoordinator.h:
     28        (WebCore::ScrollingCoordinator::reconcileScrollingState):
     29        * page/scrolling/ThreadedScrollingTree.cpp:
     30        (WebCore::ThreadedScrollingTree::scrollingTreeNodeDidScroll):
     31
    1322019-04-03  Youenn Fablet  <youenn@apple.com>
    233

  • trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp

    r243539 r243855  
    213213        return false;
    214214
    215     bool isProgrammaticScroll = frameView.inProgrammaticScroll();
    216     if (isProgrammaticScroll || frameView.frame().document()->pageCacheState() != Document::NotInPageCache)
    217         updateScrollPositionAfterAsyncScroll(frameView.scrollingNodeID(), scrollPosition, WTF::nullopt, isProgrammaticScroll, ScrollingLayerPositionAction::Set);
     215    bool inPageCache = frameView.frame().document()->pageCacheState() != Document::NotInPageCache;
     216    bool inProgrammaticScroll = frameView.inProgrammaticScroll();
     217    if (inProgrammaticScroll || inPageCache)
     218        updateScrollPositionAfterAsyncScroll(frameView.scrollingNodeID(), scrollPosition, { }, ScrollType::Programmatic, ScrollingLayerPositionAction::Set);
    218219
    219220    // If this frame view's document is being put into the page cache, we don't want to update our
    220221    // main frame scroll position. Just let the FrameView think that we did.
    221     if (frameView.frame().document()->pageCacheState() != Document::NotInPageCache)
     222    if (inPageCache)
    222223        return true;
    223224
    … …  
    226227        return false;
    227228
    228     stateNode->setRequestedScrollPosition(scrollPosition, isProgrammaticScroll);
     229    stateNode->setRequestedScrollPosition(scrollPosition, inProgrammaticScroll);
    229230    return true;
    230231}
    … …  
    235236}
    236237
    237 void AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll(ScrollingNodeID nodeID, const FloatPoint& scrollPosition, const Optional<FloatPoint>& layoutViewportOrigin, bool programmaticScroll, ScrollingLayerPositionAction scrollingLayerPositionAction)
    238 {
    239     ScheduledScrollUpdate scrollUpdate(nodeID, scrollPosition, layoutViewportOrigin, programmaticScroll, scrollingLayerPositionAction);
    240    
    241     // For programmatic scrolls, requestScrollPositionUpdate() has already called updateScrollPositionAfterAsyncScroll().
    242     if (programmaticScroll)
    243         return;
    244 
     238void AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll(ScrollingNodeID nodeID, const FloatPoint& scrollPosition, const Optional<FloatPoint>& layoutViewportOrigin, ScrollingLayerPositionAction scrollingLayerPositionAction)
     239{
     240    ScheduledScrollUpdate scrollUpdate(nodeID, scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction);
     241   
    245242    if (m_updateNodeScrollPositionTimer.isActive()) {
    246243        if (m_scheduledScrollUpdate.matchesUpdateType(scrollUpdate)) {
    … …  
    252249        // If the parameters don't match what was previously scheduled, dispatch immediately.
    253250        m_updateNodeScrollPositionTimer.stop();
    254         updateScrollPositionAfterAsyncScroll(m_scheduledScrollUpdate.nodeID, m_scheduledScrollUpdate.scrollPosition, m_scheduledScrollUpdate.layoutViewportOrigin, m_scheduledScrollUpdate.isProgrammaticScroll, m_scheduledScrollUpdate.updateLayerPositionAction);
    255         updateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, layoutViewportOrigin, programmaticScroll, scrollingLayerPositionAction);
     251        updateScrollPositionAfterAsyncScroll(m_scheduledScrollUpdate.nodeID, m_scheduledScrollUpdate.scrollPosition, m_scheduledScrollUpdate.layoutViewportOrigin, ScrollType::User, m_scheduledScrollUpdate.updateLayerPositionAction);
     252        updateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, layoutViewportOrigin, ScrollType::User, scrollingLayerPositionAction);
    256253        return;
    257254    }
    … …  
    263260void AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScrollTimerFired()
    264261{
    265     updateScrollPositionAfterAsyncScroll(m_scheduledScrollUpdate.nodeID, m_scheduledScrollUpdate.scrollPosition, m_scheduledScrollUpdate.layoutViewportOrigin, m_scheduledScrollUpdate.isProgrammaticScroll, m_scheduledScrollUpdate.updateLayerPositionAction);
     262    updateScrollPositionAfterAsyncScroll(m_scheduledScrollUpdate.nodeID, m_scheduledScrollUpdate.scrollPosition, m_scheduledScrollUpdate.layoutViewportOrigin, ScrollType::User, m_scheduledScrollUpdate.updateLayerPositionAction);
    266263}
    267264
    … …  
    298295}
    299296
    300 void AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll(ScrollingNodeID scrollingNodeID, const FloatPoint& scrollPosition, Optional<FloatPoint> layoutViewportOrigin, bool programmaticScroll, ScrollingLayerPositionAction scrollingLayerPositionAction)
     297void AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll(ScrollingNodeID scrollingNodeID, const FloatPoint& scrollPosition, Optional<FloatPoint> layoutViewportOrigin, ScrollType scrollType, ScrollingLayerPositionAction scrollingLayerPositionAction)
    301298{
    302299    ASSERT(isMainThread());
    … …  
    314311
    315312    if (scrollingNodeID == frameView.scrollingNodeID()) {
    316         reconcileScrollingState(frameView, scrollPosition, layoutViewportOrigin, programmaticScroll, ViewportRectStability::Stable, scrollingLayerPositionAction);
     313        reconcileScrollingState(frameView, scrollPosition, layoutViewportOrigin, scrollType, ViewportRectStability::Stable, scrollingLayerPositionAction);
    317314
    318315#if PLATFORM(COCOA)
    … …  
    345342}
    346343
    347 void AsyncScrollingCoordinator::reconcileScrollingState(FrameView& frameView, const FloatPoint& scrollPosition, const LayoutViewportOriginOrOverrideRect& layoutViewportOriginOrOverrideRect, bool programmaticScroll, ViewportRectStability viewportRectStability, ScrollingLayerPositionAction scrollingLayerPositionAction)
     344void AsyncScrollingCoordinator::reconcileScrollingState(FrameView& frameView, const FloatPoint& scrollPosition, const LayoutViewportOriginOrOverrideRect& layoutViewportOriginOrOverrideRect, ScrollType scrollType, ViewportRectStability viewportRectStability, ScrollingLayerPositionAction scrollingLayerPositionAction)
    348345{
    349346    bool oldProgrammaticScroll = frameView.inProgrammaticScroll();
    350     frameView.setInProgrammaticScroll(programmaticScroll);
    351 
    352     LOG_WITH_STREAM(Scrolling, stream << getCurrentProcessID() << " AsyncScrollingCoordinator " << this << " reconcileScrollingState scrollPosition " << scrollPosition << " programmaticScroll " << programmaticScroll << " stability " << viewportRectStability << " " << scrollingLayerPositionAction);
     347    frameView.setInProgrammaticScroll(scrollType == ScrollType::Programmatic);
     348
     349    LOG_WITH_STREAM(Scrolling, stream << getCurrentProcessID() << " AsyncScrollingCoordinator " << this << " reconcileScrollingState scrollPosition " << scrollPosition << " type " << scrollType << " stability " << viewportRectStability << " " << scrollingLayerPositionAction);
    353350
    354351    Optional<FloatRect> layoutViewportRect;
    … …  
    373370    frameView.setInProgrammaticScroll(oldProgrammaticScroll);
    374371
    375     if (!programmaticScroll && scrollingLayerPositionAction != ScrollingLayerPositionAction::Set) {
     372    if (scrollType == ScrollType::User && scrollingLayerPositionAction != ScrollingLayerPositionAction::Set) {
    376373        auto scrollingNodeID = frameView.scrollingNodeID();
    377374        if (viewportRectStability == ViewportRectStability::Stable)
    … …  
    404401        FrameView::yPositionForFooterLayer(scrollPosition, topContentInset, frameView.totalContentsSize().height(), frameView.footerHeight()));
    405402
    406     if (programmaticScroll || scrollingLayerPositionAction == ScrollingLayerPositionAction::Set) {
     403    if (scrollType == ScrollType::Programmatic || scrollingLayerPositionAction == ScrollingLayerPositionAction::Set) {
    407404        reconcileScrollPosition(frameView, ScrollingLayerPositionAction::Set);
    408405

  • trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h

    r242913 r243855  
    5252    void scrollingStateTreePropertiesChanged();
    5353
    54     WEBCORE_EXPORT void scheduleUpdateScrollPositionAfterAsyncScroll(ScrollingNodeID, const FloatPoint&, const Optional<FloatPoint>& layoutViewportOrigin, bool programmaticScroll, ScrollingLayerPositionAction);
     54    WEBCORE_EXPORT void scheduleUpdateScrollPositionAfterAsyncScroll(ScrollingNodeID, const FloatPoint&, const Optional<FloatPoint>& layoutViewportOrigin, ScrollingLayerPositionAction);
    5555
    5656#if PLATFORM(COCOA)
    … …  
    7878    RefPtr<ScrollingTree> releaseScrollingTree() { return WTFMove(m_scrollingTree); }
    7979
    80     void updateScrollPositionAfterAsyncScroll(ScrollingNodeID, const FloatPoint&, Optional<FloatPoint> layoutViewportOrigin, bool programmaticScroll, ScrollingLayerPositionAction);
     80    void updateScrollPositionAfterAsyncScroll(ScrollingNodeID, const FloatPoint&, Optional<FloatPoint> layoutViewportOrigin, ScrollType, ScrollingLayerPositionAction);
    8181
    8282    WEBCORE_EXPORT String scrollingStateTreeAsText(ScrollingStateTreeAsTextBehavior = ScrollingStateTreeAsTextBehaviorNormal) const override;
    … …  
    119119    WEBCORE_EXPORT void setRelatedOverflowScrollingNodes(ScrollingNodeID, Vector<ScrollingNodeID>&&) override;
    120120
    121     WEBCORE_EXPORT void reconcileScrollingState(FrameView&, const FloatPoint&, const LayoutViewportOriginOrOverrideRect&, bool programmaticScroll, ViewportRectStability, ScrollingLayerPositionAction) override;
     121    WEBCORE_EXPORT void reconcileScrollingState(FrameView&, const FloatPoint&, const LayoutViewportOriginOrOverrideRect&, ScrollType, ViewportRectStability, ScrollingLayerPositionAction) override;
    122122    void reconcileScrollPosition(FrameView&, ScrollingLayerPositionAction);
    123123
    … …  
    148148    struct ScheduledScrollUpdate {
    149149        ScheduledScrollUpdate() = default;
    150         ScheduledScrollUpdate(ScrollingNodeID scrollingNodeID, FloatPoint point, Optional<FloatPoint> viewportOrigin, bool isProgrammatic, ScrollingLayerPositionAction udpateAction)
     150        ScheduledScrollUpdate(ScrollingNodeID scrollingNodeID, FloatPoint point, Optional<FloatPoint> viewportOrigin, ScrollingLayerPositionAction udpateAction)
    151151            : nodeID(scrollingNodeID)
    152152            , scrollPosition(point)
    153153            , layoutViewportOrigin(viewportOrigin)
    154             , isProgrammaticScroll(isProgrammatic)
    155154            , updateLayerPositionAction(udpateAction)
    156155        { }
    … …  
    159158        FloatPoint scrollPosition;
    160159        Optional<FloatPoint> layoutViewportOrigin;
    161         bool isProgrammaticScroll { false };
    162160        ScrollingLayerPositionAction updateLayerPositionAction { ScrollingLayerPositionAction::Sync };
    163161       
    164162        bool matchesUpdateType(const ScheduledScrollUpdate& other) const
    165163        {
    166             return nodeID == other.nodeID
    167                 && isProgrammaticScroll == other.isProgrammaticScroll
    168                 && updateLayerPositionAction == other.updateLayerPositionAction;
     164            return nodeID == other.nodeID && updateLayerPositionAction == other.updateLayerPositionAction;
    169165        }
    170166    };

  • trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp

    r243416 r243855  
    510510}
    511511
     512TextStream& operator<<(TextStream& ts, ScrollType scrollType)
     513{
     514    switch (scrollType) {
     515    case ScrollType::User: ts << "user"; break;
     516    case ScrollType::Programmatic: ts << "programmatic"; break;
     517    }
     518    return ts;
     519}
     520
    512521} // namespace WebCore

  • trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h

    r242913 r243855  
    8888
    8989    using LayoutViewportOriginOrOverrideRect = WTF::Variant<Optional<FloatPoint>, Optional<FloatRect>>;
    90     virtual void reconcileScrollingState(FrameView&, const FloatPoint&, const LayoutViewportOriginOrOverrideRect&, bool /* programmaticScroll */, ViewportRectStability, ScrollingLayerPositionAction) { }
     90    virtual void reconcileScrollingState(FrameView&, const FloatPoint&, const LayoutViewportOriginOrOverrideRect&, ScrollType, ViewportRectStability, ScrollingLayerPositionAction) { }
    9191
    9292    // Should be called whenever the slow repaint objects counter changes between zero and one.
    … …  
    215215WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, ScrollingLayerPositionAction);
    216216WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, ViewportRectStability);
     217WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, ScrollType);
    217218
    218219} // namespace WebCore

  • trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp

    r243607 r243855  
    104104        setMainFrameScrollPosition(scrollPosition);
    105105
     106    if (isHandlingProgrammaticScroll())
     107        return;
     108
    106109    Optional<FloatPoint> layoutViewportOrigin;
    107110    if (is<ScrollingTreeFrameScrollingNode>(node))
    108111        layoutViewportOrigin = downcast<ScrollingTreeFrameScrollingNode>(node).layoutViewport().location();
    109112
    110     RunLoop::main().dispatch([scrollingCoordinator = m_scrollingCoordinator, nodeID = node.scrollingNodeID(), scrollPosition, layoutViewportOrigin, localIsHandlingProgrammaticScroll = isHandlingProgrammaticScroll(), scrollingLayerPositionAction] {
    111         scrollingCoordinator->scheduleUpdateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, layoutViewportOrigin, localIsHandlingProgrammaticScroll, scrollingLayerPositionAction);
     113    RunLoop::main().dispatch([scrollingCoordinator = m_scrollingCoordinator, nodeID = node.scrollingNodeID(), scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction] {
     114        scrollingCoordinator->scheduleUpdateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction);
    112115    });
    113116}

  • trunk/Source/WebKit/ChangeLog

    r243848 r243855  
     12019-04-03  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Simplify some "programmaticScroll" code paths
     4        https://bugs.webkit.org/show_bug.cgi?id=196589
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        * WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm: Remove the parameter.
     9        (WebKit::RemoteScrollingCoordinator::scrollPositionChangedForNode): Use the enum type.
     10        * WebProcess/WebPage/ios/WebPageIOS.mm:
     11        (WebKit::WebPage::updateVisibleContentRects):
     12
    1132019-04-03  Chris Dumez  <cdumez@apple.com>
    214

  • trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm

    r239427 r243855  
    9797void RemoteScrollingCoordinator::scrollPositionChangedForNode(ScrollingNodeID nodeID, const FloatPoint& scrollPosition, bool syncLayerPosition)
    9898{
    99     scheduleUpdateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, WTF::nullopt, false /* FIXME */, syncLayerPosition ? ScrollingLayerPositionAction::Sync : ScrollingLayerPositionAction::Set);
     99    scheduleUpdateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, WTF::nullopt, syncLayerPosition ? ScrollingLayerPositionAction::Sync : ScrollingLayerPositionAction::Set);
    100100}
    101101

  • trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

    r243798 r243855  
    32483248            layerAction = ScrollingLayerPositionAction::SetApproximate;
    32493249        }
    3250         scrollingCoordinator->reconcileScrollingState(frameView, scrollPosition, visibleContentRectUpdateInfo.customFixedPositionRect(), false, viewportStability, layerAction);
     3250        scrollingCoordinator->reconcileScrollingState(frameView, scrollPosition, visibleContentRectUpdateInfo.customFixedPositionRect(), ScrollType::User, viewportStability, layerAction);
    32513251    }
    32523252}
Note: See TracChangeset for help on using the changeset viewer.