Bug 1422654: stylo: Avoid restyling XBL-bound element if the binding doesn't have stylesheets. r=heycam

MozReview-Commit-ID: An2McUbpCLk

--HG--
extra : rebase_source : 1c869057d032773fbfd426d6f392cc936c81029c

dom/xbl/nsXBLService.cpp

@@ -443,14 +443,18 @@ private:
 class MOZ_RAII AutoStyleElement
 {
 public:
-  explicit AutoStyleElement(Element* aElement)
+  AutoStyleElement(Element* aElement, bool* aResolveStyle)
     : mElement(aElement)
     , mHadData(aElement->HasServoData())
+    , mResolveStyle(aResolveStyle)
   {
+    MOZ_ASSERT(mResolveStyle);
     if (mHadData) {
-      ServoRestyleManager::ClearServoDataFromSubtree(mElement);
+      ServoRestyleManager::ClearServoDataFromSubtree(
+        mElement, ServoRestyleManager::IncludeRoot::No);
     }
   }
+
   ~AutoStyleElement()
   {
     nsIPresShell* presShell = mElement->OwnerDoc()->GetShell();
@@ -458,13 +462,18 @@ public:
       return;
     }
 
-    ServoStyleSet* servoSet = presShell->StyleSet()->AsServo();
-    servoSet->StyleNewSubtree(mElement);
+    if (*mResolveStyle) {
+      mElement->ClearServoData();
+
+      ServoStyleSet* servoSet = presShell->StyleSet()->AsServo();
+      servoSet->StyleNewSubtree(mElement);
+    }
   }
 
 private:
   Element* mElement;
   bool mHadData;
+  bool* mResolveStyle;
 };
 
 // This function loads a particular XBL file and installs all of the bindings
@@ -532,7 +541,7 @@ nsXBLService::LoadBindings(nsIContent* aContent, nsIURI* aURL,
     return NS_ERROR_ILLEGAL_VALUE;
   }
 
-  AutoStyleElement styleElement(aContent->AsElement());
+  AutoStyleElement styleElement(aContent->AsElement(), aResolveStyle);
 
   // We loaded a style binding.  It goes on the end.
   // Install the binding on the content node.

layout/base/PresShell.cpp

@@ -2963,14 +2963,8 @@ PresShell::DestroyFramesForAndRestyle(Element* aElement)
       //
       // FIXME(emilio): It'd be more ergonomic to just map the no data -> data
       // case to a reframe from the style system.
-      StyleChildrenIterator iter(aElement);
-      for (nsIContent* child = iter.GetNextChild();
-           child;
-           child = iter.GetNextChild()) {
-        if (child->IsElement()) {
-          ServoRestyleManager::ClearServoDataFromSubtree(child->AsElement());
-        }
-      }
+      ServoRestyleManager::ClearServoDataFromSubtree(
+          aElement, ServoRestyleManager::IncludeRoot::No);
     } else {
       // This is the case of an element that was redistributed but is no longer
       // bound to any insertion point. Just forget about all the data.

layout/base/ServoRestyleManager.cpp

@@ -427,7 +427,7 @@ ServoRestyleManager::PostRebuildAllStyleDataEvent(nsChangeHint aExtraHint,
 }
 
 /* static */ void
-ServoRestyleManager::ClearServoDataFromSubtree(Element* aElement)
+ServoRestyleManager::ClearServoDataFromSubtree(Element* aElement, IncludeRoot aIncludeRoot)
 {
   if (!aElement->HasServoData()) {
     MOZ_ASSERT(!aElement->HasDirtyDescendantsForServo());
@@ -438,11 +438,13 @@ ServoRestyleManager::ClearServoDataFromSubtree(Element* aElement)
   StyleChildrenIterator it(aElement);
   for (nsIContent* n = it.GetNextChild(); n; n = it.GetNextChild()) {
     if (n->IsElement()) {
-      ClearServoDataFromSubtree(n->AsElement());
+      ClearServoDataFromSubtree(n->AsElement(), IncludeRoot::Yes);
     }
   }
 
-  aElement->ClearServoData();
+  if (MOZ_LIKELY(aIncludeRoot == IncludeRoot::Yes)) {
+    aElement->ClearServoData();
+  }
 }
 
 /* static */ void

layout/base/ServoRestyleManager.h

@@ -242,11 +242,20 @@ private:
                                 ServoStyleSet& aStyleSet);
 
 public:
+  /**
+   * Whether to clear all the style data (including the element itself), or just
+   * the descendants' data.
+   */
+  enum class IncludeRoot {
+    Yes,
+    No,
+  };
+
   /**
    * Clears the ServoElementData and HasDirtyDescendants from all elements
    * in the subtree rooted at aElement.
    */
-  static void ClearServoDataFromSubtree(Element* aElement);
+  static void ClearServoDataFromSubtree(Element*, IncludeRoot = IncludeRoot::Yes);
 
   /**
    * Clears HasDirtyDescendants and RestyleData from all elements in the

layout/base/nsCSSFrameConstructor.cpp

@@ -2599,15 +2599,10 @@ nsCSSFrameConstructor::ConstructDocElementFrame(Element*                 aDocEle
       mDocument->BindingManager()->AddToAttachedQueue(binding);
     }
 
-    if (resolveStyle || styleContext->IsServo()) {
+    if (resolveStyle) {
       // FIXME: Should this use ResolveStyleContext?  (The calls in this
       // function are the only case in nsCSSFrameConstructor where we don't do
       // so for the construction of a style context for an element.)
-      //
-      // NOTE(emilio): In the case of Servo, even though resolveStyle returns
-      // false, we re-get the style context to avoid tripping otherwise-useful
-      // assertions when resolving pseudo-elements. Note that this operation in
-      // Servo is cheap.
       styleContext = mPresShell->StyleSet()->ResolveStyleFor(
           aDocElement, nullptr, LazyComputeBehavior::Assert);
       display = styleContext->StyleDisplay();
@@ -5906,16 +5901,18 @@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState
         aState.AddPendingBinding(newPendingBinding.forget());
       }
 
-      // See the comment in the similar-looking block in
-      // ConstructDocElementFrame to see why we always re-fetch the style
-      // context in Servo.
-      if (styleContext->IsServo()) {
-        styleContext =
-          mPresShell->StyleSet()->AsServo()->ResolveServoStyle(aContent->AsElement());
-      } else if (resolveStyle) {
-        styleContext =
-          ResolveStyleContext(styleContext->AsGecko()->GetParent(),
-                              aContent, &aState);
+      if (resolveStyle) {
+        // Need to take a different path (Servo directly grabs the style from
+        // the element, Gecko needs to actually re-resolve it using the parent
+        // style context).
+        if (styleContext->IsServo()) {
+          styleContext =
+            mPresShell->StyleSet()->AsServo()->ResolveServoStyle(aContent->AsElement());
+        } else {
+          styleContext =
+            ResolveStyleContext(styleContext->AsGecko()->GetParent(),
+                                aContent, &aState);
+        }
       }
 
       display = styleContext->StyleDisplay();