Bug 795221 part 3. Implement cycle collection for GroupRule objects. r=smaug,dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 10 Oct 2012 22:16:50 -0400
changeset 110036 b604d43c28cd5b217d35d1ebb0ab8218730c0bd1
parent 110035 83000067bfe9fa4c2a4f11ebb2a1a603f59faf20
child 110037 a985c5ececcb480f554433b63ca7cdb760e80015
push idunknown
push userunknown
push dateunknown
reviewerssmaug, dbaron
bugs795221
milestone17.0
Bug 795221 part 3. Implement cycle collection for GroupRule objects. r=smaug,dbaron
content/html/content/crashtests/795221-2.html
content/html/content/crashtests/crashtests.list
layout/style/GroupRule.h
layout/style/ImportRule.h
layout/style/NameSpaceRule.h
layout/style/Rule.h
layout/style/StyleRule.cpp
layout/style/StyleRule.h
layout/style/nsCSSRules.cpp
layout/style/nsCSSRules.h
copy from content/html/content/crashtests/795221-1.html
copy to content/html/content/crashtests/795221-2.html
--- a/content/html/content/crashtests/795221-1.html
+++ b/content/html/content/crashtests/795221-2.html
@@ -1,7 +1,9 @@
 <!DOCTYPE html>
 <style>
-  div { }
+  @media all {
+    div { }
+  }
 </style>
 <script>
-  document.styleSheets[0].cssRules[0].style.foo = document;
+  document.styleSheets[0].cssRules[0].cssRules[0].style.foo = document;
 </script>
--- a/content/html/content/crashtests/crashtests.list
+++ b/content/html/content/crashtests/crashtests.list
@@ -32,8 +32,9 @@ load 620078-1.html
 load 620078-2.html
 load 680922-1.xul
 load 682058.xhtml
 load 682460.html
 load 673853.html
 load 738744.xhtml
 load 741250.xhtml
 load 795221-1.html
+load 795221-2.html
--- a/layout/style/GroupRule.h
+++ b/layout/style/GroupRule.h
@@ -9,16 +9,17 @@
  */
 
 #ifndef mozilla_css_GroupRule_h__
 #define mozilla_css_GroupRule_h__
 
 #include "mozilla/css/Rule.h"
 #include "nsCOMArray.h"
 #include "nsAutoPtr.h"
+#include "nsCycleCollectionParticipant.h"
 
 class nsPresContext;
 class nsMediaQueryResultCacheKey;
 
 namespace mozilla {
 namespace css {
 
 class GroupRuleRuleList;
@@ -28,16 +29,19 @@ class GroupRuleRuleList;
 class GroupRule : public Rule
 {
 protected:
   GroupRule();
   GroupRule(const GroupRule& aCopy);
   virtual ~GroupRule();
 public:
 
+  NS_DECL_CYCLE_COLLECTION_CLASS(GroupRule)
+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
+
   // implement part of nsIStyleRule and Rule
   DECL_STYLE_RULE_INHERIT_NO_DOMRULE
   virtual void SetStyleSheet(nsCSSStyleSheet* aSheet);
 
   // to help implement nsIStyleRule
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const;
 #endif
--- a/layout/style/ImportRule.h
+++ b/layout/style/ImportRule.h
@@ -25,17 +25,17 @@ class ImportRule MOZ_FINAL : public Rule
 {
 public:
   ImportRule(nsMediaList* aMedia, const nsString& aURLSpec);
 private:
   // for |Clone|
   ImportRule(const ImportRule& aCopy);
   ~ImportRule();
 public:
-  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_ISUPPORTS
 
   DECL_STYLE_RULE_INHERIT
 
 #ifdef HAVE_CPP_AMBIGUITY_RESOLVING_USING
   using Rule::GetStyleSheet; // unhide since nsIDOMCSSImportRule has its own GetStyleSheet
 #endif
 
   // nsIStyleRule methods
--- a/layout/style/NameSpaceRule.h
+++ b/layout/style/NameSpaceRule.h
@@ -30,17 +30,17 @@ public:
   NameSpaceRule(nsIAtom* aPrefix, const nsString& aURLSpec);
 private:
   // for |Clone|
   NameSpaceRule(const NameSpaceRule& aCopy);
   ~NameSpaceRule();
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_CSS_NAMESPACE_RULE_IMPL_CID)
 
-  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_ISUPPORTS
 
   DECL_STYLE_RULE_INHERIT
 
   // nsIStyleRule methods
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const;
 #endif
 
--- a/layout/style/Rule.h
+++ b/layout/style/Rule.h
@@ -41,23 +41,16 @@ protected:
     : mSheet(aCopy.mSheet),
       mParentRule(aCopy.mParentRule)
   {
   }
 
   virtual ~Rule() {}
 
 public:
-  // for implementing nsISupports
-  NS_IMETHOD_(nsrefcnt) AddRef();
-  NS_IMETHOD_(nsrefcnt) Release();
-protected:
-  nsAutoRefCnt mRefCnt;
-  NS_DECL_OWNINGTHREAD
-public:
 
   // The constants in this list must maintain the following invariants:
   //   If a rule of type N must appear before a rule of type M in stylesheets
   //   then N < M
   // Note that nsCSSStyleSheet::RebuildChildList assumes that no other kinds of
   // rules can come between two rules of type IMPORT_RULE.
   enum {
     UNKNOWN_RULE = 0,
--- a/layout/style/StyleRule.cpp
+++ b/layout/style/StyleRule.cpp
@@ -1356,18 +1356,18 @@ NS_INTERFACE_MAP_BEGIN(StyleRule)
     NS_ADDREF_THIS();
     return NS_OK;
   }
   else
   NS_INTERFACE_MAP_ENTRY(nsIStyleRule)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStyleRule)
 NS_INTERFACE_MAP_END
 
-NS_IMPL_ADDREF_INHERITED(StyleRule, Rule)
-NS_IMPL_RELEASE_INHERITED(StyleRule, Rule)
+NS_IMPL_ADDREF(StyleRule)
+NS_IMPL_RELEASE(StyleRule)
 
 void
 StyleRule::RuleMatched()
 {
   if (!mWasMatched) {
     NS_ABORT_IF_FALSE(!mImportantRule, "should not have important rule yet");
 
     mWasMatched = true;
--- a/layout/style/StyleRule.h
+++ b/layout/style/StyleRule.h
@@ -300,17 +300,17 @@ private:
   // for |Clone|
   StyleRule(const StyleRule& aCopy);
   // for |DeclarationChanged|
   StyleRule(StyleRule& aCopy,
             Declaration *aDeclaration);
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_CSS_STYLE_RULE_IMPL_CID)
 
-  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_ISUPPORTS
 
   // null for style attribute
   nsCSSSelectorList* Selector() { return mSelector; }
 
   uint32_t GetLineNumber() const { return mLineNumber; }
   void SetLineNumber(uint32_t aLineNumber) { mLineNumber = aLineNumber; }
 
   Declaration* GetDeclaration() const { return mDeclaration; }
--- a/layout/style/nsCSSRules.cpp
+++ b/layout/style/nsCSSRules.cpp
@@ -52,19 +52,16 @@ namespace css = mozilla::css;
 IMPL_STYLE_RULE_INHERIT_GET_DOM_RULE_WEAK(class_, super_) \
 IMPL_STYLE_RULE_INHERIT_MAP_RULE_INFO_INTO(class_, super_)
 
 // base class for all rule types in a CSS style sheet
 
 namespace mozilla {
 namespace css {
 
-NS_IMPL_ADDREF(Rule)
-NS_IMPL_RELEASE(Rule)
-
 /* virtual */ void
 Rule::SetStyleSheet(nsCSSStyleSheet* aSheet)
 {
   // We don't reference count this up reference. The style sheet
   // will tell us when it's going away or when we're detached from
   // it.
   mSheet = aSheet;
 }
@@ -208,18 +205,18 @@ CharsetRule::CharsetRule(const nsAString
 }
 
 CharsetRule::CharsetRule(const CharsetRule& aCopy)
   : Rule(aCopy),
     mEncoding(aCopy.mEncoding)
 {
 }
 
-NS_IMPL_ADDREF_INHERITED(CharsetRule, Rule)
-NS_IMPL_RELEASE_INHERITED(CharsetRule, Rule)
+NS_IMPL_ADDREF(CharsetRule)
+NS_IMPL_RELEASE(CharsetRule)
 
 // QueryInterface implementation for CharsetRule
 NS_INTERFACE_MAP_BEGIN(CharsetRule)
   NS_INTERFACE_MAP_ENTRY(nsIStyleRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSCharsetRule)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStyleRule)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(CSSCharsetRule)
@@ -342,18 +339,18 @@ ImportRule::ImportRule(const ImportRule&
 
 ImportRule::~ImportRule()
 {
   if (mChildSheet) {
     mChildSheet->SetOwnerRule(nullptr);
   }
 }
 
-NS_IMPL_ADDREF_INHERITED(ImportRule, Rule)
-NS_IMPL_RELEASE_INHERITED(ImportRule, Rule)
+NS_IMPL_ADDREF(ImportRule)
+NS_IMPL_RELEASE(ImportRule)
 
 // QueryInterface implementation for ImportRule
 NS_INTERFACE_MAP_BEGIN(ImportRule)
   NS_INTERFACE_MAP_ENTRY(nsIStyleRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSImportRule)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStyleRule)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(CSSImportRule)
@@ -532,31 +529,71 @@ GroupRule::~GroupRule()
 {
   NS_ABORT_IF_FALSE(!mSheet, "SetStyleSheet should have been called");
   mRules.EnumerateForwards(SetParentRuleReference, nullptr);
   if (mRuleCollection) {
     mRuleCollection->DropReference();
   }
 }
 
+NS_IMPL_CYCLE_COLLECTION_CLASS(GroupRule)
+NS_IMPL_CYCLE_COLLECTING_ADDREF(GroupRule)
+NS_IMPL_CYCLE_COLLECTING_RELEASE(GroupRule)
+
+NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(GroupRule)
+NS_INTERFACE_MAP_END
+
 IMPL_STYLE_RULE_INHERIT_MAP_RULE_INFO_INTO(GroupRule, Rule)
 
 static bool
 SetStyleSheetReference(Rule* aRule, void* aSheet)
 {
   nsCSSStyleSheet* sheet = (nsCSSStyleSheet*)aSheet;
   aRule->SetStyleSheet(sheet);
   return true;
 }
 
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(GroupRule)
+  tmp->mRules.EnumerateForwards(SetParentRuleReference, nullptr);
+  // If tmp does not have a stylesheet, neither do its descendants.  In that
+  // case, don't try to null out their stylesheet, to avoid O(N^2) behavior in
+  // depth of group rule nesting.  But if tmp _does_ have a stylesheet (which
+  // can happen if it gets unlinked earlier than its owning stylesheet), then we
+  // need to null out the stylesheet pointer on descendants now, before we clear
+  // tmp->mRules.
+  if (tmp->GetStyleSheet()) {
+    tmp->mRules.EnumerateForwards(SetStyleSheetReference, nullptr);
+  }
+  tmp->mRules.Clear();
+  if (tmp->mRuleCollection) {
+    tmp->mRuleCollection->DropReference();
+    tmp->mRuleCollection = nullptr;
+  }
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
+
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(GroupRule)
+  const nsCOMArray<Rule>& rules = tmp->mRules;
+  for (int32_t i = 0, count = rules.Count(); i < count; ++i) {
+    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mRules[i]");
+    cb.NoteXPCOMChild(rules[i]->GetExistingDOMRule());
+  }
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mRuleCollection)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
+
 /* virtual */ void
 GroupRule::SetStyleSheet(nsCSSStyleSheet* aSheet)
 {
-  mRules.EnumerateForwards(SetStyleSheetReference, aSheet);
-  Rule::SetStyleSheet(aSheet);
+  // Don't set the sheet on the kids if it's already the same as the sheet we
+  // already have.  This is needed to avoid O(N^2) behavior in group nesting
+  // depth when seting the sheet to null during unlink, if we happen to unlin in
+  // order from most nested rule up to least nested rule.
+  if (aSheet != GetStyleSheet()) {
+    mRules.EnumerateForwards(SetStyleSheetReference, aSheet);
+    Rule::SetStyleSheet(aSheet);
+  }
 }
 
 #ifdef DEBUG
 /* virtual */ void
 GroupRule::List(FILE* out, int32_t aIndent) const
 {
   fputs(" {\n", out);
 
@@ -741,17 +778,17 @@ NS_IMPL_RELEASE_INHERITED(MediaRule, Gro
 
 // QueryInterface implementation for MediaRule
 NS_INTERFACE_MAP_BEGIN(MediaRule)
   NS_INTERFACE_MAP_ENTRY(nsIStyleRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSMediaRule)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStyleRule)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(CSSMediaRule)
-NS_INTERFACE_MAP_END
+NS_INTERFACE_MAP_END_INHERITING(GroupRule)
 
 /* virtual */ void
 MediaRule::SetStyleSheet(nsCSSStyleSheet* aSheet)
 {
   if (mMedia) {
     // Set to null so it knows it's leaving one sheet and joining another.
     mMedia->SetStyleSheet(nullptr);
     mMedia->SetStyleSheet(aSheet);
@@ -921,17 +958,17 @@ NS_IMPL_RELEASE_INHERITED(DocumentRule, 
 
 // QueryInterface implementation for DocumentRule
 NS_INTERFACE_MAP_BEGIN(DocumentRule)
   NS_INTERFACE_MAP_ENTRY(nsIStyleRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSMozDocumentRule)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStyleRule)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(CSSMozDocumentRule)
-NS_INTERFACE_MAP_END
+NS_INTERFACE_MAP_END_INHERITING(GroupRule)
 
 #ifdef DEBUG
 /* virtual */ void
 DocumentRule::List(FILE* out, int32_t aIndent) const
 {
   for (int32_t indent = aIndent; --indent >= 0; ) fputs("  ", out);
 
   nsCAutoString str;
@@ -1141,18 +1178,18 @@ NameSpaceRule::NameSpaceRule(const NameS
     mURLSpec(aCopy.mURLSpec)
 {
 }
 
 NameSpaceRule::~NameSpaceRule()
 {
 }
 
-NS_IMPL_ADDREF_INHERITED(NameSpaceRule, Rule)
-NS_IMPL_RELEASE_INHERITED(NameSpaceRule, Rule)
+NS_IMPL_ADDREF(NameSpaceRule)
+NS_IMPL_RELEASE(NameSpaceRule)
 
 // QueryInterface implementation for NameSpaceRule
 NS_INTERFACE_MAP_BEGIN(NameSpaceRule)
   if (aIID.Equals(NS_GET_IID(css::NameSpaceRule))) {
     *aInstancePtr = this;
     NS_ADDREF_THIS();
     return NS_OK;
   }
@@ -1884,18 +1921,18 @@ nsCSSKeyframeRule::~nsCSSKeyframeRule()
 
 /* virtual */ already_AddRefed<css::Rule>
 nsCSSKeyframeRule::Clone() const
 {
   nsRefPtr<css::Rule> clone = new nsCSSKeyframeRule(*this);
   return clone.forget();
 }
 
-NS_IMPL_ADDREF_INHERITED(nsCSSKeyframeRule, Rule)
-NS_IMPL_RELEASE_INHERITED(nsCSSKeyframeRule, Rule)
+NS_IMPL_ADDREF(nsCSSKeyframeRule)
+NS_IMPL_RELEASE(nsCSSKeyframeRule)
 
 DOMCI_DATA(MozCSSKeyframeRule, nsCSSKeyframeRule)
 
 // QueryInterface implementation for nsCSSKeyframeRule
 NS_INTERFACE_MAP_BEGIN(nsCSSKeyframeRule)
   NS_INTERFACE_MAP_ENTRY(nsIStyleRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMMozCSSKeyframeRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSRule)
@@ -2077,17 +2114,17 @@ DOMCI_DATA(MozCSSKeyframesRule, nsCSSKey
 
 // QueryInterface implementation for nsCSSKeyframesRule
 NS_INTERFACE_MAP_BEGIN(nsCSSKeyframesRule)
   NS_INTERFACE_MAP_ENTRY(nsIStyleRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMMozCSSKeyframesRule)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStyleRule)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(MozCSSKeyframesRule)
-NS_INTERFACE_MAP_END
+NS_INTERFACE_MAP_END_INHERITING(GroupRule)
 
 #ifdef DEBUG
 void
 nsCSSKeyframesRule::List(FILE* out, int32_t aIndent) const
 {
   // FIXME: WRITE ME
 }
 #endif
@@ -2313,17 +2350,17 @@ NS_IMPL_RELEASE_INHERITED(CSSSupportsRul
 
 // QueryInterface implementation for CSSSupportsRule
 NS_INTERFACE_MAP_BEGIN(CSSSupportsRule)
   NS_INTERFACE_MAP_ENTRY(nsIStyleRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSRule)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSSupportsRule)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStyleRule)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(CSSSupportsRule)
-NS_INTERFACE_MAP_END
+NS_INTERFACE_MAP_END_INHERITING(GroupRule)
 
 // nsIDOMCSSRule methods
 NS_IMETHODIMP
 CSSSupportsRule::GetType(uint16_t* aType)
 {
   *aType = nsIDOMCSSRule::SUPPORTS_RULE;
   return NS_OK;
 }
--- a/layout/style/nsCSSRules.h
+++ b/layout/style/nsCSSRules.h
@@ -274,17 +274,17 @@ class CharsetRule MOZ_FINAL : public Rul
 public:
   CharsetRule(const nsAString& aEncoding);
 private:
   // For |Clone|
   CharsetRule(const CharsetRule& aCopy);
   ~CharsetRule() {}
 
 public:
-  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_ISUPPORTS
 
   DECL_STYLE_RULE_INHERIT
 
   // nsIStyleRule methods
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const;
 #endif
 
@@ -345,17 +345,17 @@ public:
     : mDeclaration(aDeclaration)
   {
     mKeys.SwapElements(aKeys);
   }
 private:
   nsCSSKeyframeRule(const nsCSSKeyframeRule& aCopy);
   ~nsCSSKeyframeRule();
 public:
-  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_ISUPPORTS
 
   // nsIStyleRule methods
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const;
 #endif
 
   // Rule methods
   DECL_STYLE_RULE_INHERIT