Bug 194973

Summary: Have a single notion of scroll position in the scrolling tree and derive layoutViewport from it
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, ews-watchlist, fred.wang, jamesr, koivisto, luiz, simon.fraser, tonikitoo, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=196115
Bug Depends on: 194968, 194984    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
koivisto: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Patch
none
Patch
none
Patch
ews-watchlist: commit-queue-
Archive of layout-test-results from ews123 for ios-simulator-wk2 none

Description Simon Fraser (smfr) 2019-02-22 17:57:20 PST
Have a single notion of scroll position in the scrolling tree and derive layoutViewport from it
Comment 1 Simon Fraser (smfr) 2019-02-22 18:01:31 PST
Created attachment 362809 [details]
Patch
Comment 2 Antti Koivisto 2019-02-22 19:08:23 PST
Comment on attachment 362809 [details]
Patch

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

> Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:63
> +    FloatPoint scrollPosition() const { return m_scrollPosition; }

As discussed, it would be good to clarify in naming this is the current scroll position (here and elsewhere).
Comment 3 Simon Fraser (smfr) 2019-02-24 21:54:08 PST
Created attachment 362883 [details]
Patch
Comment 4 Simon Fraser (smfr) 2019-02-24 21:55:17 PST
This new patch goes further down the same path, cleaning up a lot of cruft in the scrolling tree.
Comment 5 Radar WebKit Bug Importer 2019-02-24 21:55:58 PST
<rdar://problem/48353121>
Comment 6 Simon Fraser (smfr) 2019-02-24 22:08:12 PST
Created attachment 362884 [details]
Patch
Comment 7 EWS Watchlist 2019-02-24 22:10:57 PST
Attachment 362884 [details] did not pass style-queue:


ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:56:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:57:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:97:  Inline functions should not be annotated with WEBCORE_EXPORT. Remove the macro, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:98:  Inline functions should not be annotated with WEBCORE_EXPORT. Remove the macro, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 4 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 EWS Watchlist 2019-02-24 23:59:57 PST
Comment on attachment 362884 [details]
Patch

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

New failing tests:
fast/scrolling/ios/scroll-iframe.html
Comment 9 EWS Watchlist 2019-02-24 23:59:59 PST
Created attachment 362887 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 10 Antti Koivisto 2019-02-25 10:24:54 PST
Comment on attachment 362884 [details]
Patch

r=me
Comment 11 Simon Fraser (smfr) 2019-02-26 21:52:16 PST
Created attachment 363068 [details]
Patch
Comment 12 EWS Watchlist 2019-02-26 21:54:36 PST
Attachment 363068 [details] did not pass style-queue:


ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:56:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:57:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:97:  Inline functions should not be annotated with WEBCORE_EXPORT. Remove the macro, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:98:  Inline functions should not be annotated with WEBCORE_EXPORT. Remove the macro, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 4 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Simon Fraser (smfr) 2019-02-26 22:05:42 PST
Created attachment 363069 [details]
Patch
Comment 14 EWS Watchlist 2019-02-26 22:08:10 PST
Attachment 363069 [details] did not pass style-queue:


ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:56:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:57:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:97:  Inline functions should not be annotated with WEBCORE_EXPORT. Remove the macro, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:98:  Inline functions should not be annotated with WEBCORE_EXPORT. Remove the macro, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 4 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Simon Fraser (smfr) 2019-02-26 22:23:59 PST
Created attachment 363071 [details]
Patch
Comment 16 EWS Watchlist 2019-02-26 22:26:23 PST
Attachment 363071 [details] did not pass style-queue:


ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:56:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:57:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:97:  Inline functions should not be annotated with WEBCORE_EXPORT. Remove the macro, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:98:  Inline functions should not be annotated with WEBCORE_EXPORT. Remove the macro, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 4 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 EWS Watchlist 2019-02-27 00:31:09 PST
Comment on attachment 363071 [details]
Patch

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

New failing tests:
fast/scrolling/ios/mixing-user-and-programmatic-scroll-001.html
fast/scrolling/ios/mixing-user-and-programmatic-scroll-004.html
fast/scrolling/ios/mixing-user-and-programmatic-scroll-005.html
fast/scrolling/ios/mixing-user-and-programmatic-scroll-006.html
fast/scrolling/ios/scroll-iframe-004.html
Comment 18 EWS Watchlist 2019-02-27 00:31:11 PST
Created attachment 363079 [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 19 Frédéric Wang (:fredw) 2019-02-27 09:36:38 PST
Comment on attachment 363071 [details]
Patch

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

> Source/WebCore/ChangeLog:113
> +2019-02-22  Simon Fraser  <simon.fraser@apple.com>

Duplicate change
Comment 20 Truitt Savell 2019-02-27 15:39:32 PST
The changes in https://trac.webkit.org/changeset/242132/webkit

has caused tiled-drawing/scrolling/scroll-snap/scroll-snap-proximity-mainframe.html

to start timing out flakily.

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=tiled-drawing%2Fscrolling%2Fscroll-snap%2Fscroll-snap-proximity-mainframe.html

reproduced with: 
run-webkit-tests tiled-drawing/scrolling/scroll-snap/scroll-snap-proximity-mainframe.html --iterations 200 -f

this test times out on r242132 but not on r242131.
Comment 21 Simon Fraser (smfr) 2019-03-02 09:58:32 PST
https://trac.webkit.org/changeset/242132/webkit