Bug 795221 part 3. Implement cycle collection for GroupRule objects. r=smaug,dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Sun, 07 Oct 2012 22:39:09 -0400
changeset 109605 baed277d76547b080abe940bb297be36721a0383
parent 109604 a2ab9f871f2f24f433eac3b7949a1aac7bb7b0ca
child 109606 8cd475094c5d65440d34b2e81bb4086f3bbde589
push id23636
push usergsharp@mozilla.com
push dateMon, 08 Oct 2012 08:08:19 +0000
treeherdermozilla-central@24cf40690042 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, dbaron
bugs795221
milestone18.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 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
@@ -42,23 +42,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
@@ -302,17 +302,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)
-
 nsCSSStyleSheet*
 Rule::GetStyleSheet() const
 {
   if (!(mSheet & 0x1)) {
     return reinterpret_cast<nsCSSStyleSheet*>(mSheet);
   }
 
   return nullptr;
@@ -238,18 +235,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)
@@ -372,18 +369,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)
@@ -562,31 +559,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);
 
@@ -774,17 +811,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);
@@ -954,17 +991,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);
 
   nsAutoCString str;
@@ -1174,18 +1211,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;
   }
@@ -1917,18 +1954,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)
@@ -2112,17 +2149,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
@@ -2350,17 +2387,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