Bug 195254

Summary: Share more code for updating the state of frame scrolling nodes
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, fred.wang, koivisto, rniwa, simon.fraser, sroberts, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 195258    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews106 for mac-highsierra-wk2
none
Patch
koivisto: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews123 for ios-simulator-wk2 none

Description Simon Fraser (smfr) 2019-03-02 20:32:12 PST
Share more code for updating the state of frame scrolling nodes
Comment 1 Simon Fraser (smfr) 2019-03-02 20:37:43 PST
Created attachment 363440 [details]
Patch
Comment 2 EWS Watchlist 2019-03-02 21:58:23 PST Comment hidden (obsolete)
Comment 3 EWS Watchlist 2019-03-02 21:58:24 PST Comment hidden (obsolete)
Comment 4 Simon Fraser (smfr) 2019-03-03 09:12:56 PST
Created attachment 363458 [details]
Patch
Comment 5 EWS Watchlist 2019-03-03 11:30:36 PST
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
Comment 6 EWS Watchlist 2019-03-03 11:30:38 PST
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
Comment 7 Antti Koivisto 2019-03-03 11:46:31 PST
Comment on attachment 363458 [details]
Patch

r=me with the iOS test fixed
Comment 8 Simon Fraser (smfr) 2019-03-04 08:22:06 PST
http://trac.webkit.org/r242333
Comment 9 Radar WebKit Bug Importer 2019-03-04 08:25:36 PST
<rdar://problem/48562627>
Comment 10 Frédéric Wang (:fredw) 2019-03-05 06:15:43 PST
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.
Comment 11 Frédéric Wang (:fredw) 2019-03-05 06:47:56 PST
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.
Comment 12 Frédéric Wang (:fredw) 2019-03-05 06:54:23 PST
Committed r242465: <https://trac.webkit.org/changeset/242465>
Comment 13 Simon Fraser (smfr) 2019-03-05 09:33:26 PST
(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.
Comment 14 Shawn Roberts 2019-04-03 13:54:07 PDT
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.