Bug 1180118 - Part 9: Clear nsCSSSelector pointers in the pending restyle tracker if they might be stale. r=bzbarsky
authorCameron McCormack <cam@mcc.id.au>
Wed, 05 Aug 2015 22:42:21 +1000
changeset 287964 9f7deabf4068705258a8eaa97d374cef359fd300
parent 287963 b7367a66c33a87e52da228f819ffad5cc8d3c265
child 287965 68bef2d8c8bfd412a5831896fa523ecbcc19266c
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1180118
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1180118 - Part 9: Clear nsCSSSelector pointers in the pending restyle tracker if they might be stale. r=bzbarsky
layout/base/RestyleManager.h
layout/base/RestyleTracker.cpp
layout/base/RestyleTracker.h
layout/style/CSSStyleSheet.cpp
layout/style/nsStyleSet.cpp
layout/style/nsStyleSet.h
--- a/layout/base/RestyleManager.h
+++ b/layout/base/RestyleManager.h
@@ -122,16 +122,20 @@ public:
    * aFrame must be changed to the new parent before this function is called;
    * the new parent style context will be automatically computed based on the
    * new position in the frame tree.
    *
    * @param aFrame the root of the subtree to reparent.  Must not be null.
    */
   nsresult ReparentStyleContext(nsIFrame* aFrame);
 
+  void ClearSelectors() {
+    mPendingRestyles.ClearSelectors();
+  }
+
 private:
   // Used when restyling an element with a frame.
   void ComputeAndProcessStyleChange(nsIFrame*              aFrame,
                                     nsChangeHint           aMinChange,
                                     RestyleTracker&        aRestyleTracker,
                                     nsRestyleHint          aRestyleHint,
                                     const RestyleHintData& aRestyleHintData);
   // Used when restyling a display:contents element.
--- a/layout/base/RestyleTracker.cpp
+++ b/layout/base/RestyleTracker.cpp
@@ -442,16 +442,19 @@ RestyleTracker::DoProcessRestyles()
                                                 currentRestyle->mRestyleHint);
             TimelineConsumers::AddMarkerForDocShell(docShell, Move(marker));
           }
         }
       }
     }
   }
 
+  // mPendingRestyles is now empty.
+  mHaveSelectors = false;
+
   mRestyleManager->EndProcessingRestyles();
 }
 
 bool
 RestyleTracker::GetRestyleData(Element* aElement, nsAutoPtr<RestyleData>& aData)
 {
   NS_PRECONDITION(aElement->GetCrossShadowCurrentDoc() == Document(),
                   "Unexpected document; this will lead to incorrect behavior!");
@@ -507,9 +510,28 @@ RestyleTracker::AddRestyleRootsIfAwaitin
   for (size_t i = 0; i < aElements.Length(); i++) {
     Element* element = aElements[i];
     if (element->HasFlag(RestyleBit())) {
       mRestyleRoots.AppendElement(element);
     }
   }
 }
 
+void
+RestyleTracker::ClearSelectors()
+{
+  if (!mHaveSelectors) {
+    return;
+  }
+  for (auto it = mPendingRestyles.Iter(); !it.Done(); it.Next()) {
+    RestyleData* data = it.Data();
+    if (data->mRestyleHint & eRestyle_SomeDescendants) {
+      data->mRestyleHint =
+        (data->mRestyleHint & ~eRestyle_SomeDescendants) | eRestyle_Subtree;
+      data->mRestyleHintData.mSelectorsForDescendants.Clear();
+    } else {
+      MOZ_ASSERT(data->mRestyleHintData.mSelectorsForDescendants.IsEmpty());
+    }
+  }
+  mHaveSelectors = false;
+}
+
 } // namespace mozilla
--- a/layout/base/RestyleTracker.h
+++ b/layout/base/RestyleTracker.h
@@ -226,16 +226,17 @@ class RestyleTracker {
 public:
   typedef mozilla::dom::Element Element;
 
   friend class ElementRestyler; // for AddPendingRestyleToTable
 
   explicit RestyleTracker(Element::FlagsType aRestyleBits)
     : mRestyleBits(aRestyleBits)
     , mHaveLaterSiblingRestyles(false)
+    , mHaveSelectors(false)
   {
     NS_PRECONDITION((mRestyleBits & ~ELEMENT_ALL_RESTYLE_FLAGS) == 0,
                     "Why do we have these bits set?");
     NS_PRECONDITION((mRestyleBits & ELEMENT_PENDING_RESTYLE_FLAGS) != 0,
                     "Must have a restyle flag");
     NS_PRECONDITION((mRestyleBits & ELEMENT_PENDING_RESTYLE_FLAGS) !=
                       ELEMENT_PENDING_RESTYLE_FLAGS,
                     "Shouldn't have both restyle flags set");
@@ -348,16 +349,24 @@ public:
    * This function must be called with elements in order such that
    * appending them to mRestyleRoots maintains its ordering invariant that
    * ancestors appear after descendants.
    */
   void AddRestyleRootsIfAwaitingRestyle(
                                   const nsTArray<nsRefPtr<Element>>& aElements);
 
   /**
+   * Converts any eRestyle_SomeDescendants restyle hints in the pending restyle
+   * table into eRestyle_Subtree hints and clears out the associated arrays of
+   * nsCSSSelector pointers.  This is called in response to a style sheet change
+   * that might have cause an nsCSSSelector to be destroyed.
+   */
+  void ClearSelectors();
+
+  /**
    * The document we're associated with.
    */
   inline nsIDocument* Document() const;
 
 #ifdef RESTYLE_LOGGING
   // Defined in RestyleTrackerInlines.h.
   inline bool ShouldLogRestyle();
   inline int32_t& LoggingDepth();
@@ -397,26 +406,34 @@ private:
   // We maintain this invariant by checking whether an element has an
   // ancestor with the restyle root bit set before appending it to the
   // array.
   RestyleRootArray mRestyleRoots;
   // True if we have some entries with the eRestyle_LaterSiblings
   // flag.  We need this to avoid enumerating the hashtable looking
   // for such entries when we can't possibly have any.
   bool mHaveLaterSiblingRestyles;
+  // True if we have some entries with selectors in the restyle hint data.
+  // We use this to skip iterating over mPendingRestyles in ClearSelectors.
+  bool mHaveSelectors;
 };
 
 inline bool
 RestyleTracker::AddPendingRestyleToTable(Element* aElement,
                                          nsRestyleHint aRestyleHint,
                                          nsChangeHint aMinChangeHint,
                                          const RestyleHintData* aRestyleHintData)
 {
   RestyleData* existingData;
 
+  if (aRestyleHintData &&
+      !aRestyleHintData->mSelectorsForDescendants.IsEmpty()) {
+    mHaveSelectors = true;
+  }
+
   // Check the RestyleBit() flag before doing the hashtable Get, since
   // it's possible that the data in the hashtable isn't actually
   // relevant anymore (if the flag is not set).
   if (aElement->HasFlag(RestyleBit())) {
     mPendingRestyles.Get(aElement, &existingData);
   } else {
     aElement->SetFlags(RestyleBit());
     existingData = nullptr;
--- a/layout/style/CSSStyleSheet.cpp
+++ b/layout/style/CSSStyleSheet.cpp
@@ -1704,16 +1704,25 @@ CSSStyleSheet::List(FILE* out, int32_t a
   fprintf_stderr(out, "%s", "Rules in source order:\n");
   ListRules(mInner->mOrderedRules, out, aIndent);
 }
 #endif
 
 void 
 CSSStyleSheet::ClearRuleCascades()
 {
+  // We might be in ClearRuleCascades because we had a modification
+  // to the sheet that resulted in an nsCSSSelector being destroyed.
+  // Tell the RestyleManager for each document we're used in
+  // so that they can drop any nsCSSSelector pointers (used for
+  // eRestyle_SomeDescendants) in their mPendingRestyles.
+  for (nsStyleSet* styleSet : mStyleSets) {
+    styleSet->ClearSelectors();
+  }
+
   bool removedSheetFromRuleProcessorCache = false;
   if (mRuleProcessors) {
     nsCSSRuleProcessor **iter = mRuleProcessors->Elements(),
                        **end = iter + mRuleProcessors->Length();
     for(; iter != end; ++iter) {
       if (!removedSheetFromRuleProcessorCache && (*iter)->IsShared()) {
         // Since the sheet has been modified, we need to remove all
         // RuleProcessorCache entries that contain this sheet, as the
--- a/layout/style/nsStyleSet.cpp
+++ b/layout/style/nsStyleSet.cpp
@@ -380,16 +380,24 @@ SortStyleSheetsByScope(nsTArray<CSSStyle
   for (uint32_t i = 0; i < n; i++) {
     aSheets[i] = sheets[i].mSheet;
   }
 }
 
 nsresult
 nsStyleSet::GatherRuleProcessors(sheetType aType)
 {
+  // We might be in GatherRuleProcessors because we are dropping a sheet,
+  // resulting in an nsCSSSelector being destroyed.  Tell the
+  // RestyleManager for each document we're used in so that they can
+  // drop any nsCSSSelector pointers (used for eRestyle_SomeDescendants)
+  // in their mPendingRestyles.
+  if (IsCSSSheetType(aType)) {
+    ClearSelectors();
+  }
   nsCOMPtr<nsIStyleRuleProcessor> oldRuleProcessor(mRuleProcessors[aType]);
   nsTArray<nsCOMPtr<nsIStyleRuleProcessor>> oldScopedDocRuleProcessors;
   if (aType == eAgentSheet || aType == eUserSheet) {
     // drop reference to cached rule processor
     nsCSSRuleProcessor* rp =
       static_cast<nsCSSRuleProcessor*>(mRuleProcessors[aType].get());
     if (rp) {
       MOZ_ASSERT(rp->IsShared());
@@ -2493,8 +2501,18 @@ nsStyleSet::HasRuleProcessorUsedByMultip
   MOZ_ASSERT(size_t(aSheetType) < ArrayLength(mRuleProcessors));
   if (!IsCSSSheetType(aSheetType) || !mRuleProcessors[aSheetType]) {
     return false;
   }
   nsCSSRuleProcessor* rp =
     static_cast<nsCSSRuleProcessor*>(mRuleProcessors[aSheetType].get());
   return rp->IsUsedByMultipleStyleSets();
 }
+
+void
+nsStyleSet::ClearSelectors()
+{
+  // We might be called before we've done our first rule tree construction.
+  if (!mRuleTree) {
+    return;
+  }
+  PresContext()->RestyleManager()->ClearSelectors();
+}
--- a/layout/style/nsStyleSet.h
+++ b/layout/style/nsStyleSet.h
@@ -394,16 +394,20 @@ class nsStyleSet final
   void SetNeedsRestyleAfterEnsureUniqueInner() {
     mNeedsRestyleAfterEnsureUniqueInner = true;
   }
 
   nsIStyleRule* InitialStyleRule();
 
   bool HasRuleProcessorUsedByMultipleStyleSets(sheetType aSheetType);
 
+  // Tells the RestyleManager for the document using this style set
+  // to drop any nsCSSSelector pointers it has.
+  void ClearSelectors();
+
  private:
   nsStyleSet(const nsStyleSet& aCopy) = delete;
   nsStyleSet& operator=(const nsStyleSet& aCopy) = delete;
 
   // Run mark-and-sweep GC on mRuleTree and mOldRuleTrees, based on mRoots.
   void GCRuleTrees();
 
   // Update the rule processor list after a change to the style sheet list.