Bug 867487. Make sure we don't think we're cloning a complete sheet when we're actually cloning a loading one because the complete sheet was dirty. r=heycam, a=lsblakk
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 23 May 2013 09:20:52 -0400
changeset 142726 915b309c6cd521033df5804fe12bf6411222cf48
parent 142725 6664ebea6eae95c8390668aba7dd609f121a3e90
child 142727 b6929d3b4d9f54b00f892dd300bf787985d74d49
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam, lsblakk
bugs867487
milestone23.0a2
Bug 867487. Make sure we don't think we're cloning a complete sheet when we're actually cloning a loading one because the complete sheet was dirty. r=heycam, a=lsblakk
layout/style/Loader.cpp
layout/style/crashtests/867487.html
layout/style/crashtests/crashtests.list
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -1100,16 +1100,17 @@ Loader::CreateSheet(nsIURI* aURI,
       NS_ASSERTION(sheet->IsComplete(),
                    "Sheet thinks it's not complete while we think it is");
 
       // Make sure it hasn't been modified; if it has, we can't use it
       if (sheet->IsModified()) {
         LOG(("  Not cloning completed sheet %p because it's been modified",
              sheet.get()));
         sheet = nullptr;
+        fromCompleteSheets = false;
       }
     }
 
     // Then loading sheets
     if (!sheet && !aSyncLoad) {
       aSheetState = eSheetLoading;
       SheetLoadData* loadData = nullptr;
       URIPrincipalAndCORSModeHashKey key(aURI, aLoaderPrincipal, aCORSMode);
@@ -1161,16 +1162,18 @@ Loader::CreateSheet(nsIURI* aURI,
       *aSheet = sheet->Clone(nullptr, nullptr, nullptr, nullptr).get();
       if (*aSheet && fromCompleteSheets &&
           !sheet->GetOwnerNode() && !sheet->GetParentSheet()) {
         // The sheet we're cloning isn't actually referenced by
         // anyone.  Replace it in the cache, so that if our CSSOM is
         // later modified we don't end up with two copies of our inner
         // hanging around.
         URIPrincipalAndCORSModeHashKey key(aURI, aLoaderPrincipal, aCORSMode);
+        NS_ASSERTION((*aSheet)->IsComplete(),
+                     "Should only be caching complete sheets");
         mCompleteSheets.Put(&key, *aSheet);
       }
     }
   }
 
   if (!*aSheet) {
     aSheetState = eSheetNeedsParser;
     nsIURI *sheetURI;
@@ -1785,25 +1788,29 @@ Loader::DoSheetComplete(SheetLoadData* a
       data = data->mNext;
     }
 #ifdef MOZ_XUL
     if (IsChromeURI(aLoadData->mURI)) {
       nsXULPrototypeCache* cache = nsXULPrototypeCache::GetInstance();
       if (cache && cache->IsEnabled()) {
         if (!cache->GetStyleSheet(aLoadData->mURI)) {
           LOG(("  Putting sheet in XUL prototype cache"));
+          NS_ASSERTION(sheet->IsComplete(),
+                       "Should only be caching complete sheets");
           cache->PutStyleSheet(sheet);
         }
       }
     }
     else {
 #endif
       URIPrincipalAndCORSModeHashKey key(aLoadData->mURI,
                                          aLoadData->mLoaderPrincipal,
                                          aLoadData->mSheet->GetCORSMode());
+      NS_ASSERTION(sheet->IsComplete(),
+                   "Should only be caching complete sheets");
       mCompleteSheets.Put(&key, sheet);
 #ifdef MOZ_XUL
     }
 #endif
   }
 
   NS_RELEASE(aLoadData);  // this will release parents and siblings and all that
 }
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/867487.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="stylesheet" href="data:text/css,">
+<script>
+
+function boom()
+{
+    var s = document.styleSheets[0];
+    var n = s.ownerNode;
+    var p = n.parentNode;
+
+    s.insertRule("#a { }", 0);
+
+    for (var i = 0; i < 3; ++i) {
+        p.removeChild(n);
+        p.appendChild(n);
+    }
+}
+
+</script>
+</head>
+<body onload="boom();"></body>
+</html>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -85,8 +85,9 @@ load 788836.html
 load 806310-1.html
 load 812824.html
 load 822842.html
 load 822766-1.html
 load 827591-1.html
 load 840898.html
 load 842134.html
 load 862113.html
+load 867487.html