Bug 1462618: The restyle root machinery could work better. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 17 May 2018 19:55:48 +0200
changeset 796876 e728198ba3decb01201f3c68e41b7f2c81e209b8
parent 796372 8b1c2ae868814e5de9502145364d26264ebb71dc
push id110371
push userbmo:emilio@crisal.io
push dateFri, 18 May 2018 12:21:53 +0000
reviewersheycam
bugs1462618
milestone62.0a1
Bug 1462618: The restyle root machinery could work better. r?heycam Consider the test-case where we have: <div> <span id=a /> <span id=b /> </div> We try to set one bit on "a", and a different one on "b". Ideally we'll end up with <div> as the root with both bits. But with the current code we'd go all the way to the document unnecessarily. This fixes it by checking the bit's we've propagated up to the top instead of existingBits. MozReview-Commit-ID: GfwjwCBpkuy
dom/base/Element.cpp
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -4384,27 +4384,31 @@ AssertNoBitsPropagatedFrom(nsINode* aRoo
     element = element->GetFlattenedTreeParentElementForStyle();
   }
 #endif
 }
 
 // Sets |aBits| on aElement and all of its flattened-tree ancestors up to and
 // including aStopAt or the root element (whichever is encountered first).
 static inline Element*
-PropagateBits(Element* aElement, uint32_t aBits, nsINode* aStopAt)
+PropagateBits(Element* aElement, uint32_t aBits, nsINode* aStopAt, uint32_t aBitsToStopAt)
 {
   Element* curr = aElement;
-  while (curr && !curr->HasAllFlags(aBits)) {
+  while (curr && !curr->HasAllFlags(aBitsToStopAt)) {
     curr->SetFlags(aBits);
     if (curr == aStopAt) {
       break;
     }
     curr = curr->GetFlattenedTreeParentElementForStyle();
   }
 
+  if (aBitsToStopAt != aBits && curr) {
+    curr->SetFlags(aBits);
+  }
+
   return curr;
 }
 
 // Notes that a given element is "dirty" with respect to the given descendants
 // bit (which may be one of dirty descendants, dirty animation descendants, or
 // need frame construction for descendants).
 //
 // This function operates on the dirty element itself, despite the fact that the
@@ -4505,40 +4509,41 @@ NoteDirtyElement(Element* aElement, uint
     doc->SetServoRestyleRoot(aElement, aBits);
     return;
   }
 
   // There is an existing restyle root - walk up the tree from our element,
   // propagating bits as we go.
   const bool reachedDocRoot =
     !parent->IsElement() ||
-    !PropagateBits(parent->AsElement(), aBits, existingRoot);
+    !PropagateBits(parent->AsElement(), aBits, existingRoot, aBits);
 
   uint32_t existingBits = doc->GetServoRestyleRootDirtyBits();
   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
     // existing root, extending bits along the way.
     Element* rootParent = existingRoot->GetFlattenedTreeParentElementForStyle();
-    if (Element* commonAncestor = PropagateBits(rootParent, existingBits, aElement)) {
+    if (Element* commonAncestor = PropagateBits(rootParent, existingBits, aElement, aBits)) {
       MOZ_ASSERT(commonAncestor == aElement ||
                  commonAncestor == nsContentUtils::GetCommonFlattenedTreeAncestorForStyle(aElement, rootParent));
 
       // We found a common ancestor. Make that the new style root, and clear the
       // bits between the new style root and the document root.
       doc->SetServoRestyleRoot(commonAncestor, existingBits | aBits);
       Element* curr = commonAncestor;
       while ((curr = curr->GetFlattenedTreeParentElementForStyle())) {
         MOZ_ASSERT(curr->HasAllFlags(aBits));
         curr->UnsetFlags(aBits);
       }
+      AssertNoBitsPropagatedFrom(commonAncestor);
     } else {
       // We didn't find a common ancestor element. That means we're descended
       // from two different document style roots, so the common ancestor is the
       // document.
       doc->SetServoRestyleRoot(doc, existingBits | aBits);
     }
   }
 
@@ -4567,17 +4572,18 @@ Element::NoteDirtySubtreeForServo()
 
   if (existingRoot &&
       existingRoot->IsElement() &&
       existingRoot != this &&
       nsContentUtils::ContentIsFlattenedTreeDescendantOfForStyle(
         existingRoot->AsElement(), this)) {
     PropagateBits(existingRoot->AsElement()->GetFlattenedTreeParentElementForStyle(),
                   existingBits,
-                  this);
+                  this,
+                  existingBits);
 
     doc->ClearServoRestyleRoot();
   }
 
   NoteDirtyElement(this, existingBits | ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO);
 }
 
 void