Bug 1412714 - Don't clone inner of XBL stylesheet in Servo. r?bz draft
authorXidorn Quan <me@upsuper.org>
Mon, 30 Oct 2017 15:16:20 +1100
changeset 691870 d4d65f4f00fd90f2004a6624bfc595b191de644b
parent 691869 dad8774fd59aaa1a4c857705811a616772045ab0
child 738610 53df92e0cb35ad40aafaee9c3cd62b29eaac5192
push id87339
push userxquan@mozilla.com
push dateThu, 02 Nov 2017 05:50:26 +0000
reviewersbz
bugs1412714
milestone58.0a1
Bug 1412714 - Don't clone inner of XBL stylesheet in Servo. r?bz MozReview-Commit-ID: Kzrod3SBt1k
layout/inspector/ServoStyleRuleMap.cpp
layout/style/ServoStyleSet.h
layout/style/ServoStyleSheet.cpp
layout/style/ServoStyleSheet.h
layout/style/StyleSheet.cpp
--- a/layout/inspector/ServoStyleRuleMap.cpp
+++ b/layout/inspector/ServoStyleRuleMap.cpp
@@ -171,16 +171,13 @@ ServoStyleRuleMap::FillTableFromRuleList
     FillTableFromRule(aRuleList->GetRule(i));
   }
 }
 
 void
 ServoStyleRuleMap::FillTableFromStyleSheet(ServoStyleSheet* aSheet)
 {
   if (aSheet->IsComplete()) {
-    // XBL stylesheets are not expected to ever change, so it's a waste
-    // to make its inner unique.
-    FillTableFromRuleList(aSheet->GetCssRulesInternal(
-        /* aRequireUniqueInner = */ !mStyleSet->IsForXBL()));
+    FillTableFromRuleList(aSheet->GetCssRulesInternal());
   }
 }
 
 } // namespace mozilla
--- a/layout/style/ServoStyleSet.h
+++ b/layout/style/ServoStyleSet.h
@@ -444,16 +444,17 @@ public:
 
   // Returns true if a restyle of the document is needed due to cloning
   // sheet inners.
   bool EnsureUniqueInnerOnCSSSheets();
 
   // Called by StyleSheet::EnsureUniqueInner to let us know it cloned
   // its inner.
   void SetNeedsRestyleAfterEnsureUniqueInner() {
+    MOZ_ASSERT(!IsForXBL(), "Should not be cloning things for XBL stylesheet");
     mNeedsRestyleAfterEnsureUniqueInner = true;
   }
 
   // Returns the style rule map.
   ServoStyleRuleMap* StyleRuleMap();
 
   // Return whether this is the last PresContext which uses this XBL styleset.
   bool IsPresContextChanged(nsPresContext* aPresContext) const {
--- a/layout/style/ServoStyleSheet.cpp
+++ b/layout/style/ServoStyleSheet.cpp
@@ -388,25 +388,20 @@ ServoStyleSheet::Clone(StyleSheet* aClon
     static_cast<ServoStyleSheet*>(aCloneParent),
     aCloneOwnerRule,
     aCloneDocument,
     aCloneOwningNode);
   return clone.forget();
 }
 
 ServoCSSRuleList*
-ServoStyleSheet::GetCssRulesInternal(bool aRequireUniqueInner)
+ServoStyleSheet::GetCssRulesInternal()
 {
   if (!mRuleList) {
-    MOZ_ASSERT(aRequireUniqueInner || !mDocument,
-               "Not requiring unique inner for stylesheet associated "
-               "with document may have undesired behavior");
-    if (aRequireUniqueInner) {
-      EnsureUniqueInner();
-    }
+    EnsureUniqueInner();
 
     RefPtr<ServoCssRules> rawRules =
       Servo_StyleSheet_GetRules(Inner()->mContents).Consume();
     MOZ_ASSERT(rawRules);
     mRuleList = new ServoCSSRuleList(rawRules.forget(), this);
   }
   return mRuleList;
 }
--- a/layout/style/ServoStyleSheet.h
+++ b/layout/style/ServoStyleSheet.h
@@ -115,17 +115,17 @@ public:
     nsINode* aCloneOwningNode) const final;
 
   // nsICSSLoaderObserver interface
   NS_IMETHOD StyleSheetLoaded(StyleSheet* aSheet, bool aWasAlternate,
                               nsresult aStatus) final;
 
   // Internal GetCssRules method which do not have security check and
   // completelness check.
-  ServoCSSRuleList* GetCssRulesInternal(bool aRequireUniqueInner = true);
+  ServoCSSRuleList* GetCssRulesInternal();
 
   // Returns the stylesheet's Servo origin as an OriginFlags value.
   OriginFlags GetOrigin();
 
 protected:
   virtual ~ServoStyleSheet();
 
   void LastRelease();
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -449,16 +449,27 @@ StyleSheet::EnsureUniqueInner()
   MOZ_ASSERT(mInner->mSheets.Length() != 0,
              "unexpected number of outers");
   mDirty = true;
 
   if (HasUniqueInner()) {
     // already unique
     return;
   }
+  // If this stylesheet is for XBL with Servo, don't bother cloning
+  // it, as it may break ServoStyleRuleMap. XBL stylesheets are not
+  // supposed to change anyway.
+  // The mDocument check is used as a fast reject path because no
+  // XBL stylesheets would have associated document, but in normal
+  // cases, content stylesheets should usually have one.
+  if (!mDocument && IsServo() &&
+      mStyleSets.Length() == 1 &&
+      mStyleSets[0]->AsServo()->IsForXBL()) {
+    return;
+  }
 
   StyleSheetInfo* clone = mInner->CloneFor(this);
   MOZ_ASSERT(clone);
   mInner->RemoveSheet(this);
   mInner = clone;
 
   if (CSSStyleSheet* geckoSheet = GetAsGecko()) {
     // Ensure we're using the new rules.