Attachment 371574 Details for Bug 198647 – patch

[patch] patch

fixed-inside-positioning-node.patch (text/plain), 21.98 KB, created by Antti Koivisto on 2019-06-07 01:07:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2019-06-07 01:07:13 PDT

Size: 21.98 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 246191)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,32 @@
>+2019-06-07  Antti Koivisto  <antti@apple.com>
>+
>+        position:fixed inside overflow positioning nodes is jumpy
>+        https://bugs.webkit.org/show_bug.cgi?id=198647
>+        <rdar://problem/51514437>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Tests: scrollingcoordinator/ios/fixed-overflow-no-stacking-context-1.html
>+               scrollingcoordinator/ios/fixed-overflow-no-stacking-context-2.html
>+               scrollingcoordinator/ios/fixed-overflow-stacking-context-stationary.html
>+
>+        * page/scrolling/ScrollingTreeScrollingNode.h:
>+        * page/scrolling/cocoa/ScrollingTreeFixedNode.mm:
>+        (WebCore::ScrollingTreeFixedNode::applyLayerPositions):
>+
>+        Take deltas from positioning nodes into account.
>+
>+        * page/scrolling/cocoa/ScrollingTreePositionedNode.h:
>+        * page/scrolling/cocoa/ScrollingTreePositionedNode.mm:
>+        (WebCore::ScrollingTreePositionedNode::scrollDeltaSinceLastCommit const):
>+
>+        Rename since 'scrollOffset' has other meaning.
>+
>+        (WebCore::ScrollingTreePositionedNode::applyLayerPositions):
>+        (WebCore::ScrollingTreePositionedNode::scrollOffsetSinceLastCommit const): Deleted.
>+        * page/scrolling/cocoa/ScrollingTreeStickyNode.mm:
>+        (WebCore::ScrollingTreeStickyNode::applyLayerPositions):
>+
> 2019-06-06  Andy Estes  <aestes@apple.com>
> 
>         process-swap-on-navigation error when loading blocked website on iOS 12.2 only.
>Index: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h	(revision 246186)
>+++ Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h	(working copy)
>@@ -55,6 +55,7 @@ public:
> 
>     FloatPoint currentScrollPosition() const { return m_currentScrollPosition; }
>     FloatPoint lastCommittedScrollPosition() const { return m_lastCommittedScrollPosition; }
>+    FloatSize scrollDeltaSinceLastCommit() const { return m_currentScrollPosition - m_lastCommittedScrollPosition; }
> 
>     // These are imperative; they adjust the scrolling layers.
>     void scrollTo(const FloatPoint&, ScrollType = ScrollType::User, ScrollPositionClamp = ScrollPositionClamp::ToContentEdges);
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm	(revision 246186)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm	(working copy)
>@@ -33,6 +33,7 @@
> #import "ScrollingTree.h"
> #import "ScrollingTreeFrameScrollingNode.h"
> #import "ScrollingTreeOverflowScrollingNode.h"
>+#import "ScrollingTreePositionedNode.h"
> #import "WebCoreCALayerExtras.h"
> #import <wtf/text/TextStream.h>
> 
>@@ -69,21 +70,24 @@ void ScrollingTreeFixedNode::applyLayerP
> {
>     auto computeLayerPosition = [&] {
>         FloatSize overflowScrollDelta;
>-        // FIXME: This code is wrong in complex cases where the fixed element is inside a positioned node as
>-        //        the scroll container order does not match the scrolling tree ancestor order.
>-        for (auto* node = parent(); node; node = node->parent()) {
>-            if (is<ScrollingTreeFrameScrollingNode>(*node)) {
>+        for (auto* ancestor = parent(); ancestor; ancestor = ancestor->parent()) {
>+            if (is<ScrollingTreePositionedNode>(*ancestor)) {
>+                auto& positioningAncestor = downcast<ScrollingTreePositionedNode>(*ancestor);
>+                if (positioningAncestor.layer() != m_layer)
>+                    overflowScrollDelta -= positioningAncestor.scrollDeltaSinceLastCommit();
>+            }
>+
>+            if (is<ScrollingTreeFrameScrollingNode>(*ancestor)) {
>                 // Fixed nodes are positioned relative to the containing frame scrolling node.
>                 // We bail out after finding one.
>-                auto layoutViewport = downcast<ScrollingTreeFrameScrollingNode>(*node).layoutViewport();
>+                auto layoutViewport = downcast<ScrollingTreeFrameScrollingNode>(*ancestor).layoutViewport();
>                 return m_constraints.layerPositionForViewportRect(layoutViewport) - overflowScrollDelta;
>             }
> 
>-            if (is<ScrollingTreeOverflowScrollingNode>(*node)) {
>+            if (is<ScrollingTreeOverflowScrollingNode>(*ancestor)) {
>                 // To keep the layer still during async scrolling we adjust by how much the position has changed since layout.
>-                auto& overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(*node);
>-                auto localDelta = overflowNode.lastCommittedScrollPosition() - overflowNode.currentScrollPosition();
>-                overflowScrollDelta += localDelta;
>+                auto& overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(*ancestor);
>+                overflowScrollDelta -= overflowNode.scrollDeltaSinceLastCommit();
>             }
>         }
>         ASSERT_NOT_REACHED();
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.h	(revision 246186)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.h	(working copy)
>@@ -46,7 +46,7 @@ public:
>     ScrollPositioningBehavior scrollPositioningBehavior() const { return m_constraints.scrollPositioningBehavior(); }
>     const Vector<ScrollingNodeID>& relatedOverflowScrollingNodes() const { return m_relatedOverflowScrollingNodes; }
> 
>-    FloatSize scrollOffsetSinceLastCommit() const;
>+    FloatSize scrollDeltaSinceLastCommit() const;
> 
> private:
>     ScrollingTreePositionedNode(ScrollingTree&, ScrollingNodeID);
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm	(revision 246186)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm	(working copy)
>@@ -76,31 +76,31 @@ void ScrollingTreePositionedNode::commit
>         scrollingTree().positionedNodesWithRelatedOverflow().add(scrollingNodeID());
> }
> 
>-FloatSize ScrollingTreePositionedNode::scrollOffsetSinceLastCommit() const
>+FloatSize ScrollingTreePositionedNode::scrollDeltaSinceLastCommit() const
> {
>-    FloatSize offset;
>+    FloatSize delta;
>     for (auto nodeID : m_relatedOverflowScrollingNodes) {
>         if (auto* node = scrollingTree().nodeForID(nodeID)) {
>             if (is<ScrollingTreeOverflowScrollingNode>(node)) {
>                 auto& overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(*node);
>-                offset += overflowNode.currentScrollPosition() - overflowNode.lastCommittedScrollPosition();
>+                delta += overflowNode.scrollDeltaSinceLastCommit();
>             }
>         }
>     }
>     if (m_constraints.scrollPositioningBehavior() == ScrollPositioningBehavior::Stationary) {
>         // Stationary nodes move in the opposite direction.
>-        return -offset;
>+        return -delta;
>     }
> 
>-    return offset;
>+    return delta;
> }
> 
> void ScrollingTreePositionedNode::applyLayerPositions()
> {
>-    auto offset = scrollOffsetSinceLastCommit();
>-    auto layerPosition = m_constraints.layerPositionAtLastLayout() - offset;
>+    auto delta = scrollDeltaSinceLastCommit();
>+    auto layerPosition = m_constraints.layerPositionAtLastLayout() - delta;
> 
>-    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreePositionedNode " << scrollingNodeID() << " applyLayerPositions: overflow delta " << offset << " moving layer to " << layerPosition);
>+    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreePositionedNode " << scrollingNodeID() << " applyLayerPositions: overflow delta " << delta << " moving layer to " << layerPosition);
> 
>     [m_layer _web_setLayerTopLeftPosition:layerPosition - m_constraints.alignmentOffset()];
> }
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm	(revision 246186)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm	(working copy)
>@@ -97,7 +97,7 @@ void ScrollingTreeStickyNode::applyLayer
> 
>                     if (positioningAncestor.layer() == m_layer) {
>                         // We'll also do the adjustment the positioning node would do.
>-                        position -= positioningAncestor.scrollOffsetSinceLastCommit();
>+                        position -= positioningAncestor.scrollDeltaSinceLastCommit();
>                     }
>                     
>                     return position;
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 246186)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2019-06-07  Antti Koivisto  <antti@apple.com>
>+
>+        position:fixed inside overflow positioning nodes is jumpy
>+        https://bugs.webkit.org/show_bug.cgi?id=198647
>+        <rdar://problem/51514437>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * scrollingcoordinator/ios/fixed-overflow-no-stacking-context-1-expected.html: Added.
>+        * scrollingcoordinator/ios/fixed-overflow-no-stacking-context-1.html: Added.
>+        * scrollingcoordinator/ios/fixed-overflow-no-stacking-context-2-expected.html: Added.
>+        * scrollingcoordinator/ios/fixed-overflow-no-stacking-context-2.html: Added.
>+        * scrollingcoordinator/ios/fixed-overflow-stacking-context-stationary-expected.html: Added.
>+        * scrollingcoordinator/ios/fixed-overflow-stacking-context-stationary.html: Added.
>+
> 2019-06-06  Youenn Fablet  <youenn@apple.com>
> 
>         Allow WebKitTestRunner to terminate network process after it finishes service worker file operations
>Index: LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-1-expected.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-1-expected.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-1-expected.html	(working copy)
>@@ -0,0 +1,59 @@
>+<!DOCTYPE html> 
>+<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;
>+        }
>+        
>+        .fixed {
>+            position: fixed;
>+            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();
>+
>+            testRunner.notifyDone();
>+        }
>+        
>+        window.addEventListener('load', doTest, false);
>+    </script>
>+</head>
>+<body>
>+    <div class="scroller">
>+        <div class="container">
>+            <div class="fixed"></div>
>+        </div>
>+    </div>
>+</body>
>+</html>
>Index: LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-1.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-1.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-1.html	(working copy)
>@@ -0,0 +1,57 @@
>+<!DOCTYPE html> 
>+<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;
>+        }
>+        
>+        .fixed {
>+            position: fixed;
>+            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="scroller">
>+        <div class="container">
>+            <div class="fixed"></div>
>+        </div>
>+    </div>
>+</body>
>+</html>
>Index: LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-2-expected.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-2-expected.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-2-expected.html	(working copy)
>@@ -0,0 +1,60 @@
>+<!DOCTYPE html> 
>+<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;
>+        }
>+        
>+        .fixed {
>+            position: fixed;
>+            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();
>+
>+            testRunner.notifyDone();
>+        }
>+        
>+        window.addEventListener('load', doTest, false);
>+    </script>
>+</head>
>+<body>
>+    <div class="scroller">
>+        <div class="container">
>+            <div class="fixed"></div>
>+        </div>
>+    </div>
>+</body>
>+</html>
>Index: LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-2.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-2.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/fixed-overflow-no-stacking-context-2.html	(working copy)
>@@ -0,0 +1,58 @@
>+<!DOCTYPE html> 
>+<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;
>+        }
>+        
>+        .fixed {
>+            position: fixed;
>+            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="scroller">
>+        <div class="container">
>+            <div class="fixed"></div>
>+        </div>
>+    </div>
>+</body>
>+</html>
>Index: LayoutTests/scrollingcoordinator/ios/fixed-overflow-stacking-context-stationary-expected.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/fixed-overflow-stacking-context-stationary-expected.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/fixed-overflow-stacking-context-stationary-expected.html	(working copy)
>@@ -0,0 +1,69 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+    <meta name="viewport" content="initial-scale=1.0">
>+    <style>
>+        .scroller {
>+            margin: 10px;
>+            height: 300px;
>+            width: 300px;
>+            border: 1px solid black;
>+            overflow: scroll;
>+            will-change: transform;
>+        }
>+        
>+        .fixed {
>+            position: fixed;
>+            top: 0px;
>+            width: 200px;
>+            height: 200px;
>+            background-color: green;
>+        }
>+
>+        .container {
>+            top: 0px;
>+            margin: 40px;
>+            border: 2px solid red;
>+            width: 200px;
>+            height: 5000px;
>+            position: absolute;
>+            will-change: transform;
>+        }
>+        .spacer {
>+            height: 1000px;
>+            border: 2px solid blue;
>+
>+        }
>+    </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();
>+
>+            testRunner.notifyDone();
>+        }
>+        
>+        window.addEventListener('load', doTest, false);
>+    </script>
>+</head>
>+<body>
>+    <div class="scroller">
>+        <div class="container">
>+            <div class="fixed"></div>
>+        </div>
>+        <div class="spacer"></div>
>+    </div>
>+</body>
>+</html>
>Index: LayoutTests/scrollingcoordinator/ios/fixed-overflow-stacking-context-stationary.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/fixed-overflow-stacking-context-stationary.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/fixed-overflow-stacking-context-stationary.html	(working copy)
>@@ -0,0 +1,67 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+    <meta name="viewport" content="initial-scale=1.0">
>+    <style>
>+        .scroller {
>+            margin: 10px;
>+            height: 300px;
>+            width: 300px;
>+            border: 1px solid black;
>+            overflow: scroll;
>+            will-change: transform;
>+        }
>+        
>+        .fixed {
>+            position: fixed;
>+            top: 0px;
>+            width: 200px;
>+            height: 200px;
>+            background-color: green;
>+        }
>+
>+        .container {
>+            top: 0px;
>+            margin: 40px;
>+            border: 2px solid red;
>+            width: 200px;
>+            height: 5000px;
>+            position: absolute;
>+            will-change: transform;
>+        }
>+        .spacer {
>+            height: 1000px;
>+            border: 2px solid blue;
>+
>+        }
>+    </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="scroller">
>+        <div class="container">
>+            <div class="fixed"></div>
>+        </div>
>+        <div class="spacer"></div>
>+    </div>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 198647: 371574