Bug 195254 – Share more code for updating the state of frame scrolling nodes

• Component: New Bugs
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Share more code for updating the state of frame scrolling nodes

[Simon Fraser (smfr)]

Created attachment 363440 [details]
Patch

[EWS Watchlist]

Comment on attachment 363440 [details]
Patch

Attachment 363440 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11348622

New failing tests:
fast/visual-viewport/tiled-drawing/zoomed-fixed-scrolled-down.html
fast/visual-viewport/tiled-drawing/zoomed-fixed-scrolled-down-then-up.html

[EWS Watchlist]

Created attachment 363446 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Simon Fraser (smfr)]

Created attachment 363458 [details]
Patch

[EWS Watchlist]

Comment on attachment 363458 [details]
Patch

Attachment 363458 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11353961

New failing tests:
scrollingcoordinator/scrolling-tree/fixed-inside-frame.html

[EWS Watchlist]

Created attachment 363461 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Antti Koivisto]

Comment on attachment 363458 [details]
Patch

r=me with the iOS test fixed

[Simon Fraser (smfr)]

http://trac.webkit.org/r242333

[Radar WebKit Bug Importer]

<rdar://problem/48562627>

[Frédéric Wang (:fredw)]

Comment on attachment 363458 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363458&action=review

> Source/WebCore/ChangeLog:15
> +        ScrollingStateOverflowScrollingNodews, since both can be updated from a ScrollableArea.

too late, but s/ScrollingStateOverflowScrollingNodews/ScrollingStateOverflowScrollingNodes/

> Source/WebCore/ChangeLog:18
> +        on macOS.

Does this patch add rubber-stamping support on macOS or only prepares the support? In the former case, I think we need a test.

[Frédéric Wang (:fredw)]

Comment on attachment 363458 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363458&action=review

> Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp:133
>          return;

frameView is no longer used on other platforms, so we should add ASSERT_UNUSED.

[Frédéric Wang (:fredw)]

Committed r242465: <https://trac.webkit.org/changeset/242465>

[Simon Fraser (smfr)]

(In reply to Frédéric Wang (:fredw) from comment #12)
> Committed r242465: <https://trac.webkit.org/changeset/242465>

Thanks for the fix.

I agree a test would be good (also a test that shows that scrollbars become visible), but overflow scrolling on macOS isn't my focus now, and we're debating whether we'll enable the threaded overflow scroll on macOS, so I didn't want to invest a lot of time in it.

[Shawn Roberts]

This change caused scrollingcoordinator/scrolling-tree/fixed-inside-frame.html to fail.

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=scrollingcoordinator%2Fscrolling-tree%2Ffixed-inside-frame.html

Did not notice before because I had marked this test flaky prior to this change. 

I rebaselined the test in https://trac.webkit.org/changeset/243825/webkit to fix it.