Bug 1180118 - Part 9: Clear nsCSSSelector pointers in the pending restyle tracker if they might be stale. r=bzbarsky
☠☠ backed out by 01576b408ea7 ☠ ☠
authorCameron McCormack <cam@mcc.id.au>
Tue, 04 Aug 2015 17:27:53 +1000
changeset 276144 9b41cd9f2bc5822f27f287f515d8fd16a4d3af6e
parent 276143 37493f6eef20c1dbf6d6c042a609474e93e1d996
child 276145 15ad6049b940ffe57b13941bfed374155528eb32
push id8304
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 19:25:01 +0000
treeherdermozilla-aurora@7308dd0a6c3b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1180118
milestone42.0a1
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,14 @@ 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()
+{
+  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.