From 529cae9883bbcdbb008581fdb5a0d84df8f7cdbc Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Sun, 7 Oct 2012 22:39:09 -0400 Subject: [PATCH] Bug 795221 part 3. Implement cycle collection for GroupRule objects. r=smaug,dbaron --HG-- rename : content/html/content/crashtests/795221-1.html => content/html/content/crashtests/795221-2.html --- content/html/content/crashtests/795221-2.html | 9 +++ .../html/content/crashtests/crashtests.list | 1 + layout/style/GroupRule.h | 4 ++ layout/style/ImportRule.h | 2 +- layout/style/NameSpaceRule.h | 2 +- layout/style/Rule.h | 7 -- layout/style/StyleRule.cpp | 4 +- layout/style/StyleRule.h | 2 +- layout/style/nsCSSRules.cpp | 71 ++++++++++++++----- layout/style/nsCSSRules.h | 4 +- 10 files changed, 75 insertions(+), 31 deletions(-) create mode 100644 content/html/content/crashtests/795221-2.html diff --git a/content/html/content/crashtests/795221-2.html b/content/html/content/crashtests/795221-2.html new file mode 100644 index 00000000000..fc2aa7d5063 --- /dev/null +++ b/content/html/content/crashtests/795221-2.html @@ -0,0 +1,9 @@ + + + diff --git a/content/html/content/crashtests/crashtests.list b/content/html/content/crashtests/crashtests.list index b342ec64e4a..c38cb7de6be 100644 --- a/content/html/content/crashtests/crashtests.list +++ b/content/html/content/crashtests/crashtests.list @@ -37,3 +37,4 @@ load 673853.html load 738744.xhtml load 741250.xhtml load 795221-1.html +load 795221-2.html diff --git a/layout/style/GroupRule.h b/layout/style/GroupRule.h index c6ad553da0f..7bcc0ef0d57 100644 --- a/layout/style/GroupRule.h +++ b/layout/style/GroupRule.h @@ -14,6 +14,7 @@ #include "mozilla/css/Rule.h" #include "nsCOMArray.h" #include "nsAutoPtr.h" +#include "nsCycleCollectionParticipant.h" class nsPresContext; class nsMediaQueryResultCacheKey; @@ -33,6 +34,9 @@ protected: 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); diff --git a/layout/style/ImportRule.h b/layout/style/ImportRule.h index e7ec1060360..90f9b0d9f7a 100644 --- a/layout/style/ImportRule.h +++ b/layout/style/ImportRule.h @@ -30,7 +30,7 @@ private: ImportRule(const ImportRule& aCopy); ~ImportRule(); public: - NS_DECL_ISUPPORTS_INHERITED + NS_DECL_ISUPPORTS DECL_STYLE_RULE_INHERIT diff --git a/layout/style/NameSpaceRule.h b/layout/style/NameSpaceRule.h index e3e0bdfad21..7e7c4942ef7 100644 --- a/layout/style/NameSpaceRule.h +++ b/layout/style/NameSpaceRule.h @@ -35,7 +35,7 @@ private: public: NS_DECLARE_STATIC_IID_ACCESSOR(NS_CSS_NAMESPACE_RULE_IMPL_CID) - NS_DECL_ISUPPORTS_INHERITED + NS_DECL_ISUPPORTS DECL_STYLE_RULE_INHERIT diff --git a/layout/style/Rule.h b/layout/style/Rule.h index 502da83198c..04eec174890 100644 --- a/layout/style/Rule.h +++ b/layout/style/Rule.h @@ -46,13 +46,6 @@ protected: 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: diff --git a/layout/style/StyleRule.cpp b/layout/style/StyleRule.cpp index 3f54901da08..8f36002f0b9 100644 --- a/layout/style/StyleRule.cpp +++ b/layout/style/StyleRule.cpp @@ -1361,8 +1361,8 @@ NS_INTERFACE_MAP_BEGIN(StyleRule) 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() diff --git a/layout/style/StyleRule.h b/layout/style/StyleRule.h index d38de030c19..94b6ca237a3 100644 --- a/layout/style/StyleRule.h +++ b/layout/style/StyleRule.h @@ -307,7 +307,7 @@ private: 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; } diff --git a/layout/style/nsCSSRules.cpp b/layout/style/nsCSSRules.cpp index db16905529b..f9cbaefbb22 100644 --- a/layout/style/nsCSSRules.cpp +++ b/layout/style/nsCSSRules.cpp @@ -57,9 +57,6 @@ IMPL_STYLE_RULE_INHERIT_MAP_RULE_INFO_INTO(class_, super_) namespace mozilla { namespace css { -NS_IMPL_ADDREF(Rule) -NS_IMPL_RELEASE(Rule) - nsCSSStyleSheet* Rule::GetStyleSheet() const { @@ -243,8 +240,8 @@ CharsetRule::CharsetRule(const CharsetRule& aCopy) { } -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) @@ -377,8 +374,8 @@ ImportRule::~ImportRule() } } -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) @@ -567,6 +564,13 @@ GroupRule::~GroupRule() } } +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 @@ -577,11 +581,44 @@ SetStyleSheetReference(Rule* aRule, void* aSheet) 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& 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 @@ -779,7 +816,7 @@ NS_INTERFACE_MAP_BEGIN(MediaRule) 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) @@ -959,7 +996,7 @@ NS_INTERFACE_MAP_BEGIN(DocumentRule) 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 @@ -1179,8 +1216,8 @@ 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) @@ -1922,8 +1959,8 @@ nsCSSKeyframeRule::Clone() const 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) @@ -2117,7 +2154,7 @@ NS_INTERFACE_MAP_BEGIN(nsCSSKeyframesRule) 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 @@ -2355,7 +2392,7 @@ NS_INTERFACE_MAP_BEGIN(CSSSupportsRule) 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 diff --git a/layout/style/nsCSSRules.h b/layout/style/nsCSSRules.h index 960d8ccbe13..0a4e7b22448 100644 --- a/layout/style/nsCSSRules.h +++ b/layout/style/nsCSSRules.h @@ -279,7 +279,7 @@ private: ~CharsetRule() {} public: - NS_DECL_ISUPPORTS_INHERITED + NS_DECL_ISUPPORTS DECL_STYLE_RULE_INHERIT @@ -350,7 +350,7 @@ private: nsCSSKeyframeRule(const nsCSSKeyframeRule& aCopy); ~nsCSSKeyframeRule(); public: - NS_DECL_ISUPPORTS_INHERITED + NS_DECL_ISUPPORTS // nsIStyleRule methods #ifdef DEBUG