Attachment 370738 Details for Bug 198292 – patch

[patch] patch

cumulative-delta-6.patch (text/plain), 32.97 KB, created by Antti Koivisto on 2019-05-28 07:41:51 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2019-05-28 07:41:51 PDT

Size: 32.97 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 245809)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,54 @@
>+2019-05-28  Antti Koivisto  <antti@apple.com>
>+
>+        [async scrolling] Fixed positioning inside stacking context overflow scroll is jumpy
>+        https://bugs.webkit.org/show_bug.cgi?id=198292
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Tests: scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-2.html
>+               scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll.html
>+
>+        We were computing delta from the layout scroll position in ScrollingTree::notifyRelatedNodesAfterScrollPositionChange
>+        based on the passed in node only. If any other node had deltas they were not taken into account at all. This would occur
>+        frequently since the function is always invoked for the root node after layer tree commit.
>+
>+        Fix by moving the delta computation (and fetching layoutViewport) to ScrollingTreeFixedNode.
>+
>+        * page/scrolling/ScrollingTree.cpp:
>+        (WebCore::ScrollingTree::applyLayerPositions):
>+
>+        No need to pass offset and layoutViewport around anymore.
>+
>+        (WebCore::ScrollingTree::applyLayerPositionsRecursive):
>+        (WebCore::ScrollingTree::notifyRelatedNodesAfterScrollPositionChange):
>+
>+        Remove the offset and layoutViewport computations.
>+
>+        (WebCore::ScrollingTree::notifyRelatedNodesRecursive):
>+        * page/scrolling/ScrollingTree.h:
>+        * page/scrolling/ScrollingTreeFrameHostingNode.cpp:
>+        (WebCore::ScrollingTreeFrameHostingNode::applyLayerPositions):
>+        * page/scrolling/ScrollingTreeFrameHostingNode.h:
>+        * page/scrolling/ScrollingTreeNode.cpp:
>+        (WebCore::ScrollingTreeNode::relatedNodeScrollPositionDidChange):
>+        * page/scrolling/ScrollingTreeNode.h:
>+        * page/scrolling/ScrollingTreeScrollingNode.cpp:
>+        (WebCore::ScrollingTreeScrollingNode::applyLayerPositions):
>+        * page/scrolling/ScrollingTreeScrollingNode.h:
>+        * page/scrolling/cocoa/ScrollingTreeFixedNode.h:
>+        * page/scrolling/cocoa/ScrollingTreeFixedNode.mm:
>+        (WebCore::ScrollingTreeFixedNode::applyLayerPositions):
>+
>+        Compute them here instead, always taking all overflow scrollers up to the closest frame into account.
>+
>+        * page/scrolling/cocoa/ScrollingTreePositionedNode.h:
>+        * page/scrolling/cocoa/ScrollingTreePositionedNode.mm:
>+        (WebCore::ScrollingTreePositionedNode::applyLayerPositions):
>+        (WebCore::ScrollingTreePositionedNode::relatedNodeScrollPositionDidChange):
>+        * page/scrolling/cocoa/ScrollingTreeStickyNode.h:
>+        * page/scrolling/cocoa/ScrollingTreeStickyNode.mm:
>+        (WebCore::ScrollingTreeStickyNode::applyLayerPositions):
>+
> 2019-05-27  Antoine Quint  <graouts@apple.com>
> 
>         [Pointer Events] Check that capturing data managed by the PointerCaptureController gets cleared upon navigation
>Index: Source/WebCore/page/scrolling/ScrollingTree.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTree.cpp	(revision 245809)
>+++ Source/WebCore/page/scrolling/ScrollingTree.cpp	(working copy)
>@@ -264,23 +264,18 @@ void ScrollingTree::applyLayerPositions(
> 
>     LOG(Scrolling, "\nScrollingTree %p applyLayerPositions", this);
> 
>-    applyLayerPositionsRecursive(*m_rootNode, { }, { });
>+    applyLayerPositionsRecursive(*m_rootNode);
> 
>     LOG(Scrolling, "ScrollingTree %p applyLayerPositions - done\n", this);
> }
> 
>-void ScrollingTree::applyLayerPositionsRecursive(ScrollingTreeNode& currNode, FloatRect layoutViewport, FloatSize cumulativeDelta)
>+void ScrollingTree::applyLayerPositionsRecursive(ScrollingTreeNode& currNode)
> {
>-    if (is<ScrollingTreeFrameScrollingNode>(currNode)) {
>-        layoutViewport = downcast<ScrollingTreeFrameScrollingNode>(currNode).layoutViewport();
>-        cumulativeDelta = { };
>-    }
>-
>-    currNode.applyLayerPositions(layoutViewport, cumulativeDelta);
>+    currNode.applyLayerPositions();
> 
>     if (auto children = currNode.children()) {
>         for (auto& child : *children)
>-            applyLayerPositionsRecursive(*child, layoutViewport, cumulativeDelta);
>+            applyLayerPositionsRecursive(*child);
>     }
> }
> 
>@@ -296,41 +291,31 @@ void ScrollingTree::notifyRelatedNodesAf
> {
>     Vector<ScrollingNodeID> additionalUpdateRoots;
>     
>-    FloatSize deltaFromLastCommittedScrollPosition;
>-    FloatRect currentFrameLayoutViewport;
>-    if (is<ScrollingTreeFrameScrollingNode>(changedNode))
>-        currentFrameLayoutViewport = downcast<ScrollingTreeFrameScrollingNode>(changedNode).layoutViewport();
>-    else if (is<ScrollingTreeOverflowScrollingNode>(changedNode)) {
>-        deltaFromLastCommittedScrollPosition = changedNode.lastCommittedScrollPosition() - changedNode.currentScrollPosition();
>-
>-        if (auto* frameScrollingNode = changedNode.enclosingFrameNodeIncludingSelf())
>-            currentFrameLayoutViewport = frameScrollingNode->layoutViewport();
>-        
>+    if (is<ScrollingTreeOverflowScrollingNode>(changedNode))
>         additionalUpdateRoots = overflowRelatedNodes().get(changedNode.scrollingNodeID());
>-    }
> 
>-    notifyRelatedNodesRecursive(changedNode, changedNode, currentFrameLayoutViewport, deltaFromLastCommittedScrollPosition);
>+    notifyRelatedNodesRecursive(changedNode, changedNode);
>     
>     for (auto positionedNodeID : additionalUpdateRoots) {
>         auto* positionedNode = nodeForID(positionedNodeID);
>         if (positionedNode)
>-            notifyRelatedNodesRecursive(changedNode, *positionedNode, currentFrameLayoutViewport, deltaFromLastCommittedScrollPosition);
>+            notifyRelatedNodesRecursive(changedNode, *positionedNode);
>     }
> }
> 
>-void ScrollingTree::notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode, const FloatRect& layoutViewport, FloatSize cumulativeDelta)
>+void ScrollingTree::notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode)
> {
>-    currNode.relatedNodeScrollPositionDidChange(changedNode, layoutViewport, cumulativeDelta);
>+    currNode.relatedNodeScrollPositionDidChange(changedNode);
> 
>     if (!currNode.children())
>         return;
>-    
>+
>     for (auto& child : *currNode.children()) {
>         // Never need to cross frame boundaries, since scroll layer adjustments are isolated to each document.
>         if (is<ScrollingTreeFrameScrollingNode>(child))
>             continue;
> 
>-        notifyRelatedNodesRecursive(changedNode, *child, layoutViewport, cumulativeDelta);
>+        notifyRelatedNodesRecursive(changedNode, *child);
>     }
> }
> 
>Index: Source/WebCore/page/scrolling/ScrollingTree.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTree.h	(revision 245809)
>+++ Source/WebCore/page/scrolling/ScrollingTree.h	(working copy)
>@@ -165,9 +165,9 @@ private:
>     using OrphanScrollingNodeMap = HashMap<ScrollingNodeID, RefPtr<ScrollingTreeNode>>;
>     void updateTreeFromStateNode(const ScrollingStateNode*, OrphanScrollingNodeMap&, HashSet<ScrollingNodeID>& unvisitedNodes);
> 
>-    void applyLayerPositionsRecursive(ScrollingTreeNode&, FloatRect layoutViewport, FloatSize cumulativeDelta);
>+    void applyLayerPositionsRecursive(ScrollingTreeNode&);
> 
>-    void notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode, const FloatRect& layoutViewport, FloatSize cumulativeDelta);
>+    void notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode);
> 
>     Lock m_treeMutex; // Protects the scrolling tree.
> 
>Index: Source/WebCore/page/scrolling/ScrollingTreeFrameHostingNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTreeFrameHostingNode.cpp	(revision 245809)
>+++ Source/WebCore/page/scrolling/ScrollingTreeFrameHostingNode.cpp	(working copy)
>@@ -57,7 +57,7 @@ void ScrollingTreeFrameHostingNode::comm
>         m_parentRelativeScrollableRect = frameHostingStateNode.parentRelativeScrollableRect();
> }
> 
>-void ScrollingTreeFrameHostingNode::applyLayerPositions(const FloatRect&, FloatSize&)
>+void ScrollingTreeFrameHostingNode::applyLayerPositions()
> {
> }
> 
>Index: Source/WebCore/page/scrolling/ScrollingTreeFrameHostingNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTreeFrameHostingNode.h	(revision 245809)
>+++ Source/WebCore/page/scrolling/ScrollingTreeFrameHostingNode.h	(working copy)
>@@ -42,7 +42,7 @@ private:
>     ScrollingTreeFrameHostingNode(ScrollingTree&, ScrollingNodeID);
> 
>     void commitStateBeforeChildren(const ScrollingStateNode&) final;
>-    void applyLayerPositions(const FloatRect&, FloatSize&) final;
>+    void applyLayerPositions() final;
> 
>     const LayoutRect& parentRelativeScrollableRect() const { return m_parentRelativeScrollableRect; }
> 
>Index: Source/WebCore/page/scrolling/ScrollingTreeNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTreeNode.cpp	(revision 245809)
>+++ Source/WebCore/page/scrolling/ScrollingTreeNode.cpp	(working copy)
>@@ -77,9 +77,9 @@ bool ScrollingTreeNode::isRootNode() con
>     return m_scrollingTree.rootNode() == this;
> }
> 
>-void ScrollingTreeNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode&, const FloatRect& layoutViewport, FloatSize& cumulativeDelta)
>+void ScrollingTreeNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode&)
> {
>-    applyLayerPositions(layoutViewport, cumulativeDelta);
>+    applyLayerPositions();
> }
> 
> void ScrollingTreeNode::dumpProperties(TextStream& ts, ScrollingStateTreeAsTextBehavior behavior) const
>Index: Source/WebCore/page/scrolling/ScrollingTreeNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTreeNode.h	(revision 245809)
>+++ Source/WebCore/page/scrolling/ScrollingTreeNode.h	(working copy)
>@@ -84,9 +84,9 @@ protected:
>     ScrollingTreeNode(ScrollingTree&, ScrollingNodeType, ScrollingNodeID);
>     ScrollingTree& scrollingTree() const { return m_scrollingTree; }
> 
>-    WEBCORE_EXPORT virtual void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode, const FloatRect& layoutViewport, FloatSize& cumulativeDelta);
>+    WEBCORE_EXPORT virtual void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode);
> 
>-    virtual void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) = 0;
>+    virtual void applyLayerPositions() = 0;
> 
>     WEBCORE_EXPORT virtual void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const;
> 
>Index: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp	(revision 245809)
>+++ Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp	(working copy)
>@@ -188,7 +188,7 @@ bool ScrollingTreeScrollingNode::scrollP
>     return position == m_currentScrollPosition;
> }
> 
>-void ScrollingTreeScrollingNode::applyLayerPositions(const FloatRect&, FloatSize&)
>+void ScrollingTreeScrollingNode::applyLayerPositions()
> {
>     repositionScrollingLayers();
>     repositionRelatedLayers();
>Index: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h	(revision 245809)
>+++ Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h	(working copy)
>@@ -107,7 +107,7 @@ protected:
>     virtual void repositionScrollingLayers() { }
>     virtual void repositionRelatedLayers() { }
> 
>-    void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
>+    void applyLayerPositions() override;
> 
>     const FloatSize& reachableContentsSize() const { return m_reachableContentsSize; }
>     const LayoutRect& parentRelativeScrollableRect() const { return m_parentRelativeScrollableRect; }
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.h	(revision 245809)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.h	(working copy)
>@@ -47,7 +47,7 @@ private:
>     ScrollingTreeFixedNode(ScrollingTree&, ScrollingNodeID);
> 
>     void commitStateBeforeChildren(const ScrollingStateNode&) override;
>-    void applyLayerPositions(const FloatRect&, FloatSize&) override;
>+    void applyLayerPositions() override;
> 
>     void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
> 
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm	(revision 245809)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm	(working copy)
>@@ -31,6 +31,8 @@
> #import "Logging.h"
> #import "ScrollingStateFixedNode.h"
> #import "ScrollingTree.h"
>+#import "ScrollingTreeFrameScrollingNode.h"
>+#import "ScrollingTreeOverflowScrollingNode.h"
> #import "WebCoreCALayerExtras.h"
> #import <wtf/text/TextStream.h>
> 
>@@ -63,16 +65,36 @@ void ScrollingTreeFixedNode::commitState
>         m_constraints = fixedStateNode.viewportConstraints();
> }
> 
>-void ScrollingTreeFixedNode::applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta)
>+void ScrollingTreeFixedNode::applyLayerPositions()
> {
>-    FloatPoint layerPosition = m_constraints.layerPositionForViewportRect(layoutViewport);
>+    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)) {
>+                // Fixed nodes are positioned relative to the containing frame scrolling node.
>+                // We bail out after finding one.
>+                auto layoutViewport = downcast<ScrollingTreeFrameScrollingNode>(*node).layoutViewport();
>+                return m_constraints.layerPositionForViewportRect(layoutViewport) - overflowScrollDelta;
>+            }
>+
>+            if (is<ScrollingTreeOverflowScrollingNode>(*node)) {
>+                // 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;
>+            }
>+        }
>+        ASSERT_NOT_REACHED();
>+        return FloatPoint();
>+    };
> 
>-    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeFixedNode " << scrollingNodeID() << " relatedNodeScrollPositionDidChange: new viewport " << layoutViewport << " viewportRectAtLastLayout " << m_constraints.viewportRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout() << " new offset from top " << (layoutViewport.y() - layerPosition.y()));
>+    auto layerPosition = computeLayerPosition();
> 
>-    layerPosition -= cumulativeDelta;
>+    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeFixedNode " << scrollingNodeID() << " relatedNodeScrollPositionDidChange: viewportRectAtLastLayout " << m_constraints.viewportRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout() << " layerPosition " << layerPosition);
> 
>     [m_layer _web_setLayerTopLeftPosition:layerPosition - m_constraints.alignmentOffset()];
>-    cumulativeDelta += layerPosition - m_constraints.layerPositionAtLastLayout();
> }
> 
> void ScrollingTreeFixedNode::dumpProperties(TextStream& ts, ScrollingStateTreeAsTextBehavior behavior) const
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.h	(revision 245809)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.h	(working copy)
>@@ -50,9 +50,9 @@ private:
>     ScrollingTreePositionedNode(ScrollingTree&, ScrollingNodeID);
> 
>     void commitStateBeforeChildren(const ScrollingStateNode&) override;
>-    void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode, const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
>+    void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode) override;
> 
>-    void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
>+    void applyLayerPositions() override;
> 
>     WEBCORE_EXPORT void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
> 
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm	(revision 245809)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm	(working copy)
>@@ -76,10 +76,8 @@ void ScrollingTreePositionedNode::commit
>         scrollingTree().positionedNodesWithRelatedOverflow().add(scrollingNodeID());
> }
> 
>-void ScrollingTreePositionedNode::applyLayerPositions(const FloatRect&, FloatSize& cumulativeDelta)
>+void ScrollingTreePositionedNode::applyLayerPositions()
> {
>-    // Note that we ignore cumulativeDelta because it will contain the delta for ancestor scrollers,
>-    // but not non-ancestor ones, so it's simpler to just recompute from the scrollers we know about here.
>     FloatSize scrollOffsetSinceLastCommit;
>     for (auto nodeID : m_relatedOverflowScrollingNodes) {
>         if (auto* node = scrollingTree().nodeForID(nodeID)) {
>@@ -100,17 +98,14 @@ void ScrollingTreePositionedNode::applyL
>     LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreePositionedNode " << scrollingNodeID() << " applyLayerPositions: overflow delta " << scrollOffsetSinceLastCommit << " moving layer to " << layerPosition);
> 
>     [m_layer _web_setLayerTopLeftPosition:layerPosition - m_constraints.alignmentOffset()];
>-
>-    // FIXME: Should our scroller deltas propagate to descendants?
>-    cumulativeDelta = layerPosition - m_constraints.layerPositionAtLastLayout();
> }
> 
>-void ScrollingTreePositionedNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode, const FloatRect& layoutViewport, FloatSize& cumulativeDelta)
>+void ScrollingTreePositionedNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode)
> {
>     if (!m_relatedOverflowScrollingNodes.contains(changedNode.scrollingNodeID()))
>         return;
> 
>-    applyLayerPositions(layoutViewport, cumulativeDelta);
>+    applyLayerPositions();
> }
> 
> void ScrollingTreePositionedNode::dumpProperties(TextStream& ts, ScrollingStateTreeAsTextBehavior behavior) const
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.h	(revision 245809)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.h	(working copy)
>@@ -45,7 +45,7 @@ private:
>     ScrollingTreeStickyNode(ScrollingTree&, ScrollingNodeID);
> 
>     void commitStateBeforeChildren(const ScrollingStateNode&) override;
>-    void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
>+    void applyLayerPositions() override;
> 
>     void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
> 
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm	(revision 245809)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm	(working copy)
>@@ -65,7 +65,7 @@ void ScrollingTreeStickyNode::commitStat
>         m_constraints = stickyStateNode.viewportConstraints();
> }
> 
>-void ScrollingTreeStickyNode::applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta)
>+void ScrollingTreeStickyNode::applyLayerPositions()
> {
>     FloatRect constrainingRect;
> 
>@@ -73,16 +73,15 @@ void ScrollingTreeStickyNode::applyLayer
>     if (is<ScrollingTreeOverflowScrollingNode>(enclosingScrollingNode))
>         constrainingRect = FloatRect(downcast<ScrollingTreeOverflowScrollingNode>(*enclosingScrollingNode).currentScrollPosition(), m_constraints.constrainingRectAtLastLayout().size());
>     else if (is<ScrollingTreeFrameScrollingNode>(enclosingScrollingNode))
>-        constrainingRect = layoutViewport;
>+        constrainingRect = downcast<ScrollingTreeFrameScrollingNode>(enclosingScrollingNode)->layoutViewport();
>     else
>         return;
> 
>-    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeStickyNode " << scrollingNodeID() << " relatedNodeScrollPositionDidChange: new viewport " << layoutViewport << " constrainingRectAtLastLayout " << m_constraints.constrainingRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout());
>-
>     FloatPoint layerPosition = m_constraints.layerPositionForConstrainingRect(constrainingRect) - m_constraints.alignmentOffset();
>-    [m_layer _web_setLayerTopLeftPosition:layerPosition];
> 
>-    cumulativeDelta += layerPosition - m_constraints.layerPositionAtLastLayout();
>+    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];
> }
> 
> void ScrollingTreeStickyNode::dumpProperties(TextStream& ts, ScrollingStateTreeAsTextBehavior behavior) const
>Index: Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.cpp	(revision 245809)
>+++ Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.cpp	(working copy)
>@@ -54,7 +54,7 @@ void ScrollingTreeFixedNode::commitState
> {
> }
> 
>-void ScrollingTreeFixedNode::applyLayerPositions(const FloatRect&, FloatSize&)
>+void ScrollingTreeFixedNode::applyLayerPositions()
> {
> }
> 
>Index: Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.h	(revision 245809)
>+++ Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.h	(working copy)
>@@ -42,7 +42,7 @@ private:
>     ScrollingTreeFixedNode(ScrollingTree&, ScrollingNodeID);
> 
>     void commitStateBeforeChildren(const ScrollingStateNode&) override;
>-    void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
>+    void applyLayerPositions() override;
> };
> 
> } // namespace WebCore
>Index: Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.cpp	(revision 245809)
>+++ Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.cpp	(working copy)
>@@ -54,7 +54,7 @@ void ScrollingTreeStickyNode::commitStat
> {
> }
> 
>-void ScrollingTreeStickyNode::applyLayerPositions(const FloatRect&, FloatSize&)
>+void ScrollingTreeStickyNode::applyLayerPositions()
> {
> }
> 
>Index: Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.h	(revision 245809)
>+++ Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.h	(working copy)
>@@ -42,7 +42,7 @@ private:
>     ScrollingTreeStickyNode(ScrollingTree&, ScrollingNodeID);
> 
>     void commitStateBeforeChildren(const ScrollingStateNode&) override;
>-    void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
>+    void applyLayerPositions() override;
> 
> };
> 
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 245809)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2019-05-28  Antti Koivisto  <antti@apple.com>
>+
>+        [async scrolling] Fixed positioning inside stacking context overflow scroll is jumpy
>+        https://bugs.webkit.org/show_bug.cgi?id=198292
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-2-expected.html: Added.
>+        * scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-2.html: Added.
>+        * scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-expected.html: Added.
>+        * scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll.html: Added.
>+
> 2019-05-27  Takashi Komori  <Takashi.Komori@sony.com>
> 
>         [CURL] Fix crashing SocketStreamHandle.
>Index: LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-2-expected.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-2-expected.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-2-expected.html	(working copy)
>@@ -0,0 +1,56 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+    <meta name="viewport" content="initial-scale=1.0">
>+    <style>
>+        #scroller {
>+            left: 100px;
>+            margin: 10px;
>+            height: 300px;
>+            width: 300px;
>+            border: 1px solid black;
>+            overflow: scroll;
>+            z-index: 0;
>+            position: relative;
>+        }
>+        
>+        .box {
>+            position: fixed;
>+            margin-top: 200px;
>+            width: 200px;
>+            height: 200px;
>+            background-color: green;
>+        }
>+        
>+        .spacer {
>+            border: 2px solid blue;
>+            height: 5000px;
>+        }
>+    </style>
>+    <script src="../../resources/ui-helper.js"></script>
>+    <script>
>+        if (window.testRunner)
>+            testRunner.waitUntilDone();
>+
>+        async function doTest()
>+        {
>+            await UIHelper.ensurePresentationUpdate(); // Not sure why this is necessary, but it is.
>+            window.scrollTo(0, 50);
>+            await UIHelper.ensurePresentationUpdate();
>+
>+            if (window.testRunner)
>+                testRunner.notifyDone();
>+        }
>+        
>+        window.addEventListener('load', doTest, false);
>+    </script>
>+</head>
>+<body>
>+    <div class="spacer">
>+        <div id="scroller">
>+            <div class="box"></div>
>+            <div class="spacer"></div>
>+        </div>
>+    </div>
>+</body>
>+</html>
>Index: LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-2.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-2.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-2.html	(working copy)
>@@ -0,0 +1,59 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+    <meta name="viewport" content="initial-scale=1.0">
>+    <style>
>+        .scroller {
>+            left: 100px;
>+            margin: 10px;
>+            height: 300px;
>+            width: 300px;
>+            border: 1px solid black;
>+            overflow: scroll;
>+            z-index: 0;
>+            position: relative;
>+        }
>+        
>+        .box {
>+            position: fixed;
>+            margin-top: 200px;
>+            width: 200px;
>+            height: 200px;
>+            background-color: green;
>+        }
>+        
>+        .spacer {
>+            border: 2px solid blue;
>+            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, 50, scrollUpdatesDisabled);
>+            testRunner.notifyDone();
>+        }
>+        
>+        window.addEventListener('load', doTest, false);
>+    </script>
>+</head>
>+<body>
>+    <div class="spacer">
>+        <div class="scroller">
>+            <div class="box"></div>
>+            <div class="spacer"></div>
>+        </div>
>+    </div>
>+</body>
>+</html>
>Index: LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-expected.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-expected.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll-expected.html	(working copy)
>@@ -0,0 +1,55 @@
>+<!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;
>+            z-index: 0;
>+            position: relative;
>+        }
>+        
>+        .box {
>+            position: fixed;
>+            margin-top: 200px;
>+            width: 200px;
>+            height: 200px;
>+            background-color: green;
>+        }
>+        
>+        .spacer {
>+            border: 2px solid blue;
>+            height: 5000px;
>+        }
>+    </style>
>+    <script src="../../resources/ui-helper.js"></script>
>+    <script>
>+        if (window.testRunner)
>+            testRunner.waitUntilDone();
>+
>+        async function doTest()
>+        {
>+            await UIHelper.ensurePresentationUpdate(); // Not sure why this is necessary, but it is.
>+            scroller.scrollTo(0, 200);
>+            await UIHelper.ensurePresentationUpdate();
>+
>+            if (window.testRunner)
>+                testRunner.notifyDone();
>+        }
>+        
>+        window.addEventListener('load', doTest, false);
>+    </script>
>+</head>
>+<body>
>+    <div class="spacer">
>+        <div id="scroller">
>+            <div class="box"></div>
>+            <div class="spacer"></div>
>+        </div>
>+    </div>
>+</body>
>+</html>
>Index: LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/fixed-in-stacking-context-overflow-scroll.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;
>+            z-index: 0;
>+            position: relative;
>+        }
>+        
>+        .box {
>+            position: fixed;
>+            margin-top: 200px;
>+            width: 200px;
>+            height: 200px;
>+            background-color: green;
>+        }
>+        
>+        .spacer {
>+            border: 2px solid blue;
>+            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="box"></div>
>+            <div class="spacer"></div>
>+        </div>
>+    </div>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 198292: 370727 | 370729 | 370734 | 370738 | 370769