Skip to content

Commit 6555fc3

Browse files
smfrsmfr
committed
[UI-side compositing] History swipes can fail if you swipe over an element with a wheel event handler
https://bugs.webkit.org/show_bug.cgi?id=254837 rdar://107481980 Reviewed by Tim Horton. `ThreadedScrollingCoordinator::handleWheelEventForScrolling()` checks to see if the current event can start a swipe gesture, and we need to do the same in `RemoteScrollingCoordinator::handleWheelEventForScrolling()`. However, we don't have a scrolling tree to ask in the web process, so we need to feed this information down from the UI Process. So plumb `willStartSwipe` through and check it. Tested by `LayoutTests/swipe` tests. * Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp: (WebKit::RemoteScrollingCoordinatorProxy::continueWheelEventHandling): * Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.mm: (WebKit::RemoteScrollingTreeMac::waitForEventDefaultHandlingCompletion): * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::handleWheelEvent): (WebKit::WebPageProxy::continueWheelEventHandling): (WebKit::WebPageProxy::sendWheelEvent): (WebKit::WebPageProxy::wheelEventHandlingCompleted): * Source/WebKit/UIProcess/WebPageProxy.h: * Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.h: * Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm: (WebKit::RemoteScrollingCoordinator::handleWheelEventForScrolling): * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::handleWheelEvent): * Source/WebKit/WebProcess/WebPage/WebPage.h: * Source/WebKit/WebProcess/WebPage/WebPage.messages.in: Canonical link: https://commits.webkit.org/262439@main
1 parent 9a7153a commit 6555fc3

File tree

9 files changed

+27
-17
lines changed

9 files changed

+27
-17
lines changed

‎Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ void RemoteScrollingCoordinatorProxy::handleWheelEvent(const WebWheelEvent& whee
135135

136136
void RemoteScrollingCoordinatorProxy::continueWheelEventHandling(const WebWheelEvent& wheelEvent, WheelEventHandlingResult result)
137137
{
138-
webPageProxy().continueWheelEventHandling(wheelEvent, result);
138+
bool willStartSwipe = m_scrollingTree->willWheelEventStartSwipeGesture(platform(wheelEvent));
139+
webPageProxy().continueWheelEventHandling(wheelEvent, result, willStartSwipe);
139140
}
140141

141142
void RemoteScrollingCoordinatorProxy::handleMouseEvent(const WebCore::PlatformMouseEvent& event)

‎Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.mm‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,9 @@
270270

271271
Locker locker { m_treeLock };
272272

273-
static constexpr auto maxAllowableMainThreadDelay = 50_ms;
273+
static constexpr auto maxAllowableEventProcessingDelay = 50_ms;
274274
auto startTime = MonotonicTime::now();
275-
auto timeoutTime = startTime + maxAllowableMainThreadDelay;
275+
auto timeoutTime = startTime + maxAllowableEventProcessingDelay;
276276

277277
bool receivedEvent = m_waitingForBeganEventCondition.waitUntil(m_treeLock, timeoutTime, [&] {
278278
assertIsHeld(m_treeLock);

‎Source/WebKit/UIProcess/WebPageProxy.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3052,7 +3052,7 @@ void WebPageProxy::handleWheelEvent(const WebWheelEvent& wheelEvent)
30523052
return;
30533053

30543054
if (drawingArea()->shouldSendWheelEventsToEventDispatcher()) {
3055-
continueWheelEventHandling(wheelEvent, { WheelEventProcessingSteps::SynchronousScrolling, false });
3055+
continueWheelEventHandling(wheelEvent, { WheelEventProcessingSteps::SynchronousScrolling, false }, { });
30563056
return;
30573057
}
30583058

@@ -3065,7 +3065,7 @@ void WebPageProxy::handleWheelEvent(const WebWheelEvent& wheelEvent)
30653065
#endif
30663066
}
30673067

3068-
void WebPageProxy::continueWheelEventHandling(const WebWheelEvent& wheelEvent, const WheelEventHandlingResult& result)
3068+
void WebPageProxy::continueWheelEventHandling(const WebWheelEvent& wheelEvent, const WheelEventHandlingResult& result, std::optional<bool> willStartSwipe)
30693069
{
30703070
LOG_WITH_STREAM(WheelEvents, stream << "WebPageProxy::continueWheelEventHandling - " << result);
30713071

@@ -3075,10 +3075,10 @@ void WebPageProxy::continueWheelEventHandling(const WebWheelEvent& wheelEvent, c
30753075
}
30763076

30773077
auto rubberBandableEdges = rubberBandableEdgesRespectingHistorySwipe();
3078-
sendWheelEvent(wheelEvent, result.steps, rubberBandableEdges);
3078+
sendWheelEvent(wheelEvent, result.steps, rubberBandableEdges, willStartSwipe);
30793079
}
30803080

3081-
void WebPageProxy::sendWheelEvent(const WebWheelEvent& event, OptionSet<WebCore::WheelEventProcessingSteps> processingSteps, RectEdges<bool> rubberBandableEdges)
3081+
void WebPageProxy::sendWheelEvent(const WebWheelEvent& event, OptionSet<WebCore::WheelEventProcessingSteps> processingSteps, RectEdges<bool> rubberBandableEdges, std::optional<bool> willStartSwipe)
30823082
{
30833083
#if HAVE(CVDISPLAYLINK)
30843084
m_wheelEventActivityHysteresis.impulse();
@@ -3092,7 +3092,7 @@ void WebPageProxy::sendWheelEvent(const WebWheelEvent& event, OptionSet<WebCore:
30923092
sendWheelEventScrollingAccelerationCurveIfNecessary(event);
30933093
connection->send(Messages::EventDispatcher::WheelEvent(m_webPageID, event, rubberBandableEdges), 0, { }, Thread::QOS::UserInteractive);
30943094
} else {
3095-
sendWithAsyncReply(Messages::WebPage::HandleWheelEvent(event, processingSteps), [weakThis = WeakPtr { *this }, platformWheelEvent = platform(event)](ScrollingNodeID nodeID, std::optional<WheelScrollGestureState> gestureState) {
3095+
sendWithAsyncReply(Messages::WebPage::HandleWheelEvent(event, processingSteps, willStartSwipe), [weakThis = WeakPtr { *this }, platformWheelEvent = platform(event)](ScrollingNodeID nodeID, std::optional<WheelScrollGestureState> gestureState) {
30963096
RefPtr protectedThis = weakThis.get();
30973097
if (!protectedThis)
30983098
return;
@@ -3117,7 +3117,7 @@ void WebPageProxy::wheelEventHandlingCompleted(bool wasHandled)
31173117
{
31183118
auto oldestProcessedEvent = wheelEventCoalescer().takeOldestEventBeingProcessed();
31193119

3120-
LOG_WITH_STREAM(WheelEvents, stream << "WebPageProxy::wheelEventHandlingCompleted - taking " << platform(oldestProcessedEvent) << " handled " << wasHandled);
3120+
LOG_WITH_STREAM(WheelEvents, stream << "WebPageProxy::wheelEventHandlingCompleted - finished handling " << platform(oldestProcessedEvent) << " handled " << wasHandled);
31213121

31223122
if (!wasHandled) {
31233123
m_uiClient->didNotHandleWheelEvent(this, oldestProcessedEvent);

‎Source/WebKit/UIProcess/WebPageProxy.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,7 +1091,7 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>
10911091

10921092
bool isProcessingWheelEvents() const;
10931093
void handleNativeWheelEvent(const NativeWebWheelEvent&);
1094-
void continueWheelEventHandling(const WebWheelEvent&, const WebCore::WheelEventHandlingResult&);
1094+
void continueWheelEventHandling(const WebWheelEvent&, const WebCore::WheelEventHandlingResult&, std::optional<bool> willStartSwipe);
10951095

10961096
bool isProcessingKeyboardEvents() const;
10971097
bool handleKeyboardEvent(const NativeWebKeyboardEvent&);
@@ -2630,7 +2630,7 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>
26302630
void setRenderTreeSize(uint64_t treeSize) { m_renderTreeSize = treeSize; }
26312631

26322632
void handleWheelEvent(const WebWheelEvent&);
2633-
void sendWheelEvent(const WebWheelEvent&, OptionSet<WebCore::WheelEventProcessingSteps>, WebCore::RectEdges<bool> rubberBandableEdges);
2633+
void sendWheelEvent(const WebWheelEvent&, OptionSet<WebCore::WheelEventProcessingSteps>, WebCore::RectEdges<bool> rubberBandableEdges, std::optional<bool> willStartSwipe);
26342634

26352635
void wheelEventHandlingCompleted(bool wasHandled);
26362636

‎Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ class RemoteScrollingCoordinator final : public WebCore::AsyncScrollingCoordinat
5757

5858
void addNodeWithActiveRubberBanding(WebCore::ScrollingNodeID);
5959
void removeNodeWithActiveRubberBanding(WebCore::ScrollingNodeID);
60+
61+
void setCurrentWheelEventWillStartSwipe(std::optional<bool> value) { m_currentWheelEventWillStartSwipe = value; }
6062

6163
struct NodeAndGestureState {
6264
WebCore::ScrollingNodeID wheelGestureNode { 0 };
@@ -106,6 +108,8 @@ class RemoteScrollingCoordinator final : public WebCore::AsyncScrollingCoordinat
106108
NodeAndGestureState m_currentWheelGestureInfo;
107109

108110
bool m_clearScrollLatchingInNextTransaction { false };
111+
112+
std::optional<bool> m_currentWheelEventWillStartSwipe;
109113
};
110114

111115
} // namespace WebKit

‎Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,10 @@
176176

177177
WheelEventHandlingResult RemoteScrollingCoordinator::handleWheelEventForScrolling(const PlatformWheelEvent& wheelEvent, ScrollingNodeID targetNodeID, std::optional<WheelScrollGestureState> gestureState)
178178
{
179-
LOG_WITH_STREAM(Scrolling, stream << "RemoteScrollingCoordinator::handleWheelEventForScrolling " << wheelEvent << " - node " << targetNodeID << " gestureState " << gestureState);
179+
LOG_WITH_STREAM(Scrolling, stream << "RemoteScrollingCoordinator::handleWheelEventForScrolling " << wheelEvent << " - node " << targetNodeID << " gestureState " << gestureState << " will start swipe " << (m_currentWheelEventWillStartSwipe && *m_currentWheelEventWillStartSwipe));
180180

181-
// FIXME: Need to check for swipe here, as ThreadedScrollingCoordinator::handleWheelEventForScrolling() does.
181+
if (m_currentWheelEventWillStartSwipe && *m_currentWheelEventWillStartSwipe)
182+
return WheelEventHandlingResult::unhandled();
182183

183184
m_currentWheelGestureInfo = NodeAndGestureState { targetNodeID, gestureState };
184185
return WheelEventHandlingResult::handled();

‎Source/WebKit/WebProcess/WebPage/WebPage.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3307,12 +3307,16 @@ void WebPage::mouseEvent(const WebMouseEvent& mouseEvent, std::optional<Vector<S
33073307
revokeSandboxExtensions(mouseEventSandboxExtensions);
33083308
}
33093309

3310-
void WebPage::handleWheelEvent(const WebWheelEvent& event, const OptionSet<WheelEventProcessingSteps>& processingSteps, CompletionHandler<void(WebCore::ScrollingNodeID, std::optional<WebCore::WheelScrollGestureState>)>&& completionHandler)
3310+
void WebPage::handleWheelEvent(const WebWheelEvent& event, const OptionSet<WheelEventProcessingSteps>& processingSteps, std::optional<bool> willStartSwipe, CompletionHandler<void(WebCore::ScrollingNodeID, std::optional<WebCore::WheelScrollGestureState>)>&& completionHandler)
33113311
{
3312+
auto* remoteScrollingCoordinator = dynamicDowncast<RemoteScrollingCoordinator>(scrollingCoordinator());
3313+
if (remoteScrollingCoordinator)
3314+
remoteScrollingCoordinator->setCurrentWheelEventWillStartSwipe(willStartSwipe);
3315+
33123316
bool handled = wheelEvent(event, processingSteps, EventDispatcher::WheelEventOrigin::UIProcess);
33133317
// FIXME: Ideally we'd avoid sending both a reply, and a separate WebPageProxy::DidReceiveEvent IPC, but the latter is generic to all events.
33143318
#if ENABLE(ASYNC_SCROLLING)
3315-
if (auto* remoteScrollingCoordinator = dynamicDowncast<RemoteScrollingCoordinator>(scrollingCoordinator())) {
3319+
if (remoteScrollingCoordinator) {
33163320
auto gestureInfo = remoteScrollingCoordinator->takeCurrentWheelGestureInfo();
33173321
completionHandler(gestureInfo.wheelGestureNode, gestureInfo.wheelGestureState);
33183322
} else

‎Source/WebKit/WebProcess/WebPage/WebPage.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1155,7 +1155,7 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
11551155
void startWaitingForContextMenuToShow() { m_waitingForContextMenuToShow = true; }
11561156
#endif
11571157

1158-
void handleWheelEvent(const WebWheelEvent&, const OptionSet<WebCore::WheelEventProcessingSteps>&, CompletionHandler<void(WebCore::ScrollingNodeID, std::optional<WebCore::WheelScrollGestureState>)>&&);
1158+
void handleWheelEvent(const WebWheelEvent&, const OptionSet<WebCore::WheelEventProcessingSteps>&, std::optional<bool> willStartSwipe, CompletionHandler<void(WebCore::ScrollingNodeID, std::optional<WebCore::WheelScrollGestureState>)>&&);
11591159
bool wheelEvent(const WebWheelEvent&, OptionSet<WebCore::WheelEventProcessingSteps>, EventDispatcher::WheelEventOrigin);
11601160

11611161
void wheelEventHandlersChanged(bool);

‎Source/WebKit/WebProcess/WebPage/WebPage.messages.in‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
676676
SetAppHighlightsVisibility(enum:bool WebCore::HighlightVisibility highlightVisibility)
677677
#endif
678678

679-
HandleWheelEvent(WebKit::WebWheelEvent event, OptionSet<WebCore::WheelEventProcessingSteps> processingSteps) -> (uint64_t scrollingNodeID, std::optional<WebCore::WheelScrollGestureState> gestureState)
679+
HandleWheelEvent(WebKit::WebWheelEvent event, OptionSet<WebCore::WheelEventProcessingSteps> processingSteps, std::optional<bool> willStartSwipe) -> (uint64_t scrollingNodeID, std::optional<WebCore::WheelScrollGestureState> gestureState)
680680
#if PLATFORM(IOS_FAMILY)
681681
DispatchWheelEventWithoutScrolling(WebKit::WebWheelEvent event) -> (bool handled)
682682
#endif

0 commit comments

Comments
 (0)