Bug 795221 part 4. Hook up <link> elements to cycle collect their stylesheet. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Sun, 07 Oct 2012 22:39:09 -0400
changeset 109606 8cd475094c5d65440d34b2e81bb4086f3bbde589
parent 109605 baed277d76547b080abe940bb297be36721a0383
child 109607 614a3c53c14bc88bce59beb0902c031535937acf
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
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 4. Hook up <link> elements to cycle collect their stylesheet. r=smaug
content/base/src/nsDocument.cpp
content/html/content/crashtests/795221-3.html
content/html/content/crashtests/crashtests.list
content/html/content/src/nsHTMLLinkElement.cpp
layout/style/Loader.cpp
layout/style/Loader.h
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -1892,16 +1892,20 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
   // Traverse animation components
   if (tmp->mAnimationController) {
     tmp->mAnimationController->Traverse(&cb);
   }
 
   if (tmp->mSubDocuments && tmp->mSubDocuments->ops) {
     PL_DHashTableEnumerate(tmp->mSubDocuments, SubDocTraverser, &cb);
   }
+
+  if (tmp->mCSSLoader) {
+    tmp->mCSSLoader->TraverseCachedSheets(cb);
+  }
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsDocument)
   nsINode::Trace(tmp, aCallback, aClosure);
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 
@@ -1965,16 +1969,20 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ns
   tmp->mIdentifierMap.Clear();
 
   if (tmp->mAnimationController) {
     tmp->mAnimationController->Unlink();
   }
 
   tmp->mPendingTitleChangeEvent.Revoke();
 
+  if (tmp->mCSSLoader) {
+    tmp->mCSSLoader->UnlinkCachedSheets();
+  }
+
   tmp->mInUnlinkOrDeletion = false;
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 
 nsresult
 nsDocument::Init()
 {
   if (mCSSLoader || mStyleImageLoader || mNodeInfoManager || mScriptLoader) {
copy from content/html/content/crashtests/795221-1.html
copy to content/html/content/crashtests/795221-3.html
--- a/content/html/content/crashtests/795221-1.html
+++ b/content/html/content/crashtests/795221-3.html
@@ -1,7 +1,14 @@
 <!DOCTYPE html>
-<style>
-  div { }
-</style>
+<body>
 <script>
-  document.styleSheets[0].cssRules[0].style.foo = document;
+  // Create the stylesheet via script, so that the parser's preloading doesn't
+  // make the CSS loader hold on to the sheet we're _not_ trying to form a
+  // cycle through, thus accidentally avoiding the cycle
+  var link = document.createElement("link");
+  link.setAttribute("rel", "stylesheet");
+  link.setAttribute("href", "data:text/css,div { }");
+  link.onload = function () {
+    document.styleSheets[0].cssRules[0].style.foo = document;
+  }
+  document.body.appendChild(link);
 </script>
--- a/content/html/content/crashtests/crashtests.list
+++ b/content/html/content/crashtests/crashtests.list
@@ -33,8 +33,9 @@ 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
+load 795221-3.html
--- a/content/html/content/src/nsHTMLLinkElement.cpp
+++ b/content/html/content/src/nsHTMLLinkElement.cpp
@@ -36,16 +36,20 @@ class nsHTMLLinkElement : public nsGener
 {
 public:
   nsHTMLLinkElement(already_AddRefed<nsINodeInfo> aNodeInfo);
   virtual ~nsHTMLLinkElement();
 
   // nsISupports
   NS_DECL_ISUPPORTS_INHERITED
 
+  // CC
+  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsHTMLLinkElement,
+                                           nsGenericHTMLElement)
+
   // nsIDOMNode
   NS_FORWARD_NSIDOMNODE(nsGenericHTMLElement::)
 
   // nsIDOMElement
   NS_FORWARD_NSIDOMELEMENT(nsGenericHTMLElement::)
 
   // nsIDOMHTMLElement
   NS_FORWARD_NSIDOMHTMLELEMENT(nsGenericHTMLElement::)
@@ -115,25 +119,34 @@ nsHTMLLinkElement::nsHTMLLinkElement(alr
     Link(this)
 {
 }
 
 nsHTMLLinkElement::~nsHTMLLinkElement()
 {
 }
 
+NS_IMPL_CYCLE_COLLECTION_CLASS(nsHTMLLinkElement)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsHTMLLinkElement,
+                                                  nsGenericHTMLElement)
+  tmp->nsStyleLinkElement::Traverse(cb);
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsHTMLLinkElement,
+                                                nsGenericHTMLElement)
+  tmp->nsStyleLinkElement::Unlink();
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_ADDREF_INHERITED(nsHTMLLinkElement, nsGenericElement) 
 NS_IMPL_RELEASE_INHERITED(nsHTMLLinkElement, nsGenericElement) 
 
 
 DOMCI_NODE_DATA(HTMLLinkElement, nsHTMLLinkElement)
 
 // QueryInterface implementation for nsHTMLLinkElement
-NS_INTERFACE_TABLE_HEAD(nsHTMLLinkElement)
+NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(nsHTMLLinkElement)
   NS_HTML_CONTENT_INTERFACE_TABLE5(nsHTMLLinkElement,
                                    nsIDOMHTMLLinkElement,
                                    nsIDOMLinkStyle,
                                    nsILink,
                                    nsIStyleSheetLinkingElement,
                                    Link)
   NS_HTML_CONTENT_INTERFACE_TABLE_TO_MAP_SEGUE(nsHTMLLinkElement,
                                                nsGenericHTMLElement)
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -59,16 +59,17 @@
 
 #include "nsIMediaList.h"
 #include "nsIDOMStyleSheet.h"
 #include "nsIDOMCSSStyleSheet.h"
 #include "nsError.h"
 
 #include "nsIChannelPolicy.h"
 #include "nsIContentSecurityPolicy.h"
+#include "nsCycleCollectionParticipant.h"
 
 
 /**
  * OVERALL ARCHITECTURE
  *
  * The CSS Loader gets requests to load various sorts of style sheets:
  * inline style from <style> elements, linked style, @import-ed child
  * sheets, non-document sheets.  The loader handles the following tasks:
@@ -2385,10 +2386,38 @@ Loader::StartAlternateLoads()
 
   mDatasToNotifyOn += arr.Length();
   for (uint32_t i = 0; i < arr.Length(); ++i) {
     --mDatasToNotifyOn;
     LoadSheet(arr[i], eSheetNeedsParser);
   }
 }
 
+static PLDHashOperator
+TraverseSheet(URIPrincipalAndCORSModeHashKey*,
+              nsCSSStyleSheet* aSheet,
+              void* aClosure)
+{
+  nsCycleCollectionTraversalCallback* cb =
+    static_cast<nsCycleCollectionTraversalCallback*>(aClosure);
+  NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(*cb, "Sheet cache nsCSSLoader");
+  cb->NoteXPCOMChild(NS_ISUPPORTS_CAST(nsIStyleSheet*, aSheet));
+  return PL_DHASH_NEXT;
+}
+
+void
+Loader::TraverseCachedSheets(nsCycleCollectionTraversalCallback& cb)
+{
+  if (mCompleteSheets.IsInitialized()) {
+    mCompleteSheets.EnumerateRead(TraverseSheet, &cb);
+  }
+}
+
+void
+Loader::UnlinkCachedSheets()
+{
+  if (mCompleteSheets.IsInitialized()) {
+    mCompleteSheets.Clear();
+  }
+}
+
 } // namespace css
 } // namespace mozilla
--- a/layout/style/Loader.h
+++ b/layout/style/Loader.h
@@ -24,16 +24,17 @@
 class nsIAtom;
 class nsICSSLoaderObserver;
 class nsCSSStyleSheet;
 class nsIContent;
 class nsIDocument;
 class nsCSSParser;
 class nsMediaList;
 class nsIStyleSheetLinkingElement;
+class nsCycleCollectionTraversalCallback;
 
 namespace mozilla {
 
 class URIPrincipalAndCORSModeHashKey : public nsURIHashKey
 {
 public:
   typedef URIPrincipalAndCORSModeHashKey* KeyType;
   typedef const URIPrincipalAndCORSModeHashKey* KeyTypePointer;
@@ -346,16 +347,24 @@ public:
   // within nsCSSLoader.cpp.
 
   // IsAlternate can change our currently selected style set if none
   // is selected and aHasAlternateRel is false.
   bool IsAlternate(const nsAString& aTitle, bool aHasAlternateRel);
 
   typedef nsTArray<nsRefPtr<SheetLoadData> > LoadDataArray;
 
+  // Traverse the cached stylesheets we're holding on to.  This should
+  // only be called from the document that owns this loader.
+  void TraverseCachedSheets(nsCycleCollectionTraversalCallback& cb);
+
+  // Unlink the cached stylesheets we're holding on to.  Again, this
+  // should only be called from the document that owns this loader.
+  void UnlinkCachedSheets();
+
 private:
   friend class SheetLoadData;
 
   // Note: null aSourcePrincipal indicates that the content policy and
   // CheckLoadURI checks should be skipped.
   nsresult CheckLoadAllowed(nsIPrincipal* aSourcePrincipal,
                             nsIURI* aTargetURI,
                             nsISupports* aContext);