Bug 700116. Fix the ordering if IsAlternate calls and sheet state determination so that the hashtable munging IsAlternate can do doesn't mess up our sheet states. r=sicking
☠☠ backed out by 095a745d1e72 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 07 Nov 2011 23:41:17 -0500
changeset 79987 27b8e5fb9699fb0145486ebfbfbcacf0c510431e
parent 79986 95efc21bf5af830e5df9fe3ac571d305907c3221
child 79988 5c0861ec147f45cb4bee620c2381a2c26dcf227d
push idunknown
push userunknown
push dateunknown
reviewerssicking
bugs700116
milestone10.0a1
Bug 700116. Fix the ordering if IsAlternate calls and sheet state determination so that the hashtable munging IsAlternate can do doesn't mess up our sheet states. r=sicking
layout/style/Loader.cpp
layout/style/Loader.h
layout/style/crashtests/700116.html
layout/style/crashtests/crashtests.list
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -1088,31 +1088,38 @@ Loader::CheckLoadAllowed(nsIPrincipal* a
  * are clones off; make sure to call PrepareSheet() on the result of
  * CreateSheet().
  */
 nsresult
 Loader::CreateSheet(nsIURI* aURI,
                     nsIContent* aLinkingContent,
                     nsIPrincipal* aLoaderPrincipal,
                     bool aSyncLoad,
+                    bool aHasAlternateRel,
+                    const nsAString& aTitle,                       
                     StyleSheetState& aSheetState,
+                    bool *aIsAlternate,
                     nsCSSStyleSheet** aSheet)
 {
   LOG(("css::Loader::CreateSheet"));
   NS_PRECONDITION(aSheet, "Null out param!");
 
   NS_ENSURE_TRUE((mCompleteSheets.IsInitialized() || mCompleteSheets.Init()) &&
                    (mLoadingDatas.IsInitialized() || mLoadingDatas.Init()) &&
                    (mPendingDatas.IsInitialized() || mPendingDatas.Init()),
                  NS_ERROR_OUT_OF_MEMORY);
 
   nsresult rv = NS_OK;
   *aSheet = nsnull;
   aSheetState = eSheetStateUnknown;
 
+  // Check the alternate state before doing anything else, because it
+  // can mess with our hashtables.
+  *aIsAlternate = IsAlternate(aTitle, aHasAlternateRel);
+
   if (aURI) {
     aSheetState = eSheetComplete;
     nsRefPtr<nsCSSStyleSheet> sheet;
 
     // First, the XUL cache
 #ifdef MOZ_XUL
     if (IsChromeURI(aURI)) {
       nsXULPrototypeCache* cache = nsXULPrototypeCache::GetInstance();
@@ -1236,18 +1243,17 @@ Loader::CreateSheet(nsIURI* aURI,
  * well as setting the enabled state based on the title and whether
  * the sheet had "alternate" in its rel.
  */
 nsresult
 Loader::PrepareSheet(nsCSSStyleSheet* aSheet,
                      const nsSubstring& aTitle,
                      const nsSubstring& aMediaString,
                      nsMediaList* aMediaList,
-                     bool aHasAlternateRel,
-                     bool *aIsAlternate)
+                     bool isAlternate)
 {
   NS_PRECONDITION(aSheet, "Must have a sheet!");
 
   nsresult rv;
   nsRefPtr<nsMediaList> mediaList(aMediaList);
 
   if (!aMediaString.IsEmpty()) {
     NS_ASSERTION(!aMediaList,
@@ -1262,21 +1268,17 @@ Loader::PrepareSheet(nsCSSStyleSheet* aS
     rv = mediumParser.ParseMediaList(aMediaString, nsnull, 0, mediaList,
                                      true);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   aSheet->SetMedia(mediaList);
 
   aSheet->SetTitle(aTitle);
-  bool alternate = IsAlternate(aTitle, aHasAlternateRel);
-  aSheet->SetEnabled(! alternate);
-  if (aIsAlternate) {
-    *aIsAlternate = alternate;
-  }
+  aSheet->SetEnabled(! isAlternate);
   return NS_OK;
 }
 
 /**
  * InsertSheetInDoc handles ordering of sheets in the document.  Here
  * we have two types of sheets -- those with linking elements and
  * those without.  The latter are loaded by Link: headers.
  * The following constraints are observed:
@@ -1833,25 +1835,25 @@ Loader::LoadInlineStyle(nsIContent* aEle
   NS_ENSURE_TRUE(mDocument, NS_ERROR_NOT_INITIALIZED);
 
   nsCOMPtr<nsIStyleSheetLinkingElement> owningElement(do_QueryInterface(aElement));
   NS_ASSERTION(owningElement, "Element is not a style linking element!");
 
   // Since we're not planning to load a URI, no need to hand a principal to the
   // load data or to CreateSheet().
   StyleSheetState state;
+  bool isAlternate;
   nsRefPtr<nsCSSStyleSheet> sheet;
-  nsresult rv = CreateSheet(nsnull, aElement, nsnull, false, state,
-                            getter_AddRefs(sheet));
+  nsresult rv = CreateSheet(nsnull, aElement, nsnull, false, false,
+                            aTitle, state, &isAlternate, getter_AddRefs(sheet));
   NS_ENSURE_SUCCESS(rv, rv);
   NS_ASSERTION(state == eSheetNeedsParser,
                "Inline sheets should not be cached");
 
-  rv = PrepareSheet(sheet, aTitle, aMedia, nsnull, false,
-                    aIsAlternate);
+  rv = PrepareSheet(sheet, aTitle, aMedia, nsnull, isAlternate);
   NS_ENSURE_SUCCESS(rv, rv);
 
   LOG(("  Sheet is alternate: %d", *aIsAlternate));
 
   rv = InsertSheetInDoc(sheet, aElement, mDocument);
   NS_ENSURE_SUCCESS(rv, rv);
 
   SheetLoadData* data = new SheetLoadData(this, aTitle, nsnull, sheet,
@@ -1907,23 +1909,23 @@ Loader::LoadStyleLink(nsIContent* aEleme
     context = mDocument;
   }
   nsresult rv = CheckLoadAllowed(principal, aURL, context);
   if (NS_FAILED(rv)) return rv;
 
   LOG(("  Passed load check"));
 
   StyleSheetState state;
+  bool isAlternate;
   nsRefPtr<nsCSSStyleSheet> sheet;
-  rv = CreateSheet(aURL, aElement, principal, false, state,
-                   getter_AddRefs(sheet));
+  rv = CreateSheet(aURL, aElement, principal, false, aHasAlternateRel,
+                   aTitle, state, &isAlternate, getter_AddRefs(sheet));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = PrepareSheet(sheet, aTitle, aMedia, nsnull, aHasAlternateRel,
-                    aIsAlternate);
+  rv = PrepareSheet(sheet, aTitle, aMedia, nsnull, isAlternate);
   NS_ENSURE_SUCCESS(rv, rv);
 
   LOG(("  Sheet is alternate: %d", *aIsAlternate));
 
   rv = InsertSheetInDoc(sheet, aElement, mDocument);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIStyleSheetLinkingElement> owningElement(do_QueryInterface(aElement));
@@ -2065,24 +2067,25 @@ Loader::LoadChildSheet(nsCSSStyleSheet* 
     // No parent load data, so the sheet will need to be notified when
     // we finish, if it can be, if we do the load asynchronously.
     observer = aParentSheet;
   }
 
   // Now that we know it's safe to load this (passes security check and not a
   // loop) do so
   nsRefPtr<nsCSSStyleSheet> sheet;
+  bool isAlternate;
   StyleSheetState state;
+  const nsSubstring& empty = EmptyString();
   rv = CreateSheet(aURL, nsnull, principal,
                    parentData ? parentData->mSyncLoad : false,
-                   state, getter_AddRefs(sheet));
+                   false, empty, state, &isAlternate, getter_AddRefs(sheet));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  const nsSubstring& empty = EmptyString();
-  rv = PrepareSheet(sheet, empty, empty, aMedia);
+  rv = PrepareSheet(sheet, empty, empty, aMedia, isAlternate);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = InsertChildSheet(sheet, aParentSheet, aParentRule);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (state == eSheetComplete) {
     LOG(("  Sheet already complete"));
     // We're completely done.  No need to notify, even, since the
@@ -2172,25 +2175,26 @@ Loader::InternalLoadNonDocumentSheet(nsI
   }
 
   nsresult rv = CheckLoadAllowed(aOriginPrincipal, aURL, mDocument);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   StyleSheetState state;
+  bool isAlternate;
   nsRefPtr<nsCSSStyleSheet> sheet;
   bool syncLoad = (aObserver == nsnull);
+  const nsSubstring& empty = EmptyString();
 
-  rv = CreateSheet(aURL, nsnull, aOriginPrincipal, syncLoad, state,
-                   getter_AddRefs(sheet));
+  rv = CreateSheet(aURL, nsnull, aOriginPrincipal, syncLoad, false, empty,
+                   state, &isAlternate, getter_AddRefs(sheet));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  const nsSubstring& empty = EmptyString();
-  rv = PrepareSheet(sheet, empty, empty, nsnull);
+  rv = PrepareSheet(sheet, empty, empty, nsnull, isAlternate);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (state == eSheetComplete) {
     LOG(("  Sheet already complete"));
     if (aObserver || !mObservers.IsEmpty()) {
       rv = PostLoadEvent(aURL, sheet, aObserver, false, nsnull);
     }
     if (aSheet) {
--- a/layout/style/Loader.h
+++ b/layout/style/Loader.h
@@ -379,33 +379,35 @@ private:
   nsresult CheckLoadAllowed(nsIPrincipal* aSourcePrincipal,
                             nsIURI* aTargetURI,
                             nsISupports* aContext);
 
 
   // For inline style, the aURI param is null, but the aLinkingContent
   // must be non-null then.  The loader principal must never be null
   // if aURI is not null.
+  // *aIsAlternate is set based on aTitle and aHasAlternateRel.
   nsresult CreateSheet(nsIURI* aURI,
                        nsIContent* aLinkingContent,
                        nsIPrincipal* aLoaderPrincipal,
                        bool aSyncLoad,
+                       bool aHasAlternateRel,
+                       const nsAString& aTitle,
                        StyleSheetState& aSheetState,
+                       bool *aIsAlternate,
                        nsCSSStyleSheet** aSheet);
 
   // Pass in either a media string or the nsMediaList from the
   // CSSParser.  Don't pass both.
-  // If aIsAlternate is non-null, this method will set *aIsAlternate to
-  // correspond to the sheet's enabled state (which it will set no matter what)
+  // This method will set the sheet's enabled state based on isAlternate
   nsresult PrepareSheet(nsCSSStyleSheet* aSheet,
                         const nsAString& aTitle,
                         const nsAString& aMediaString,
                         nsMediaList* aMediaList,
-                        bool aHasAlternateRel = false,
-                        bool *aIsAlternate = nsnull);
+                        bool isAlternate);
 
   nsresult InsertSheetInDoc(nsCSSStyleSheet* aSheet,
                             nsIContent* aLinkingContent,
                             nsIDocument* aDocument);
 
   nsresult InsertChildSheet(nsCSSStyleSheet* aSheet,
                             nsCSSStyleSheet* aParentSheet,
                             ImportRule* aParentRule);
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/700116.html
@@ -0,0 +1,5 @@
+<head>
+  <script>
+    document.write('<link rel="stylesheet" href="#"><link rel="alternate stylesheet" title="x" href="data:text/css,"><link rel="stylesheet" title="x" href="data:text/css,">');
+  </script>
+</head>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -69,8 +69,9 @@ load 605689-1.html
 load 645142.html
 load 611922-1.html
 == 645951-1.html 645951-1-ref.html
 load 665209-1.html
 asserts(2) load 671799-1.html
 asserts(2) load 671799-2.html
 load 690990-1.html
 load 696188-1.html
+load 700116.html