Bug 1419554: Teach the restyle root code about elements outside of the flattened tree. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 22 Nov 2017 14:15:34 +0100
changeset 701943 1040f13fa3fcab02005c56d0befb78f173a8fb18
parent 701872 5378dcb45044a160fad93b02eed0c617f3324dbc
child 702286 81547e016bf6438686f37c8e11059e5e31a5e69e
child 702652 000d5764ab64e33d8b5361e4356a4df3deaeb198
child 702675 d8d336ea1efcc27ad147541fd4fa529b4a975e7b
push id90312
push userbmo:emilio@crisal.io
push dateWed, 22 Nov 2017 13:22:31 +0000
reviewersheycam
bugs1419554
milestone59.0a1
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