Branch: refs/heads/main Home: https://github.com/WebKit/WebKit Commit: 02526276204938ef6d32be0f75f1bdb076469963 https://github.com/WebKit/WebKit/commit/02526276204938ef6d32be0f75f1bdb076469963 Author: Wenson Hsieh <wenson_hs...@apple.com> Date: 2023-04-18 (Tue, 18 Apr 2023)
Changed paths: A LayoutTests/css3/scroll-snap/scroll-snap-discrete-wheel-events-with-layout-expected.txt A LayoutTests/css3/scroll-snap/scroll-snap-discrete-wheel-events-with-layout.html M LayoutTests/platform/glib/TestExpectations M LayoutTests/platform/ios-wk2/TestExpectations M Source/WebCore/page/scrolling/ScrollingTree.cpp M Source/WebCore/page/scrolling/ScrollingTree.h M Source/WebCore/platform/mac/ScrollingEffectsController.mm M Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h M Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp M Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.h M Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingCoordinatorProxyMac.h M Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingCoordinatorProxyMac.mm Log Message: ----------- [macOS] Performing layout when scroll snapping with a physical mouse wheel snaps to the last snap position https://bugs.webkit.org/show_bug.cgi?id=255603 Reviewed by Tim Horton. Currently, when `resnapAfterLayout()` is called after a layout pass while scrolling with a physical mouse wheel in a scroll snapping container, we end up erroneously re-snapping to the last active snap position. This doesn't happen when using a trackpad to scroll because we bail here: ``` void ScrollableArea::resnapAfterLayout() { … if (!scrollAnimator || isScrollSnapInProgress() || isUserScrollInProgress()) return; ``` …due to the fact that `isUserScrollInProgress()` is `true`, since this flag is set over the course of both user-driven and momentum scrolling phases. Importantly, note that `isScrollSnapInProgress()` is only `true` in this case where UI-side compositing is *disabled* — this is because nothing currently calls `{add|remove}NodeWithActiveScrollSnap` on `RemoteScrollingUIState`, which means that we never end up propagating `m_nodesWithActiveScrollSnap` to the web process when UI-side compositing is enabled, so from the web-process' perspective, `isScrollSnapInProgress()` is always `false`. As such, in order to make physical mouse wheel scrolling work well when there are interleaved layout passes, the fix is two-fold: 1. Consider `isScrollSnapInProgress()` to be true if the discrete wheel event timer is scheduled. 2. Add plumbing to deliver `isScrollSnapInProgress()` state from the UI process to the web process through the scrolling state tree, to ensure that this bug fix is also effective when UI-side compositing is enabled. Test: css3/scroll-snap/scroll-snap-discrete-wheel-events-with-layout.html * LayoutTests/css3/scroll-snap/scroll-snap-discrete-wheel-events-with-layout-expected.txt: Added. * LayoutTests/css3/scroll-snap/scroll-snap-discrete-wheel-events-with-layout.html: Added. Add a new test case to exercise the bug fix. * LayoutTests/platform/glib/TestExpectations: * LayoutTests/platform/ios-wk2/TestExpectations: * Source/WebCore/page/scrolling/ScrollingTree.cpp: (WebCore::ScrollingTree::setNodeScrollSnapInProgress): * Source/WebCore/page/scrolling/ScrollingTree.h: (WebCore::ScrollingTree::scrollingTreeNodeDidBeginScrollSnapping): (WebCore::ScrollingTree::scrollingTreeNodeDidEndScrollSnapping): Add new override hooks to allow the client layer to know when scrolling tree nodes change "scroll snap in progress" state. See WebKit2 changes below for more information. * Source/WebCore/platform/mac/ScrollingEffectsController.mm: (WebCore::ScrollingEffectsController::stopAllTimers): (WebCore::ScrollingEffectsController::isScrollSnapInProgress const): Consider scroll snap in progress if we've scheduled a scroll snap while handling discrete wheel events. (WebCore::ScrollingEffectsController::discreteSnapTransitionTimerFired): Add a couple of call sites to `m_client.didStopScrollSnapAnimation()` in the case where the timer is either stopped early or without triggering a scroll snap animation, such that we don't end up with a node being stuck indefinitely in `nodesWithActiveScrollSnap`. * Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h: (WebKit::RemoteScrollingCoordinatorProxy::scrollingTreeNodeDidBeginScrollSnapping): (WebKit::RemoteScrollingCoordinatorProxy::scrollingTreeNodeDidEndScrollSnapping): * Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp: (WebKit::RemoteScrollingTree::scrollingTreeNodeDidBeginScrollSnapping): (WebKit::RemoteScrollingTree::scrollingTreeNodeDidEndScrollSnapping): Add plumbing from `RemoteScrollingTree` -> `RemoteScrollingCoordinatorProxy` -> `RemoteScrollingUIState` whenever a scrolling node begins or ends scroll snapping progress. * Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.h: * Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingCoordinatorProxyMac.h: * Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingCoordinatorProxyMac.mm: (WebKit::RemoteScrollingCoordinatorProxyMac::scrollingTreeNodeDidBeginScrollSnapping): (WebKit::RemoteScrollingCoordinatorProxyMac::scrollingTreeNodeDidEndScrollSnapping): Canonical link: https://commits.webkit.org/263108@main _______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes