Changeset 264190 in webkit


Ignore:
Timestamp:
Jul 9, 2020, 1:01:53 PM (5 years ago)
Author:
Simon Fraser
Message:

After a scroll gesture, content changes don't trigger re-snapping with scroll snap
​https://bugs.webkit.org/show_bug.cgi?id=214123

Reviewed by Wenson Hsieh.

Source/WebCore:

ScrollController::m_inScrollGesture could get stuck as true after a scroll gesture, because
ScrollAnimatorMac::handleWheelEvent() didn't reliably call m_scrollController.handleWheelEvent() for
the event event based on the hokey shouldForwardWheelEventsToParent() code.

Fix by explicitly calling m_scrollController.updateGestureInProgressState() which maintains
m_inScrollGesture.

When m_inScrollGesture was stuck as true, ScrollableArea::updateScrollSnapState() would return early
because isScrollSnapInProgress() would return true. In addition,
ScrollController::isScrollSnapInProgress() was a lie for non-scroll-snapping scrollable areas, so
explicitly check for snap points there, using an explicit usesScrollSnap() function.

Rename some WheelEventStatus values to use "momentum" rather than "inertia".

Test: fast/scrolling/mac/adjust-scroll-snap-after-gesture.html

  • page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm:

(WebCore::ScrollingTreeScrollingNodeDelegateMac::handleWheelEvent):

  • platform/ScrollableArea.cpp:

(WebCore::ScrollableArea::usesScrollSnap const):
(WebCore::ScrollableArea::updateScrollSnapState):

  • platform/ScrollableArea.h:
  • platform/cocoa/ScrollController.h:
  • platform/cocoa/ScrollController.mm:

(WebCore::ScrollController::handleWheelEvent):
(WebCore::ScrollController::usesScrollSnap const):
(WebCore::ScrollController::isScrollSnapInProgress const):
(WebCore::ScrollController::snapRubberBand):
(WebCore::toWheelEventStatus):
(WebCore::operator<<):
(WebCore::ScrollController::shouldOverrideMomentumScrolling const):
(WebCore::ScrollController::scheduleStatelessScrollSnap):
(WebCore::ScrollController::statelessSnapTransitionTimerFired):
(WebCore::ScrollController::processWheelEventForScrollSnap):
(WebCore::ScrollController::updateGestureInProgressState):
(WebCore::ScrollController::scrollSnapTimerFired):
(WebCore::ScrollController::activeScrollSnapIndexForAxis const):
(WebCore::ScrollController::setActiveScrollSnapIndexForAxis):
(WebCore::ScrollController::setNearestScrollSnapIndexForAxisAndOffset):
(WebCore::ScrollController::setActiveScrollSnapIndicesForOffset):
(WebCore::ScrollController::shouldOverrideInertialScrolling const): Deleted.

  • platform/mac/ScrollAnimatorMac.mm:

(WebCore::ScrollAnimatorMac::handleWheelEvent):

LayoutTests:

  • fast/scrolling/mac/adjust-scroll-snap-after-gesture-expected.txt: Added.
  • fast/scrolling/mac/adjust-scroll-snap-after-gesture.html: Added.
Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r264187 r264190  
     12020-07-09  Simon Fraser  <simon.fraser@apple.com>
     2
     3        After a scroll gesture, content changes don't trigger re-snapping with scroll snap
     4        https://bugs.webkit.org/show_bug.cgi?id=214123
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        * fast/scrolling/mac/adjust-scroll-snap-after-gesture-expected.txt: Added.
     9        * fast/scrolling/mac/adjust-scroll-snap-after-gesture.html: Added.
     10
    1112020-07-09  Wenson Hsieh  <wenson_hsieh@apple.com>
    212

  • trunk/Source/WebCore/ChangeLog

    r264188 r264190  
     12020-07-09  Simon Fraser  <simon.fraser@apple.com>
     2
     3        After a scroll gesture, content changes don't trigger re-snapping with scroll snap
     4        https://bugs.webkit.org/show_bug.cgi?id=214123
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        ScrollController::m_inScrollGesture could get stuck as true after a scroll gesture, because
     9        ScrollAnimatorMac::handleWheelEvent() didn't reliably call m_scrollController.handleWheelEvent() for
     10        the event event based on the hokey shouldForwardWheelEventsToParent() code.
     11
     12        Fix by explicitly calling m_scrollController.updateGestureInProgressState() which maintains
     13        m_inScrollGesture.
     14
     15        When m_inScrollGesture was stuck as true, ScrollableArea::updateScrollSnapState() would return early
     16        because isScrollSnapInProgress() would return true. In addition,
     17        ScrollController::isScrollSnapInProgress() was a lie for non-scroll-snapping scrollable areas, so
     18        explicitly check for snap points there, using an explicit usesScrollSnap() function.
     19
     20        Rename some WheelEventStatus values to use "momentum" rather than "inertia".
     21
     22        Test: fast/scrolling/mac/adjust-scroll-snap-after-gesture.html
     23
     24        * page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm:
     25        (WebCore::ScrollingTreeScrollingNodeDelegateMac::handleWheelEvent):
     26        * platform/ScrollableArea.cpp:
     27        (WebCore::ScrollableArea::usesScrollSnap const):
     28        (WebCore::ScrollableArea::updateScrollSnapState):
     29        * platform/ScrollableArea.h:
     30        * platform/cocoa/ScrollController.h:
     31        * platform/cocoa/ScrollController.mm:
     32        (WebCore::ScrollController::handleWheelEvent):
     33        (WebCore::ScrollController::usesScrollSnap const):
     34        (WebCore::ScrollController::isScrollSnapInProgress const):
     35        (WebCore::ScrollController::snapRubberBand):
     36        (WebCore::toWheelEventStatus):
     37        (WebCore::operator<<):
     38        (WebCore::ScrollController::shouldOverrideMomentumScrolling const):
     39        (WebCore::ScrollController::scheduleStatelessScrollSnap):
     40        (WebCore::ScrollController::statelessSnapTransitionTimerFired):
     41        (WebCore::ScrollController::processWheelEventForScrollSnap):
     42        (WebCore::ScrollController::updateGestureInProgressState):
     43        (WebCore::ScrollController::scrollSnapTimerFired):
     44        (WebCore::ScrollController::activeScrollSnapIndexForAxis const):
     45        (WebCore::ScrollController::setActiveScrollSnapIndexForAxis):
     46        (WebCore::ScrollController::setNearestScrollSnapIndexForAxisAndOffset):
     47        (WebCore::ScrollController::setActiveScrollSnapIndicesForOffset):
     48        (WebCore::ScrollController::shouldOverrideInertialScrolling const): Deleted.
     49        * platform/mac/ScrollAnimatorMac.mm:
     50        (WebCore::ScrollAnimatorMac::handleWheelEvent):
     51
    1522020-07-09  Fujii Hironori  <Hironori.Fujii@sony.com>
    253

  • trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm

    r264008 r264190  
    140140#endif
    141141
     142    m_scrollController.updateGestureInProgressState(wheelEvent);
     143
    142144    // PlatformWheelEventPhaseMayBegin fires when two fingers touch the trackpad, and is used to flash overlay scrollbars.
    143145    // We know we're scrollable at this point, so handle the event.

  • trunk/Source/WebCore/platform/ScrollableArea.cpp

    r264151 r264190  
    537537}
    538538
     539bool ScrollableArea::usesScrollSnap() const
     540{
     541    return !!m_snapOffsetsInfo;
     542}
     543
    539544IntPoint ScrollableArea::nearestActiveSnapPoint(const IntPoint& currentPosition)
    540545{
    … …  
    571576        scrollAnimator->updateScrollSnapState();
    572577
     578    if (!usesScrollSnap())
     579        return;
     580
    573581    LOG_WITH_STREAM(ScrollSnap, stream << *this << " updateScrollSnapState: isScrollSnapInProgress " << isScrollSnapInProgress() << " isUserScrollInProgress " << isUserScrollInProgress());
    574582

  • trunk/Source/WebCore/platform/ScrollableArea.h

    r264151 r264190  
    8989    WEBCORE_EXPORT bool handleWheelEvent(const PlatformWheelEvent&);
    9090
     91    bool usesScrollSnap() const;
     92
    9193#if ENABLE(CSS_SCROLL_SNAP)
    9294    WEBCORE_EXPORT const Vector<LayoutUnit>* horizontalSnapOffsets() const;

  • trunk/Source/WebCore/platform/cocoa/ScrollController.h

    r264151 r264190  
    120120    UserScrolling,
    121121    UserScrollEnd,
    122     InertialScrollBegin,
    123     InertialScrolling,
    124     InertialScrollEnd,
     122    MomentumScrollBegin,
     123    MomentumScrolling,
     124    MomentumScrollEnd,
    125125    StatelessScrollEvent,
    126126    Unknown
    … …  
    137137    bool handleWheelEvent(const PlatformWheelEvent&);
    138138#endif
     139
     140    bool usesScrollSnap() const;
    139141
    140142    bool isUserScrollInProgress() const;
    … …  
    150152    unsigned activeScrollSnapIndexForAxis(ScrollEventAxis) const;
    151153    void updateScrollSnapState(const ScrollableArea&);
     154
     155    void updateGestureInProgressState(const PlatformWheelEvent&);
     156
    152157#if PLATFORM(MAC)
    153158    bool processWheelEventForScrollSnap(const PlatformWheelEvent&);
    … …  
    180185    void stopScrollSnapTimer();
    181186
    182     bool shouldOverrideInertialScrolling() const;
     187    bool shouldOverrideMomentumScrolling() const;
    183188    void statelessSnapTransitionTimerFired();
    184189    void scheduleStatelessScrollSnap();

  • trunk/Source/WebCore/platform/cocoa/ScrollController.mm

    r264151 r264190  
    138138            return false;
    139139
    140         m_inScrollGesture = true;
    141140        m_momentumScrollInProgress = false;
    142141        m_ignoreMomentumScrolls = false;
    … …  
    456455#endif
    457456
     457bool ScrollController::usesScrollSnap() const
     458{
     459#if ENABLE(CSS_SCROLL_SNAP)
     460    return !!m_scrollSnapState;
     461#else
     462    return false;
     463#endif
     464}
     465
    458466bool ScrollController::isUserScrollInProgress() const
    459467{
    … …  
    479487bool ScrollController::isScrollSnapInProgress() const
    480488{
     489    if (!usesScrollSnap())
     490        return false;
     491
    481492#if ENABLE(CSS_SCROLL_SNAP) && PLATFORM(MAC)
    482493    if (m_inScrollGesture || m_momentumScrollInProgress || m_scrollSnapTimer)
    … …  
    518529        m_momentumVelocity = { };
    519530
    520     m_inScrollGesture = false;
    521 
    522531    if (m_snapRubberbandTimer)
    523532        return;
    … …  
    554563        switch (momentumPhase) {
    555564        case PlatformWheelEventPhaseBegan:
    556             return WheelEventStatus::InertialScrollBegin;
     565            return WheelEventStatus::MomentumScrollBegin;
    557566               
    558567        case PlatformWheelEventPhaseChanged:
    559             return WheelEventStatus::InertialScrolling;
     568            return WheelEventStatus::MomentumScrolling;
    560569               
    561570        case PlatformWheelEventPhaseEnded:
    562             return WheelEventStatus::InertialScrollEnd;
     571            return WheelEventStatus::MomentumScrollEnd;
    563572
    564573        case PlatformWheelEventPhaseNone:
    … …  
    596605    case WheelEventStatus::UserScrolling: ts << "UserScrolling"; break;
    597606    case WheelEventStatus::UserScrollEnd: ts << "UserScrollEnd"; break;
    598     case WheelEventStatus::InertialScrollBegin: ts << "InertialScrollBegin"; break;
    599     case WheelEventStatus::InertialScrolling: ts << "InertialScrolling"; break;
    600     case WheelEventStatus::InertialScrollEnd: ts << "InertialScrollEnd"; break;
     607    case WheelEventStatus::MomentumScrollBegin: ts << "MomentumScrollBegin"; break;
     608    case WheelEventStatus::MomentumScrolling: ts << "MomentumScrolling"; break;
     609    case WheelEventStatus::MomentumScrollEnd: ts << "MomentumScrollEnd"; break;
    601610    case WheelEventStatus::StatelessScrollEvent: ts << "StatelessScrollEvent"; break;
    602611    case WheelEventStatus::Unknown: ts << "Unknown"; break;
    … …  
    606615#endif
    607616
    608 bool ScrollController::shouldOverrideInertialScrolling() const
    609 {
    610     if (!m_scrollSnapState)
     617bool ScrollController::shouldOverrideMomentumScrolling() const
     618{
     619    if (!usesScrollSnap())
    611620        return false;
    612621
    … …  
    622631        m_statelessSnapTransitionTimer = nullptr;
    623632    }
    624     if (!m_scrollSnapState)
     633    if (!usesScrollSnap())
    625634        return;
    626635
    … …  
    637646    m_statelessSnapTransitionTimer = nullptr;
    638647
    639     if (!m_scrollSnapState)
     648    if (!usesScrollSnap())
    640649        return;
    641650
    … …  
    656665bool ScrollController::processWheelEventForScrollSnap(const PlatformWheelEvent& wheelEvent)
    657666{
    658     if (!m_scrollSnapState)
     667    if (!usesScrollSnap())
    659668        return true;
    660669
    … …  
    662671        return true;
    663672
    664     WheelEventStatus status = toWheelEventStatus(wheelEvent.phase(), wheelEvent.momentumPhase());
     673    auto status = toWheelEventStatus(wheelEvent.phase(), wheelEvent.momentumPhase());
    665674
    666675    LOG_WITH_STREAM(ScrollSnap, stream << "ScrollController " << this << " processWheelEventForScrollSnap: status " << status);
    667676
    668     bool isInertialScrolling = false;
     677    bool isMomentumScrolling = false;
    669678    switch (status) {
    670679    case WheelEventStatus::UserScrollBegin:
    … …  
    678687        startScrollSnapTimer();
    679688        break;
    680     case WheelEventStatus::InertialScrollBegin:
     689    case WheelEventStatus::MomentumScrollBegin:
    681690        m_scrollSnapState->transitionToGlideAnimationState(m_client.scrollExtent(), m_client.viewportSize(), m_client.pageScaleFactor(), m_client.scrollOffset(), m_dragEndedScrollingVelocity, FloatSize(-wheelEvent.deltaX(), -wheelEvent.deltaY()));
    682691        m_dragEndedScrollingVelocity = { };
    683         isInertialScrolling = true;
     692        isMomentumScrolling = true;
    684693        break;
    685     case WheelEventStatus::InertialScrolling:
    686     case WheelEventStatus::InertialScrollEnd:
    687         isInertialScrolling = true;
     694    case WheelEventStatus::MomentumScrolling:
     695    case WheelEventStatus::MomentumScrollEnd:
     696        isMomentumScrolling = true;
    688697        break;
    689698    case WheelEventStatus::StatelessScrollEvent:
    … …  
    696705    }
    697706
    698     return !(isInertialScrolling && shouldOverrideInertialScrolling());
     707    return !(isMomentumScrolling && shouldOverrideMomentumScrolling());
     708}
     709
     710void ScrollController::updateGestureInProgressState(const PlatformWheelEvent& wheelEvent)
     711{
     712    if (wheelEvent.isGestureBegin() || wheelEvent.isTransitioningToMomentumScroll())
     713        m_inScrollGesture = true;
     714    else if (wheelEvent.isEndOfNonMomentumScroll() || wheelEvent.isGestureCancel() || wheelEvent.isEndOfMomentumScroll())
     715        m_inScrollGesture = false;
    699716}
    700717
    … …  
    730747void ScrollController::scrollSnapTimerFired()
    731748{
    732     if (!m_scrollSnapState) {
     749    if (!usesScrollSnap()) {
    733750        ASSERT_NOT_REACHED();
    734751        return;
    … …  
    787804unsigned ScrollController::activeScrollSnapIndexForAxis(ScrollEventAxis axis) const
    788805{
    789     if (!m_scrollSnapState)
     806    if (!usesScrollSnap())
    790807        return 0;
    791808
    … …  
    795812void ScrollController::setActiveScrollSnapIndexForAxis(ScrollEventAxis axis, unsigned index)
    796813{
    797     if (!m_scrollSnapState)
     814    if (!usesScrollSnap())
    798815        return;
    799816
    … …  
    803820void ScrollController::setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis axis, int offset)
    804821{
    805     if (!m_scrollSnapState)
     822    if (!usesScrollSnap())
    806823        return;
    807824
    … …  
    827844void ScrollController::setActiveScrollSnapIndicesForOffset(int x, int y)
    828845{
    829     if (!m_scrollSnapState)
     846    if (!usesScrollSnap())
    830847        return;
    831848

  • trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm

    r264151 r264190  
    12731273        return ScrollAnimator::handleWheelEvent(wheelEvent);
    12741274
     1275    m_scrollController.updateGestureInProgressState(wheelEvent);
     1276
    12751277    // FIXME: This is somewhat roundabout hack to allow forwarding wheel events
    12761278    // up to the parent scrollable area. It takes advantage of the fact that
Note: See TracChangeset for help on using the changeset viewer.