[webkit-changes] [WebKit/WebKit] 025262: [macOS] Performing layout when scroll snapping wit...

[Wenson Hsieh]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 02526276204938ef6d32be0f75f1bdb076469963
      https://github.com/WebKit/WebKit/commit/02526276204938ef6d32be0f75f1bdb076469963
  Author: Wenson Hsieh <wenson_hsieh at 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