Bug 1170888 - Restyle the document in EnsureSafeToHandOutCSSRules if we previously cloned sheet inners outside of that method. r=bzbarsky

layout/base/nsPresContext.cpp

@@ -2332,14 +2332,11 @@ nsPresContext::NotifyMissingFonts()
 void
 nsPresContext::EnsureSafeToHandOutCSSRules()
 {
-  CSSStyleSheet::EnsureUniqueInnerResult res =
-    mShell->StyleSet()->EnsureUniqueInnerOnCSSSheets();
-  if (res == CSSStyleSheet::eUniqueInner_AlreadyUnique) {
+  if (!mShell->StyleSet()->EnsureUniqueInnerOnCSSSheets()) {
     // Nothing to do.
     return;
   }
 
-  MOZ_ASSERT(res == CSSStyleSheet::eUniqueInner_ClonedInner);
   RebuildAllStyleData(nsChangeHint(0), eRestyle_Subtree);
 }
 

layout/style/CSSStyleSheet.cpp

@@ -1186,6 +1186,20 @@ CSSStyleSheet::DropRuleProcessor(nsCSSRuleProcessor* aProcessor)
            : NS_ERROR_FAILURE;
 }
 
+void
+CSSStyleSheet::AddStyleSet(nsStyleSet* aStyleSet)
+{
+  NS_ASSERTION(!mStyleSets.Contains(aStyleSet),
+               "style set already registered");
+  mStyleSets.AppendElement(aStyleSet);
+}
+
+void
+CSSStyleSheet::DropStyleSet(nsStyleSet* aStyleSet)
+{
+  DebugOnly<bool> found = mStyleSets.RemoveElement(aStyleSet);
+  NS_ASSERTION(found, "didn't find style set");
+}
 
 void
 CSSStyleSheet::SetURIs(nsIURI* aSheetURI, nsIURI* aOriginalSheetURI,
@@ -1487,7 +1501,7 @@ CSSStyleSheet::StyleSheetCount() const
   return count;
 }
 
-CSSStyleSheet::EnsureUniqueInnerResult
+void
 CSSStyleSheet::EnsureUniqueInner()
 {
   mDirty = true;
@@ -1495,7 +1509,8 @@ CSSStyleSheet::EnsureUniqueInner()
   MOZ_ASSERT(mInner->mSheets.Length() != 0,
              "unexpected number of outers");
   if (mInner->mSheets.Length() == 1) {
-    return eUniqueInner_AlreadyUnique;
+    // already unique
+    return;
   }
   CSSStyleSheetInner* clone = mInner->CloneFor(this);
   MOZ_ASSERT(clone);
@@ -1505,7 +1520,12 @@ CSSStyleSheet::EnsureUniqueInner()
   // otherwise the rule processor has pointers to the old rules
   ClearRuleCascades();
 
-  return eUniqueInner_ClonedInner;
+  // let our containing style sets know that if we call
+  // nsPresContext::EnsureSafeToHandOutCSSRules we will need to restyle the
+  // document
+  for (nsStyleSet* styleSet : mStyleSets) {
+    styleSet->SetNeedsRestyleAfterEnsureUniqueInner();
+  }
 }
 
 void

layout/style/CSSStyleSheet.h

@@ -212,6 +212,9 @@ public:
   nsresult AddRuleProcessor(nsCSSRuleProcessor* aProcessor);
   nsresult DropRuleProcessor(nsCSSRuleProcessor* aProcessor);
 
+  void AddStyleSet(nsStyleSet* aStyleSet);
+  void DropStyleSet(nsStyleSet* aStyleSet);
+
   /**
    * Like the DOM insertRule() method, but doesn't do any security checks
    */
@@ -226,14 +229,7 @@ public:
   NS_IMETHOD StyleSheetLoaded(CSSStyleSheet* aSheet, bool aWasAlternate,
                               nsresult aStatus) override;
 
-  enum EnsureUniqueInnerResult {
-    // No work was needed to ensure a unique inner.
-    eUniqueInner_AlreadyUnique,
-    // A clone was done to ensure a unique inner (which means the style
-    // rules in this sheet have changed).
-    eUniqueInner_ClonedInner
-  };
-  EnsureUniqueInnerResult EnsureUniqueInner();
+  void EnsureUniqueInner();
 
   // Append all of this sheet's child sheets to aArray.
   void AppendAllChildSheets(nsTArray<CSSStyleSheet*>& aArray);
@@ -366,6 +362,7 @@ protected:
   CSSStyleSheetInner*   mInner;
 
   nsAutoTArray<nsCSSRuleProcessor*, 8>* mRuleProcessors;
+  nsTArray<nsStyleSet*> mStyleSets;
 
   friend class ::nsMediaList;
   friend class ::nsCSSRuleProcessor;

layout/style/nsStyleSet.cpp

@@ -151,6 +151,16 @@ static const nsStyleSet::sheetType gCSSSheetTypes[] = {
   nsStyleSet::eOverrideSheet
 };
 
+static bool IsCSSSheetType(nsStyleSet::sheetType aSheetType)
+{
+  for (nsStyleSet::sheetType type : gCSSSheetTypes) {
+    if (type == aSheetType) {
+      return true;
+    }
+  }
+  return false;
+}
+
 nsStyleSet::nsStyleSet()
   : mRuleTree(nullptr),
     mBatching(0),
@@ -158,11 +168,21 @@ nsStyleSet::nsStyleSet()
     mAuthorStyleDisabled(false),
     mInReconstruct(false),
     mInitFontFeatureValuesLookup(true),
+    mNeedsRestyleAfterEnsureUniqueInner(false),
     mDirty(0),
     mUnusedRuleNodeCount(0)
 {
 }
 
+nsStyleSet::~nsStyleSet()
+{
+  for (sheetType type : gCSSSheetTypes) {
+    for (uint32_t i = 0, n = mSheets[type].Length(); i < n; i++) {
+      static_cast<CSSStyleSheet*>(mSheets[type][i])->DropStyleSet(this);
+    }
+  }
+}
+
 size_t
 nsStyleSet::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
 {
@@ -500,10 +520,14 @@ nsStyleSet::AppendStyleSheet(sheetType aType, nsIStyleSheet *aSheet)
   NS_PRECONDITION(aSheet, "null arg");
   NS_ASSERTION(aSheet->IsApplicable(),
                "Inapplicable sheet being placed in style set");
-  mSheets[aType].RemoveObject(aSheet);
+  bool present = mSheets[aType].RemoveObject(aSheet);
   if (!mSheets[aType].AppendObject(aSheet))
     return NS_ERROR_OUT_OF_MEMORY;
 
+  if (!present && IsCSSSheetType(aType)) {
+    static_cast<CSSStyleSheet*>(aSheet)->AddStyleSet(this);
+  }
+
   return DirtyRuleProcessors(aType);
 }
 
@@ -513,10 +537,14 @@ nsStyleSet::PrependStyleSheet(sheetType aType, nsIStyleSheet *aSheet)
   NS_PRECONDITION(aSheet, "null arg");
   NS_ASSERTION(aSheet->IsApplicable(),
                "Inapplicable sheet being placed in style set");
-  mSheets[aType].RemoveObject(aSheet);
+  bool present = mSheets[aType].RemoveObject(aSheet);
   if (!mSheets[aType].InsertObjectAt(aSheet, 0))
     return NS_ERROR_OUT_OF_MEMORY;
 
+  if (!present && IsCSSSheetType(aType)) {
+    static_cast<CSSStyleSheet*>(aSheet)->AddStyleSet(this);
+  }
+
   return DirtyRuleProcessors(aType);
 }
 
@@ -526,7 +554,11 @@ nsStyleSet::RemoveStyleSheet(sheetType aType, nsIStyleSheet *aSheet)
   NS_PRECONDITION(aSheet, "null arg");
   NS_ASSERTION(aSheet->IsComplete(),
                "Incomplete sheet being removed from style set");
-  mSheets[aType].RemoveObject(aSheet);
+  if (mSheets[aType].RemoveObject(aSheet)) {
+    if (IsCSSSheetType(aType)) {
+      static_cast<CSSStyleSheet*>(aSheet)->DropStyleSet(this);
+    }
+  }
 
   return DirtyRuleProcessors(aType);
 }
@@ -535,10 +567,23 @@ nsresult
 nsStyleSet::ReplaceSheets(sheetType aType,
                           const nsCOMArray<nsIStyleSheet> &aNewSheets)
 {
+  bool cssSheetType = IsCSSSheetType(aType);
+  if (cssSheetType) {
+    for (uint32_t i = 0, n = mSheets[aType].Length(); i < n; i++) {
+      static_cast<CSSStyleSheet*>(mSheets[aType][i])->DropStyleSet(this);
+    }
+  }
+
   mSheets[aType].Clear();
   if (!mSheets[aType].AppendObjects(aNewSheets))
     return NS_ERROR_OUT_OF_MEMORY;
 
+  if (cssSheetType) {
+    for (uint32_t i = 0, n = mSheets[aType].Length(); i < n; i++) {
+      static_cast<CSSStyleSheet*>(mSheets[aType][i])->AddStyleSet(this);
+    }
+  }
+
   return DirtyRuleProcessors(aType);
 }
 
@@ -550,7 +595,7 @@ nsStyleSet::InsertStyleSheetBefore(sheetType aType, nsIStyleSheet *aNewSheet,
   NS_ASSERTION(aNewSheet->IsApplicable(),
                "Inapplicable sheet being placed in style set");
 
-  mSheets[aType].RemoveObject(aNewSheet);
+  bool present = mSheets[aType].RemoveObject(aNewSheet);
   int32_t idx = mSheets[aType].IndexOf(aReferenceSheet);
   if (idx < 0)
     return NS_ERROR_INVALID_ARG;
@@ -558,6 +603,10 @@ nsStyleSet::InsertStyleSheetBefore(sheetType aType, nsIStyleSheet *aNewSheet,
   if (!mSheets[aType].InsertObjectAt(aNewSheet, idx))
     return NS_ERROR_OUT_OF_MEMORY;
 
+  if (!present && IsCSSSheetType(aType)) {
+    static_cast<CSSStyleSheet*>(aNewSheet)->AddStyleSet(this);
+  }
+
   return DirtyRuleProcessors(aType);
 }
 
@@ -605,7 +654,7 @@ nsStyleSet::AddDocStyleSheet(nsIStyleSheet* aSheet, nsIDocument* aDocument)
                      eDocSheet;
   nsCOMArray<nsIStyleSheet>& sheets = mSheets[type];
 
-  sheets.RemoveObject(aSheet);
+  bool present = sheets.RemoveObject(aSheet);
   nsStyleSheetService *sheetService = nsStyleSheetService::GetInstance();
 
   // lowest index first
@@ -632,6 +681,10 @@ nsStyleSet::AddDocStyleSheet(nsIStyleSheet* aSheet, nsIDocument* aDocument)
   if (!sheets.InsertObjectAt(aSheet, index))
     return NS_ERROR_OUT_OF_MEMORY;
 
+  if (!present) {
+    static_cast<CSSStyleSheet*>(aSheet)->AddStyleSet(this);
+  }
+
   return DirtyRuleProcessors(type);
 }
 
@@ -2310,7 +2363,7 @@ nsStyleSet::MediumFeaturesChanged()
   return stylesChanged;
 }
 
-CSSStyleSheet::EnsureUniqueInnerResult
+bool
 nsStyleSet::EnsureUniqueInnerOnCSSSheets()
 {
   nsAutoTArray<CSSStyleSheet*, 32> queue;
@@ -2326,22 +2379,19 @@ nsStyleSet::EnsureUniqueInnerOnCSSSheets()
     mBindingManager->AppendAllSheets(queue);
   }
 
-  CSSStyleSheet::EnsureUniqueInnerResult res =
-    CSSStyleSheet::eUniqueInner_AlreadyUnique;
   while (!queue.IsEmpty()) {
     uint32_t idx = queue.Length() - 1;
     CSSStyleSheet* sheet = queue[idx];
     queue.RemoveElementAt(idx);
 
-    CSSStyleSheet::EnsureUniqueInnerResult sheetRes =
-      sheet->EnsureUniqueInner();
-    if (sheetRes == CSSStyleSheet::eUniqueInner_ClonedInner) {
-      res = sheetRes;
-    }
+    sheet->EnsureUniqueInner();
 
     // Enqueue all the sheet's children.
     sheet->AppendAllChildSheets(queue);
   }
+
+  bool res = mNeedsRestyleAfterEnsureUniqueInner;
+  mNeedsRestyleAfterEnsureUniqueInner = false;
   return res;
 }
 

layout/style/nsStyleSet.h

@@ -83,10 +83,11 @@ public:
 // then handed off to the PresShell.  Only the PresShell should delete a
 // style set.
 
-class nsStyleSet
+class nsStyleSet final
 {
  public:
   nsStyleSet();
+  ~nsStyleSet();
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
@@ -381,7 +382,15 @@ class nsStyleSet
     --mUnusedRuleNodeCount;
   }
 
-  mozilla::CSSStyleSheet::EnsureUniqueInnerResult EnsureUniqueInnerOnCSSSheets();
+  // Returns true if a restyle of the document is needed due to cloning
+  // sheet inners.
+  bool EnsureUniqueInnerOnCSSSheets();
+
+  // Called by CSSStyleSheet::EnsureUniqueInner to let us know it cloned
+  // its inner.
+  void SetNeedsRestyleAfterEnsureUniqueInner() {
+    mNeedsRestyleAfterEnsureUniqueInner = true;
+  }
 
   nsIStyleRule* InitialStyleRule();
 
@@ -459,8 +468,8 @@ class nsStyleSet
   // The sheets in each array in mSheets are stored with the most significant
   // sheet last.
   // The arrays for ePresHintSheet, eStyleAttrSheet, eTransitionSheet,
-  // and eAnimationSheet are always empty.  (FIXME:  We should reduce
-  // the storage needed for them.)
+  // eAnimationSheet and eSVGAttrAnimationSheet are always empty.
+  // (FIXME:  We should reduce the storage needed for them.)
   nsCOMArray<nsIStyleSheet> mSheets[eSheetTypeCount];
 
   // mRuleProcessors[eScopedDocSheet] is always null; rule processors
@@ -482,6 +491,7 @@ class nsStyleSet
   unsigned mAuthorStyleDisabled: 1;
   unsigned mInReconstruct : 1;
   unsigned mInitFontFeatureValuesLookup : 1;
+  unsigned mNeedsRestyleAfterEnsureUniqueInner : 1;
   unsigned mDirty : 10;  // one dirty bit is used per sheet type
 
   uint32_t mUnusedRuleNodeCount; // used to batch rule node GC