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

Changeset 259805 in webkit


Ignore:
Timestamp:
Apr 9, 2020, 9:25:08 AM (6 years ago)
Author:
Simon Fraser
Message:

[Async overflow scroll] Horizontal scrolls can trigger unwanted back swipes
​https://bugs.webkit.org/show_bug.cgi?id=210095
<rdar://problem/61376245>

Reviewed by Tim Horton.

Source/WebCore:

With async overflow/frame scrolling, EventDispatcher::wheelEvent() can't immediately
determine whether the scrolling tree handled the scroll; we have to wait until the
event has been processed by the scrolling thread. To allow that, add a
ScrollingEventResult::SendToScrollingThread return value and a give tryToHandleWheelEvent()
a callback that's called when the scrolling thread is done with the event. EventDispatcher
uses that to send the "didReceiveEvent" with "handled" back to the UI process, which then
proceeds with history or reading list swipes.

Various fixes were necessary to correctly determine whether the event was handled.

ScrollingTreeFrameScrollingNodeMac::handleWheelEvent() didn't return an accurate ScrollingEventResult,
and ScrollController didn't return false in cases where rubber-banding was disabled (which broke navigation swipes
and reading list navigation).

Tests: scrollingcoordinator/mac/latching/main-frame-back-swipe.html

scrollingcoordinator/mac/latching/simple-page-rubberbands.html

  • page/scrolling/ScrollingCoordinatorTypes.h:
  • page/scrolling/ScrollingThread.h:
  • page/scrolling/ScrollingTree.cpp:

(WebCore::ScrollingTree::handleWheelEvent):
(WebCore::ScrollingTree::mainFrameCanRubberBandInDirection):

  • page/scrolling/ScrollingTree.h:
  • page/scrolling/ThreadedScrollingTree.cpp:

(WebCore::ThreadedScrollingTree::tryToHandleWheelEvent):

  • page/scrolling/ThreadedScrollingTree.h:
  • page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:

(WebCore::ScrollingTreeFrameScrollingNodeMac::handleWheelEvent):

  • page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm:

(WebCore::ScrollingTreeScrollingNodeDelegateMac::allowsHorizontalStretching const):
(WebCore::ScrollingTreeScrollingNodeDelegateMac::allowsVerticalStretching const):
(WebCore::ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection const):

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

(WebCore::ScrollController::handleWheelEvent):
(WebCore::ScrollController::wheelDeltaBiasingTowardsVertical):
(WebCore::ScrollController::directionFromEvent):
(WebCore::ScrollController::shouldRubberBandInHorizontalDirection const):
(WebCore::ScrollController::shouldRubberBandInDirection const):
(WebCore::ScrollController::shouldRubberBandInHorizontalDirection): Deleted.

Source/WebKit:

With async overflow/frame scrolling, EventDispatcher::wheelEvent() can't immediately
determine whether the scrolling tree handled the scroll; we have to wait until the
event has been processed by the scrolling thread. To allow that, add a
ScrollingEventResult::SendToScrollingThread return value and a give tryToHandleWheelEvent()
a callback that's called when the scrolling thread is done with the event. EventDispatcher
uses that to send the "didReceiveEvent" with "handled" back to the UI process, which then
proceeds with history or reading list swipes.

Various fixes were necessary to correctly determine whether the event was handled.

ScrollingTreeFrameScrollingNodeMac::handleWheelEvent() didn't return an accurate ScrollingEventResult,
and ScrollController didn't return false in cases where rubber-banding was disabled (which broke navigation swipes
and reading list navigation).

  • UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:

(WebKit::RemoteScrollingCoordinatorProxy::handleWheelEvent):

  • UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp:

(WebKit::RemoteScrollingTree::tryToHandleWheelEvent):

  • UIProcess/RemoteLayerTree/RemoteScrollingTree.h:
  • WebProcess/WebPage/EventDispatcher.cpp:

(WebKit::EventDispatcher::wheelEvent):
(WebKit::EventDispatcher::sendDidReceiveEvent):

  • WebProcess/WebPage/EventDispatcher.h:

LayoutTests:

Tests that rubberbanding works on a simple page, and that edge swipes work on a simple page.

Edge swipes inside overflow:scroll are still broken by latching and will be fixed later.

  • resources/ui-helper.js:

(window.UIHelper.async mouseWheelScrollAt):

  • scrollingcoordinator/mac/latching/main-frame-back-swipe-expected.txt: Added.
  • scrollingcoordinator/mac/latching/main-frame-back-swipe.html: Added.
  • scrollingcoordinator/mac/latching/simple-page-rubberbands-expected.txt: Added.
  • scrollingcoordinator/mac/latching/simple-page-rubberbands.html: Added.
Location:
trunk
Files:
5 added
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r259802 r259805  
     12020-04-08  Simon Fraser  <simon.fraser@apple.com>
     2
     3        [Async overflow scroll] Horizontal scrolls can trigger unwanted back swipes
     4        https://bugs.webkit.org/show_bug.cgi?id=210095
     5        <rdar://problem/61376245>
     6
     7        Reviewed by Tim Horton.
     8
     9        Tests that rubberbanding works on a simple page, and that edge swipes work on a simple page.
     10
     11        Edge swipes inside overflow:scroll are still broken by latching and will be fixed later.
     12
     13        * resources/ui-helper.js:
     14        (window.UIHelper.async mouseWheelScrollAt):
     15        * scrollingcoordinator/mac/latching/main-frame-back-swipe-expected.txt: Added.
     16        * scrollingcoordinator/mac/latching/main-frame-back-swipe.html: Added.
     17        * scrollingcoordinator/mac/latching/simple-page-rubberbands-expected.txt: Added.
     18        * scrollingcoordinator/mac/latching/simple-page-rubberbands.html: Added.
     19
    1202020-04-09  Youenn Fablet  <youenn@apple.com>
    221

  • trunk/LayoutTests/resources/ui-helper.js

    r259762 r259805  
    2929        eventSender.mouseUp();
    3030    }
    31    
    32     static async mouseWheelScrollAt(x, y)
    33     {
     31
     32    static async mouseWheelScrollAt(x, y, beginX, beginY, deltaX, deltaY)
     33    {
     34        if (beginX === undefined)
     35            beginX = 0;
     36        if (beginY === undefined)
     37            beginY = -1;
     38
     39        if (deltaX === undefined)
     40            deltaX = 0;
     41        if (deltaY === undefined)
     42            deltaY = -10;
     43
    3444        eventSender.monitorWheelEvents();
    3545        eventSender.mouseMoveTo(x, y);
    36         eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "began", "none");
    37         eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -10, "changed", "none");
     46        eventSender.mouseScrollByWithWheelAndMomentumPhases(beginX, beginY, "began", "none");
     47        eventSender.mouseScrollByWithWheelAndMomentumPhases(deltaX, deltaY, "changed", "none");
    3848        eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
    3949        return new Promise(resolve => {

  • trunk/Source/WebCore/ChangeLog

    r259804 r259805  
     12020-04-08  Simon Fraser  <simon.fraser@apple.com>
     2
     3        [Async overflow scroll] Horizontal scrolls can trigger unwanted back swipes
     4        https://bugs.webkit.org/show_bug.cgi?id=210095
     5        <rdar://problem/61376245>
     6
     7        Reviewed by Tim Horton.
     8
     9        With async overflow/frame scrolling, EventDispatcher::wheelEvent() can't immediately
     10        determine whether the scrolling tree handled the scroll; we have to wait until the
     11        event has been processed by the scrolling thread. To allow that, add a
     12        ScrollingEventResult::SendToScrollingThread return value and a give tryToHandleWheelEvent()
     13        a callback that's called when the scrolling thread is done with the event. EventDispatcher
     14        uses that to send the "didReceiveEvent" with "handled" back to the UI process, which then
     15        proceeds with history or reading list swipes.
     16
     17        Various fixes were necessary to correctly determine whether the event was handled.
     18       
     19        ScrollingTreeFrameScrollingNodeMac::handleWheelEvent() didn't return an accurate ScrollingEventResult,
     20        and ScrollController didn't return false in cases where rubber-banding was disabled (which broke navigation swipes
     21        and reading list navigation).
     22
     23        Tests: scrollingcoordinator/mac/latching/main-frame-back-swipe.html
     24               scrollingcoordinator/mac/latching/simple-page-rubberbands.html
     25
     26        * page/scrolling/ScrollingCoordinatorTypes.h:
     27        * page/scrolling/ScrollingThread.h:
     28        * page/scrolling/ScrollingTree.cpp:
     29        (WebCore::ScrollingTree::handleWheelEvent):
     30        (WebCore::ScrollingTree::mainFrameCanRubberBandInDirection):
     31        * page/scrolling/ScrollingTree.h:
     32        * page/scrolling/ThreadedScrollingTree.cpp:
     33        (WebCore::ThreadedScrollingTree::tryToHandleWheelEvent):
     34        * page/scrolling/ThreadedScrollingTree.h:
     35        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
     36        (WebCore::ScrollingTreeFrameScrollingNodeMac::handleWheelEvent):
     37        * page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm:
     38        (WebCore::ScrollingTreeScrollingNodeDelegateMac::allowsHorizontalStretching const):
     39        (WebCore::ScrollingTreeScrollingNodeDelegateMac::allowsVerticalStretching const):
     40        (WebCore::ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection const):
     41        * platform/cocoa/ScrollController.h:
     42        * platform/cocoa/ScrollController.mm:
     43        (WebCore::ScrollController::handleWheelEvent):
     44        (WebCore::ScrollController::wheelDeltaBiasingTowardsVertical):
     45        (WebCore::ScrollController::directionFromEvent):
     46        (WebCore::ScrollController::shouldRubberBandInHorizontalDirection const):
     47        (WebCore::ScrollController::shouldRubberBandInDirection const):
     48        (WebCore::ScrollController::shouldRubberBandInHorizontalDirection): Deleted.
     49
    1502020-04-09  Alex Christensen  <achristensen@webkit.org>
    251

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

    r257996 r259805  
    9696    DidNotHandleEvent,
    9797    DidHandleEvent,
    98     SendToMainThread
     98    SendToScrollingThread,
     99    SendToMainThread,
    99100};
    100101

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

    r230905 r259805  
    4949
    5050public:
    51     static bool isCurrentThread();
     51    WEBCORE_EXPORT static bool isCurrentThread();
    5252    WEBCORE_EXPORT static void dispatch(Function<void ()>&&);
    5353

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

    r259672 r259805  
    102102    if (!asyncFrameOrOverflowScrollingEnabled()) {
    103103        if (m_rootNode)
    104             m_rootNode->handleWheelEvent(wheelEvent);
     104            return m_rootNode->handleWheelEvent(wheelEvent);
     105
    105106        return ScrollingEventResult::DidNotHandleEvent;
    106107    }
    … …  
    396397
    397398    m_swipeState.canRubberBand = canRubberBand;
     399}
     400
     401bool ScrollingTree::mainFrameCanRubberBandInDirection(ScrollDirection direction)
     402{
     403    LockHolder locker(m_swipeStateMutex);
     404
     405    switch (direction) {
     406    case ScrollUp: return m_swipeState.canRubberBand.top();
     407    case ScrollDown: return m_swipeState.canRubberBand.bottom();
     408    case ScrollLeft: return m_swipeState.canRubberBand.left();
     409    case ScrollRight: return m_swipeState.canRubberBand.right();
     410    };
     411
     412    return false;
    398413}
    399414

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

    r259672 r259805  
    2828#if ENABLE(ASYNC_SCROLLING)
    2929
     30#include "PageIdentifier.h"
    3031#include "PlatformWheelEvent.h"
    3132#include "RectEdges.h"
    … …  
    6364    void setAsyncFrameOrOverflowScrollingEnabled(bool);
    6465
    65     virtual ScrollingEventResult tryToHandleWheelEvent(const PlatformWheelEvent&) = 0;
     66    using CompletionFunction = WTF::Function<void (ScrollingEventResult)>;
     67    // Note that CompletionFunction may get called on a different thread.
     68    virtual ScrollingEventResult tryToHandleWheelEvent(const PlatformWheelEvent&, CompletionFunction&& = nullptr) = 0;
    6669    WEBCORE_EXPORT bool shouldHandleWheelEventSynchronously(const PlatformWheelEvent&);
    6770   
    … …  
    124127    // Can be called from any thread. Will update what edges allow rubber-banding.
    125128    WEBCORE_EXPORT void setMainFrameCanRubberBand(RectEdges<bool>);
     129    bool mainFrameCanRubberBandInDirection(ScrollDirection);
    126130
    127131    bool isHandlingProgrammaticScroll() const { return m_isHandlingProgrammaticScroll; }

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

    r259093 r259805  
    5050}
    5151
    52 ScrollingEventResult ThreadedScrollingTree::tryToHandleWheelEvent(const PlatformWheelEvent& wheelEvent)
     52ScrollingEventResult ThreadedScrollingTree::tryToHandleWheelEvent(const PlatformWheelEvent& wheelEvent, CompletionFunction&& completionFunction)
    5353{
    5454    if (shouldHandleWheelEventSynchronously(wheelEvent))
    5555        return ScrollingEventResult::SendToMainThread;
    5656
    57     if (willWheelEventStartSwipeGesture(wheelEvent))
    58         return ScrollingEventResult::DidNotHandleEvent;
    59 
    6057    RefPtr<ThreadedScrollingTree> protectedThis(this);
    61     ScrollingThread::dispatch([protectedThis, wheelEvent] {
    62         protectedThis->handleWheelEvent(wheelEvent);
     58    ScrollingThread::dispatch([protectedThis, wheelEvent, completionFunc = WTFMove(completionFunction)] {
     59        auto result = protectedThis->handleWheelEvent(wheelEvent);
     60        if (completionFunc)
     61            completionFunc(result);
    6362    });
    6463   
    65     return ScrollingEventResult::DidHandleEvent;
     64    return ScrollingEventResult::SendToScrollingThread;
    6665}
    6766

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

    r258805 r259805  
    5252    // Returns true if the wheel event can be handled on the scrolling thread and false if the
    5353    // event must be sent again to the WebCore event handler.
    54     ScrollingEventResult tryToHandleWheelEvent(const PlatformWheelEvent&) override;
     54    ScrollingEventResult tryToHandleWheelEvent(const PlatformWheelEvent&, CompletionFunction&&) override;
    5555
    5656    void invalidate() override;

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

    r259672 r259805  
    154154        return ScrollingEventResult::DidNotHandleEvent;
    155155
    156     m_delegate.handleWheelEvent(wheelEvent);
     156    bool handled = m_delegate.handleWheelEvent(wheelEvent);
    157157
    158158#if ENABLE(CSS_SCROLL_SNAP)
    … …  
    166166    scrollingTree().handleWheelEventPhase(wheelEvent.phase());
    167167   
    168     // FIXME: This needs to return whether the event was handled.
    169     return ScrollingEventResult::DidHandleEvent;
     168    return handled ? ScrollingEventResult::DidHandleEvent : ScrollingEventResult::DidNotHandleEvent;
    170169}
    171170

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

    r258679 r259805  
    112112// FIXME: We should find a way to share some of the code from newGestureIsStarting(), isAlreadyPinnedInDirectionOfGesture(),
    113113// allowsVerticalStretching(), and allowsHorizontalStretching() with the implementation in ScrollAnimatorMac.
     114// This is also the same as PlatformWheelEvent::shouldConsiderLatching().
    114115static bool newGestureIsStarting(const PlatformWheelEvent& wheelEvent)
    115116{
    … …  
    141142    case ScrollElasticityNone:
    142143        return false;
    143     case ScrollElasticityAllowed:
     144    case ScrollElasticityAllowed: {
     145        auto scrollDirection = ScrollController::directionFromEvent(wheelEvent, ScrollEventAxis::Horizontal);
     146        if (scrollDirection)
     147            return shouldRubberBandInDirection(scrollDirection.value());
    144148        return true;
     149    }
    145150    }
    146151
    … …  
    159164    case ScrollElasticityNone:
    160165        return false;
    161     case ScrollElasticityAllowed:
     166    case ScrollElasticityAllowed: {
     167        auto scrollDirection = ScrollController::directionFromEvent(wheelEvent, ScrollEventAxis::Vertical);
     168        if (scrollDirection)
     169            return shouldRubberBandInDirection(scrollDirection.value());
    162170        return true;
     171    }
    163172    }
    164173
    … …  
    185194}
    186195
     196// FIXME: Share more with ScrollingTreeScrollingNode::edgePinnedState().
    187197bool ScrollingTreeScrollingNodeDelegateMac::pinnedInDirection(const FloatSize& delta) const
    188198{
    … …  
    224234}
    225235
    226 bool ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection(ScrollDirection) const
    227 {
     236bool ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection(ScrollDirection direction) const
     237{
     238    if (scrollingNode().isRootNode())
     239        return scrollingTree().mainFrameCanRubberBandInDirection(direction);
     240
     241    // FIXME: Consult the node.
    228242    return true;
    229243}

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

    r251173 r259805  
    147147#endif
    148148
     149    static FloatSize wheelDeltaBiasingTowardsVertical(const PlatformWheelEvent&);
     150
     151    enum class WheelAxisBias { None, Vertical };
     152    static Optional<ScrollDirection> directionFromEvent(const PlatformWheelEvent&, Optional<ScrollEventAxis>, WheelAxisBias = WheelAxisBias::None);
     153
    149154private:
    150155#if ENABLE(RUBBER_BANDING)
    … …  
    154159    void snapRubberBandTimerFired();
    155160
    156     bool shouldRubberBandInHorizontalDirection(const PlatformWheelEvent&);
     161    bool shouldRubberBandInHorizontalDirection(const PlatformWheelEvent&) const;
     162    bool shouldRubberBandInDirection(ScrollDirection) const;
    157163#endif
    158164

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

    r250946 r259805  
    105105#endif
    106106    if (wheelEvent.phase() == PlatformWheelEventPhaseBegan) {
    107         // First, check if we should rubber-band at all.
    108         if (m_client.pinnedInDirection(FloatSize(-wheelEvent.deltaX(), 0))
    109             && !shouldRubberBandInHorizontalDirection(wheelEvent))
     107        // FIXME: Trying to decide if a gesture is horizontal or vertical at the "began" phase is very error-prone.
     108        auto direction = directionFromEvent(wheelEvent, ScrollEventAxis::Horizontal);
     109        // FIXME: pinnedInDirection() needs cleanup.
     110        if (direction && m_client.pinnedInDirection(FloatSize(-wheelEvent.deltaX(), 0)) && !shouldRubberBandInDirection(direction.value()))
     111            return false;
     112
     113        direction = directionFromEvent(wheelEvent, ScrollEventAxis::Vertical);
     114        if (direction && m_client.pinnedInDirection(FloatSize(0, -wheelEvent.deltaY())) && !shouldRubberBandInDirection(direction.value()))
    110115            return false;
    111116
    … …  
    165170
    166171    // Slightly prefer scrolling vertically by applying the = case to deltaY
     172    // FIXME: Use wheelDeltaBiasingTowardsVertical().
    167173    if (fabsf(deltaY) >= fabsf(deltaX))
    168174        deltaX = 0;
    … …  
    226232    }
    227233
     234    bool handled = true;
     235
    228236    if (deltaX || deltaY) {
    229237        if (!(shouldStretch || isVerticallyStretched || isHorizontallyStretched)) {
    … …  
    240248                deltaX = 0;
    241249                eventCoalescedDeltaX = 0;
     250                handled = false;
    242251            } else if (deltaX && !isHorizontallyStretched && !m_client.pinnedInDirection(FloatSize(deltaX, 0))) {
    243252                deltaX *= scrollWheelMultiplier();
    … …  
    250259                deltaY = 0;
    251260                eventCoalescedDeltaY = 0;
     261                handled = false;
    252262            } else if (deltaY && !isVerticallyStretched && !m_client.pinnedInDirection(FloatSize(0, deltaY))) {
    253263                deltaY *= scrollWheelMultiplier();
    … …  
    282292    }
    283293
    284     return true;
    285 }
    286 #endif
     294    return handled;
     295}
     296#endif // PLATFORM(MAC)
     297
     298FloatSize ScrollController::wheelDeltaBiasingTowardsVertical(const PlatformWheelEvent& wheelEvent)
     299{
     300    auto deltaX = wheelEvent.deltaX();
     301    auto deltaY = wheelEvent.deltaY();
     302
     303    if (fabsf(deltaY) >= fabsf(deltaX))
     304        deltaX = 0;
     305    else
     306        deltaY = 0;
     307
     308    return { deltaX, deltaY };
     309}
     310
     311Optional<ScrollDirection> ScrollController::directionFromEvent(const PlatformWheelEvent& wheelEvent, Optional<ScrollEventAxis> axis, WheelAxisBias bias)
     312{
     313    // FIXME: It's impossible to infer direction from a single event, since the start of a gesture is either zero or
     314    // has small deltas on both axes.
     315
     316    auto wheelDelta = FloatSize { wheelEvent.deltaX(), wheelEvent.deltaY() };
     317    if (bias == WheelAxisBias::Vertical)
     318        wheelDelta = wheelDeltaBiasingTowardsVertical(wheelEvent);
     319
     320    if (axis) {
     321        switch (axis.value()) {
     322        case ScrollEventAxis::Vertical:
     323            if (wheelDelta.height() < 0)
     324                return ScrollDown;
     325
     326            if (wheelDelta.height() > 0)
     327                return ScrollUp;
     328            break;
     329
     330        case ScrollEventAxis::Horizontal:
     331            if (wheelDelta.width() > 0)
     332                return ScrollLeft;
     333
     334            if (wheelDelta.width() < 0)
     335                return ScrollRight;
     336        }
     337
     338        return WTF::nullopt;
     339    }
     340
     341    // Check Y first because vertical scrolling dominates.
     342    if (wheelDelta.height() < 0)
     343        return ScrollDown;
     344
     345    if (wheelDelta.height() > 0)
     346        return ScrollUp;
     347
     348    if (wheelDelta.width() > 0)
     349        return ScrollLeft;
     350
     351    if (wheelDelta.width() < 0)
     352        return ScrollRight;
     353
     354    return WTF::nullopt;
     355}
    287356
    288357#if ENABLE(RUBBER_BANDING)
    … …  
    422491}
    423492
    424 bool ScrollController::shouldRubberBandInHorizontalDirection(const PlatformWheelEvent& wheelEvent)
    425 {
    426     if (wheelEvent.deltaX() > 0)
    427         return m_client.shouldRubberBandInDirection(ScrollLeft);
    428     if (wheelEvent.deltaX() < 0)
    429         return m_client.shouldRubberBandInDirection(ScrollRight);
     493bool ScrollController::shouldRubberBandInHorizontalDirection(const PlatformWheelEvent& wheelEvent) const
     494{
     495    auto direction = directionFromEvent(wheelEvent, ScrollEventAxis::Horizontal);
     496    if (direction)
     497        return shouldRubberBandInDirection(direction.value());
    430498
    431499    return true;
    432500}
    433 #endif
     501
     502bool ScrollController::shouldRubberBandInDirection(ScrollDirection direction) const
     503{
     504    return m_client.shouldRubberBandInDirection(direction);
     505}
     506
     507#endif // ENABLE(RUBBER_BANDING)
    434508
    435509#if ENABLE(CSS_SCROLL_SNAP)

  • trunk/Source/WebKit/ChangeLog

    r259804 r259805  
     12020-04-08  Simon Fraser  <simon.fraser@apple.com>
     2
     3        [Async overflow scroll] Horizontal scrolls can trigger unwanted back swipes
     4        https://bugs.webkit.org/show_bug.cgi?id=210095
     5        <rdar://problem/61376245>
     6
     7        Reviewed by Tim Horton.
     8
     9        With async overflow/frame scrolling, EventDispatcher::wheelEvent() can't immediately
     10        determine whether the scrolling tree handled the scroll; we have to wait until the
     11        event has been processed by the scrolling thread. To allow that, add a
     12        ScrollingEventResult::SendToScrollingThread return value and a give tryToHandleWheelEvent()
     13        a callback that's called when the scrolling thread is done with the event. EventDispatcher
     14        uses that to send the "didReceiveEvent" with "handled" back to the UI process, which then
     15        proceeds with history or reading list swipes.
     16
     17        Various fixes were necessary to correctly determine whether the event was handled.
     18       
     19        ScrollingTreeFrameScrollingNodeMac::handleWheelEvent() didn't return an accurate ScrollingEventResult,
     20        and ScrollController didn't return false in cases where rubber-banding was disabled (which broke navigation swipes
     21        and reading list navigation).
     22
     23        * UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:
     24        (WebKit::RemoteScrollingCoordinatorProxy::handleWheelEvent):
     25        * UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp:
     26        (WebKit::RemoteScrollingTree::tryToHandleWheelEvent):
     27        * UIProcess/RemoteLayerTree/RemoteScrollingTree.h:
     28        * WebProcess/WebPage/EventDispatcher.cpp:
     29        (WebKit::EventDispatcher::wheelEvent):
     30        (WebKit::EventDispatcher::sendDidReceiveEvent):
     31        * WebProcess/WebPage/EventDispatcher.h:
     32
    1332020-04-09  Alex Christensen  <achristensen@webkit.org>
    234

  • trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp

    r258148 r259805  
    179179bool RemoteScrollingCoordinatorProxy::handleWheelEvent(const PlatformWheelEvent& event)
    180180{
    181     ScrollingEventResult result = m_scrollingTree->tryToHandleWheelEvent(event);
     181    ScrollingEventResult result = m_scrollingTree->tryToHandleWheelEvent(event, nullptr);
    182182    return result == ScrollingEventResult::DidHandleEvent; // FIXME: handle other values.
    183183}

  • trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp

    r255037 r259805  
    6262}
    6363
    64 ScrollingEventResult RemoteScrollingTree::tryToHandleWheelEvent(const PlatformWheelEvent& wheelEvent)
     64ScrollingEventResult RemoteScrollingTree::tryToHandleWheelEvent(const PlatformWheelEvent& wheelEvent, CompletionFunction&&)
    6565{
    6666    if (shouldHandleWheelEventSynchronously(wheelEvent))

  • trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.h

    r255037 r259805  
    4646
    4747    bool isRemoteScrollingTree() const override { return true; }
    48     WebCore::ScrollingEventResult tryToHandleWheelEvent(const WebCore::PlatformWheelEvent&) override;
     48    WebCore::ScrollingEventResult tryToHandleWheelEvent(const WebCore::PlatformWheelEvent&, CompletionFunction&&) override;
    4949
    5050    void handleMouseEvent(const WebCore::PlatformMouseEvent&);

  • trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp

    r259672 r259805  
    127127            scrollingTree->setMainFrameCanRubberBand({ canRubberBandAtTop, canRubberBandAtRight, canRubberBandAtBottom, canRubberBandAtLeft });
    128128
    129         ScrollingEventResult result = scrollingTree->tryToHandleWheelEvent(platformWheelEvent);
     129        auto eventType = wheelEvent.type();
     130        ScrollingEventResult result = scrollingTree->tryToHandleWheelEvent(platformWheelEvent, [pageID, eventType](ScrollingEventResult result) {
     131            ASSERT(ScrollingThread::isCurrentThread());
     132            ASSERT(result != ScrollingEventResult::SendToScrollingThread);
     133            ASSERT(result != ScrollingEventResult::SendToMainThread);
     134           
     135            sendDidReceiveEvent(pageID, eventType, result == ScrollingEventResult::DidHandleEvent);
     136        });
     137
     138        if (result == ScrollingEventResult::SendToScrollingThread)
     139            return;
    130140
    131141        if (result == ScrollingEventResult::DidHandleEvent || result == ScrollingEventResult::DidNotHandleEvent) {
    132             sendDidReceiveEvent(pageID, wheelEvent, result == ScrollingEventResult::DidHandleEvent);
     142            sendDidReceiveEvent(pageID, wheelEvent.type(), result == ScrollingEventResult::DidHandleEvent);
    133143            return;
    134144        }
    … …  
    238248
    239249#if ENABLE(ASYNC_SCROLLING)
    240 void EventDispatcher::sendDidReceiveEvent(PageIdentifier pageID, const WebEvent& event, bool didHandleEvent)
    241 {
    242     WebProcess::singleton().parentProcessConnection()->send(Messages::WebPageProxy::DidReceiveEvent(static_cast<uint32_t>(event.type()), didHandleEvent), pageID);
     250void EventDispatcher::sendDidReceiveEvent(PageIdentifier pageID, WebEvent::Type eventType, bool didHandleEvent)
     251{
     252    WebProcess::singleton().parentProcessConnection()->send(Messages::WebPageProxy::DidReceiveEvent(static_cast<uint32_t>(eventType), didHandleEvent), pageID);
    243253}
    244254#endif

  • trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.h

    r258098 r259805  
    9595
    9696#if ENABLE(ASYNC_SCROLLING)
    97     void sendDidReceiveEvent(WebCore::PageIdentifier, const WebEvent&, bool didHandleEvent);
     97    static void sendDidReceiveEvent(WebCore::PageIdentifier, WebEvent::Type, bool didHandleEvent);
    9898#endif
    9999
Note: See TracChangeset for help on using the changeset viewer.