Bug 1412145 - Drop more backpointers of CSSOM objects in dtor and unlink. r=bz, a=gchang
authorXidorn Quan <me@upsuper.org>
Tue, 28 Nov 2017 17:06:51 -0600
changeset 445083 07afcea951ff9ff7d77452a04c6a082d2a5f1d9b
parent 445082 faef474a7ee4c5165f5d34ad5f16e535afbfbdfb
child 445084 fb7969babb002d449eb86bf91c371a9117fa0980
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, gchang
bugs1412145
milestone58.0
Bug 1412145 - Drop more backpointers of CSSOM objects in dtor and unlink. r=bz, a=gchang MozReview-Commit-ID: Ftg3WMBBNlO
layout/style/GroupRule.cpp
layout/style/GroupRule.h
layout/style/MediaList.h
layout/style/ServoCSSRuleList.cpp
layout/style/ServoKeyframeRule.cpp
layout/style/ServoKeyframesRule.cpp
layout/style/ServoMediaRule.cpp
layout/style/ServoPageRule.cpp
layout/style/ServoStyleRule.cpp
layout/style/nsCSSRules.cpp
--- a/layout/style/GroupRule.cpp
+++ b/layout/style/GroupRule.cpp
@@ -193,16 +193,23 @@ GeckoGroupRuleRules::SizeOfExcludingThis
   // - mRuleCollection
   return n;
 }
 
 // -------------------------------
 // ServoGroupRuleRules
 //
 
+ServoGroupRuleRules::~ServoGroupRuleRules()
+{
+  if (mRuleList) {
+    mRuleList->DropReference();
+  }
+}
+
 #ifdef DEBUG
 void
 ServoGroupRuleRules::List(FILE* out, int32_t aIndent) const
 {
   // TODO list something reasonable?
 }
 #endif
 
--- a/layout/style/GroupRule.h
+++ b/layout/style/GroupRule.h
@@ -82,16 +82,17 @@ struct ServoGroupRuleRules
   explicit ServoGroupRuleRules(already_AddRefed<ServoCssRules> aRawRules)
     : mRuleList(new ServoCSSRuleList(Move(aRawRules), nullptr)) {}
   ServoGroupRuleRules(ServoGroupRuleRules&& aOther)
     : mRuleList(Move(aOther.mRuleList)) {}
   ServoGroupRuleRules(const ServoGroupRuleRules& aCopy) {
     // Do we ever clone Servo rules?
     MOZ_ASSERT_UNREACHABLE("stylo: Cloning GroupRule not implemented");
   }
+  ~ServoGroupRuleRules();
 
   void SetParentRule(GroupRule* aParentRule) {
     if (mRuleList) {
       mRuleList->SetParentRule(aParentRule);
     }
   }
   void SetStyleSheet(StyleSheet* aSheet) {
     if (mRuleList) {
--- a/layout/style/MediaList.h
+++ b/layout/style/MediaList.h
@@ -79,17 +79,19 @@ public:
   {
     aRv = AppendMedium(aMedium);
   }
 
 protected:
   virtual nsresult Delete(const nsAString& aOldMedium) = 0;
   virtual nsresult Append(const nsAString& aNewMedium) = 0;
 
-  virtual ~MediaList() {}
+  virtual ~MediaList() {
+    MOZ_ASSERT(!mStyleSheet, "Backpointer should have been cleared");
+  }
 
   // not refcounted; sheet will let us know when it goes away
   // mStyleSheet is the sheet that needs to be dirtied when this
   // medialist changes
   StyleSheet* mStyleSheet = nullptr;
 
 private:
   template<typename Func>
--- a/layout/style/ServoCSSRuleList.cpp
+++ b/layout/style/ServoCSSRuleList.cpp
@@ -229,12 +229,14 @@ ServoCSSRuleList::GetDOMCSSRuleType(uint
   if (rule <= kMaxRuleType) {
     return rule;
   }
   return CastToPtr(rule)->Type();
 }
 
 ServoCSSRuleList::~ServoCSSRuleList()
 {
+  MOZ_ASSERT(!mStyleSheet, "Backpointer should have been cleared");
+  MOZ_ASSERT(!mParentRule, "Backpointer should have been cleared");
   DropAllRules();
 }
 
 } // namespace mozilla
--- a/layout/style/ServoKeyframeRule.cpp
+++ b/layout/style/ServoKeyframeRule.cpp
@@ -30,17 +30,20 @@ public:
     ServoKeyframeDeclaration, nsICSSDeclaration)
 
   NS_IMETHOD GetParentRule(nsIDOMCSSRule** aParent) final
   {
     NS_IF_ADDREF(*aParent = mRule);
     return NS_OK;
   }
 
-  void DropReference() { mRule = nullptr; }
+  void DropReference() {
+    mRule = nullptr;
+    mDecls->SetOwningRule(nullptr);
+  }
 
   DeclarationBlock* GetCSSDeclaration(Operation aOperation) final
   {
     return mDecls;
   }
   nsresult SetCSSDeclaration(DeclarationBlock* aDecls) final
   {
     if (!mRule) {
@@ -76,17 +79,19 @@ public:
   size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
   {
     size_t n = aMallocSizeOf(this);
     // TODO we may want to add size of mDecls as well
     return n;
   }
 
 private:
-  virtual ~ServoKeyframeDeclaration() {}
+  virtual ~ServoKeyframeDeclaration() {
+    MOZ_ASSERT(!mRule, "Backpointer should have been cleared");
+  }
 
   ServoKeyframeRule* mRule;
   RefPtr<ServoDeclarationBlock> mDecls;
 };
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(ServoKeyframeDeclaration)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(ServoKeyframeDeclaration)
 
@@ -97,16 +102,19 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
 NS_INTERFACE_MAP_END_INHERITING(nsDOMCSSDeclaration)
 
 // -------------------------------------------
 // ServoKeyframeRule
 //
 
 ServoKeyframeRule::~ServoKeyframeRule()
 {
+  if (mDeclaration) {
+    mDeclaration->DropReference();
+  }
 }
 
 NS_IMPL_ADDREF_INHERITED(ServoKeyframeRule, dom::CSSKeyframeRule)
 NS_IMPL_RELEASE_INHERITED(ServoKeyframeRule, dom::CSSKeyframeRule)
 
 // QueryInterface implementation for nsCSSKeyframeRule
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ServoKeyframeRule)
 NS_INTERFACE_MAP_END_INHERITING(dom::CSSKeyframeRule)
--- a/layout/style/ServoKeyframesRule.cpp
+++ b/layout/style/ServoKeyframesRule.cpp
@@ -85,38 +85,44 @@ public:
   }
 
   uint32_t Length() final { return mRules.Length(); }
 
   void DropReference()
   {
     mStyleSheet = nullptr;
     mParentRule = nullptr;
-    DropAllRules();
+    for (css::Rule* rule : mRules) {
+      if (rule) {
+        rule->SetStyleSheet(nullptr);
+        rule->SetParentRule(nullptr);
+      }
+    }
   }
 
   size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
   {
     size_t n = aMallocSizeOf(this);
     for (const css::Rule* rule : mRules) {
       n += rule ? rule->SizeOfIncludingThis(aMallocSizeOf) : 0;
     }
     return n;
   }
 
 private:
-  virtual ~ServoKeyframeList() {}
+  virtual ~ServoKeyframeList() {
+    MOZ_ASSERT(!mParentRule, "Backpointer should have been cleared");
+    MOZ_ASSERT(!mStyleSheet, "Backpointer should have been cleared");
+    DropAllRules();
+  }
 
   void DropAllRules()
   {
-    for (css::Rule* rule : mRules) {
-      if (rule) {
-        rule->SetStyleSheet(nullptr);
-        rule->SetParentRule(nullptr);
-      }
+    if (mParentRule || mStyleSheet) {
+      DropReference();
     }
     mRules.Clear();
     mRawRule = nullptr;
   }
 
   // may be nullptr when the style sheet drops the reference to us.
   ServoStyleSheet* mStyleSheet = nullptr;
   ServoKeyframesRule* mParentRule = nullptr;
@@ -158,16 +164,19 @@ ServoKeyframesRule::ServoKeyframesRule(R
   // rid of nsCSSKeyframeRule.
   : dom::CSSKeyframesRule(aLine, aColumn)
   , mRawRule(Move(aRawRule))
 {
 }
 
 ServoKeyframesRule::~ServoKeyframesRule()
 {
+  if (mKeyframeList) {
+    mKeyframeList->DropReference();
+  }
 }
 
 NS_IMPL_ADDREF_INHERITED(ServoKeyframesRule, dom::CSSKeyframesRule)
 NS_IMPL_RELEASE_INHERITED(ServoKeyframesRule, dom::CSSKeyframesRule)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ServoKeyframesRule)
 NS_INTERFACE_MAP_END_INHERITING(dom::CSSKeyframesRule)
 
--- a/layout/style/ServoMediaRule.cpp
+++ b/layout/style/ServoMediaRule.cpp
@@ -19,27 +19,40 @@ ServoMediaRule::ServoMediaRule(RefPtr<Ra
                                uint32_t aLine, uint32_t aColumn)
   : CSSMediaRule(Servo_MediaRule_GetRules(aRawRule).Consume(), aLine, aColumn)
   , mRawRule(Move(aRawRule))
 {
 }
 
 ServoMediaRule::~ServoMediaRule()
 {
+  if (mMediaList) {
+    mMediaList->SetStyleSheet(nullptr);
+  }
 }
 
 NS_IMPL_ADDREF_INHERITED(ServoMediaRule, CSSMediaRule)
 NS_IMPL_RELEASE_INHERITED(ServoMediaRule, CSSMediaRule)
 
 // QueryInterface implementation for MediaRule
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ServoMediaRule)
 NS_INTERFACE_MAP_END_INHERITING(CSSMediaRule)
 
-NS_IMPL_CYCLE_COLLECTION_INHERITED(ServoMediaRule, CSSMediaRule,
-                                   mMediaList)
+NS_IMPL_CYCLE_COLLECTION_CLASS(ServoMediaRule)
+
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(ServoMediaRule, CSSMediaRule)
+  if (tmp->mMediaList) {
+    tmp->mMediaList->SetStyleSheet(nullptr);
+    tmp->mMediaList = nullptr;
+  }
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
+
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(ServoMediaRule, CSSMediaRule)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMediaList)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 /* virtual */ already_AddRefed<css::Rule>
 ServoMediaRule::Clone() const
 {
   // Rule::Clone is only used when CSSStyleSheetInner is cloned in
   // preparation of being mutated. However, ServoStyleSheet never clones
   // anything, so this method should never be called.
   MOZ_ASSERT_UNREACHABLE("Shouldn't be cloning ServoMediaRule");
--- a/layout/style/ServoPageRule.cpp
+++ b/layout/style/ServoPageRule.cpp
@@ -21,16 +21,17 @@ namespace mozilla {
 ServoPageRuleDeclaration::ServoPageRuleDeclaration(
   already_AddRefed<RawServoDeclarationBlock> aDecls)
   : mDecls(new ServoDeclarationBlock(Move(aDecls)))
 {
 }
 
 ServoPageRuleDeclaration::~ServoPageRuleDeclaration()
 {
+  mDecls->SetOwningRule(nullptr);
 }
 
 // QueryInterface implementation for ServoPageRuleDeclaration
 NS_INTERFACE_MAP_BEGIN(ServoPageRuleDeclaration)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   // We forward the cycle collection interfaces to Rule(), which is
   // never null (in fact, we're part of that object!)
   if (aIID.Equals(NS_GET_IID(nsCycleCollectionISupports)) ||
@@ -136,16 +137,17 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(ServoPageRule, CSSPageRule)
   // Keep this in sync with IsCCLeaf.
 
   // Unlink the wrapper for our declaraton.  This just expands out
   // NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER which we can't use
   // directly because the wrapper is on the declaration, not on us.
   tmp->mDecls.ReleaseWrapper(static_cast<nsISupports*>(p));
+  tmp->mDecls.mDecls->SetOwningRule(nullptr);
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(ServoPageRule, CSSPageRule)
   // Keep this in sync with IsCCLeaf.
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 bool
 ServoPageRule::IsCCLeaf() const
--- a/layout/style/ServoStyleRule.cpp
+++ b/layout/style/ServoStyleRule.cpp
@@ -23,16 +23,17 @@ namespace mozilla {
 ServoStyleRuleDeclaration::ServoStyleRuleDeclaration(
   already_AddRefed<RawServoDeclarationBlock> aDecls)
   : mDecls(new ServoDeclarationBlock(Move(aDecls)))
 {
 }
 
 ServoStyleRuleDeclaration::~ServoStyleRuleDeclaration()
 {
+  mDecls->SetOwningRule(nullptr);
 }
 
 // QueryInterface implementation for ServoStyleRuleDeclaration
 NS_INTERFACE_MAP_BEGIN(ServoStyleRuleDeclaration)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   // We forward the cycle collection interfaces to Rule(), which is
   // never null (in fact, we're part of that object!)
   if (aIID.Equals(NS_GET_IID(nsCycleCollectionISupports)) ||
--- a/layout/style/nsCSSRules.cpp
+++ b/layout/style/nsCSSRules.cpp
@@ -207,22 +207,35 @@ ImportRule::~ImportRule()
   if (mChildSheet) {
     mChildSheet->SetOwnerRule(nullptr);
   }
 }
 
 NS_IMPL_ADDREF_INHERITED(ImportRule, CSSImportRule)
 NS_IMPL_RELEASE_INHERITED(ImportRule, CSSImportRule)
 
-NS_IMPL_CYCLE_COLLECTION_INHERITED(ImportRule, CSSImportRule, mMedia, mChildSheet)
-
 // QueryInterface implementation for ImportRule
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ImportRule)
 NS_INTERFACE_MAP_END_INHERITING(CSSImportRule)
 
+NS_IMPL_CYCLE_COLLECTION_CLASS(ImportRule)
+
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(ImportRule, CSSImportRule)
+  if (tmp->mChildSheet) {
+    tmp->mChildSheet->SetOwnerRule(nullptr);
+    tmp->mChildSheet = nullptr;
+  }
+  tmp->mMedia = nullptr;
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
+
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(ImportRule, CSSImportRule)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMedia)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mChildSheet)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
+
 #ifdef DEBUG
 /* virtual */ void
 ImportRule::List(FILE* out, int32_t aIndent) const
 {
   nsAutoCString str;
   // Indent
   for (int32_t indent = aIndent; --indent >= 0; ) {
     str.AppendLiteral("  ");
@@ -337,18 +350,28 @@ MediaRule::~MediaRule()
 
 NS_IMPL_ADDREF_INHERITED(MediaRule, CSSMediaRule)
 NS_IMPL_RELEASE_INHERITED(MediaRule, CSSMediaRule)
 
 // QueryInterface implementation for MediaRule
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(MediaRule)
 NS_INTERFACE_MAP_END_INHERITING(CSSMediaRule)
 
-NS_IMPL_CYCLE_COLLECTION_INHERITED(MediaRule, CSSMediaRule,
-                                   mMedia)
+NS_IMPL_CYCLE_COLLECTION_CLASS(MediaRule)
+
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(MediaRule, CSSMediaRule)
+  if (tmp->mMedia) {
+    tmp->mMedia->SetStyleSheet(nullptr);
+    tmp->mMedia = nullptr;
+  }
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
+
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(MediaRule, CSSMediaRule)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMedia)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 /* virtual */ void
 MediaRule::SetStyleSheet(StyleSheet* aSheet)
 {
   if (mMedia) {
     // Set to null so it knows it's leaving one sheet and joining another.
     mMedia->SetStyleSheet(nullptr);
     if (aSheet) {