Bug 795221 part 1. Implement cycle collection for nsCSSStyleSheet objects, so we don't leak through them. r=smaug,dbaron

Each nsCSSStyleSheet has a pointer to a nsCSSStyleSheetInner.  The
nsCSSStyleSheetInner is shared across multiple stylesheets, in
general.  The nsCSSStyleSheetInner owns the rules and the child
stylesheets.

What this means is that a given rule object is effectively owned by
multiple sheets.  However, cycles can only form through rule objects
that have been JS-wrapped, and if we're JS-wrapping a rule object that
means we have ensured that it's owned by only one stylesheet.
Therefore, we only traverse and unlink mInner if it's uniquely owned
by our sheet.

Similarly, if our child sheets or any of their rules have been
JS-wrapped, that means that we must have an mInner that we own
outright.

layout/style/Rule.h

@@ -25,9 +25,10 @@ class GroupRule;
 #define DECL_STYLE_RULE_INHERIT_NO_DOMRULE  \
 virtual void MapRuleInfoInto(nsRuleData* aRuleData);
 
-#define DECL_STYLE_RULE_INHERIT  \
-DECL_STYLE_RULE_INHERIT_NO_DOMRULE \
-virtual nsIDOMCSSRule* GetDOMRule();
+#define DECL_STYLE_RULE_INHERIT                   \
+  DECL_STYLE_RULE_INHERIT_NO_DOMRULE              \
+  virtual nsIDOMCSSRule* GetDOMRule();            \
+  virtual nsIDOMCSSRule* GetExistingDOMRule();
 
 class Rule : public nsIStyleRule {
 protected:
@@ -107,6 +108,9 @@ public:
   // supposed to have a DOM rule representation (and our code wouldn't work).
   virtual nsIDOMCSSRule* GetDOMRule() = 0;
 
+  // Like GetDOMRule(), but won't create one if we don't have one yet
+  virtual nsIDOMCSSRule* GetExistingDOMRule() = 0;
+
   // to implement methods on nsIDOMCSSRule
   nsresult GetParentRule(nsIDOMCSSRule** aParentRule);
   nsresult GetParentStyleSheet(nsIDOMCSSStyleSheet** aSheet);

layout/style/StyleRule.cpp

@@ -1406,6 +1406,12 @@ StyleRule::GetDOMRule()
   return mDOMRule;
 }
 
+/* virtual */ nsIDOMCSSRule*
+StyleRule::GetExistingDOMRule()
+{
+  return mDOMRule;
+}
+
 /* virtual */ already_AddRefed<StyleRule>
 StyleRule::DeclarationChanged(Declaration* aDecl,
                               bool aHandleContainer)

layout/style/StyleRule.h

@@ -349,6 +349,8 @@ public:
 
   virtual nsIDOMCSSRule* GetDOMRule();
 
+  virtual nsIDOMCSSRule* GetExistingDOMRule();
+
   // The new mapping function.
   virtual void MapRuleInfoInto(nsRuleData* aRuleData);
 

layout/style/nsCSSRules.cpp

@@ -40,7 +40,9 @@
 namespace css = mozilla::css;
 
 #define IMPL_STYLE_RULE_INHERIT_GET_DOM_RULE_WEAK(class_, super_) \
-/* virtual */ nsIDOMCSSRule* class_::GetDOMRule() \
+  /* virtual */ nsIDOMCSSRule* class_::GetDOMRule()               \
+  { return this; }                                                \
+  /* virtual */ nsIDOMCSSRule* class_::GetExistingDOMRule()       \
   { return this; }
 #define IMPL_STYLE_RULE_INHERIT_MAP_RULE_INFO_INTO(class_, super_) \
 /* virtual */ void class_::MapRuleInfoInto(nsRuleData* aRuleData) \

layout/style/nsCSSRules.h

@@ -65,6 +65,10 @@ public:
   {
     return this;
   }
+  virtual nsIDOMCSSRule* GetExistingDOMRule()
+  {
+    return this;
+  }
 
   // nsIDOMCSSRule interface
   NS_DECL_NSIDOMCSSRULE
@@ -110,6 +114,10 @@ public:
   {
     return this;
   }
+  virtual nsIDOMCSSRule* GetExistingDOMRule()
+  {
+    return this;
+  }
 
   // nsIDOMCSSRule interface
   NS_DECL_NSIDOMCSSRULE
@@ -400,6 +408,10 @@ public:
   {
     return this;
   }
+  virtual nsIDOMCSSRule* GetExistingDOMRule()
+  {
+    return this;
+  }
 
   // nsIDOMCSSRule interface
   NS_DECL_NSIDOMCSSRULE
@@ -444,6 +456,10 @@ public:
   {
     return this;
   }
+  virtual nsIDOMCSSRule* GetExistingDOMRule()
+  {
+    return this;
+  }
 
   NS_DECL_ISUPPORTS_INHERITED
 

layout/style/nsCSSStyleSheet.cpp

@@ -1075,14 +1075,8 @@ nsCSSStyleSheet::~nsCSSStyleSheet()
       child->mDocument = nullptr;
     }
   }
-  if (nullptr != mRuleCollection) {
-    mRuleCollection->DropReference();
-    NS_RELEASE(mRuleCollection);
-  }
-  if (mMedia) {
-    mMedia->SetStyleSheet(nullptr);
-    mMedia = nullptr;
-  }
+  DropRuleCollection();
+  DropMedia();
   mInner->RemoveSheet(this);
   // XXX The document reference is not reference counted and should
   // not be released. The document will let us know when it is going
@@ -1093,11 +1087,85 @@ nsCSSStyleSheet::~nsCSSStyleSheet()
   }
 }
 
+void
+nsCSSStyleSheet::DropRuleCollection()
+{
+  if (mRuleCollection) {
+    mRuleCollection->DropReference();
+    NS_RELEASE(mRuleCollection);
+  }
+}
+
+void
+nsCSSStyleSheet::DropMedia()
+{
+  if (mMedia) {
+    mMedia->SetStyleSheet(nullptr);
+    mMedia = nullptr;
+  }
+}
+
+void
+nsCSSStyleSheet::UnlinkInner()
+{
+  // We can only have a cycle through our inner if we have a unique inner,
+  // because otherwise there are no JS wrappers for anything in the inner.
+  if (mInner->mSheets.Length() != 1) {
+    return;
+  }
+
+  mInner->mOrderedRules.EnumerateForwards(SetStyleSheetReference, nullptr);
+  mInner->mOrderedRules.Clear();
+
+  // Have to be a bit careful with child sheets, because we want to
+  // drop their mNext pointers and null out their mParent and
+  // mDocument, but don't want to work with deleted objects.  And we
+  // don't want to do any addrefing in the process, just to make sure
+  // we don't confuse the cycle collector (though on the face of it,
+  // addref/release pairs during unlink should probably be ok).
+  nsRefPtr<nsCSSStyleSheet> child;
+  child.swap(mInner->mFirstChild);
+  while (child) {
+    MOZ_ASSERT(child->mParent == this, "We have a unique inner!");
+    child->mParent = nullptr;
+    child->mDocument = nullptr;
+    nsRefPtr<nsCSSStyleSheet> next;
+    // Null out child->mNext, but don't let it die yet
+    next.swap(child->mNext);
+    // Switch to looking at the old value of child->mNext next iteration
+    child.swap(next);
+    // "next" is now our previous value of child; it'll get released
+    // as we loop around.
+  }
+}
+
+void
+nsCSSStyleSheet::TraverseInner(nsCycleCollectionTraversalCallback &cb)
+{
+  // We can only have a cycle through our inner if we have a unique inner,
+  // because otherwise there are no JS wrappers for anything in the inner.
+  if (mInner->mSheets.Length() != 1) {
+    return;
+  }
+
+  nsRefPtr<nsCSSStyleSheet>* childSheetSlot = &mInner->mFirstChild;
+  while (*childSheetSlot) {
+    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "child sheet");
+    cb.NoteXPCOMChild(NS_ISUPPORTS_CAST(nsIStyleSheet*, childSheetSlot->get()));
+    childSheetSlot = &(*childSheetSlot)->mNext;
+  }
+
+  const nsCOMArray<css::Rule>& rules = mInner->mOrderedRules;
+  for (int32_t i = 0, count = rules.Count(); i < count; ++i) {
+    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mOrderedRules[i]");
+    cb.NoteXPCOMChild(rules[i]->GetExistingDOMRule());
+  }
+}
 
 DOMCI_DATA(CSSStyleSheet, nsCSSStyleSheet)
 
 // QueryInterface implementation for nsCSSStyleSheet
-NS_INTERFACE_MAP_BEGIN(nsCSSStyleSheet)
+NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsCSSStyleSheet)
   NS_INTERFACE_MAP_ENTRY(nsIStyleSheet)
   NS_INTERFACE_MAP_ENTRY(nsIDOMStyleSheet)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSStyleSheet)
@@ -1110,8 +1178,26 @@ NS_INTERFACE_MAP_BEGIN(nsCSSStyleSheet)
 NS_INTERFACE_MAP_END
 
 
-NS_IMPL_ADDREF(nsCSSStyleSheet)
-NS_IMPL_RELEASE(nsCSSStyleSheet)
+NS_IMPL_CYCLE_COLLECTING_ADDREF(nsCSSStyleSheet)
+NS_IMPL_CYCLE_COLLECTING_RELEASE(nsCSSStyleSheet)
+
+NS_IMPL_CYCLE_COLLECTION_CLASS(nsCSSStyleSheet)
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsCSSStyleSheet)
+  tmp->DropMedia();
+  // We do not unlink mNext; our parent will handle that.  If we
+  // unlinked it here, our parent would not be able to walk its list
+  // of child sheets and null out the back-references to it, if we got
+  // unlinked before it does.
+  tmp->DropRuleCollection();
+  tmp->UnlinkInner();
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsCSSStyleSheet)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mMedia)
+  // We do not traverse mNext; our parent will handle that.  See
+  // comments in Unlink for why.
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_RAWPTR(mRuleCollection)
+  tmp->TraverseInner(cb);
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 
 nsresult

layout/style/nsCSSStyleSheet.h

@@ -21,6 +21,7 @@
 #include "nsTArray.h"
 #include "nsString.h"
 #include "mozilla/CORSMode.h"
+#include "nsCycleCollectionParticipant.h"
 
 class nsXMLNameSpaceMap;
 class nsCSSRuleProcessor;
@@ -110,7 +111,9 @@ class nsCSSStyleSheet MOZ_FINAL : public nsIStyleSheet,
 public:
   nsCSSStyleSheet(mozilla::CORSMode aCORSMode);
 
-  NS_DECL_ISUPPORTS
+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
+  NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsCSSStyleSheet,
+                                           nsIStyleSheet)
 
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_CSS_STYLE_SHEET_IMPL_CID)
 
@@ -272,6 +275,17 @@ protected:
   // Add the namespace mapping from this @namespace rule to our namespace map
   nsresult RegisterNamespaceRule(mozilla::css::Rule* aRule);
 
+  // Drop our reference to mRuleCollection
+  void DropRuleCollection();
+
+  // Drop our reference to mMedia
+  void DropMedia();
+
+  // Unlink our inner, if needed, for cycle collection
+  void UnlinkInner();
+  // Traverse our inner, if needed, for cycle collection
+  void TraverseInner(nsCycleCollectionTraversalCallback &);
+
 protected:
   nsString              mTitle;
   nsRefPtr<nsMediaList> mMedia;