Attachment 372448 Details for Bug 198994 – patch

[patch] patch

scroll-position-reattach-2.patch (text/plain), 18.88 KB, created by Antti Koivisto on 2019-06-19 02:34:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2019-06-19 02:34:59 PDT

Size: 18.88 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 246582)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,54 @@
>+2019-06-19  Antti Koivisto  <antti@apple.com>
>+
>+        RequestedScrollPosition shouldn't be applied after node reattach
>+        https://bugs.webkit.org/show_bug.cgi?id=198994
>+        <rdar://problem/51439685>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: scrollingcoordinator/ios/scroll-position-after-reattach.html
>+
>+        If a scrolling node gets reattached, its scroll position resets to (0,0) or whatever the previous
>+        requestedScrollPosition was, and the current position is lost.
>+
>+        * page/scrolling/ScrollingStateFixedNode.cpp:
>+        (WebCore::ScrollingStateFixedNode::setPropertyChangedBitsAfterReattach):
>+        (WebCore::ScrollingStateFixedNode::setAllPropertiesChanged): Deleted.
>+
>+        Rename to better reflect what this is for.
>+
>+        * page/scrolling/ScrollingStateFixedNode.h:
>+        * page/scrolling/ScrollingStateFrameHostingNode.cpp:
>+        (WebCore::ScrollingStateFrameHostingNode::setPropertyChangedBitsAfterReattach):
>+        (WebCore::ScrollingStateFrameHostingNode::setAllPropertiesChanged): Deleted.
>+        * page/scrolling/ScrollingStateFrameHostingNode.h:
>+        * page/scrolling/ScrollingStateFrameScrollingNode.cpp:
>+        (WebCore::ScrollingStateFrameScrollingNode::setPropertyChangedBitsAfterReattach):
>+        (WebCore::ScrollingStateFrameScrollingNode::setAllPropertiesChanged): Deleted.
>+        * page/scrolling/ScrollingStateFrameScrollingNode.h:
>+        * page/scrolling/ScrollingStateNode.cpp:
>+        (WebCore::ScrollingStateNode::setPropertyChangedBitsAfterReattach):
>+        (WebCore::ScrollingStateNode::setAllPropertiesChanged): Deleted.
>+        * page/scrolling/ScrollingStateNode.h:
>+        * page/scrolling/ScrollingStatePositionedNode.cpp:
>+        (WebCore::ScrollingStatePositionedNode::setPropertyChangedBitsAfterReattach):
>+        (WebCore::ScrollingStatePositionedNode::setAllPropertiesChanged): Deleted.
>+        * page/scrolling/ScrollingStatePositionedNode.h:
>+        * page/scrolling/ScrollingStateScrollingNode.cpp:
>+        (WebCore::ScrollingStateScrollingNode::setPropertyChangedBitsAfterReattach):
>+
>+        Don't set RequestedScrollPosition. It is a special property that is applied only once on request
>+        and shouldn't get reapplied. Nodes should keep their existing scroll position on reattach.
>+
>+        (WebCore::ScrollingStateScrollingNode::setAllPropertiesChanged): Deleted.
>+        * page/scrolling/ScrollingStateScrollingNode.h:
>+        * page/scrolling/ScrollingStateStickyNode.cpp:
>+        (WebCore::ScrollingStateStickyNode::setPropertyChangedBitsAfterReattach):
>+        (WebCore::ScrollingStateStickyNode::setAllPropertiesChanged): Deleted.
>+        * page/scrolling/ScrollingStateStickyNode.h:
>+        * page/scrolling/ScrollingStateTree.cpp:
>+        (WebCore::ScrollingStateTree::nodeWasReattachedRecursive):
>+
> 2019-06-18  Saam Barati  <sbarati@apple.com>
> 
>         [WHLSL] Support matrices
>Index: Source/WebCore/page/scrolling/ScrollingStateFixedNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateFixedNode.cpp	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateFixedNode.cpp	(working copy)
>@@ -58,10 +58,10 @@ Ref<ScrollingStateNode> ScrollingStateFi
>     return adoptRef(*new ScrollingStateFixedNode(*this, adoptiveTree));
> }
> 
>-void ScrollingStateFixedNode::setAllPropertiesChanged()
>+void ScrollingStateFixedNode::setPropertyChangedBitsAfterReattach()
> {
>     setPropertyChangedBit(ViewportConstraints);
>-    ScrollingStateNode::setAllPropertiesChanged();
>+    ScrollingStateNode::setPropertyChangedBitsAfterReattach();
> }
> 
> void ScrollingStateFixedNode::updateConstraints(const FixedPositionViewportConstraints& constraints)
>Index: Source/WebCore/page/scrolling/ScrollingStateFixedNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateFixedNode.h	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateFixedNode.h	(working copy)
>@@ -55,7 +55,7 @@ private:
>     ScrollingStateFixedNode(ScrollingStateTree&, ScrollingNodeID);
>     ScrollingStateFixedNode(const ScrollingStateFixedNode&, ScrollingStateTree&);
> 
>-    void setAllPropertiesChanged() override;
>+    void setPropertyChangedBitsAfterReattach() override;
> 
>     void reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction) override;
> 
>Index: Source/WebCore/page/scrolling/ScrollingStateFrameHostingNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateFrameHostingNode.cpp	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateFrameHostingNode.cpp	(working copy)
>@@ -57,11 +57,11 @@ Ref<ScrollingStateNode> ScrollingStateFr
>     return adoptRef(*new ScrollingStateFrameHostingNode(*this, adoptiveTree));
> }
> 
>-void ScrollingStateFrameHostingNode::setAllPropertiesChanged()
>+void ScrollingStateFrameHostingNode::setPropertyChangedBitsAfterReattach()
> {
>     setPropertyChangedBit(ParentRelativeScrollableRect);
> 
>-    ScrollingStateNode::setAllPropertiesChanged();
>+    ScrollingStateNode::setPropertyChangedBitsAfterReattach();
> }
> 
> void ScrollingStateFrameHostingNode::setParentRelativeScrollableRect(const LayoutRect& parentRelativeScrollableRect)
>Index: Source/WebCore/page/scrolling/ScrollingStateFrameHostingNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateFrameHostingNode.h	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateFrameHostingNode.h	(working copy)
>@@ -53,7 +53,7 @@ private:
>     ScrollingStateFrameHostingNode(ScrollingStateTree&, ScrollingNodeID);
>     ScrollingStateFrameHostingNode(const ScrollingStateFrameHostingNode&, ScrollingStateTree&);
> 
>-    void setAllPropertiesChanged() override;
>+    void setPropertyChangedBitsAfterReattach() override;
> 
>     LayoutRect m_parentRelativeScrollableRect;
> };
>Index: Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.cpp	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.cpp	(working copy)
>@@ -85,7 +85,7 @@ Ref<ScrollingStateNode> ScrollingStateFr
>     return adoptRef(*new ScrollingStateFrameScrollingNode(*this, adoptiveTree));
> }
> 
>-void ScrollingStateFrameScrollingNode::setAllPropertiesChanged()
>+void ScrollingStateFrameScrollingNode::setPropertyChangedBitsAfterReattach()
> {
>     setPropertyChangedBit(FrameScaleFactor);
>     setPropertyChangedBit(EventTrackingRegion);
>@@ -107,7 +107,7 @@ void ScrollingStateFrameScrollingNode::s
>     setPropertyChangedBit(MinLayoutViewportOrigin);
>     setPropertyChangedBit(MaxLayoutViewportOrigin);
> 
>-    ScrollingStateScrollingNode::setAllPropertiesChanged();
>+    ScrollingStateScrollingNode::setPropertyChangedBitsAfterReattach();
> }
> 
> void ScrollingStateFrameScrollingNode::setFrameScaleFactor(float scaleFactor)
>Index: Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.h	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.h	(working copy)
>@@ -135,7 +135,7 @@ private:
>     ScrollingStateFrameScrollingNode(ScrollingStateTree&, ScrollingNodeType, ScrollingNodeID);
>     ScrollingStateFrameScrollingNode(const ScrollingStateFrameScrollingNode&, ScrollingStateTree&);
> 
>-    void setAllPropertiesChanged() override;
>+    void setPropertyChangedBitsAfterReattach() override;
> 
>     LayerRepresentation m_rootContentsLayer;
>     LayerRepresentation m_counterScrollingLayer;
>Index: Source/WebCore/page/scrolling/ScrollingStateNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateNode.cpp	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateNode.cpp	(working copy)
>@@ -67,7 +67,7 @@ void ScrollingStateNode::setPropertyChan
>     m_scrollingStateTree.setHasChangedProperties();
> }
> 
>-void ScrollingStateNode::setAllPropertiesChanged()
>+void ScrollingStateNode::setPropertyChangedBitsAfterReattach()
> {
>     setPropertyChangedBit(Layer);
>     setPropertyChangedBit(ChildNodes);
>Index: Source/WebCore/page/scrolling/ScrollingStateNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateNode.h	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateNode.h	(working copy)
>@@ -212,7 +212,7 @@ public:
>     bool hasChangedProperty(unsigned propertyBit) const { return m_changedProperties & (static_cast<ChangedProperties>(1) << propertyBit); }
>     void resetChangedProperties() { m_changedProperties = 0; }
>     void setPropertyChanged(unsigned propertyBit);
>-    virtual void setAllPropertiesChanged();
>+    virtual void setPropertyChangedBitsAfterReattach();
> 
>     ChangedProperties changedProperties() const { return m_changedProperties; }
>     void setChangedProperties(ChangedProperties changedProperties) { m_changedProperties = changedProperties; }
>Index: Source/WebCore/page/scrolling/ScrollingStatePositionedNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStatePositionedNode.cpp	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStatePositionedNode.cpp	(working copy)
>@@ -59,11 +59,11 @@ Ref<ScrollingStateNode> ScrollingStatePo
>     return adoptRef(*new ScrollingStatePositionedNode(*this, adoptiveTree));
> }
> 
>-void ScrollingStatePositionedNode::setAllPropertiesChanged()
>+void ScrollingStatePositionedNode::setPropertyChangedBitsAfterReattach()
> {
>     setPropertyChangedBit(RelatedOverflowScrollingNodes);
>     setPropertyChangedBit(LayoutConstraintData);
>-    ScrollingStateNode::setAllPropertiesChanged();
>+    ScrollingStateNode::setPropertyChangedBitsAfterReattach();
> }
> 
> void ScrollingStatePositionedNode::setRelatedOverflowScrollingNodes(Vector<ScrollingNodeID>&& nodes)
>Index: Source/WebCore/page/scrolling/ScrollingStatePositionedNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStatePositionedNode.h	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStatePositionedNode.h	(working copy)
>@@ -61,7 +61,7 @@ private:
>     ScrollingStatePositionedNode(ScrollingStateTree&, ScrollingNodeID);
>     ScrollingStatePositionedNode(const ScrollingStatePositionedNode&, ScrollingStateTree&);
> 
>-    void setAllPropertiesChanged() override;
>+    void setPropertyChangedBitsAfterReattach() override;
> 
>     void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
> 
>Index: Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp	(working copy)
>@@ -73,7 +73,7 @@ ScrollingStateScrollingNode::ScrollingSt
> 
> ScrollingStateScrollingNode::~ScrollingStateScrollingNode() = default;
> 
>-void ScrollingStateScrollingNode::setAllPropertiesChanged()
>+void ScrollingStateScrollingNode::setPropertyChangedBitsAfterReattach()
> {
>     setPropertyChangedBit(ScrollableAreaSize);
>     setPropertyChangedBit(TotalContentsSize);
>@@ -82,7 +82,6 @@ void ScrollingStateScrollingNode::setAll
>     setPropertyChangedBit(ScrollPosition);
>     setPropertyChangedBit(ScrollOrigin);
>     setPropertyChangedBit(ScrollableAreaParams);
>-    setPropertyChangedBit(RequestedScrollPosition);
> #if ENABLE(CSS_SCROLL_SNAP)
>     setPropertyChangedBit(HorizontalSnapOffsets);
>     setPropertyChangedBit(VerticalSnapOffsets);
>@@ -98,7 +97,7 @@ void ScrollingStateScrollingNode::setAll
>     setPropertyChangedBit(VerticalScrollbarLayer);
>     setPropertyChangedBit(PainterForScrollbar);
> 
>-    ScrollingStateNode::setAllPropertiesChanged();
>+    ScrollingStateNode::setPropertyChangedBitsAfterReattach();
> }
> 
> void ScrollingStateScrollingNode::setScrollableAreaSize(const FloatSize& size)
>Index: Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h	(working copy)
>@@ -139,7 +139,7 @@ protected:
>     ScrollingStateScrollingNode(ScrollingStateTree&, ScrollingNodeType, ScrollingNodeID);
>     ScrollingStateScrollingNode(const ScrollingStateScrollingNode&, ScrollingStateTree&);
> 
>-    void setAllPropertiesChanged() override;
>+    void setPropertyChangedBitsAfterReattach() override;
> 
>     void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
> 
>Index: Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp	(working copy)
>@@ -58,10 +58,10 @@ Ref<ScrollingStateNode> ScrollingStateSt
>     return adoptRef(*new ScrollingStateStickyNode(*this, adoptiveTree));
> }
> 
>-void ScrollingStateStickyNode::setAllPropertiesChanged()
>+void ScrollingStateStickyNode::setPropertyChangedBitsAfterReattach()
> {
>     setPropertyChangedBit(ViewportConstraints);
>-    ScrollingStateNode::setAllPropertiesChanged();
>+    ScrollingStateNode::setPropertyChangedBitsAfterReattach();
> }
> 
> void ScrollingStateStickyNode::updateConstraints(const StickyPositionViewportConstraints& constraints)
>Index: Source/WebCore/page/scrolling/ScrollingStateStickyNode.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateStickyNode.h	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateStickyNode.h	(working copy)
>@@ -55,7 +55,7 @@ private:
>     ScrollingStateStickyNode(ScrollingStateTree&, ScrollingNodeID);
>     ScrollingStateStickyNode(const ScrollingStateStickyNode&, ScrollingStateTree&);
> 
>-    void setAllPropertiesChanged() override;
>+    void setPropertyChangedBitsAfterReattach() override;
> 
>     void reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction) override;
> 
>Index: Source/WebCore/page/scrolling/ScrollingStateTree.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateTree.cpp	(revision 246430)
>+++ Source/WebCore/page/scrolling/ScrollingStateTree.cpp	(working copy)
>@@ -274,7 +274,7 @@ void ScrollingStateTree::clear()
> void ScrollingStateTree::nodeWasReattachedRecursive(ScrollingStateNode& node)
> {
>     // When a node is re-attached, the ScrollingTree is recreating the ScrollingNode from scratch, so we need to set all the dirty bits.
>-    node.setAllPropertiesChanged();
>+    node.setPropertyChangedBitsAfterReattach();
> 
>     if (auto* children = node.children()) {
>         for (auto& child : *children)
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 246430)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2019-06-19  Antti Koivisto  <antti@apple.com>
>+
>+        RequestedScrollPosition shouldn't be applied after node reattach
>+        https://bugs.webkit.org/show_bug.cgi?id=198994
>+        <rdar://problem/51439685>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * scrollingcoordinator/ios/scroll-position-after-reattach-expected.html: Added.
>+        * scrollingcoordinator/ios/scroll-position-after-reattach.html: Added.
>+
> 2019-06-13  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: REGRESSION(r246178): extra spaces added in at-rules when formatting CSS
>Index: LayoutTests/scrollingcoordinator/ios/scroll-position-after-reattach-expected.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/scroll-position-after-reattach-expected.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/scroll-position-after-reattach-expected.html	(working copy)
>@@ -0,0 +1,47 @@
>+<!DOCTYPE html> 
>+<style>
>+#outer {
>+    overflow: scroll;
>+    width: 400px;
>+    height: 400px;
>+    border: 2px solid yellow;
>+}
>+#inner {
>+    overflow: auto;
>+    width: 300px;
>+    height: 300px;
>+    border: 2px solid blue;
>+}
>+#innercontent {
>+    width: 1000px;
>+    height: 1000px;
>+    border: 10px solid green;
>+}
>+#outercontent {
>+    width: 1000px;
>+    height: 1000px;
>+    border: 10px solid red;
>+}
>+</style>
>+<script src="../../resources/ui-helper.js"></script>
>+<script>
>+if (window.testRunner)
>+    testRunner.waitUntilDone();
>+
>+async function doTest() {
>+    if (!window.testRunner)
>+        return;
>+    inner.scrollTo(0, 200);
>+    testRunner.notifyDone();
>+}
>+
>+window.addEventListener('load', doTest, false);
>+</script>
>+
>+<div id=outer>
>+    <div id=inner>
>+        <div id=innercontent>This should be scrollable</div>
>+    </div>
>+    <div id=outercontent class=content></div>
>+</div>
>+
>Index: LayoutTests/scrollingcoordinator/ios/scroll-position-after-reattach.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/ios/scroll-position-after-reattach.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/ios/scroll-position-after-reattach.html	(working copy)
>@@ -0,0 +1,56 @@
>+<!DOCTYPE html> 
>+<style>
>+#outer {
>+    overflow: scroll;
>+    width: 400px;
>+    height: 400px;
>+    border: 2px solid yellow;
>+}
>+#inner {
>+    overflow: auto;
>+    width: 300px;
>+    height: 300px;
>+    border: 2px solid blue;
>+}
>+#innercontent {
>+    width: 1000px;
>+    height: 1000px;
>+    border: 10px solid green;
>+}
>+#outercontent {
>+    width: 1000px;
>+    height: 1000px;
>+    border: 10px solid red;
>+}
>+</style>
>+<script src="../../resources/ui-helper.js"></script>
>+<script>
>+if (window.testRunner)
>+    testRunner.waitUntilDone();
>+
>+function scroll() {
>+    let removed = outercontent;
>+    outercontent.remove();
>+    document.offsetLeft;
>+    setTimeout(() => outer.appendChild(removed));
>+}
>+
>+async function doTest() {
>+    if (!window.testRunner)
>+        return;
>+    inner.addEventListener("scroll", scroll);
>+
>+    await UIHelper.immediateScrollElementAtContentPointToOffset(50, 50, 0, 200);
>+    testRunner.notifyDone();
>+}
>+
>+window.addEventListener('load', doTest, false);
>+</script>
>+
>+<div id=outer>
>+    <div id=inner>
>+        <div id=innercontent>This should be scrollable</div>
>+    </div>
>+    <div id=outercontent class=content></div>
>+</div>
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 198994: 372447 | 372448