Bug 1402684 - Clear the servo data early, but the flags later, in UnbindFromTree. r=bholley, a=sledru
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 24 Sep 2017 19:16:10 +0200
changeset 431840 3fa652b178a23dd826594355a18d008f0b976bf1
parent 431839 99992ed2b3f326a71044f04106a4c7795c5fdb40
child 431841 fd78928b00d87444d1aefe942d0489e8682e0981
push id7821
push userryanvm@gmail.com
push dateMon, 25 Sep 2017 14:09:06 +0000
treeherdermozilla-beta@fd78928b00d8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley, sledru
bugs1402684
milestone57.0
Bug 1402684 - Clear the servo data early, but the flags later, in UnbindFromTree. r=bholley, a=sledru MozReview-Commit-ID: 5rRGKq39smW
dom/base/Element.cpp
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1893,16 +1893,29 @@ Element::UnbindFromTree(bool aDeep, bool
     DeleteProperty(nsGkAtoms::transitionsOfBeforeProperty);
     DeleteProperty(nsGkAtoms::transitionsOfAfterProperty);
     DeleteProperty(nsGkAtoms::transitionsProperty);
     DeleteProperty(nsGkAtoms::animationsOfBeforeProperty);
     DeleteProperty(nsGkAtoms::animationsOfAfterProperty);
     DeleteProperty(nsGkAtoms::animationsProperty);
   }
 
+  // Computed style data isn't useful for detached nodes, and we'll need to
+  // recompute it anyway if we ever insert the nodes back into a document.
+  if (IsStyledByServo()) {
+    if (document) {
+      ClearServoData(document);
+    } else {
+      MOZ_ASSERT(!HasServoData());
+      MOZ_ASSERT(!HasAnyOfFlags(kAllServoDescendantBits | NODE_NEEDS_FRAME));
+    }
+  } else {
+    MOZ_ASSERT(!HasServoData());
+  }
+
   // Editable descendant count only counts descendants that
   // are in the uncomposed document.
   ResetEditableDescendantCount();
 
   if (aNullParent || !mParent->IsInShadowTree()) {
     UnsetFlags(NODE_IS_IN_SHADOW_TREE);
 
     // Begin keeping track of our subtree root.
@@ -1987,28 +2000,31 @@ Element::UnbindFromTree(bool aDeep, bool
     for (nsIContent* child = shadowRoot->GetFirstChild(); child;
          child = child->GetNextSibling()) {
       child->UnbindFromTree(true, false);
     }
 
     shadowRoot->SetIsComposedDocParticipant(false);
   }
 
-  // Computed style data isn't useful for detached nodes, and we'll need to
-  // recompute it anyway if we ever insert the nodes back into a document.
-  if (IsStyledByServo()) {
-    if (document) {
-      ClearServoData(document);
-    } else {
-      MOZ_ASSERT(!HasServoData());
-      MOZ_ASSERT(!HasAnyOfFlags(kAllServoDescendantBits | NODE_NEEDS_FRAME));
-    }
-  } else {
-    MOZ_ASSERT(!HasServoData());
-  }
+  // Unbinding of children is the only point in time where we don't enforce the
+  // "child has style data implies parent has it too" invariant.
+  //
+  // As such, the restyle root tracking may incorrectly end up setting dirty
+  // bits on the parent chain when moving from a not yet unbound root with
+  // already unbound parents to a root higher up in the tree, so we clear those
+  // (again, since they're also cleared in ClearServoData) here.
+  //
+  // This can happen when the element changes the state of some ancestor up in
+  // the tree, for example.
+  //
+  // Note that clearing the data itself here would have its own set of problems,
+  // since the invariant we'd be breaking in that case is "HasServoData()
+  // implies InComposedDoc()", which we rely on in various places.
+  UnsetFlags(kAllServoDescendantBits);
 }
 
 nsICSSDeclaration*
 Element::GetSMILOverrideStyle()
 {
   Element::nsExtendedDOMSlots* slots = ExtendedDOMSlots();
 
   if (!slots->mSMILOverrideStyle) {