Fix scrolling tree dumping
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 May 2016 19:31:22 +0000 (19:31 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 May 2016 19:31:22 +0000 (19:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157529

Reviewed by Tim Horton.

Source/WebCore:

Scrolling tree dumps cannot contain layerIDs because they are not stable between
runs. Fix by adding ScrollingStateTreeAsTextBehavior flags, and not dumping
the layerID for tests.

Sadly RemoteScrollingCoordinatorTransaction has a lot of duplicated code for dumping
the scrolling state tree, which should be converted to dumpProperties() at some point.

Fix the one test that suffered from this problem, and unskip it.

Test: fast/scrolling/ios/remove-scrolling-role.html

* page/scrolling/ScrollingStateFixedNode.cpp:
(WebCore::ScrollingStateFixedNode::dumpProperties):
* page/scrolling/ScrollingStateFixedNode.h:
* page/scrolling/ScrollingStateFrameScrollingNode.cpp:
(WebCore::ScrollingStateFrameScrollingNode::dumpProperties):
* page/scrolling/ScrollingStateFrameScrollingNode.h:
* page/scrolling/ScrollingStateNode.cpp:
(WebCore::ScrollingStateNode::dump):
(WebCore::ScrollingStateNode::scrollingStateTreeAsText):
* page/scrolling/ScrollingStateNode.h:
* page/scrolling/ScrollingStateOverflowScrollingNode.cpp:
(WebCore::ScrollingStateOverflowScrollingNode::dumpProperties):
* page/scrolling/ScrollingStateOverflowScrollingNode.h:
* page/scrolling/ScrollingStateScrollingNode.cpp:
(WebCore::ScrollingStateScrollingNode::dumpProperties):
* page/scrolling/ScrollingStateScrollingNode.h:
* page/scrolling/ScrollingStateStickyNode.cpp:
(WebCore::ScrollingStateStickyNode::dumpProperties):
* page/scrolling/ScrollingStateStickyNode.h:

LayoutTests:

* fast/scrolling/ios/remove-scrolling-role-expected.txt: Renamed from LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt.
* fast/scrolling/ios/remove-scrolling-role.html: Renamed from LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html.
* platform/ios-simulator-wk2/TestExpectations:

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/scrolling/ios/remove-scrolling-role-expected.txt [moved from LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt with 85% similarity]
LayoutTests/fast/scrolling/ios/remove-scrolling-role.html [moved from LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html with 100% similarity]
LayoutTests/platform/ios-simulator-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingStateFixedNode.cpp
Source/WebCore/page/scrolling/ScrollingStateFixedNode.h
Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.cpp
Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.h
Source/WebCore/page/scrolling/ScrollingStateNode.cpp
Source/WebCore/page/scrolling/ScrollingStateNode.h
Source/WebCore/page/scrolling/ScrollingStateOverflowScrollingNode.cpp
Source/WebCore/page/scrolling/ScrollingStateOverflowScrollingNode.h
Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp
Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h
Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp
Source/WebCore/page/scrolling/ScrollingStateStickyNode.h

index 9e6adf5..94b0620 100644 (file)
@@ -1,5 +1,16 @@
 2016-05-10  Simon Fraser  <simon.fraser@apple.com>
 
+        Fix scrolling tree dumping
+        https://bugs.webkit.org/show_bug.cgi?id=157529
+
+        Reviewed by Tim Horton.
+
+        * fast/scrolling/ios/remove-scrolling-role-expected.txt: Renamed from LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt.
+        * fast/scrolling/ios/remove-scrolling-role.html: Renamed from LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html.
+        * platform/ios-simulator-wk2/TestExpectations:
+
+2016-05-10  Simon Fraser  <simon.fraser@apple.com>
+
         Mark fast/scrolling/ios/scroll-events-back-forward-after-pageshow.html as flakey.
 
         * platform/ios-simulator-wk2/TestExpectations:
index 5c029c0..f1ee8b7 100644 (file)
@@ -1664,7 +1664,7 @@ fast/text/simple-line-layout-text-stroke-width.html [ Failure ]
 http/tests/loading/preload-img-srcset-sizes.html [ Failure ]
 http/tests/navigation/postredirect-basic.html [ Failure ]
 http/tests/navigation/postredirect-goback1.html [ Failure ]
-platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html [ Failure ]
+
 platform/ios-simulator/ios/fast/text/underline-scaling.html [ Failure ]
 compositing/masks/compositing-clip-path-on-subpixel-position.html [ ImageOnlyFailure ]
 fast/borders/dashed-border-on-subpixel-position.html [ ImageOnlyFailure ]
index 407860f..d7d0f67 100644 (file)
@@ -1,3 +1,41 @@
+2016-05-10  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix scrolling tree dumping
+        https://bugs.webkit.org/show_bug.cgi?id=157529
+
+        Reviewed by Tim Horton.
+
+        Scrolling tree dumps cannot contain layerIDs because they are not stable between
+        runs. Fix by adding ScrollingStateTreeAsTextBehavior flags, and not dumping
+        the layerID for tests.
+
+        Sadly RemoteScrollingCoordinatorTransaction has a lot of duplicated code for dumping
+        the scrolling state tree, which should be converted to dumpProperties() at some point.
+        
+        Fix the one test that suffered from this problem, and unskip it.
+
+        Test: fast/scrolling/ios/remove-scrolling-role.html
+
+        * page/scrolling/ScrollingStateFixedNode.cpp:
+        (WebCore::ScrollingStateFixedNode::dumpProperties):
+        * page/scrolling/ScrollingStateFixedNode.h:
+        * page/scrolling/ScrollingStateFrameScrollingNode.cpp:
+        (WebCore::ScrollingStateFrameScrollingNode::dumpProperties):
+        * page/scrolling/ScrollingStateFrameScrollingNode.h:
+        * page/scrolling/ScrollingStateNode.cpp:
+        (WebCore::ScrollingStateNode::dump):
+        (WebCore::ScrollingStateNode::scrollingStateTreeAsText):
+        * page/scrolling/ScrollingStateNode.h:
+        * page/scrolling/ScrollingStateOverflowScrollingNode.cpp:
+        (WebCore::ScrollingStateOverflowScrollingNode::dumpProperties):
+        * page/scrolling/ScrollingStateOverflowScrollingNode.h:
+        * page/scrolling/ScrollingStateScrollingNode.cpp:
+        (WebCore::ScrollingStateScrollingNode::dumpProperties):
+        * page/scrolling/ScrollingStateScrollingNode.h:
+        * page/scrolling/ScrollingStateStickyNode.cpp:
+        (WebCore::ScrollingStateStickyNode::dumpProperties):
+        * page/scrolling/ScrollingStateStickyNode.h:
+
 2016-05-10  Csaba Osztrogon├íc  <ossy@webkit.org>
 
         Fix the !ENABLE(CSS_REGIONS) build after r198990
index ccdc832..82469c8 100644 (file)
@@ -75,7 +75,7 @@ void ScrollingStateFixedNode::syncLayerPositionForViewportRect(const LayoutRect&
         static_cast<GraphicsLayer*>(layer())->syncPosition(position);
 }
 
-void ScrollingStateFixedNode::dumpProperties(TextStream& ts, int indent) const
+void ScrollingStateFixedNode::dumpProperties(TextStream& ts, int indent, ScrollingStateTreeAsTextBehavior) const
 {
     ts << "(" << "Fixed node" << "\n";
 
index 877e0a2..4b6474f 100644 (file)
@@ -58,7 +58,7 @@ private:
 
     void syncLayerPositionForViewportRect(const LayoutRect& viewportRect) override;
 
-    void dumpProperties(TextStream&, int indent) const override;
+    void dumpProperties(TextStream&, int indent, ScrollingStateTreeAsTextBehavior) const override;
 
     FixedPositionViewportConstraints m_constraints;
 };
index 065706a..dc22ee5 100644 (file)
@@ -221,11 +221,11 @@ void ScrollingStateFrameScrollingNode::setScrollerImpsFromScrollbars(Scrollbar*,
 }
 #endif
 
-void ScrollingStateFrameScrollingNode::dumpProperties(TextStream& ts, int indent) const
+void ScrollingStateFrameScrollingNode::dumpProperties(TextStream& ts, int indent, ScrollingStateTreeAsTextBehavior behavior) const
 {
     ts << "(Frame scrolling node" << "\n";
     
-    ScrollingStateScrollingNode::dumpProperties(ts, indent);
+    ScrollingStateScrollingNode::dumpProperties(ts, indent, behavior);
     
     if (m_frameScaleFactor != 1) {
         writeIndent(ts, indent + 1);
index 3959b15..29d1d15 100644 (file)
@@ -119,7 +119,7 @@ public:
 #endif
     void setScrollerImpsFromScrollbars(Scrollbar* verticalScrollbar, Scrollbar* horizontalScrollbar);
 
-    void dumpProperties(TextStream&, int indent) const override;
+    void dumpProperties(TextStream&, int indent, ScrollingStateTreeAsTextBehavior) const override;
 
 private:
     ScrollingStateFrameScrollingNode(ScrollingStateTree&, ScrollingNodeID);
index d3e4f76..dfd5285 100644 (file)
@@ -112,17 +112,17 @@ void ScrollingStateNode::setLayer(const LayerRepresentation& layerRepresentation
     setPropertyChanged(ScrollLayer);
 }
 
-void ScrollingStateNode::dump(TextStream& ts, int indent) const
+void ScrollingStateNode::dump(TextStream& ts, int indent, ScrollingStateTreeAsTextBehavior behavior) const
 {
     writeIndent(ts, indent);
-    dumpProperties(ts, indent);
+    dumpProperties(ts, indent, behavior);
 
     if (m_children) {
         writeIndent(ts, indent + 1);
         ts << "(children " << children()->size() << "\n";
         
         for (auto& child : *m_children)
-            child->dump(ts, indent + 2);
+            child->dump(ts, indent + 2, behavior);
         writeIndent(ts, indent + 1);
         ts << ")\n";
     }
@@ -135,7 +135,7 @@ String ScrollingStateNode::scrollingStateTreeAsText() const
 {
     TextStream ts;
 
-    dump(ts, 0);
+    dump(ts, 0, ScrollingStateTreeAsTextBehaviorNormal);
     return ts.release();
 }
 
index acb601c..2c8cd6a 100644 (file)
@@ -40,6 +40,13 @@ class GraphicsLayer;
 class ScrollingStateTree;
 class TextStream;
 
+enum ScrollingStateTreeAsTextBehaviorFlags {
+    ScrollingStateTreeAsTextBehaviorNormal               = 0,
+    ScrollingStateTreeAsTextBehaviorIncludeLayerIDs      = 1 << 0,
+    ScrollingStateTreeAsTextBehaviorDebug                = ScrollingStateTreeAsTextBehaviorIncludeLayerIDs
+};
+typedef unsigned ScrollingStateTreeAsTextBehavior;
+
 // Used to allow ScrollingStateNodes to refer to layers in various contexts:
 // a) Async scrolling, main thread: ScrollingStateNode holds onto a GraphicsLayer, and uses m_layerID
 //    to detect whether that GraphicsLayer's underlying PlatformLayer changed.
@@ -238,9 +245,9 @@ protected:
     ScrollingStateNode(const ScrollingStateNode&, ScrollingStateTree&);
 
 private:
-    void dump(TextStream&, int indent) const;
+    void dump(TextStream&, int indent, ScrollingStateTreeAsTextBehavior) const;
 
-    virtual void dumpProperties(TextStream&, int indent) const = 0;
+    virtual void dumpProperties(TextStream&, int indent, ScrollingStateTreeAsTextBehavior) const = 0;
 
     const ScrollingNodeType m_nodeType;
     ScrollingNodeID m_nodeID;
index 84d25ef..346459a 100644 (file)
@@ -68,13 +68,13 @@ void ScrollingStateOverflowScrollingNode::setScrolledContentsLayer(const LayerRe
     setPropertyChanged(ScrolledContentsLayer);
 }
 
-void ScrollingStateOverflowScrollingNode::dumpProperties(TextStream& ts, int indent) const
+void ScrollingStateOverflowScrollingNode::dumpProperties(TextStream& ts, int indent, ScrollingStateTreeAsTextBehavior behavior) const
 {
     ts << "(" << "Overflow scrolling node" << "\n";
     
-    ScrollingStateScrollingNode::dumpProperties(ts, indent);
+    ScrollingStateScrollingNode::dumpProperties(ts, indent, behavior);
     
-    if (m_scrolledContentsLayer.layerID()) {
+    if ((behavior & ScrollingStateTreeAsTextBehaviorIncludeLayerIDs) && m_scrolledContentsLayer.layerID()) {
         writeIndent(ts, indent + 1);
         ts << "(scrolled contents layer " << m_scrolledContentsLayer.layerID() << ")\n";
     }
index 31c01b0..49dfdaf 100644 (file)
@@ -48,7 +48,7 @@ public:
     const LayerRepresentation& scrolledContentsLayer() const { return m_scrolledContentsLayer; }
     WEBCORE_EXPORT void setScrolledContentsLayer(const LayerRepresentation&);
     
-    void dumpProperties(TextStream&, int indent) const override;
+    void dumpProperties(TextStream&, int indent, ScrollingStateTreeAsTextBehavior) const override;
 
 private:
     ScrollingStateOverflowScrollingNode(ScrollingStateTree&, ScrollingNodeID);
index c53b9b5..a5cfa12 100644 (file)
@@ -168,7 +168,7 @@ void ScrollingStateScrollingNode::setExpectsWheelEventTestTrigger(bool expectsTe
     setPropertyChanged(ExpectsWheelEventTestTrigger);
 }
 
-void ScrollingStateScrollingNode::dumpProperties(TextStream& ts, int indent) const
+void ScrollingStateScrollingNode::dumpProperties(TextStream& ts, int indent, ScrollingStateTreeAsTextBehavior) const
 {
     if (m_scrollPosition != FloatPoint()) {
         writeIndent(ts, indent + 1);
index acaf020..96acae5 100644 (file)
@@ -95,11 +95,11 @@ public:
     bool expectsWheelEventTestTrigger() const { return m_expectsWheelEventTestTrigger; }
     WEBCORE_EXPORT void setExpectsWheelEventTestTrigger(bool);
 
-    void dumpProperties(TextStream&, int indent) const override;
-    
 protected:
     ScrollingStateScrollingNode(ScrollingStateTree&, ScrollingNodeType, ScrollingNodeID);
     ScrollingStateScrollingNode(const ScrollingStateScrollingNode&, ScrollingStateTree&);
+
+    void dumpProperties(TextStream&, int indent, ScrollingStateTreeAsTextBehavior) const override;
     
 private:
     FloatSize m_scrollableAreaSize;
index cee476e..ee14678 100644 (file)
@@ -75,7 +75,7 @@ void ScrollingStateStickyNode::syncLayerPositionForViewportRect(const LayoutRect
         static_cast<GraphicsLayer*>(layer())->syncPosition(position);
 }
 
-void ScrollingStateStickyNode::dumpProperties(TextStream& ts, int indent) const
+void ScrollingStateStickyNode::dumpProperties(TextStream& ts, int indent, ScrollingStateTreeAsTextBehavior) const
 {
     ts << "(" << "Sticky node" << "\n";
 
index 0833759..7fb31ef 100644 (file)
@@ -58,7 +58,7 @@ private:
 
     void syncLayerPositionForViewportRect(const LayoutRect& viewportRect) override;
 
-    void dumpProperties(TextStream&, int indent) const override;
+    void dumpProperties(TextStream&, int indent, ScrollingStateTreeAsTextBehavior) const override;
 
     StickyPositionViewportConstraints m_constraints;
 };