Bug 1403615: Also follow the NODE_DESCENDANTS_NEED_FRAMES bit in ClearRestyleStateFromSubtree. r=bholley draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 27 Sep 2017 19:19:12 +0200
changeset 671249 239b2290ac7b4280e0d915fb74b9a143d8438c93
parent 671247 632ff5e943fabc82829d011abdf1eca090154e4c
child 733474 e862ea935fde77e280a7f9c8cb358f5386306503
push id81891
push userbmo:emilio@crisal.io
push dateWed, 27 Sep 2017 17:56:59 +0000
reviewersbholley
bugs1403615
milestone58.0a1
Bug 1403615: Also follow the NODE_DESCENDANTS_NEED_FRAMES bit in ClearRestyleStateFromSubtree. r=bholley We don't follow this bit intentionally because we know that even if it's set, when none of the other two bits are set there are no other restyle / change hints down the tree. We rely on the frame constructor to clean the mess up, though, and it doesn't really do a good work about it. In particular, the case we're hitting on the test-case is: <body descendant-need-frames change=reconstruct style="display: table-column-group"> <div descendant-need-frames> <div descendant-need-frames> <span needs-frame></span> </div> </div> </body> When we see we need to reconstruct the body, we call ClearRestyleStateFromSubtree, but that doesn't do much now, since we don't follow the descendant-need-frames bits. Then, when we reconstruct the content, we arrive at[1] when constructing the first child <div>. The <div> flags have been cleared, but not the children's! Then a text-node is inserted in a <div>, breaking all sorts of invariants. This is the easiest fix. Other fixes include clearing the flags on SetAsUndisplayedContent. But that implies not clearing them in ShouldCreateItemsForChild, and doing that somewhere more sensible. I suspect it's not too hard, but that's a slightly more risky change, will do it if you prefer it. [1]: http://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/layout/base/nsCSSFrameConstructor.cpp#6092 MozReview-Commit-ID: 7026wkQLQkz
layout/base/ServoRestyleManager.cpp
layout/style/crashtests/1403615.html
layout/style/crashtests/crashtests.list
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -410,18 +410,17 @@ ServoRestyleManager::ClearServoDataFromS
   }
 
   aElement->ClearServoData();
 }
 
 /* static */ void
 ServoRestyleManager::ClearRestyleStateFromSubtree(Element* aElement)
 {
-  if (aElement->HasDirtyDescendantsForServo() ||
-      aElement->HasAnimationOnlyDirtyDescendantsForServo()) {
+  if (aElement->HasAnyOfFlags(Element::kAllServoDescendantBits)) {
     StyleChildrenIterator it(aElement);
     for (nsIContent* n = it.GetNextChild(); n; n = it.GetNextChild()) {
       if (n->IsElement()) {
         ClearRestyleStateFromSubtree(n->AsElement());
       }
     }
   }
 
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/1403615.html
@@ -0,0 +1,24 @@
+<html class="reftest-wait">
+<script>
+window.onload = () => {
+  a = document.createElement("body")
+  b = document.createElement("span")
+  a.appendChild(b)
+  c = document.createElement("div")
+  d = document.createElement("div")
+  c.appendChild(d)
+  a.appendChild(c)
+  document.documentElement.appendChild(a)
+  requestIdleCallback(() => {
+    a.style.display = "table-column-group"
+    d.appendChild(document.createTextNode("\u05D2"))
+    requestIdleCallback(() => {
+      d.appendChild(document.createElement("span"))
+      b.style.zIndex = "1073741824"
+      document.documentElement.offsetTop;
+      document.documentElement.removeClass("reftest-wait");
+    })
+  })
+}
+</script>
+</html>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -226,8 +226,9 @@ load 1400926.html
 load 1401801.html
 load 1401256.html
 load 1402419.html
 load 1401706.html
 load 1400936-1.html
 load 1400936-2.html
 load 1402472.html
 load 1402366.html
+load 1403615.html