Sticky positioning is jumpy in many overflow cases
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Jun 2019 21:53:57 +0000 (21:53 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Jun 2019 21:53:57 +0000 (21:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198532
<rdar://problem/51400532>

Reviewed by Simon Fraser.

Source/WebCore:

Tests: scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1.html
       scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2.html
       scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1.html
       scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2.html
       scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick.html
       scrollingcoordinator/ios/sticky-overflow-stacking-context-stick.html

* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::notifyRelatedNodesAfterScrollPositionChange):
(WebCore::ScrollingTree::notifyRelatedNodesRecursive):

Simplify for relatedNodeScrollPositionDidChange removal.

* page/scrolling/ScrollingTree.h:
* page/scrolling/ScrollingTreeNode.cpp:
(WebCore::ScrollingTreeNode::relatedNodeScrollPositionDidChange): Deleted.
* page/scrolling/ScrollingTreeNode.h:
* page/scrolling/cocoa/ScrollingTreeFixedNode.mm:
(WebCore::ScrollingTreeFixedNode::applyLayerPositions):
* page/scrolling/cocoa/ScrollingTreePositionedNode.h:
* page/scrolling/cocoa/ScrollingTreePositionedNode.mm:
(WebCore::ScrollingTreePositionedNode::scrollOffsetSinceLastCommit const):

Factor into a function.

(WebCore::ScrollingTreePositionedNode::applyLayerPositions):
(WebCore::ScrollingTreePositionedNode::relatedNodeScrollPositionDidChange): Deleted.

We can't bail out based on changed node as that makes us compute different positions based on what the change root is.
Since all relatedNodeScrollPositionDidChange functions now always simply call applyLayerPositions we can remove the whole thing.

* page/scrolling/cocoa/ScrollingTreeStickyNode.mm:
(WebCore::ScrollingTreeStickyNode::applyLayerPositions):

Implement taking into account that the containing scroller may not be our ancestor.

LayoutTests:

* scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1-expected.html: Added.
* scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1.html: Added.
* scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2-expected.html: Added.
* scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2.html: Added.
* scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1-expected.html: Added.
* scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1.html: Added.
* scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2-expected.html: Added.
* scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2.html: Added.
* scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick-expected.html: Added.
* scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick.html: Added.
* scrollingcoordinator/ios/sticky-overflow-stacking-context-stick-expected.html: Added.
* scrollingcoordinator/ios/sticky-overflow-stacking-context-stick.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246083 268f45cc-cd09-0410-ab3c-d52691b4dbfc

21 files changed:
LayoutTests/ChangeLog
LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1-expected.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2-expected.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1-expected.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2-expected.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick-expected.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-stick-expected.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-stick.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingTree.cpp
Source/WebCore/page/scrolling/ScrollingTree.h
Source/WebCore/page/scrolling/ScrollingTreeNode.cpp
Source/WebCore/page/scrolling/ScrollingTreeNode.h
Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.h
Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm
Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm

index 99432ed..ddf3ddd 100644 (file)
@@ -1,3 +1,24 @@
+2019-06-04  Antti Koivisto  <antti@apple.com>
+
+        Sticky positioning is jumpy in many overflow cases
+        https://bugs.webkit.org/show_bug.cgi?id=198532
+        <rdar://problem/51400532>
+
+        Reviewed by Simon Fraser.
+
+        * scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1-expected.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2-expected.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1-expected.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2-expected.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick-expected.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-stacking-context-stick-expected.html: Added.
+        * scrollingcoordinator/ios/sticky-overflow-stacking-context-stick.html: Added.
+
 2019-06-04  Takashi Komori  <Takashi.Komori@sony.com>
 
         [WinCairo] Implement cpu and memory measuring functions.
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1-expected.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1-expected.html
new file mode 100644 (file)
index 0000000..e9309d6
--- /dev/null
@@ -0,0 +1,62 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+            
+            await UIHelper.ensurePresentationUpdate();
+            document.querySelector('.scroller').scrollTo(0, 20);
+            await UIHelper.ensurePresentationUpdate();
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1.html
new file mode 100644 (file)
index 0000000..a98749e
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+
+            const scrollUpdatesDisabled = true;
+            await UIHelper.immediateScrollElementAtContentPointToOffset(50, 50, 0, 20, scrollUpdatesDisabled);
+            testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2-expected.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2-expected.html
new file mode 100644 (file)
index 0000000..6442115
--- /dev/null
@@ -0,0 +1,64 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            z-index: 0;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+            will-change: transform;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+            
+            await UIHelper.ensurePresentationUpdate();
+            document.querySelector('.scroller').scrollTo(0, 20);
+            await UIHelper.ensurePresentationUpdate();
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2.html
new file mode 100644 (file)
index 0000000..42a4f1b
--- /dev/null
@@ -0,0 +1,60 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+            will-change: transform;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+
+            const scrollUpdatesDisabled = true;
+            await UIHelper.immediateScrollElementAtContentPointToOffset(50, 50, 0, 20, scrollUpdatesDisabled);
+            testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1-expected.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1-expected.html
new file mode 100644 (file)
index 0000000..677451f
--- /dev/null
@@ -0,0 +1,62 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+            
+            await UIHelper.ensurePresentationUpdate();
+            document.querySelector('.scroller').scrollTo(0, 200);
+            await UIHelper.ensurePresentationUpdate();
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1.html
new file mode 100644 (file)
index 0000000..67131b9
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+
+            const scrollUpdatesDisabled = true;
+            await UIHelper.immediateScrollElementAtContentPointToOffset(50, 50, 0, 200, scrollUpdatesDisabled);
+            testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2-expected.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2-expected.html
new file mode 100644 (file)
index 0000000..28c9714
--- /dev/null
@@ -0,0 +1,64 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            z-index: 0;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+            will-change: transform;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+            
+            await UIHelper.ensurePresentationUpdate();
+            document.querySelector('.scroller').scrollTo(0, 200);
+            await UIHelper.ensurePresentationUpdate();
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2.html
new file mode 100644 (file)
index 0000000..dc90ab7
--- /dev/null
@@ -0,0 +1,60 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+            will-change: transform;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+
+            const scrollUpdatesDisabled = true;
+            await UIHelper.immediateScrollElementAtContentPointToOffset(50, 50, 0, 200, scrollUpdatesDisabled);
+            testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick-expected.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick-expected.html
new file mode 100644 (file)
index 0000000..de54df1
--- /dev/null
@@ -0,0 +1,63 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            z-index: 0;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+            
+            await UIHelper.ensurePresentationUpdate();
+            document.querySelector('.scroller').scrollTo(0, 20);
+            await UIHelper.ensurePresentationUpdate();
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick.html
new file mode 100644 (file)
index 0000000..2cc84ff
--- /dev/null
@@ -0,0 +1,60 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            z-index: 0;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+
+            const scrollUpdatesDisabled = true;
+            await UIHelper.immediateScrollElementAtContentPointToOffset(50, 50, 0, 20, scrollUpdatesDisabled);
+            testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-stick-expected.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-stick-expected.html
new file mode 100644 (file)
index 0000000..2e8a347
--- /dev/null
@@ -0,0 +1,63 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            z-index: 0;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+            
+            await UIHelper.ensurePresentationUpdate();
+            document.querySelector('.scroller').scrollTo(0, 200);
+            await UIHelper.ensurePresentationUpdate();
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-stick.html b/LayoutTests/scrollingcoordinator/ios/sticky-overflow-stacking-context-stick.html
new file mode 100644 (file)
index 0000000..4d00f84
--- /dev/null
@@ -0,0 +1,60 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+            z-index: 0;
+            position: relative;
+        }
+        
+        .sticky {
+            position: -webkit-sticky;
+            top: 0px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+
+        .container {
+            margin: 40px;
+            border: 2px solid red;
+            height: 5000px;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+
+            const scrollUpdatesDisabled = true;
+            await UIHelper.immediateScrollElementAtContentPointToOffset(50, 50, 0, 200, scrollUpdatesDisabled);
+            testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="spacer">
+        <div class="scroller">
+            <div class="container">
+                <div class="sticky"></div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
index fd3a463..18993da 100644 (file)
@@ -1,3 +1,47 @@
+2019-06-04  Antti Koivisto  <antti@apple.com>
+
+        Sticky positioning is jumpy in many overflow cases
+        https://bugs.webkit.org/show_bug.cgi?id=198532
+        <rdar://problem/51400532>
+
+        Reviewed by Simon Fraser.
+
+        Tests: scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-1.html
+               scrollingcoordinator/ios/sticky-overflow-no-stacking-context-no-stick-2.html
+               scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-1.html
+               scrollingcoordinator/ios/sticky-overflow-no-stacking-context-stick-2.html
+               scrollingcoordinator/ios/sticky-overflow-stacking-context-no-stick.html
+               scrollingcoordinator/ios/sticky-overflow-stacking-context-stick.html
+
+        * page/scrolling/ScrollingTree.cpp:
+        (WebCore::ScrollingTree::notifyRelatedNodesAfterScrollPositionChange):
+        (WebCore::ScrollingTree::notifyRelatedNodesRecursive):
+
+        Simplify for relatedNodeScrollPositionDidChange removal.
+
+        * page/scrolling/ScrollingTree.h:
+        * page/scrolling/ScrollingTreeNode.cpp:
+        (WebCore::ScrollingTreeNode::relatedNodeScrollPositionDidChange): Deleted.
+        * page/scrolling/ScrollingTreeNode.h:
+        * page/scrolling/cocoa/ScrollingTreeFixedNode.mm:
+        (WebCore::ScrollingTreeFixedNode::applyLayerPositions):
+        * page/scrolling/cocoa/ScrollingTreePositionedNode.h:
+        * page/scrolling/cocoa/ScrollingTreePositionedNode.mm:
+        (WebCore::ScrollingTreePositionedNode::scrollOffsetSinceLastCommit const):
+
+        Factor into a function.
+
+        (WebCore::ScrollingTreePositionedNode::applyLayerPositions):
+        (WebCore::ScrollingTreePositionedNode::relatedNodeScrollPositionDidChange): Deleted.
+
+        We can't bail out based on changed node as that makes us compute different positions based on what the change root is.
+        Since all relatedNodeScrollPositionDidChange functions now always simply call applyLayerPositions we can remove the whole thing.
+
+        * page/scrolling/cocoa/ScrollingTreeStickyNode.mm:
+        (WebCore::ScrollingTreeStickyNode::applyLayerPositions):
+
+        Implement taking into account that the containing scroller may not be our ancestor.
+
 2019-06-04  Takashi Komori  <Takashi.Komori@sony.com>
 
         [WinCairo] Implement cpu and memory measuring functions.
index 046713b..5eac7b2 100644 (file)
@@ -294,28 +294,28 @@ void ScrollingTree::notifyRelatedNodesAfterScrollPositionChange(ScrollingTreeScr
     if (is<ScrollingTreeOverflowScrollingNode>(changedNode))
         additionalUpdateRoots = overflowRelatedNodes().get(changedNode.scrollingNodeID());
 
-    notifyRelatedNodesRecursive(changedNode, changedNode);
+    notifyRelatedNodesRecursive(changedNode);
     
     for (auto positionedNodeID : additionalUpdateRoots) {
         auto* positionedNode = nodeForID(positionedNodeID);
         if (positionedNode)
-            notifyRelatedNodesRecursive(changedNode, *positionedNode);
+            notifyRelatedNodesRecursive(*positionedNode);
     }
 }
 
-void ScrollingTree::notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode)
+void ScrollingTree::notifyRelatedNodesRecursive(ScrollingTreeNode& node)
 {
-    currNode.relatedNodeScrollPositionDidChange(changedNode);
+    node.applyLayerPositions();
 
-    if (!currNode.children())
+    if (!node.children())
         return;
 
-    for (auto& child : *currNode.children()) {
+    for (auto& child : *node.children()) {
         // Never need to cross frame boundaries, since scroll layer adjustments are isolated to each document.
         if (is<ScrollingTreeFrameScrollingNode>(child))
             continue;
 
-        notifyRelatedNodesRecursive(changedNode, *child);
+        notifyRelatedNodesRecursive(*child);
     }
 }
 
index 6f8362e..17a6ae7 100644 (file)
@@ -167,7 +167,7 @@ private:
 
     void applyLayerPositionsRecursive(ScrollingTreeNode&);
 
-    void notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode);
+    void notifyRelatedNodesRecursive(ScrollingTreeNode&);
 
     Lock m_treeMutex; // Protects the scrolling tree.
 
index 872b43f..ea74e2c 100644 (file)
@@ -77,11 +77,6 @@ bool ScrollingTreeNode::isRootNode() const
     return m_scrollingTree.rootNode() == this;
 }
 
-void ScrollingTreeNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode&)
-{
-    applyLayerPositions();
-}
-
 void ScrollingTreeNode::dumpProperties(TextStream& ts, ScrollingStateTreeAsTextBehavior behavior) const
 {
     if (behavior & ScrollingStateTreeAsTextBehaviorIncludeNodeIDs)
index 09e9a09..3b1b749 100644 (file)
@@ -84,8 +84,6 @@ protected:
     ScrollingTreeNode(ScrollingTree&, ScrollingNodeType, ScrollingNodeID);
     ScrollingTree& scrollingTree() const { return m_scrollingTree; }
 
-    WEBCORE_EXPORT virtual void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode);
-
     virtual void applyLayerPositions() = 0;
 
     WEBCORE_EXPORT virtual void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const;
index 8cc6b73..cfbc07e 100644 (file)
@@ -46,11 +46,12 @@ public:
     ScrollPositioningBehavior scrollPositioningBehavior() const { return m_constraints.scrollPositioningBehavior(); }
     const Vector<ScrollingNodeID>& relatedOverflowScrollingNodes() const { return m_relatedOverflowScrollingNodes; }
 
+    FloatSize scrollOffsetSinceLastCommit() const;
+
 private:
     ScrollingTreePositionedNode(ScrollingTree&, ScrollingNodeID);
 
     void commitStateBeforeChildren(const ScrollingStateNode&) override;
-    void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode) override;
 
     void applyLayerPositions() override;
 
index 66a8a7a..b29fcd6 100644 (file)
@@ -76,36 +76,33 @@ void ScrollingTreePositionedNode::commitStateBeforeChildren(const ScrollingState
         scrollingTree().positionedNodesWithRelatedOverflow().add(scrollingNodeID());
 }
 
-void ScrollingTreePositionedNode::applyLayerPositions()
+FloatSize ScrollingTreePositionedNode::scrollOffsetSinceLastCommit() const
 {
-    FloatSize scrollOffsetSinceLastCommit;
+    FloatSize offset;
     for (auto nodeID : m_relatedOverflowScrollingNodes) {
         if (auto* node = scrollingTree().nodeForID(nodeID)) {
             if (is<ScrollingTreeOverflowScrollingNode>(node)) {
                 auto& overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(*node);
-                scrollOffsetSinceLastCommit += overflowNode.lastCommittedScrollPosition() - overflowNode.currentScrollPosition();
+                offset += overflowNode.currentScrollPosition() - overflowNode.lastCommittedScrollPosition();
             }
         }
     }
-    auto layerOffset = -scrollOffsetSinceLastCommit;
     if (m_constraints.scrollPositioningBehavior() == ScrollPositioningBehavior::Stationary) {
         // Stationary nodes move in the opposite direction.
-        layerOffset = -layerOffset;
+        return -offset;
     }
 
-    FloatPoint layerPosition = m_constraints.layerPositionAtLastLayout() - layerOffset;
-
-    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreePositionedNode " << scrollingNodeID() << " applyLayerPositions: overflow delta " << scrollOffsetSinceLastCommit << " moving layer to " << layerPosition);
-
-    [m_layer _web_setLayerTopLeftPosition:layerPosition - m_constraints.alignmentOffset()];
+    return offset;
 }
 
-void ScrollingTreePositionedNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode)
+void ScrollingTreePositionedNode::applyLayerPositions()
 {
-    if (!m_relatedOverflowScrollingNodes.contains(changedNode.scrollingNodeID()))
-        return;
+    auto offset = scrollOffsetSinceLastCommit();
+    auto layerPosition = m_constraints.layerPositionAtLastLayout() - offset;
+
+    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreePositionedNode " << scrollingNodeID() << " applyLayerPositions: overflow delta " << offset << " moving layer to " << layerPosition);
 
-    applyLayerPositions();
+    [m_layer _web_setLayerTopLeftPosition:layerPosition - m_constraints.alignmentOffset()];
 }
 
 void ScrollingTreePositionedNode::dumpProperties(TextStream& ts, ScrollingStateTreeAsTextBehavior behavior) const
index 7efe26d..cc51e0b 100644 (file)
@@ -31,6 +31,7 @@
 #import "Logging.h"
 #import "ScrollingStateStickyNode.h"
 #import "ScrollingTree.h"
+#import "ScrollingTreeFixedNode.h"
 #import "ScrollingTreeFrameScrollingNode.h"
 #import "ScrollingTreeOverflowScrollingNode.h"
 #import "WebCoreCALayerExtras.h"
@@ -67,21 +68,58 @@ void ScrollingTreeStickyNode::commitStateBeforeChildren(const ScrollingStateNode
 
 void ScrollingTreeStickyNode::applyLayerPositions()
 {
-    FloatRect constrainingRect;
-
-    auto* enclosingScrollingNode = parent();
-    if (is<ScrollingTreeOverflowScrollingNode>(enclosingScrollingNode))
-        constrainingRect = FloatRect(downcast<ScrollingTreeOverflowScrollingNode>(*enclosingScrollingNode).currentScrollPosition(), m_constraints.constrainingRectAtLastLayout().size());
-    else if (is<ScrollingTreeFrameScrollingNode>(enclosingScrollingNode))
-        constrainingRect = downcast<ScrollingTreeFrameScrollingNode>(enclosingScrollingNode)->layoutViewport();
-    else
-        return;
-
-    FloatPoint layerPosition = m_constraints.layerPositionForConstrainingRect(constrainingRect) - m_constraints.alignmentOffset();
-
-    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeStickyNode " << scrollingNodeID() << " constrainingRect " << constrainingRect << " constrainingRectAtLastLayout " << m_constraints.constrainingRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout() << " layerPosition " << layerPosition);
-
-    [m_layer _web_setLayerTopLeftPosition:layerPosition];
+    auto computeLayerPositionForScrollingNode = [&](ScrollingTreeNode& scrollingNode) {
+        FloatRect constrainingRect;
+        if (is<ScrollingTreeFrameScrollingNode>(scrollingNode)) {
+            auto& frameScrollingNode = downcast<ScrollingTreeFrameScrollingNode>(scrollingNode);
+            constrainingRect = frameScrollingNode.layoutViewport();
+        } else {
+            auto& overflowScrollingNode = downcast<ScrollingTreeOverflowScrollingNode>(scrollingNode);
+            constrainingRect = FloatRect(overflowScrollingNode.currentScrollPosition(), m_constraints.constrainingRectAtLastLayout().size());
+        }
+        return m_constraints.layerPositionForConstrainingRect(constrainingRect);
+    };
+
+    auto computeLayerPosition = [&] {
+        for (auto* ancestor = parent(); ancestor; ancestor = ancestor->parent()) {
+            if (is<ScrollingTreePositionedNode>(*ancestor)) {
+                auto& positioningAncestor = downcast<ScrollingTreePositionedNode>(*ancestor);
+
+                // FIXME: Do we need to do anything for ScrollPositioningBehavior::Stationary?
+                if (positioningAncestor.scrollPositioningBehavior() == ScrollPositioningBehavior::Moves) {
+                    if (positioningAncestor.relatedOverflowScrollingNodes().isEmpty())
+                        break;
+                    auto overflowNode = scrollingTree().nodeForID(positioningAncestor.relatedOverflowScrollingNodes()[0]);
+                    if (!overflowNode)
+                        break;
+
+                    auto position = computeLayerPositionForScrollingNode(*overflowNode);
+
+                    if (positioningAncestor.layer() == m_layer) {
+                        // We'll also do the adjustment the positioning node would do.
+                        position -= positioningAncestor.scrollOffsetSinceLastCommit();
+                    }
+                    
+                    return position;
+                }
+            }
+            if (is<ScrollingTreeScrollingNode>(*ancestor))
+                return computeLayerPositionForScrollingNode(*ancestor);
+
+            if (is<ScrollingTreeFixedNode>(*ancestor) || is<ScrollingTreeStickyNode>(*ancestor)) {
+                // FIXME: Do we need scrolling tree nodes at all for nested cases?
+                return m_constraints.layerPositionAtLastLayout();
+            }
+        }
+        ASSERT_NOT_REACHED();
+        return m_constraints.layerPositionAtLastLayout();
+    };
+
+    auto layerPosition = computeLayerPosition();
+
+    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeStickyNode " << scrollingNodeID() << " constrainingRectAtLastLayout " << m_constraints.constrainingRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout() << " layerPosition " << layerPosition);
+
+    [m_layer _web_setLayerTopLeftPosition:layerPosition - m_constraints.alignmentOffset()];
 }
 
 void ScrollingTreeStickyNode::dumpProperties(TextStream& ts, ScrollingStateTreeAsTextBehavior behavior) const