[webkit-changes] [WebKit/WebKit] e94fbc: REGRESSION (262041 at main): Keyboard scrolling anima...

[Wenson Hsieh]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: e94fbc9c6f2ad75e234c67d3642c0756c19679c8
      https://github.com/WebKit/WebKit/commit/e94fbc9c6f2ad75e234c67d3642c0756c19679c8
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2023-03-24 (Fri, 24 Mar 2023)

  Changed paths:
    A LayoutTests/fast/scrolling/programmatic-smooth-scroll-from-nonzero-offset-expected.txt
    A LayoutTests/fast/scrolling/programmatic-smooth-scroll-from-nonzero-offset.html
    M LayoutTests/platform/glib/TestExpectations
    M Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.cpp
    M Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp
    M Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h
    M Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp

  Log Message:
  -----------
  REGRESSION (262041 at main): Keyboard scrolling animates from (0, 0) when smooth keyboard scrolling is disabled and UI-side compositing is enabled
https://bugs.webkit.org/show_bug.cgi?id=254428

Reviewed by Tim Horton.

My previous patch in 262041 at main had a serious flaw — when decoding `ScrollingStateScrollingNode`,
we'll decode a `RequestedScrollData` and use `ScrollingStateScrollingNode::setRequestedScrollData`
to set the decoded data on the scrolling node. However, since:

- `setRequestedScrollData()` contains the request merging logic I added in 262041 at main, and...
- We decode the changed property flags before calling `setRequestedScrollData`.

...we end up treating all animated scrolling as starting from the origin. This patch fixes that by
keeping the merging logic in the web process, by passing in a `CanMergeScrollData::No` flag when
deserializing the scrolling state node.

Test: fast/scrolling/programmatic-smooth-scroll-from-nonzero-offset.html

* LayoutTests/fast/scrolling/programmatic-smooth-scroll-from-nonzero-offset-expected.txt: Added.
* LayoutTests/fast/scrolling/programmatic-smooth-scroll-from-nonzero-offset.html: Added.

Add a new layout test to exercise the bug, by scrolling (without animation) to a nonzero offset and
then performing an animated scroll from that offset, verifying that we don't attempt to start from
(0, 0) when performing the animated scroll.

* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.cpp:
(WebCore::RequestedScrollData::merge):

After fixing the above issue, I also discovered that with UI-side compositing enabled, the original
tests in `imported/w3c/web-platform-tests/css/cssom-view` that I tried to fix started failing again;
this is because of another subtle corner case that wasn't covered in when merging `RequestedScrollData`,
wherein the following sequence of programmatic scroll requests:

1. ScrollRequestType::PositionUpdate (Non-Animated)
2. ScrollRequestType::CancelAnimatedScroll
3. ScrollRequestType::PositionUpdate (Animated)

...caused us to stomp over the `requestedDataBeforeAnimatedScroll` in step (2), when merging the
animated scroll cancellation request into the position update. To address this and keep the WPT
passing with UI-side compositing enabled, I've adjusted the merging logic to preserve `scrollPosition`
after step (2), so that we can stash it in the `requestedDataBeforeAnimatedScroll` in step (3).

* Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp:
(WebCore::ScrollingStateScrollingNode::setRequestedScrollData):
* Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
(ArgumentCoder<ScrollingStateScrollingNode>::decode):

Canonical link: https://commits.webkit.org/262105@main