Bug 1373484 - Drop all reference to StyleSheet when last strong reference is dropped. r?bz draft
authorXidorn Quan <me@upsuper.org>
Mon, 19 Jun 2017 10:15:49 +1000
changeset 597098 0a1d65f2dc007fbf4785a524fabcfb8622384fb0
parent 597097 02d6c6eaae2e236f999945050a49e915661b1e69
child 634136 793a1e0cd9ebdd12932c1698df805c46bff9dba7
push id64830
push userxquan@mozilla.com
push dateTue, 20 Jun 2017 05:10:10 +0000
reviewersbz
bugs1373484
milestone56.0a1
Bug 1373484 - Drop all reference to StyleSheet when last strong reference is dropped. r?bz MozReview-Commit-ID: 2RrNZDIjx3s
layout/style/CSSStyleSheet.cpp
layout/style/CSSStyleSheet.h
layout/style/ServoStyleSheet.cpp
layout/style/ServoStyleSheet.h
layout/style/StyleSheet.cpp
layout/style/StyleSheet.h
--- 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