Bug 1290218 Part 4b: Fixup cycle collector declarations for mInner moving into StyleSheetInfo. r=heycam
authorBrad Werth <bwerth@mozilla.com>
Wed, 15 Feb 2017 16:19:33 -0800
changeset 373586 f42cbc7439d02019fb663a49010e62c3af620598
parent 373585 b433bbbd6228fa4a0bdfb3fc4b0fcdc562fb419b
child 373587 bc83f0653f30c1be0d68b7a87c52af39241b33c6
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1290218
milestone54.0a1
Bug 1290218 Part 4b: Fixup cycle collector declarations for mInner moving into StyleSheetInfo. r=heycam MozReview-Commit-ID: 9cAvEf7y8JN
layout/style/CSSStyleSheet.cpp
layout/style/CSSStyleSheet.h
layout/style/StyleSheet.cpp
layout/style/StyleSheet.h
--- a/layout/style/CSSStyleSheet.cpp
+++ b/layout/style/CSSStyleSheet.cpp
@@ -435,62 +435,37 @@ CSSStyleSheet::UnlinkInner()
   // because otherwise there are no JS wrappers for anything in the inner.
   if (mInner->mSheets.Length() != 1) {
     return;
   }
 
   Inner()->mOrderedRules.EnumerateForwards(SetStyleSheetReference, nullptr);
   Inner()->mOrderedRules.Clear();
 
-  // Have to be a bit careful with child sheets, because we want to
-  // drop their mNext pointers and null out their mParent and
-  // mDocument, but don't want to work with deleted objects.  And we
-  // don't want to do any addrefing in the process, just to make sure
-  // we don't confuse the cycle collector (though on the face of it,
-  // addref/release pairs during unlink should probably be ok).
-  RefPtr<StyleSheet> child;
-  child.swap(SheetInfo().mFirstChild);
-  while (child) {
-    MOZ_ASSERT(child->mParent == this, "We have a unique inner!");
-    child->mParent = nullptr;
-    child->mDocument = nullptr;
-
-    RefPtr<StyleSheet> next;
-    // Null out child->mNext, but don't let it die yet
-    next.swap(child->mNext);
-    // Switch to looking at the old value of child->mNext next iteration
-    child.swap(next);
-    // "next" is now our previous value of child; it'll get released
-    // as we loop around.
-  }
+  StyleSheet::UnlinkInner();
 }
 
 void
 CSSStyleSheet::TraverseInner(nsCycleCollectionTraversalCallback &cb)
 {
   // We can only have a cycle through our inner if we have a unique inner,
   // because otherwise there are no JS wrappers for anything in the inner.
   if (mInner->mSheets.Length() != 1) {
     return;
   }
 
-  StyleSheet* childSheet = GetFirstChild();
-  while (childSheet) {
-    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "child sheet");
-    cb.NoteXPCOMChild(NS_ISUPPORTS_CAST(nsIDOMCSSStyleSheet*, childSheet));
-    childSheet = childSheet->mNext;
-  }
-
   const nsCOMArray<css::Rule>& rules = Inner()->mOrderedRules;
   for (int32_t i = 0, count = rules.Count(); i < count; ++i) {
     if (!rules[i]->IsCCLeaf()) {
       NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mOrderedRules[i]");
       cb.NoteXPCOMChild(rules[i]);
     }
   }
+
+  StyleSheet::TraverseInner(cb);
 }
 
 // QueryInterface implementation for CSSStyleSheet
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(CSSStyleSheet)
   NS_INTERFACE_MAP_ENTRY(nsICSSLoaderObserver)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, StyleSheet)
   if (aIID.Equals(NS_GET_IID(CSSStyleSheet)))
     foundInterface = reinterpret_cast<nsISupports*>(this);
@@ -503,25 +478,23 @@ NS_IMPL_RELEASE_INHERITED(CSSStyleSheet,
 NS_IMPL_CYCLE_COLLECTION_CLASS(CSSStyleSheet)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(CSSStyleSheet)
   // We do not unlink mNext; our parent will handle that.  If we
   // unlinked it here, our parent would not be able to walk its list
   // of child sheets and null out the back-references to it, if we got
   // unlinked before it does.
   tmp->DropRuleCollection();
-  tmp->UnlinkInner();
   tmp->mScopeElement = nullptr;
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END_INHERITED(StyleSheet)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(CSSStyleSheet, StyleSheet)
   // We do not traverse mNext; our parent will handle that.  See
   // comments in Unlink for why.
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRuleCollection)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScopeElement)
-  tmp->TraverseInner(cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 nsresult
 CSSStyleSheet::AddRuleProcessor(nsCSSRuleProcessor* aProcessor)
 {
   if (! mRuleProcessors) {
     mRuleProcessors = new AutoTArray<nsCSSRuleProcessor*, 8>();
     if (!mRuleProcessors)
--- a/layout/style/CSSStyleSheet.h
+++ b/layout/style/CSSStyleSheet.h
@@ -207,19 +207,19 @@ protected:
   void DropRuleCollection();
 
   CSSStyleSheetInner* Inner() const
   {
     return static_cast<CSSStyleSheetInner*>(mInner);
   }
 
   // Unlink our inner, if needed, for cycle collection
-  void UnlinkInner();
+  virtual void UnlinkInner() override;
   // Traverse our inner, if needed, for cycle collection
-  void TraverseInner(nsCycleCollectionTraversalCallback &);
+  virtual void TraverseInner(nsCycleCollectionTraversalCallback &) override;
 
 protected:
   // Internal methods which do not have security check and completeness check.
   dom::CSSRuleList* GetCssRulesInternal(ErrorResult& aRv);
   uint32_t InsertRuleInternal(const nsAString& aRule,
                               uint32_t aIndex, ErrorResult& aRv);
   void DeleteRuleInternal(uint32_t aIndex, ErrorResult& aRv);
 
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -60,34 +60,85 @@ StyleSheet::~StyleSheet()
   MOZ_ASSERT(mInner, "Should have an mInner at time of destruction.");
   MOZ_ASSERT(mInner->mSheets.Contains(this), "Our mInner should include us.");
   mInner->RemoveSheet(this);
   mInner = nullptr;
 
   DropMedia();
 }
 
+void
+StyleSheet::UnlinkInner()
+{
+  // We can only have a cycle through our inner if we have a unique inner,
+  // because otherwise there are no JS wrappers for anything in the inner.
+  if (mInner->mSheets.Length() != 1) {
+    return;
+  }
+
+  // Have to be a bit careful with child sheets, because we want to
+  // drop their mNext pointers and null out their mParent and
+  // mDocument, but don't want to work with deleted objects.  And we
+  // don't want to do any addrefing in the process, just to make sure
+  // we don't confuse the cycle collector (though on the face of it,
+  // addref/release pairs during unlink should probably be ok).
+  RefPtr<StyleSheet> child;
+  child.swap(SheetInfo().mFirstChild);
+  while (child) {
+    MOZ_ASSERT(child->mParent == this, "We have a unique inner!");
+    child->mParent = nullptr;
+    child->mDocument = nullptr;
+
+    RefPtr<StyleSheet> next;
+    // Null out child->mNext, but don't let it die yet
+    next.swap(child->mNext);
+    // Switch to looking at the old value of child->mNext next iteration
+    child.swap(next);
+    // "next" is now our previous value of child; it'll get released
+    // as we loop around.
+  }
+}
+
+void
+StyleSheet::TraverseInner(nsCycleCollectionTraversalCallback &cb)
+{
+  // We can only have a cycle through our inner if we have a unique inner,
+  // because otherwise there are no JS wrappers for anything in the inner.
+  if (mInner->mSheets.Length() != 1) {
+    return;
+  }
+
+  StyleSheet* childSheet = GetFirstChild();
+  while (childSheet) {
+    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "child sheet");
+    cb.NoteXPCOMChild(NS_ISUPPORTS_CAST(nsIDOMCSSStyleSheet*, childSheet));
+    childSheet = childSheet->mNext;
+  }
+}
+
 // QueryInterface implementation for StyleSheet
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(StyleSheet)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY(nsIDOMStyleSheet)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSStyleSheet)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(StyleSheet)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(StyleSheet)
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(StyleSheet)
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(StyleSheet)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMedia)
+  tmp->TraverseInner(cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(StyleSheet)
   tmp->DropMedia();
+  tmp->UnlinkInner();
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(StyleSheet)
 
 mozilla::dom::CSSStyleSheetParsingMode
 StyleSheet::ParsingModeDOM()
 {
--- a/layout/style/StyleSheet.h
+++ b/layout/style/StyleSheet.h
@@ -230,16 +230,21 @@ protected:
                                      ErrorResult& aRv);
 
   // Drop our reference to mMedia
   void DropMedia();
 
   // Called from SetEnabled when the enabled state changed.
   void EnabledStateChanged();
 
+  // Unlink our inner, if needed, for cycle collection
+  virtual void UnlinkInner();
+  // Traverse our inner, if needed, for cycle collection
+  virtual void TraverseInner(nsCycleCollectionTraversalCallback &);
+
   StyleSheet*           mParent;    // weak ref
 
   nsString              mTitle;
   nsIDocument*          mDocument; // weak ref; parents maintain this for their children
   nsINode*              mOwningNode; // weak ref
 
   RefPtr<nsMediaList> mMedia;