imported/w3c/web-platform-tests/css/cssom-view/scroll-behavior-main-frame-window.html imported/w3c/web-platform-tests/css/cssom-view/scroll-behavior-main-frame-root.html are constantly failing on Catalina, BigSur wk2 Release since r283871 (https://trac.webkit.org/changeset/283871/webkit). History: https://results.webkit.org/?platform=ios&platform=mac&suite=layout-tests&suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcssom-view%2Fscroll-behavior-main-frame-root.html&test=imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcssom-view%2Fscroll-behavior-main-frame-window.html Build: https://build.webkit.org/#/builders/70/builds/5814 Results: https://build.webkit.org/results/Apple-BigSur-Release-WK2-Tests/r283982%20(5814)/results.html Diff: --- /Volumes/Data/worker/bigsur-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/css/cssom-view/scroll-behavior-main-frame-root-expected.txt +++ /Volumes/Data/worker/bigsur-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/css/cssom-view/scroll-behavior-main-frame-root-actual.txt @@ -36,6 +36,6 @@ PASS Set scrollLeft to frame with smooth scroll-behavior PASS Set scrollTop to frame with auto scroll-behavior PASS Set scrollTop to frame with smooth scroll-behavior -PASS Aborting an ongoing smooth scrolling on the main frame with another smooth scrolling -PASS Aborting an ongoing smooth scrolling on the main frame with an instant scrolling +FAIL Aborting an ongoing smooth scrolling on the main frame with another smooth scrolling assert_equals: Final value of scrollLeft expected 1215 but got 2430 +FAIL Aborting an ongoing smooth scrolling on the main frame with an instant scrolling assert_equals: Final value of scrollLeft expected 0 but got 2430 --- /Volumes/Data/worker/bigsur-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/css/cssom-view/scroll-behavior-main-frame-window-expected.txt +++ /Volumes/Data/worker/bigsur-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/css/cssom-view/scroll-behavior-main-frame-window-actual.txt @@ -25,5 +25,5 @@ PASS Main frame with smooth scroll-behavior ; scrollBy() with instant behavior PASS Main frame with smooth scroll-behavior ; scrollBy() with smooth behavior PASS Aborting an ongoing smooth scrolling on the main frame with another smooth scrolling -PASS Aborting an ongoing smooth scrolling on the main frame with an instant scrolling +FAIL Aborting an ongoing smooth scrolling on the main frame with an instant scrolling assert_equals: Final value of scrollLeft expected 0 but got 2430 The failure is flaky, but frequent on scroll-behavior-main-frame-window.html and constant on scroll-behavior-main-frame-root.html. Reproduced test failure locally using run-webkit-tests --iterations 1000 --exit-after-n-failures 1 -f --force --clobber-old-results --release imported/w3c/web-platform-tests/css/cssom-view/scroll-behavior-main-frame-window.html and run-webkit-tests --iterations 1000 --exit-after-n-failures 1 -f --force --clobber-old-results --release imported/w3c/web-platform-tests/css/cssom-view/scroll-behavior-main-frame-root.html
<rdar://problem/84158655>
Updated test expectations at https://trac.webkit.org/changeset/284000/webkit
Created attachment 443854 [details] Patch
Created attachment 443883 [details] Patch
Created attachment 443886 [details] Patch
Comment on attachment 443886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443886&action=review I think the real bug might be that ThreadedScrollingTree::scrollingTreeNodeRequestsScroll() should use m_nodesWithPendingScrollAnimations.set() instead of m_nodesWithPendingScrollAnimations.add() (see HashMap.h) > Source/WebCore/ChangeLog:9 > + Temporary fix for issue where second scroll doesn't cancel a current > + scroll animation. Is it temporary?
Comment on attachment 443886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443886&action=review > Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp:256 > + scrollingTree().clearAnimationsForScrollingNodeID(scrollingNodeID()); > > if (requestedScrollData.requestType == ScrollRequestType::CancelAnimatedScroll) > return; But maybe we do need to clear the pending animation if we hit the CancelAnimatedScroll clause.
Created attachment 443911 [details] Patch
Comment on attachment 443911 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443911&action=review > Source/WebCore/ChangeLog:10 > + request has come in (first animation still in m_nodesWithPendingScrollAnimations). For the "come in" could be more specific. Maybe "committed to the scrolling tree" or "received by the scrolling thread", whichever it is. > Source/WebCore/page/scrolling/ThreadedScrollingTree.h:67 > + void removePendingScrollAnimationForNode(ScrollingNodeID nodeID) WTF_REQUIRES_LOCK(m_treeLock) override { m_nodesWithPendingScrollAnimations.remove(nodeID); } Since this is virtual, I would move the implementation to the .cpp file. I think it can be 'final' instead of 'override'.
Created attachment 443990 [details] Patch
Committed r285669 (244156@main): <https://commits.webkit.org/244156@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 443990 [details].