Bug 1422654: stylo: Avoid restyling XBL-bound element if the binding doesn't have stylesheets. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 04 Dec 2017 02:47:54 +0100
changeset 706901 554017140331f774b9a3e67c4d9613b95587b3d6
parent 706900 8e076dd05bd63a1fd1754e836a0b7ba1f1f4c1bc
child 742785 5cada38f143f5bfb5bd25dd2299095a321f951fa
push id91945
push userbmo:emilio@crisal.io
push dateMon, 04 Dec 2017 10:48:58 +0000
reviewersheycam
bugs1422654
milestone59.0a1
Bug 1422654: stylo: Avoid restyling XBL-bound element if the binding doesn't have stylesheets. r?heycam MozReview-Commit-ID: An2McUbpCLk
dom/xbl/nsXBLService.cpp
layout/base/PresShell.cpp
layout/base/ServoRestyleManager.cpp
layout/base/ServoRestyleManager.h
layout/base/nsCSSFrameConstructor.cpp
--- a/dom/xbl/nsXBLService.cpp
+++ b/dom/xbl/nsXBLService.cpp
@@ -438,38 +438,47 @@ public:
 private:
   Element* mElement;
 };
 
 // RAII class to restyle the XBL bound element when it shuffles the flat tree.
 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();
     if (!mHadData || !presShell || !presShell->DidInitialize()) {
       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
 // onto the element.
 nsresult
 nsXBLService::LoadBindings(nsIContent* aContent, nsIURI* aURL,
                            nsIPrincipal* aOriginPrincipal,
                            nsXBLBinding** aBinding, bool* aResolveStyle)
@@ -527,17 +536,17 @@ nsXBLService::LoadBindings(nsIContent* a
 #endif
     return NS_OK;
   }
 
   if (::IsAncestorBinding(document, aURL, aContent)) {
     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.
   aContent->SetXBLBinding(newBinding);
 
   {
     nsAutoScriptBlocker scriptBlocker;
 
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -2958,24 +2958,18 @@ PresShell::DestroyFramesForAndRestyle(El
       // This is the case of a new shadow root or XBL binding about to be
       // attached.
       //
       // Clear the style data from all the flattened tree descendants, but _not_
       // from us, since otherwise we wouldn't see the reframe.
       //
       // 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.
       ServoRestyleManager::ClearServoDataFromSubtree(aElement);
     }
   }
 
   auto changeHint = didReconstruct
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -422,32 +422,34 @@ ServoRestyleManager::PostRebuildAllStyle
   }
 
   // TODO(emilio, bz): Extensions can add/remove stylesheets that can affect
   // non-inheriting anon boxes. It's not clear if we want to support that, but
   // if we do, we need to re-selector-match them here.
 }
 
 /* static */ void
-ServoRestyleManager::ClearServoDataFromSubtree(Element* aElement)
+ServoRestyleManager::ClearServoDataFromSubtree(Element* aElement, IncludeRoot aIncludeRoot)
 {
   if (!aElement->HasServoData()) {
     MOZ_ASSERT(!aElement->HasDirtyDescendantsForServo());
     MOZ_ASSERT(!aElement->HasAnimationOnlyDirtyDescendantsForServo());
     return;
   }
 
   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
 ServoRestyleManager::ClearRestyleStateFromSubtree(Element* aElement)
 {
   if (aElement->HasAnyOfFlags(Element::kAllServoDescendantBits)) {
     StyleChildrenIterator it(aElement);
     for (nsIContent* n = it.GetNextChild(); n; n = it.GetNextChild()) {
--- a/layout/base/ServoRestyleManager.h
+++ b/layout/base/ServoRestyleManager.h
@@ -238,20 +238,29 @@ private:
    * child that was the style parent for aFrame and hence shouldn't be
    * reparented.
    */
   void ReparentFrameDescendants(nsIFrame* aFrame, nsIFrame* aProviderChild,
                                 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
    * subtree rooted at aElement.
    */
   static void ClearRestyleStateFromSubtree(Element* aElement);
 
   /**
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -2594,25 +2594,20 @@ nsCSSFrameConstructor::ConstructDocEleme
 
     if (binding) {
       // For backwards compat, keep firing the root's constructor
       // after all of its kids' constructors.  So tell the binding
       // manager about it right now.
       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();
     }
   }
 
   // --------- IF SCROLLABLE WRAP IN SCROLLFRAME --------
 
@@ -5901,26 +5896,28 @@ nsCSSFrameConstructor::AddFrameConstruct
         return;
 
       if (newPendingBinding->mBinding) {
         pendingBinding = newPendingBinding;
         // aState takes over owning newPendingBinding
         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();
       aStyleContext = styleContext;
       aTag = mDocument->BindingManager()->ResolveTag(aContent, &aNameSpaceID);
     }
   }