Bug 1373484 - Drop all reference to StyleSheet when last strong reference is dropped. r?bz
MozReview-Commit-ID: 2RrNZDIjx3s
--- a/layout/style/CSSStyleSheet.cpp
+++ b/layout/style/CSSStyleSheet.cpp
@@ -372,25 +372,29 @@ CSSStyleSheet::CSSStyleSheet(const CSSSt
// NOTE: It's important to call this from the subclass, since it could
// access uninitialized members otherwise.
EnsureUniqueInner();
}
}
CSSStyleSheet::~CSSStyleSheet()
{
- UnparentChildren();
+}
+void
+CSSStyleSheet::LastRelease()
+{
DropRuleCollection();
// XXX The document reference is not reference counted and should
// not be released. The document will let us know when it is going
// away.
if (mRuleProcessors) {
NS_ASSERTION(mRuleProcessors->Length() == 0, "destructing sheet with rule processor reference");
delete mRuleProcessors; // weak refs, should be empty here anyway
+ mRuleProcessors = nullptr;
}
if (mInRuleProcessorCache) {
RuleProcessorCache::RemoveSheet(this);
}
}
void
CSSStyleSheet::DropRuleCollection()
--- a/layout/style/CSSStyleSheet.h
+++ b/layout/style/CSSStyleSheet.h
@@ -162,16 +162,18 @@ private:
nsINode* aOwningNodeToUse);
CSSStyleSheet(const CSSStyleSheet& aCopy) = delete;
CSSStyleSheet& operator=(const CSSStyleSheet& aCopy) = delete;
protected:
virtual ~CSSStyleSheet();
+ void LastRelease();
+
void ClearRuleCascades();
// Add the namespace mapping from this @namespace rule to our namespace map
nsresult RegisterNamespaceRule(css::Rule* aRule);
// Drop our reference to mRuleCollection
void DropRuleCollection();
--- a/layout/style/ServoStyleSheet.cpp
+++ b/layout/style/ServoStyleSheet.cpp
@@ -101,18 +101,21 @@ ServoStyleSheet::ServoStyleSheet(const S
// NOTE: It's important to call this from the subclass, since this could
// access uninitialized members otherwise.
EnsureUniqueInner();
}
}
ServoStyleSheet::~ServoStyleSheet()
{
- UnparentChildren();
+}
+void
+ServoStyleSheet::LastRelease()
+{
DropRuleList();
}
// QueryInterface implementation for ServoStyleSheet
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(ServoStyleSheet)
NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIDOMCSSStyleSheet)
if (aIID.Equals(NS_GET_IID(ServoStyleSheet)))
foundInterface = reinterpret_cast<nsISupports*>(this);
--- a/layout/style/ServoStyleSheet.h
+++ b/layout/style/ServoStyleSheet.h
@@ -120,16 +120,18 @@ public:
// Internal GetCssRules method which do not have security check and
// completelness check.
ServoCSSRuleList* GetCssRulesInternal();
protected:
virtual ~ServoStyleSheet();
+ void LastRelease();
+
ServoStyleSheetInner* Inner() const
{
return static_cast<ServoStyleSheetInner*>(mInner);
}
// Internal methods which do not have security check and completeness check.
uint32_t InsertRuleInternal(const nsAString& aRule,
uint32_t aIndex, ErrorResult& aRv);
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -60,18 +60,32 @@ StyleSheet::StyleSheet(const StyleSheet&
// XXX This is wrong; we should be keeping @import rules and
// sheets in sync!
mMedia = aCopy.mMedia->Clone();
}
}
StyleSheet::~StyleSheet()
{
+ MOZ_ASSERT(!mInner, "Inner should have been dropped in LastRelease");
+}
+
+void
+StyleSheet::LastRelease()
+{
MOZ_ASSERT(mInner, "Should have an mInner at time of destruction.");
MOZ_ASSERT(mInner->mSheets.Contains(this), "Our mInner should include us.");
+
+ UnparentChildren();
+ if (IsGecko()) {
+ AsGecko()->LastRelease();
+ } else {
+ AsServo()->LastRelease();
+ }
+
mInner->RemoveSheet(this);
mInner = nullptr;
DropMedia();
}
void
StyleSheet::UnlinkInner()
@@ -131,17 +145,22 @@ StyleSheet::TraverseInner(nsCycleCollect
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(StyleSheet)
NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
NS_INTERFACE_MAP_ENTRY(nsICSSLoaderObserver)
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)
+// We want to disconnect from our inner as soon as our refcount drops to zero,
+// without waiting for async deletion by the cycle collector. Otherwise we
+// might end up cloning the inner if someone mutates another sheet that shares
+// it with us, even though there is only one such sheet and we're about to go
+// away. This situation arises easily with sheet preloading.
+NS_IMPL_CYCLE_COLLECTING_RELEASE_WITH_LAST_RELEASE(StyleSheet, LastRelease())
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
--- a/layout/style/StyleSheet.h
+++ b/layout/style/StyleSheet.h
@@ -287,16 +287,18 @@ protected:
void SetParentLinks(StyleSheet* aSheet);
static void ReparentChildList(StyleSheet* aPrimarySheet,
StyleSheet* aFirstChild);
};
void UnparentChildren();
+ void LastRelease();
+
// Return success if the subject principal subsumes the principal of our
// inner, error otherwise. This will also succeed if the subject has
// UniversalXPConnect or if access is allowed by CORS. In the latter case,
// it will set the principal of the inner to the subject principal.
void SubjectSubsumesInnerPrincipal(nsIPrincipal& aSubjectPrincipal,
ErrorResult& aRv);
// Drop our reference to mMedia