- Revision
- 206712
- Author
- [email protected]
- Date
- 2016-10-01 18:05:06 -0700 (Sat, 01 Oct 2016)
Log Message
Bad cast when CSS position programmatically changed from -webkit-sticky to fixed
https://bugs.webkit.org/show_bug.cgi?id=160826
Reviewed by Zalan Bujtas.
Source/WebCore:
If a scrolling state tree node changed type (e.g. from sticky to fixed), we'd fail
to recreate the node so keep a node with the wrong type.
Fix by destroying the node and making a new one with a new ID in this case. The
new ID is necessary to ensure that the scrolling tree is updated.
Test: fast/scrolling/sticky-to-fixed.html
* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::nodeTypeAndParentMatch):
(WebCore::ScrollingStateTree::attachNode):
(WebCore::ScrollingStateTree::stateNodeForID):
* page/scrolling/ScrollingStateTree.h:
LayoutTests:
* fast/scrolling/sticky-to-fixed-expected.txt: Added.
* fast/scrolling/sticky-to-fixed.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (206711 => 206712)
--- trunk/LayoutTests/ChangeLog 2016-10-01 23:36:21 UTC (rev 206711)
+++ trunk/LayoutTests/ChangeLog 2016-10-02 01:05:06 UTC (rev 206712)
@@ -1,3 +1,13 @@
+2016-10-01 Simon Fraser <[email protected]>
+
+ Bad cast when CSS position programmatically changed from -webkit-sticky to fixed
+ https://bugs.webkit.org/show_bug.cgi?id=160826
+
+ Reviewed by Zalan Bujtas.
+
+ * fast/scrolling/sticky-to-fixed-expected.txt: Added.
+ * fast/scrolling/sticky-to-fixed.html: Added.
+
2016-09-30 Joseph Pecoraro <[email protected]>
Web Inspector: Stepping to a line with an autoContinue breakpoint should still pause
Added: trunk/LayoutTests/fast/scrolling/sticky-to-fixed-expected.txt (0 => 206712)
--- trunk/LayoutTests/fast/scrolling/sticky-to-fixed-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/scrolling/sticky-to-fixed-expected.txt 2016-10-02 01:05:06 UTC (rev 206712)
@@ -0,0 +1 @@
+This test should not assert in debug.
Added: trunk/LayoutTests/fast/scrolling/sticky-to-fixed.html (0 => 206712)
--- trunk/LayoutTests/fast/scrolling/sticky-to-fixed.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/sticky-to-fixed.html 2016-10-02 01:05:06 UTC (rev 206712)
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+ height: 2000px;
+}
+.masthead {
+ position: -webkit-sticky;
+ width: 200px;
+ height: 100px;
+ background-color: rgba(0, 0, 0, 0.2);
+}
+
+.make-fixed .masthead {
+ position: fixed;
+}
+
+.fixed {
+ position: fixed;
+ width: 100px;
+ height: 100px;
+}
+</style>
+<script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ function doTest()
+ {
+ window.setTimeout(function() {
+ document.body.classList.add("make-fixed")
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+ }
+
+ window.addEventListener('load', doTest, false);
+</script>
+</head>
+<body>
+<div class="masthead">This test should not assert in debug.
+ <div class="fixed">
+ </div>
+</div>
+</body>
+</html>
+
Modified: trunk/Source/WebCore/ChangeLog (206711 => 206712)
--- trunk/Source/WebCore/ChangeLog 2016-10-01 23:36:21 UTC (rev 206711)
+++ trunk/Source/WebCore/ChangeLog 2016-10-02 01:05:06 UTC (rev 206712)
@@ -1,3 +1,24 @@
+2016-10-01 Simon Fraser <[email protected]>
+
+ Bad cast when CSS position programmatically changed from -webkit-sticky to fixed
+ https://bugs.webkit.org/show_bug.cgi?id=160826
+
+ Reviewed by Zalan Bujtas.
+
+ If a scrolling state tree node changed type (e.g. from sticky to fixed), we'd fail
+ to recreate the node so keep a node with the wrong type.
+
+ Fix by destroying the node and making a new one with a new ID in this case. The
+ new ID is necessary to ensure that the scrolling tree is updated.
+
+ Test: fast/scrolling/sticky-to-fixed.html
+
+ * page/scrolling/ScrollingStateTree.cpp:
+ (WebCore::ScrollingStateTree::nodeTypeAndParentMatch):
+ (WebCore::ScrollingStateTree::attachNode):
+ (WebCore::ScrollingStateTree::stateNodeForID):
+ * page/scrolling/ScrollingStateTree.h:
+
2016-10-01 Youenn Fablet <[email protected]>
removing FetchBoyd::m_type
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (206711 => 206712)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2016-10-01 23:36:21 UTC (rev 206711)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2016-10-02 01:05:06 UTC (rev 206712)
@@ -83,21 +83,31 @@
return nullptr;
}
+bool ScrollingStateTree::nodeTypeAndParentMatch(ScrollingStateNode& node, ScrollingNodeType nodeType, ScrollingNodeID parentID) const
+{
+ if (node.nodeType() != nodeType)
+ return false;
+
+ ScrollingStateNode* parent = stateNodeForID(parentID);
+ if (!parent)
+ return true;
+
+ return node.parent() == parent;
+}
+
ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID)
{
ASSERT(newNodeID);
+
if (ScrollingStateNode* node = stateNodeForID(newNodeID)) {
- if (!parentID)
+ if (nodeTypeAndParentMatch(*node, nodeType, parentID))
return newNodeID;
+
+ // If the type has changed, we need to destroy and recreate the node with a new ID.
+ if (nodeType != node->nodeType())
+ newNodeID = m_scrollingCoordinator->uniqueScrollLayerID();
- ScrollingStateNode* parent = stateNodeForID(parentID);
- if (!parent)
- return newNodeID;
-
- if (node->parent() == parent)
- return newNodeID;
-
- // The node is being re-parented. To do that, we'll remove it, and then re-create a new node.
+ // The node is being re-parented. To do that, we'll remove it, and then create a new node.
removeNodeAndAllDescendants(node, SubframeNodeRemoval::Orphan);
}
@@ -236,7 +246,7 @@
m_nodesRemovedSinceLastCommit = WTFMove(nodes);
}
-ScrollingStateNode* ScrollingStateTree::stateNodeForID(ScrollingNodeID scrollLayerID)
+ScrollingStateNode* ScrollingStateTree::stateNodeForID(ScrollingNodeID scrollLayerID) const
{
if (!scrollLayerID)
return 0;
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h (206711 => 206712)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2016-10-01 23:36:21 UTC (rev 206711)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2016-10-02 01:05:06 UTC (rev 206712)
@@ -48,7 +48,7 @@
WEBCORE_EXPORT ~ScrollingStateTree();
ScrollingStateFrameScrollingNode* rootStateNode() const { return m_rootStateNode.get(); }
- WEBCORE_EXPORT ScrollingStateNode* stateNodeForID(ScrollingNodeID);
+ WEBCORE_EXPORT ScrollingStateNode* stateNodeForID(ScrollingNodeID) const;
WEBCORE_EXPORT ScrollingNodeID attachNode(ScrollingNodeType, ScrollingNodeID, ScrollingNodeID parentID);
void detachNode(ScrollingNodeID);
@@ -79,7 +79,9 @@
void addNode(ScrollingStateNode*);
PassRefPtr<ScrollingStateNode> createNode(ScrollingNodeType, ScrollingNodeID);
-
+
+ bool nodeTypeAndParentMatch(ScrollingStateNode&, ScrollingNodeType, ScrollingNodeID parentID) const;
+
enum class SubframeNodeRemoval {
Delete,
Orphan
