Skip to content

Commit e5047d3

Browse files
smfrsmfr
committed
Dispatch main-thread overflow scrolls to the scrolling thread as we do for page scrolls
https://bugs.webkit.org/show_bug.cgi?id=219213 Reviewed by Tim Horton. Source/WebCore: Unify the behavior of overflow scrolling and page scrolling for synchronous scrolls. Somewhat surprisingly, synchronous page scrolls move layers via a dispatch to the scrolling thread in FrameView::handleWheelEventForScrolling(), but overflow scrolls just did main thread compositing updates to set the new layer positions. A future patch will require that the "began" event for a synchronous scroll gets back to the scrolling thread, so unify these code paths to have overflow scrolls also leverage handleWheelEventAfterMainThread(), via RenderLayer::handleWheelEventForScrolling(). There's some fallout from this. ThreadedScrollingTree::handleWheelEventAfterMainThread() calls into handleWheelEventWithNode(), but in this special case that codepath needs to know that we're in a special "post main thread" mode that should 1. Behave as if the node is latched, i.e. don't propagate to parent nodes, and target the node if if it's scrolled to a the min or max to allow rubberbanding 2. Scroll even if the node has synchronous scrolling reasons This mode is represented by the EventTargeting value. Finally, EventHandler::handleWheelEventInternal() should only clear latching state if the content has called preventDefault() on the event. * page/EventHandler.cpp: (WebCore::EventHandler::handleWheelEventInternal): * page/scrolling/ScrollLatchingController.cpp: (WebCore::ScrollLatchingController::updateAndFetchLatchingStateForFrame): * page/scrolling/ScrollingTree.cpp: (WebCore::ScrollingTree::handleWheelEventWithNode): * page/scrolling/ScrollingTree.h: * page/scrolling/ScrollingTreeScrollingNode.cpp: (WebCore::ScrollingTreeScrollingNode::canHandleWheelEvent const): (WebCore::ScrollingTreeScrollingNode::handleWheelEvent): * page/scrolling/ScrollingTreeScrollingNode.h: * page/scrolling/ThreadedScrollingTree.cpp: (WebCore::ThreadedScrollingTree::handleWheelEventAfterMainThread): * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h: * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm: (WebCore::ScrollingTreeFrameScrollingNodeMac::handleWheelEvent): * page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.h: * page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.mm: (WebCore::ScrollingTreeOverflowScrollingNodeMac::handleWheelEvent): * platform/cocoa/ScrollController.mm: (WebCore::ScrollController::handleWheelEvent): * rendering/RenderLayer.cpp: (WebCore::RenderLayer::handleWheelEventForScrolling): * rendering/RenderLayer.h: Source/WebKit: Unify the behavior of overflow scrolling and page scrolling for synchronous scrolls. Somewhat surprisingly, synchronous page scrolls move layers via a dispatch to the scrolling thread in FrameView::handleWheelEventForScrolling(), but overflow scrolls just did main thread compositing updates to set the new layer positions. A future patch will require that the "began" event for a synchronous scroll gets back to the scrolling thread, so unify these code paths to have overflow scrolls also leverage handleWheelEventAfterMainThread(), via RenderLayer::handleWheelEventForScrolling(). There's some fallout from this. ThreadedScrollingTree::handleWheelEventAfterMainThread() calls into handleWheelEventWithNode(), but in this special case that codepath needs to know that we're in a special "post main thread" mode that should 1. Behave as if the node is latched, i.e. don't propagate to parent nodes, and target the node if if it's scrolled to a the min or max to allow rubberbanding 2. Scroll even if the node has synchronous scrolling reasons This mode is represented by the EventTargeting value. Finally, EventHandler::handleWheelEventInternal() should only clear latching state if the content has called preventDefault() on the event. * UIProcess/RemoteLayerTree/mac/ScrollingTreeFrameScrollingNodeRemoteMac.cpp: (WebKit::ScrollingTreeFrameScrollingNodeRemoteMac::handleWheelEvent): * UIProcess/RemoteLayerTree/mac/ScrollingTreeFrameScrollingNodeRemoteMac.h: * UIProcess/RemoteLayerTree/mac/ScrollingTreeOverflowScrollingNodeRemoteMac.cpp: (WebKit::ScrollingTreeOverflowScrollingNodeRemoteMac::handleWheelEvent): * UIProcess/RemoteLayerTree/mac/ScrollingTreeOverflowScrollingNodeRemoteMac.h: Canonical link: https://commits.webkit.org/231839@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@270130 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 42a20f8 commit e5047d3

24 files changed

Lines changed: 142 additions & 34 deletions

‎Source/WebCore/ChangeLog‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,55 @@
1+
2020-11-20 Simon Fraser <simon.fraser@apple.com>
2+
3+
Dispatch main-thread overflow scrolls to the scrolling thread as we do for page scrolls
4+
https://bugs.webkit.org/show_bug.cgi?id=219213
5+
6+
Reviewed by Tim Horton.
7+
8+
Unify the behavior of overflow scrolling and page scrolling for synchronous scrolls.
9+
10+
Somewhat surprisingly, synchronous page scrolls move layers via a dispatch to the scrolling thread in
11+
FrameView::handleWheelEventForScrolling(), but overflow scrolls just did main thread compositing updates
12+
to set the new layer positions.
13+
14+
A future patch will require that the "began" event for a synchronous scroll gets back to the scrolling thread,
15+
so unify these code paths to have overflow scrolls also leverage handleWheelEventAfterMainThread(), via
16+
RenderLayer::handleWheelEventForScrolling().
17+
18+
There's some fallout from this. ThreadedScrollingTree::handleWheelEventAfterMainThread() calls into handleWheelEventWithNode(),
19+
but in this special case that codepath needs to know that we're in a special "post main thread" mode that should
20+
1. Behave as if the node is latched, i.e. don't propagate to parent nodes,
21+
and target the node if if it's scrolled to a the min or max to allow rubberbanding
22+
2. Scroll even if the node has synchronous scrolling reasons
23+
This mode is represented by the EventTargeting value.
24+
25+
Finally, EventHandler::handleWheelEventInternal() should only clear latching state if the content has called preventDefault()
26+
on the event.
27+
28+
* page/EventHandler.cpp:
29+
(WebCore::EventHandler::handleWheelEventInternal):
30+
* page/scrolling/ScrollLatchingController.cpp:
31+
(WebCore::ScrollLatchingController::updateAndFetchLatchingStateForFrame):
32+
* page/scrolling/ScrollingTree.cpp:
33+
(WebCore::ScrollingTree::handleWheelEventWithNode):
34+
* page/scrolling/ScrollingTree.h:
35+
* page/scrolling/ScrollingTreeScrollingNode.cpp:
36+
(WebCore::ScrollingTreeScrollingNode::canHandleWheelEvent const):
37+
(WebCore::ScrollingTreeScrollingNode::handleWheelEvent):
38+
* page/scrolling/ScrollingTreeScrollingNode.h:
39+
* page/scrolling/ThreadedScrollingTree.cpp:
40+
(WebCore::ThreadedScrollingTree::handleWheelEventAfterMainThread):
41+
* page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
42+
* page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
43+
(WebCore::ScrollingTreeFrameScrollingNodeMac::handleWheelEvent):
44+
* page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.h:
45+
* page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.mm:
46+
(WebCore::ScrollingTreeOverflowScrollingNodeMac::handleWheelEvent):
47+
* platform/cocoa/ScrollController.mm:
48+
(WebCore::ScrollController::handleWheelEvent):
49+
* rendering/RenderLayer.cpp:
50+
(WebCore::RenderLayer::handleWheelEventForScrolling):
51+
* rendering/RenderLayer.h:
52+
153
2020-11-20 Zalan Bujtas <zalan@apple.com>
254

355
[LFC][Integration] Rename LineIteratorLegacy::logicalLeft/right to contentLeft/right

‎Source/WebCore/page/EventHandler.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2941,7 +2941,8 @@ bool EventHandler::handleWheelEventInternal(const PlatformWheelEvent& event, Opt
29412941
m_isHandlingWheelEvent = false;
29422942
if (scrollableArea && scrollableArea->scrollShouldClearLatchedState()) {
29432943
// Web developer is controlling scrolling, so don't attempt to latch.
2944-
clearLatchedState();
2944+
if (handling.containsAll({ EventHandling::DispatchedToDOM, EventHandling::DefaultPrevented }))
2945+
clearLatchedState();
29452946
scrollableArea->setScrollShouldClearLatchedState(false);
29462947
}
29472948

‎Source/WebCore/page/scrolling/ScrollLatchingController.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ void ScrollLatchingController::updateAndFetchLatchingStateForFrame(Frame& frame,
120120
state.scrollableArea = scrollableArea;
121121
state.isOverWidget = isOverWidget;
122122

123+
LOG_WITH_STREAM(ScrollLatching, stream << "ScrollLatchingController::updateAndFetchLatchingStateForFrame() - pushing state for Frame " << &frame << " element " << state.wheelEventElement.get() << " scrollableArea " << state.scrollableArea);
123124
m_frameStateStack.append(WTFMove(state));
124125
return;
125126
}
@@ -143,6 +144,8 @@ void ScrollLatchingController::updateAndFetchLatchingStateForFrame(Frame& frame,
143144
latchedElement = state.wheelEventElement.get();
144145
scrollableArea = state.scrollableArea;
145146
isOverWidget = state.isOverWidget;
147+
148+
LOG_WITH_STREAM(ScrollLatching, stream << "ScrollLatchingController::updateAndFetchLatchingStateForFrame() - using state for Frame " << &frame << " element " << latchedElement.get() << " scrollableArea " << scrollableArea);
146149
}
147150
}
148151
}

‎Source/WebCore/page/scrolling/ScrollingTree.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,20 +168,20 @@ WheelEventHandlingResult ScrollingTree::handleWheelEvent(const PlatformWheelEven
168168
return result;
169169
}
170170

171-
WheelEventHandlingResult ScrollingTree::handleWheelEventWithNode(const PlatformWheelEvent& wheelEvent, OptionSet<WheelEventProcessingSteps> processingSteps, ScrollingTreeNode* node)
171+
WheelEventHandlingResult ScrollingTree::handleWheelEventWithNode(const PlatformWheelEvent& wheelEvent, OptionSet<WheelEventProcessingSteps> processingSteps, ScrollingTreeNode* node, EventTargeting eventTargeting)
172172
{
173173
while (node) {
174174
if (is<ScrollingTreeScrollingNode>(*node)) {
175175
auto& scrollingNode = downcast<ScrollingTreeScrollingNode>(*node);
176-
auto result = scrollingNode.handleWheelEvent(wheelEvent);
176+
auto result = scrollingNode.handleWheelEvent(wheelEvent, eventTargeting);
177177

178178
if (result.wasHandled) {
179179
m_latchingController.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), processingSteps, wheelEvent, m_allowLatching);
180180
m_gestureState.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), wheelEvent);
181181
return result;
182182
}
183183

184-
if (result.needsMainThreadProcessing())
184+
if (result.needsMainThreadProcessing() || eventTargeting != EventTargeting::Propagate)
185185
return result;
186186
}
187187

‎Source/WebCore/page/scrolling/ScrollingTree.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ struct WheelEventHandlingResult {
7575
}
7676
};
7777

78+
enum class EventTargeting : uint8_t { NodeOnly, Propagate };
79+
7880
class ScrollingTree : public ThreadSafeRefCounted<ScrollingTree> {
7981
friend class ScrollingTreeLatchingController;
8082
public:
@@ -233,7 +235,7 @@ friend class ScrollingTreeLatchingController;
233235
Vector<ScrollUpdate> takePendingScrollUpdates();
234236

235237
protected:
236-
WheelEventHandlingResult handleWheelEventWithNode(const PlatformWheelEvent&, OptionSet<WheelEventProcessingSteps>, ScrollingTreeNode*);
238+
WheelEventHandlingResult handleWheelEventWithNode(const PlatformWheelEvent&, OptionSet<WheelEventProcessingSteps>, ScrollingTreeNode*, EventTargeting = EventTargeting::Propagate);
237239

238240
FloatPoint mainFrameScrollPosition() const;
239241
void setMainFrameScrollPosition(FloatPoint);

‎Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ bool ScrollingTreeScrollingNode::isLatchedNode() const
134134
return scrollingTree().latchedNodeID() == scrollingNodeID();
135135
}
136136

137-
bool ScrollingTreeScrollingNode::canHandleWheelEvent(const PlatformWheelEvent& wheelEvent) const
137+
bool ScrollingTreeScrollingNode::canHandleWheelEvent(const PlatformWheelEvent& wheelEvent, EventTargeting eventTargeting) const
138138
{
139139
if (!canHaveScrollbars())
140140
return false;
@@ -145,13 +145,13 @@ bool ScrollingTreeScrollingNode::canHandleWheelEvent(const PlatformWheelEvent& w
145145

146146
// We always rubber-band the latched node, or the root node.
147147
// The stateless wheel event doesn't trigger rubber-band.
148-
if (isLatchedNode() || (isRootNode() && !wheelEvent.isNonGestureEvent()))
148+
if (isLatchedNode() || eventTargeting == EventTargeting::NodeOnly || (isRootNode() && !wheelEvent.isNonGestureEvent()))
149149
return true;
150150

151151
return eventCanScrollContents(wheelEvent);
152152
}
153153

154-
WheelEventHandlingResult ScrollingTreeScrollingNode::handleWheelEvent(const PlatformWheelEvent&)
154+
WheelEventHandlingResult ScrollingTreeScrollingNode::handleWheelEvent(const PlatformWheelEvent&, EventTargeting)
155155
{
156156
return WheelEventHandlingResult::unhandled();
157157
}

‎Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,9 @@
2828
#if ENABLE(ASYNC_SCROLLING)
2929

3030
#include "IntRect.h"
31-
#include "RectEdges.h"
3231
#include "ScrollSnapOffsetsInfo.h"
33-
#include "ScrollTypes.h"
3432
#include "ScrollableArea.h"
35-
#include "ScrollingCoordinator.h"
33+
#include "ScrollingTree.h"
3634
#include "ScrollingTreeNode.h"
3735

3836
namespace WebCore {
@@ -55,8 +53,8 @@ class WEBCORE_EXPORT ScrollingTreeScrollingNode : public ScrollingTreeNode {
5553
void commitStateAfterChildren(const ScrollingStateNode&) override;
5654
void didCompleteCommitForNode() final;
5755

58-
virtual bool canHandleWheelEvent(const PlatformWheelEvent&) const;
59-
virtual WheelEventHandlingResult handleWheelEvent(const PlatformWheelEvent&);
56+
virtual bool canHandleWheelEvent(const PlatformWheelEvent&, EventTargeting) const;
57+
virtual WheelEventHandlingResult handleWheelEvent(const PlatformWheelEvent&, EventTargeting = EventTargeting::Propagate);
6058

6159
FloatPoint currentScrollPosition() const { return m_currentScrollPosition; }
6260
FloatPoint currentScrollOffset() const { return ScrollableArea::scrollOffsetFromPosition(m_currentScrollPosition, toFloatSize(m_scrollOrigin)); }

‎Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,13 @@ WheelEventHandlingResult ThreadedScrollingTree::handleWheelEvent(const PlatformW
6666

6767
bool ThreadedScrollingTree::handleWheelEventAfterMainThread(const PlatformWheelEvent& wheelEvent, ScrollingNodeID targetNodeID)
6868
{
69-
LOG_WITH_STREAM(Scrolling, stream << "ThreadedScrollingTree::handleWheelEventAfterMainThread " << wheelEvent);
69+
LOG_WITH_STREAM(Scrolling, stream << "ThreadedScrollingTree::handleWheelEventAfterMainThread " << wheelEvent << " node " << targetNodeID);
7070

7171
LockHolder locker(m_treeMutex);
7272

7373
SetForScope<bool> disallowLatchingScope(m_allowLatching, false);
7474
RefPtr<ScrollingTreeNode> targetNode = nodeForID(targetNodeID);
75-
auto result = handleWheelEventWithNode(wheelEvent, { }, targetNode.get());
75+
auto result = handleWheelEventWithNode(wheelEvent, { }, targetNode.get(), EventTargeting::NodeOnly);
7676
return result.wasHandled;
7777
}
7878

‎Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class WEBCORE_EXPORT ScrollingTreeFrameScrollingNodeMac : public ScrollingTreeFr
5151
void commitStateBeforeChildren(const ScrollingStateNode&) override;
5252
void commitStateAfterChildren(const ScrollingStateNode&) override;
5353

54-
WheelEventHandlingResult handleWheelEvent(const PlatformWheelEvent&) override;
54+
WheelEventHandlingResult handleWheelEvent(const PlatformWheelEvent&, EventTargeting) override;
5555

5656
WEBCORE_EXPORT void repositionRelatedLayers() override;
5757

‎Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,9 @@
117117
updateMainFramePinAndRubberbandState();
118118
}
119119

120-
WheelEventHandlingResult ScrollingTreeFrameScrollingNodeMac::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
120+
WheelEventHandlingResult ScrollingTreeFrameScrollingNodeMac::handleWheelEvent(const PlatformWheelEvent& wheelEvent, EventTargeting eventTargeting)
121121
{
122-
if (!canHandleWheelEvent(wheelEvent))
122+
if (!canHandleWheelEvent(wheelEvent, eventTargeting))
123123
return WheelEventHandlingResult::unhandled();
124124

125125
bool handled = m_delegate.handleWheelEvent(wheelEvent);

0 commit comments

Comments
 (0)