Bug 1419554: Teach the restyle root code about elements outside of the flattened tree. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 22 Nov 2017 14:15:34 +0100
changeset 447509 c009848f854e8cfe733bb2f2a1ddc53737a82c33
parent 447508 6286ca9b5067a317c812a473fd583ebafc3229b7
child 447510 14e8c75ef6489e9b853dae9cacc77fb22b1b3ecf
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1419554
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1419554: Teach the restyle root code about elements outside of the flattened tree. r=heycam The textarea is inserted under a Shadow host, with no matching insertion point, so its flattened tree parent node is null. We're treating this case in the restyle root code as "the parent is the document", but that's very wrong. MozReview-Commit-ID: JlzUMRIYaYZ
dom/base/Element.cpp
layout/style/ServoStyleSet.cpp
layout/style/crashtests/1419554.html
layout/style/crashtests/crashtests.list
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -4655,22 +4655,27 @@ PropagateBits(Element* aElement, uint32_
 // element under the current root is dirtied, that's why we don't trivially use
 // `nsContentUtils::GetCommonFlattenedTreeAncestorForStyle`.
 static void
 NoteDirtyElement(Element* aElement, uint32_t aBits)
 {
   MOZ_ASSERT(aElement->IsInComposedDoc());
   MOZ_ASSERT(aElement->IsStyledByServo());
 
-  Element* parent = aElement->GetFlattenedTreeParentElementForStyle();
-  if (MOZ_LIKELY(parent)) {
+  nsINode* parent = aElement->GetFlattenedTreeParentNodeForStyle();
+  if (!parent) {
+    // The element is not in the flattened tree, bail.
+    return;
+  }
+
+  if (MOZ_LIKELY(parent->IsElement())) {
     // If our parent is unstyled, we can inductively assume that it will be
     // traversed when the time is right, and that the traversal will reach us
     // when it happens. Nothing left to do.
-    if (!parent->HasServoData()) {
+    if (!parent->AsElement()->HasServoData()) {
       return;
     }
 
     // Similarly, if our parent already has the bit we're propagating, we can
     // assume everything is already set up.
     if (parent->HasAllFlags(aBits)) {
       return;
     }
@@ -4683,43 +4688,47 @@ NoteDirtyElement(Element* aElement, uint
     // The servo traversal doesn't keep style data under display: none subtrees, 
     // so in order for it to not need to cleanup each time anything happens in a
     // display: none subtree, we keep it clean.
     //
     // Also, we can't be much more smarter about using the parent's frame in
     // order to avoid work here, because since the style system keeps style data
     // in, e.g., subtrees under a leaf frame, missing restyles and such in there
     // has observable behavior via getComputedStyle, for example.
-    if (Servo_Element_IsDisplayNone(parent)) {
+    if (Servo_Element_IsDisplayNone(parent->AsElement())) {
       return;
     }
   }
 
   nsIDocument* doc = aElement->GetComposedDoc();
   if (nsIPresShell* shell = doc->GetShell()) {
     shell->EnsureStyleFlush();
   }
 
+  MOZ_ASSERT(parent->IsElement() || parent == doc);
+
   nsINode* existingRoot = doc->GetServoRestyleRoot();
   uint32_t existingBits = existingRoot ? doc->GetServoRestyleRootDirtyBits() : 0;
 
   // The bit checks below rely on this to arrive to useful conclusions about the
   // shape of the tree.
   AssertNoBitsPropagatedFrom(existingRoot);
 
   // If there's no existing restyle root, or if the root is already aElement,
   // just note the root+bits and return.
   if (!existingRoot || existingRoot == aElement) {
     doc->SetServoRestyleRoot(aElement, existingBits | aBits);
     return;
   }
 
   // There is an existing restyle root - walk up the tree from our element,
   // propagating bits as we go.
-  const bool reachedDocRoot = !parent || !PropagateBits(parent, aBits, existingRoot);
+  const bool reachedDocRoot =
+    !parent->IsElement() ||
+    !PropagateBits(parent->AsElement(), aBits, existingRoot);
 
   if (!reachedDocRoot || existingRoot == doc) {
       // We're a descendant of the existing root. All that's left to do is to
       // make sure the bit we propagated is also registered on the root.
       doc->SetServoRestyleRoot(existingRoot, existingBits | aBits);
   } else {
     // We reached the root without crossing the pre-existing restyle root. We
     // now need to find the nearest common ancestor, so climb up from the
@@ -4748,17 +4757,18 @@ NoteDirtyElement(Element* aElement, uint
   // See the comment in nsIDocument::SetServoRestyleRoot about the !IsElement()
   // check there. Same justification here.
   MOZ_ASSERT(aElement == doc->GetServoRestyleRoot() ||
              !doc->GetServoRestyleRoot()->IsElement() ||
              nsContentUtils::ContentIsFlattenedTreeDescendantOfForStyle(
                aElement, doc->GetServoRestyleRoot()));
   MOZ_ASSERT(aElement == doc->GetServoRestyleRoot() ||
              !doc->GetServoRestyleRoot()->IsElement() ||
-             BitsArePropagated(parent, aBits, doc->GetServoRestyleRoot()));
+             !parent->IsElement() ||
+             BitsArePropagated(parent->AsElement(), aBits, doc->GetServoRestyleRoot()));
   MOZ_ASSERT(doc->GetServoRestyleRootDirtyBits() & aBits);
 }
 
 void
 Element::NoteDirtySubtreeForServo()
 {
   MOZ_ASSERT(IsInComposedDoc());
   MOZ_ASSERT(HasServoData());
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -1339,26 +1339,31 @@ ServoStyleSet::MaybeGCRuleTree()
   MOZ_ASSERT(NS_IsMainThread());
   Servo_MaybeGCRuleTree(mRawSet.get());
 }
 
 /* static */ bool
 ServoStyleSet::MayTraverseFrom(const Element* aElement)
 {
   MOZ_ASSERT(aElement->IsInComposedDoc());
-  Element* parent = aElement->GetFlattenedTreeParentElementForStyle();
+  nsINode* parent = aElement->GetFlattenedTreeParentNodeForStyle();
   if (!parent) {
+    return false;
+  }
+
+  if (!parent->IsElement()) {
+    MOZ_ASSERT(parent->IsNodeOfType(nsINode::eDOCUMENT));
     return true;
   }
 
-  if (!parent->HasServoData()) {
+  if (!parent->AsElement()->HasServoData()) {
     return false;
   }
 
-  return !Servo_Element_IsDisplayNone(parent);
+  return !Servo_Element_IsDisplayNone(parent->AsElement());
 }
 
 bool
 ServoStyleSet::ShouldTraverseInParallel() const
 {
   return mPresContext->PresShell()->IsActive();
 }
 
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/1419554.html
@@ -0,0 +1,9 @@
+<script>
+function go() {
+  a.attachShadow({ mode: "open" });
+  a.appendChild(b);
+}
+</script>
+<body onload=go()>
+<div id="b"></div>
+<div id="a"></div>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -260,8 +260,9 @@ load 1411143.html
 load 1411478.html
 load 1413288.html
 load 1413361.html
 load 1415663.html
 pref(dom.webcomponents.enabled,true) load 1415353.html
 load 1415021.html # This should have dom.webcomponents.enabled=true, but it leaks the world, see bug 1416296.
 load 1418059.html
 skip-if(!stylo) test-pref(dom.animations-api.core.enabled,true) load 1418867.html
+pref(dom.webcomponents.enabled,true) load 1419554.html