Changeset 246593 in webkit


Ignore:
Timestamp:
Jun 19, 2019 10:21:11 AM (5 years ago)
Author:
Antti Koivisto
Message:

RequestedScrollPosition shouldn't be applied after node reattach
https://bugs.webkit.org/show_bug.cgi?id=198994
<rdar://problem/51439685>

Reviewed by Simon Fraser.

Source/WebCore:

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):

LayoutTests:

  • scrollingcoordinator/ios/scroll-position-after-reattach-expected.html: Added.
  • scrollingcoordinator/ios/scroll-position-after-reattach.html: Added.
Location:
trunk
Files:
2 added
17 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r246591 r246593  
     12019-06-19  Antti Koivisto  <antti@apple.com>
     2
     3        RequestedScrollPosition shouldn't be applied after node reattach
     4        https://bugs.webkit.org/show_bug.cgi?id=198994
     5        <rdar://problem/51439685>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * scrollingcoordinator/ios/scroll-position-after-reattach-expected.html: Added.
     10        * scrollingcoordinator/ios/scroll-position-after-reattach.html: Added.
     11
    1122019-06-19  Truitt Savell  <tsavell@apple.com>
    213
  • trunk/Source/WebCore/ChangeLog

    r246579 r246593  
     12019-06-19  Antti Koivisto  <antti@apple.com>
     2
     3        RequestedScrollPosition shouldn't be applied after node reattach
     4        https://bugs.webkit.org/show_bug.cgi?id=198994
     5        <rdar://problem/51439685>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Test: scrollingcoordinator/ios/scroll-position-after-reattach.html
     10
     11        If a scrolling node gets reattached, its scroll position resets to (0,0) or whatever the previous
     12        requestedScrollPosition was, and the current position is lost.
     13
     14        * page/scrolling/ScrollingStateFixedNode.cpp:
     15        (WebCore::ScrollingStateFixedNode::setPropertyChangedBitsAfterReattach):
     16        (WebCore::ScrollingStateFixedNode::setAllPropertiesChanged): Deleted.
     17
     18        Rename to better reflect what this is for.
     19
     20        * page/scrolling/ScrollingStateFixedNode.h:
     21        * page/scrolling/ScrollingStateFrameHostingNode.cpp:
     22        (WebCore::ScrollingStateFrameHostingNode::setPropertyChangedBitsAfterReattach):
     23        (WebCore::ScrollingStateFrameHostingNode::setAllPropertiesChanged): Deleted.
     24        * page/scrolling/ScrollingStateFrameHostingNode.h:
     25        * page/scrolling/ScrollingStateFrameScrollingNode.cpp:
     26        (WebCore::ScrollingStateFrameScrollingNode::setPropertyChangedBitsAfterReattach):
     27        (WebCore::ScrollingStateFrameScrollingNode::setAllPropertiesChanged): Deleted.
     28        * page/scrolling/ScrollingStateFrameScrollingNode.h:
     29        * page/scrolling/ScrollingStateNode.cpp:
     30        (WebCore::ScrollingStateNode::setPropertyChangedBitsAfterReattach):
     31        (WebCore::ScrollingStateNode::setAllPropertiesChanged): Deleted.
     32        * page/scrolling/ScrollingStateNode.h:
     33        * page/scrolling/ScrollingStatePositionedNode.cpp:
     34        (WebCore::ScrollingStatePositionedNode::setPropertyChangedBitsAfterReattach):
     35        (WebCore::ScrollingStatePositionedNode::setAllPropertiesChanged): Deleted.
     36        * page/scrolling/ScrollingStatePositionedNode.h:
     37        * page/scrolling/ScrollingStateScrollingNode.cpp:
     38        (WebCore::ScrollingStateScrollingNode::setPropertyChangedBitsAfterReattach):
     39
     40        Don't set RequestedScrollPosition. It is a special property that is applied only once on request
     41        and shouldn't get reapplied. Nodes should keep their existing scroll position on reattach.
     42
     43        (WebCore::ScrollingStateScrollingNode::setAllPropertiesChanged): Deleted.
     44        * page/scrolling/ScrollingStateScrollingNode.h:
     45        * page/scrolling/ScrollingStateStickyNode.cpp:
     46        (WebCore::ScrollingStateStickyNode::setPropertyChangedBitsAfterReattach):
     47        (WebCore::ScrollingStateStickyNode::setAllPropertiesChanged): Deleted.
     48        * page/scrolling/ScrollingStateStickyNode.h:
     49        * page/scrolling/ScrollingStateTree.cpp:
     50        (WebCore::ScrollingStateTree::nodeWasReattachedRecursive):
     51
    1522019-06-18  Saam Barati  <sbarati@apple.com>
    253
  • trunk/Source/WebCore/page/scrolling/ScrollingStateFixedNode.cpp

    r246033 r246593  
    5959}
    6060
    61 void ScrollingStateFixedNode::setAllPropertiesChanged()
     61void ScrollingStateFixedNode::setPropertyChangedBitsAfterReattach()
    6262{
    6363    setPropertyChangedBit(ViewportConstraints);
    64     ScrollingStateNode::setAllPropertiesChanged();
     64    ScrollingStateNode::setPropertyChangedBitsAfterReattach();
    6565}
    6666
  • trunk/Source/WebCore/page/scrolling/ScrollingStateFixedNode.h

    r246033 r246593  
    5656    ScrollingStateFixedNode(const ScrollingStateFixedNode&, ScrollingStateTree&);
    5757
    58     void setAllPropertiesChanged() override;
     58    void setPropertyChangedBitsAfterReattach() override;
    5959
    6060    void reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction) override;
  • trunk/Source/WebCore/page/scrolling/ScrollingStateFrameHostingNode.cpp

    r246033 r246593  
    5858}
    5959
    60 void ScrollingStateFrameHostingNode::setAllPropertiesChanged()
     60void ScrollingStateFrameHostingNode::setPropertyChangedBitsAfterReattach()
    6161{
    6262    setPropertyChangedBit(ParentRelativeScrollableRect);
    6363
    64     ScrollingStateNode::setAllPropertiesChanged();
     64    ScrollingStateNode::setPropertyChangedBitsAfterReattach();
    6565}
    6666
  • trunk/Source/WebCore/page/scrolling/ScrollingStateFrameHostingNode.h

    r246033 r246593  
    5454    ScrollingStateFrameHostingNode(const ScrollingStateFrameHostingNode&, ScrollingStateTree&);
    5555
    56     void setAllPropertiesChanged() override;
     56    void setPropertyChangedBitsAfterReattach() override;
    5757
    5858    LayoutRect m_parentRelativeScrollableRect;
  • trunk/Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.cpp

    r246033 r246593  
    8686}
    8787
    88 void ScrollingStateFrameScrollingNode::setAllPropertiesChanged()
     88void ScrollingStateFrameScrollingNode::setPropertyChangedBitsAfterReattach()
    8989{
    9090    setPropertyChangedBit(FrameScaleFactor);
     
    108108    setPropertyChangedBit(MaxLayoutViewportOrigin);
    109109
    110     ScrollingStateScrollingNode::setAllPropertiesChanged();
     110    ScrollingStateScrollingNode::setPropertyChangedBitsAfterReattach();
    111111}
    112112
  • trunk/Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.h

    r246033 r246593  
    136136    ScrollingStateFrameScrollingNode(const ScrollingStateFrameScrollingNode&, ScrollingStateTree&);
    137137
    138     void setAllPropertiesChanged() override;
     138    void setPropertyChangedBitsAfterReattach() override;
    139139
    140140    LayerRepresentation m_rootContentsLayer;
  • trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp

    r246033 r246593  
    6868}
    6969
    70 void ScrollingStateNode::setAllPropertiesChanged()
     70void ScrollingStateNode::setPropertyChangedBitsAfterReattach()
    7171{
    7272    setPropertyChangedBit(Layer);
  • trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h

    r246033 r246593  
    213213    void resetChangedProperties() { m_changedProperties = 0; }
    214214    void setPropertyChanged(unsigned propertyBit);
    215     virtual void setAllPropertiesChanged();
     215    virtual void setPropertyChangedBitsAfterReattach();
    216216
    217217    ChangedProperties changedProperties() const { return m_changedProperties; }
  • trunk/Source/WebCore/page/scrolling/ScrollingStatePositionedNode.cpp

    r246033 r246593  
    6060}
    6161
    62 void ScrollingStatePositionedNode::setAllPropertiesChanged()
     62void ScrollingStatePositionedNode::setPropertyChangedBitsAfterReattach()
    6363{
    6464    setPropertyChangedBit(RelatedOverflowScrollingNodes);
    6565    setPropertyChangedBit(LayoutConstraintData);
    66     ScrollingStateNode::setAllPropertiesChanged();
     66    ScrollingStateNode::setPropertyChangedBitsAfterReattach();
    6767}
    6868
  • trunk/Source/WebCore/page/scrolling/ScrollingStatePositionedNode.h

    r246033 r246593  
    6262    ScrollingStatePositionedNode(const ScrollingStatePositionedNode&, ScrollingStateTree&);
    6363
    64     void setAllPropertiesChanged() override;
     64    void setPropertyChangedBitsAfterReattach() override;
    6565
    6666    void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
  • trunk/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp

    r246033 r246593  
    7474ScrollingStateScrollingNode::~ScrollingStateScrollingNode() = default;
    7575
    76 void ScrollingStateScrollingNode::setAllPropertiesChanged()
     76void ScrollingStateScrollingNode::setPropertyChangedBitsAfterReattach()
    7777{
    7878    setPropertyChangedBit(ScrollableAreaSize);
     
    8383    setPropertyChangedBit(ScrollOrigin);
    8484    setPropertyChangedBit(ScrollableAreaParams);
    85     setPropertyChangedBit(RequestedScrollPosition);
    8685#if ENABLE(CSS_SCROLL_SNAP)
    8786    setPropertyChangedBit(HorizontalSnapOffsets);
     
    9998    setPropertyChangedBit(PainterForScrollbar);
    10099
    101     ScrollingStateNode::setAllPropertiesChanged();
     100    ScrollingStateNode::setPropertyChangedBitsAfterReattach();
    102101}
    103102
  • trunk/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h

    r246033 r246593  
    140140    ScrollingStateScrollingNode(const ScrollingStateScrollingNode&, ScrollingStateTree&);
    141141
    142     void setAllPropertiesChanged() override;
     142    void setPropertyChangedBitsAfterReattach() override;
    143143
    144144    void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
  • trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp

    r246033 r246593  
    5959}
    6060
    61 void ScrollingStateStickyNode::setAllPropertiesChanged()
     61void ScrollingStateStickyNode::setPropertyChangedBitsAfterReattach()
    6262{
    6363    setPropertyChangedBit(ViewportConstraints);
    64     ScrollingStateNode::setAllPropertiesChanged();
     64    ScrollingStateNode::setPropertyChangedBitsAfterReattach();
    6565}
    6666
  • trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.h

    r246033 r246593  
    5656    ScrollingStateStickyNode(const ScrollingStateStickyNode&, ScrollingStateTree&);
    5757
    58     void setAllPropertiesChanged() override;
     58    void setPropertyChangedBitsAfterReattach() override;
    5959
    6060    void reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction) override;
  • trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp

    r246033 r246593  
    275275{
    276276    // When a node is re-attached, the ScrollingTree is recreating the ScrollingNode from scratch, so we need to set all the dirty bits.
    277     node.setAllPropertiesChanged();
     277    node.setPropertyChangedBitsAfterReattach();
    278278
    279279    if (auto* children = node.children()) {
Note: See TracChangeset for help on using the changeset viewer.