Changeset 268544 in webkit


Ignore:
Timestamp:
Oct 15, 2020 12:24:40 PM (4 years ago)
Author:
Simon Fraser
Message:

Scrolls in the passive event region only send one wheel event to the DOM
​https://bugs.webkit.org/show_bug.cgi?id=217719

Reviewed by Tim Horton.
Source/WebCore:

When we hit the scrolling thread latching codepath, we'd always use { WheelEventProcessingSteps::ScrollingThread }
for the steps, but that caused us to not send DOM events.

Latching has to store the steps from when latching occurs (we should not re-hit-test
to get the region state on every event), so fix up ScrollingTreeLatchingController to
do that.

Test: fast/scrolling/latching/latched-scroll-in-passive-region.html

  • page/scrolling/ScrollingTree.cpp:

(WebCore::ScrollingTree::determineWheelEventProcessing):
(WebCore::ScrollingTree::handleWheelEvent):
(WebCore::ScrollingTree::handleWheelEventWithNode):

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

(WebCore::ScrollingTreeLatchingController::receivedWheelEvent):
(WebCore::ScrollingTreeLatchingController::latchingDataForEvent const):
(WebCore::ScrollingTreeLatchingController::latchedNodeID const):
(WebCore::ScrollingTreeLatchingController::latchedNodeAndSteps const):
(WebCore::ScrollingTreeLatchingController::nodeDidHandleEvent):
(WebCore::ScrollingTreeLatchingController::nodeWasRemoved):
(WebCore::ScrollingTreeLatchingController::clearLatchedNode):
(WebCore::ScrollingTreeLatchingController::latchedNodeForEvent const): Deleted.

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

(WebCore::ThreadedScrollingTree::handleWheelEventAfterMainThread):

LayoutTests:

  • fast/scrolling/latching/latched-scroll-in-passive-region-expected.txt: Added.
  • fast/scrolling/latching/latched-scroll-in-passive-region.html: Added.
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r268541 r268544  
     12020-10-15  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Scrolls in the passive event region only send one wheel event to the DOM
     4        https://bugs.webkit.org/show_bug.cgi?id=217719
     5
     6        Reviewed by Tim Horton.
     7
     8        * fast/scrolling/latching/latched-scroll-in-passive-region-expected.txt: Added.
     9        * fast/scrolling/latching/latched-scroll-in-passive-region.html: Added.
     10
    1112020-10-15  Andres Gonzalez  <andresg_22@apple.com>
    212

  • trunk/Source/WebCore/ChangeLog

    r268541 r268544  
     12020-10-15  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Scrolls in the passive event region only send one wheel event to the DOM
     4        https://bugs.webkit.org/show_bug.cgi?id=217719
     5
     6        Reviewed by Tim Horton.
     7       
     8        When we hit the scrolling thread latching codepath, we'd always use { WheelEventProcessingSteps::ScrollingThread }
     9        for the steps, but that caused us to not send DOM events.
     10
     11        Latching has to store the steps from when latching occurs (we should not re-hit-test
     12        to get the region state on every event), so fix up ScrollingTreeLatchingController to
     13        do that.
     14
     15        Test: fast/scrolling/latching/latched-scroll-in-passive-region.html
     16
     17        * page/scrolling/ScrollingTree.cpp:
     18        (WebCore::ScrollingTree::determineWheelEventProcessing):
     19        (WebCore::ScrollingTree::handleWheelEvent):
     20        (WebCore::ScrollingTree::handleWheelEventWithNode):
     21        * page/scrolling/ScrollingTree.h:
     22        * page/scrolling/ScrollingTreeLatchingController.cpp:
     23        (WebCore::ScrollingTreeLatchingController::receivedWheelEvent):
     24        (WebCore::ScrollingTreeLatchingController::latchingDataForEvent const):
     25        (WebCore::ScrollingTreeLatchingController::latchedNodeID const):
     26        (WebCore::ScrollingTreeLatchingController::latchedNodeAndSteps const):
     27        (WebCore::ScrollingTreeLatchingController::nodeDidHandleEvent):
     28        (WebCore::ScrollingTreeLatchingController::nodeWasRemoved):
     29        (WebCore::ScrollingTreeLatchingController::clearLatchedNode):
     30        (WebCore::ScrollingTreeLatchingController::latchedNodeForEvent const): Deleted.
     31        * page/scrolling/ScrollingTreeLatchingController.h:
     32        * page/scrolling/ThreadedScrollingTree.cpp:
     33        (WebCore::ThreadedScrollingTree::handleWheelEventAfterMainThread):
     34
    1352020-10-15  Andres Gonzalez  <andresg_22@apple.com>
    236

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

    r268476 r268544  
    6969    LockHolder lock(m_treeStateMutex);
    7070
    71     auto latchedNode = m_latchingController.latchedNodeForEvent(wheelEvent, m_allowLatching);
    72     LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTree::shouldHandleWheelEventSynchronously " << wheelEvent << " have latched node " << latchedNode);
    73     if (latchedNode)
    74         return { WheelEventProcessingSteps::ScrollingThread };
     71    auto latchedNodeAndSteps = m_latchingController.latchingDataForEvent(wheelEvent, m_allowLatching);
     72    if (latchedNodeAndSteps) {
     73        LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTree::determineWheelEventProcessing " << wheelEvent << " have latched node " << latchedNodeAndSteps->scrollingNodeID << " steps " << latchedNodeAndSteps->processingSteps);
     74        return latchedNodeAndSteps->processingSteps;
     75    }
    7576
    7677    m_latchingController.receivedWheelEvent(wheelEvent, m_allowLatching);
    … …  
    136137        m_gestureState.receivedWheelEvent(wheelEvent);
    137138
    138         if (auto latchedNodeID = m_latchingController.latchedNodeForEvent(wheelEvent, m_allowLatching)) {
    139             LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTree::handleWheelEvent: has latched node " << latchedNodeID);
    140             auto* node = nodeForID(*latchedNodeID);
     139        if (auto latchedNodeAndSteps = m_latchingController.latchingDataForEvent(wheelEvent, m_allowLatching)) {
     140            LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTree::handleWheelEvent: has latched node " << latchedNodeAndSteps->scrollingNodeID);
     141            auto* node = nodeForID(latchedNodeAndSteps->scrollingNodeID);
    141142            if (is<ScrollingTreeScrollingNode>(node)) {
    142143                auto result = downcast<ScrollingTreeScrollingNode>(*node).handleWheelEvent(wheelEvent);
    143144                if (result.wasHandled) {
    144                     m_latchingController.nodeDidHandleEvent(*latchedNodeID, wheelEvent, m_allowLatching);
    145                     m_gestureState.nodeDidHandleEvent(*latchedNodeID, wheelEvent);
     145                    m_latchingController.nodeDidHandleEvent(latchedNodeAndSteps->scrollingNodeID, processingSteps, wheelEvent, m_allowLatching);
     146                    m_gestureState.nodeDidHandleEvent(latchedNodeAndSteps->scrollingNodeID, wheelEvent);
    146147                }
    147148                return result;
    … …  
    155156        LOG_WITH_STREAM(Scrolling, stream << "ScrollingTree::handleWheelEvent found node " << (node ? node->scrollingNodeID() : 0) << " for point " << position);
    156157
    157         return handleWheelEventWithNode(wheelEvent, node.get());
     158        return handleWheelEventWithNode(wheelEvent, processingSteps, node.get());
    158159    }();
    159160
    … …  
    162163}
    163164
    164 WheelEventHandlingResult ScrollingTree::handleWheelEventWithNode(const PlatformWheelEvent& wheelEvent, ScrollingTreeNode* node)
     165WheelEventHandlingResult ScrollingTree::handleWheelEventWithNode(const PlatformWheelEvent& wheelEvent, OptionSet<WheelEventProcessingSteps> processingSteps, ScrollingTreeNode* node)
    165166{
    166167    while (node) {
    … …  
    170171
    171172            if (result.wasHandled) {
    172                 m_latchingController.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), wheelEvent, m_allowLatching);
     173                m_latchingController.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), processingSteps, wheelEvent, m_allowLatching);
    173174                m_gestureState.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), wheelEvent);
    174175                return result;

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

    r268417 r268544  
    213213
    214214protected:
    215     WheelEventHandlingResult handleWheelEventWithNode(const PlatformWheelEvent&, ScrollingTreeNode*);
     215    WheelEventHandlingResult handleWheelEventWithNode(const PlatformWheelEvent&, OptionSet<WheelEventProcessingSteps>, ScrollingTreeNode*);
    216216
    217217    FloatPoint mainFrameScrollPosition() const;

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

    r266333 r268544  
    4848
    4949    LockHolder locker(m_latchedNodeMutex);
    50     if (wheelEvent.isGestureStart() && m_latchedNodeID && !latchedNodeIsRelevant()) {
     50    if (wheelEvent.isGestureStart() && m_latchedNodeAndSteps && !latchedNodeIsRelevant()) {
    5151        LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " receivedWheelEvent - " << (MonotonicTime::now() - m_lastLatchedNodeInterationTime).milliseconds() << "ms since last event, clearing latched node");
    52         m_latchedNodeID.reset();
     52        m_latchedNodeAndSteps.reset();
    5353    }
    5454}
    5555
    56 Optional<ScrollingNodeID> ScrollingTreeLatchingController::latchedNodeForEvent(const PlatformWheelEvent& wheelEvent, bool allowLatching) const
     56Optional<ScrollingTreeLatchingController::ScrollingNodeAndProcessingSteps> ScrollingTreeLatchingController::latchingDataForEvent(const PlatformWheelEvent& wheelEvent, bool allowLatching) const
    5757{
    5858    if (!allowLatching)
    … …  
    6262
    6363    // If we have a latched node, use it.
    64     if (wheelEvent.useLatchedEventElement() && m_latchedNodeID && latchedNodeIsRelevant()) {
    65         LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " latchedNodeForEvent: returning " << m_latchedNodeID);
    66         return m_latchedNodeID.value();
     64    if (wheelEvent.useLatchedEventElement() && m_latchedNodeAndSteps && latchedNodeIsRelevant()) {
     65        LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " latchedNodeForEvent: returning " << m_latchedNodeAndSteps->scrollingNodeID);
     66        return m_latchedNodeAndSteps;
    6767    }
    6868
    … …  
    7373{
    7474    LockHolder locker(m_latchedNodeMutex);
    75     return m_latchedNodeID;
     75    if (m_latchedNodeAndSteps)
     76        return m_latchedNodeAndSteps->scrollingNodeID;
     77
     78    return WTF::nullopt;
    7679}
    7780
    78 void ScrollingTreeLatchingController::nodeDidHandleEvent(ScrollingNodeID scrollingNodeID, const PlatformWheelEvent& wheelEvent, bool allowLatching)
     81Optional<ScrollingTreeLatchingController::ScrollingNodeAndProcessingSteps> ScrollingTreeLatchingController::latchedNodeAndSteps() const
     82{
     83    LockHolder locker(m_latchedNodeMutex);
     84    return m_latchedNodeAndSteps;
     85}
     86
     87void ScrollingTreeLatchingController::nodeDidHandleEvent(ScrollingNodeID scrollingNodeID, OptionSet<WheelEventProcessingSteps> processingSteps, const PlatformWheelEvent& wheelEvent, bool allowLatching)
    7988{
    8089    if (!allowLatching)
    … …  
    8392    LockHolder locker(m_latchedNodeMutex);
    8493
    85     if (wheelEvent.useLatchedEventElement() && m_latchedNodeID == scrollingNodeID) {
     94    if (wheelEvent.useLatchedEventElement() && m_latchedNodeAndSteps && m_latchedNodeAndSteps->scrollingNodeID == scrollingNodeID) {
    8695        m_lastLatchedNodeInterationTime = MonotonicTime::now();
    8796        return;
    … …  
    92101
    93102    LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " nodeDidHandleEvent: latching to " << scrollingNodeID);
    94     m_latchedNodeID = scrollingNodeID;
     103    m_latchedNodeAndSteps = ScrollingNodeAndProcessingSteps { scrollingNodeID, processingSteps };
    95104    m_lastLatchedNodeInterationTime = MonotonicTime::now();
    96105}
    … …  
    99108{
    100109    LockHolder locker(m_latchedNodeMutex);
    101     if (nodeID == m_latchedNodeID)
    102         m_latchedNodeID.reset();
     110    if (m_latchedNodeAndSteps && m_latchedNodeAndSteps->scrollingNodeID == nodeID)
     111        m_latchedNodeAndSteps.reset();
    103112}
    104113
    … …  
    106115{
    107116    LockHolder locker(m_latchedNodeMutex);
    108     LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " clearLatchedNode (was " << m_latchedNodeID << ")");
    109     m_latchedNodeID.reset();
     117    LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " clearLatchedNode");
     118    m_latchedNodeAndSteps.reset();
    110119}
    111120

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

    r266262 r268544  
    4040class ScrollingTreeLatchingController {
    4141public:
     42    struct ScrollingNodeAndProcessingSteps {
     43        ScrollingNodeID scrollingNodeID;
     44        OptionSet<WheelEventProcessingSteps> processingSteps;
     45    };
     46
    4247    ScrollingTreeLatchingController();
    4348
    4449    void receivedWheelEvent(const PlatformWheelEvent&, bool allowLatching);
    45     Optional<ScrollingNodeID> latchedNodeForEvent(const PlatformWheelEvent&, bool allowLatching) const;
    46     void nodeDidHandleEvent(ScrollingNodeID, const PlatformWheelEvent&, bool allowLatching);
     50
     51    Optional<ScrollingNodeAndProcessingSteps> latchingDataForEvent(const PlatformWheelEvent&, bool allowLatching) const;
     52    void nodeDidHandleEvent(ScrollingNodeID, OptionSet<WheelEventProcessingSteps>, const PlatformWheelEvent&, bool allowLatching);
    4753
    4854    Optional<ScrollingNodeID> latchedNodeID() const;
     55    Optional<ScrollingNodeAndProcessingSteps> latchedNodeAndSteps() const;
    4956
    5057    void nodeWasRemoved(ScrollingNodeID);
    … …  
    5562
    5663    mutable Lock m_latchedNodeMutex;
    57     Markable<ScrollingNodeID, IntegralMarkableTraits<ScrollingNodeID, 0>> m_latchedNodeID;
     64    Optional<ScrollingNodeAndProcessingSteps> m_latchedNodeAndSteps;
    5865    MonotonicTime m_lastLatchedNodeInterationTime;
    5966};

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

    r268522 r268544  
    7171
    7272    RefPtr<ScrollingTreeNode> targetNode = nodeForID(targetNodeID);
    73     auto result = handleWheelEventWithNode(wheelEvent, targetNode.get());
     73    auto result = handleWheelEventWithNode(wheelEvent, { }, targetNode.get());
    7474    return result.wasHandled;
    7575}
Note: See TracChangeset for help on using the changeset viewer.