Bug 645998. Improve the recursion detection in the CSS loader to detect mutual recursion scenarios. r=sicking
authorBoris Zbarsky <bzbarsky@mit.edu>
Sun, 07 Aug 2011 22:23:51 -0400
changeset 73981 3b5da2d845387075947c25c91a554d01803b70e1
parent 73980 fd30726145d5db2f40e82e81c656402cc336ed81
child 73982 9954f0235bc83ce3d0ea4c70e05f6998c8e892b4
push id20936
push userkhuey@mozilla.com
push dateMon, 08 Aug 2011 12:19:50 +0000
treeherdermozilla-central@9c7ab0a15292 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs645998
milestone8.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 645998. Improve the recursion detection in the CSS loader to detect mutual recursion scenarios. r=sicking
layout/style/Loader.cpp
layout/style/test/Makefile.in
layout/style/test/file_bug645998-1.css
layout/style/test/file_bug645998-2.css
layout/style/test/test_bug645998.html
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -1864,16 +1864,46 @@ Loader::LoadStyleLink(nsIContent* aEleme
   // Load completion will free the data
   rv = LoadSheet(data, state);
   NS_ENSURE_SUCCESS(rv, rv);
 
   data->mMustNotify = PR_TRUE;
   return rv;
 }
 
+static PRBool
+HaveAncestorDataWithURI(SheetLoadData *aData, nsIURI *aURI)
+{
+  if (!aData->mURI) {
+    // Inline style; this won't have any ancestors
+    NS_ABORT_IF_FALSE(!aData->mParentData,
+                      "How does inline style have a parent?");
+    return PR_FALSE;
+  }
+
+  PRBool equal;
+  if (NS_FAILED(aData->mURI->Equals(aURI, &equal)) || equal) {
+    return PR_TRUE;
+  }
+
+  // Datas down the mNext chain have the same URI as aData, so we
+  // don't have to compare to them.  But they might have different
+  // parents, and we have to check all of those.
+  while (aData) {
+    if (aData->mParentData &&
+        HaveAncestorDataWithURI(aData->mParentData, aURI)) {
+      return PR_TRUE;
+    }
+
+    aData = aData->mNext;
+  }
+
+  return PR_FALSE;
+}
+
 nsresult
 Loader::LoadChildSheet(nsCSSStyleSheet* aParentSheet,
                        nsIURI* aURL,
                        nsMediaList* aMedia,
                        ImportRule* aParentRule)
 {
   LOG(("css::Loader::LoadChildSheet"));
   NS_PRECONDITION(aURL, "Must have a URI to load");
@@ -1918,26 +1948,21 @@ Loader::LoadChildSheet(nsCSSStyleSheet* 
   SheetLoadData* parentData = nsnull;
   nsCOMPtr<nsICSSLoaderObserver> observer;
 
   PRInt32 count = mParsingDatas.Length();
   if (count > 0) {
     LOG(("  Have a parent load"));
     parentData = mParsingDatas.ElementAt(count - 1);
     // Check for cycles
-    SheetLoadData* data = parentData;
-    while (data && data->mURI) {
-      PRBool equal;
-      if (NS_SUCCEEDED(data->mURI->Equals(aURL, &equal)) && equal) {
-        // Houston, we have a loop, blow off this child and pretend this never
-        // happened
-        LOG_ERROR(("  @import cycle detected, dropping load"));
-        return NS_OK;
-      }
-      data = data->mParentData;
+    if (HaveAncestorDataWithURI(parentData, aURL)) {
+      // Houston, we have a loop, blow off this child and pretend this never
+      // happened
+      LOG_ERROR(("  @import cycle detected, dropping load"));
+      return NS_OK;
     }
 
     NS_ASSERTION(parentData->mSheet == aParentSheet,
                  "Unexpected call to LoadChildSheet");
   } else {
     LOG(("  No parent load; must be CSSOM"));
     // 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.
--- a/layout/style/test/Makefile.in
+++ b/layout/style/test/Makefile.in
@@ -124,16 +124,19 @@ GARBAGE += css_properties.js
 		test_bug534804.html \
 		test_bug573255.html \
 		test_bug580685.html \
 		test_bug635286.html \
 		test_bug652486.html \
 		test_bug657143.html \
 		test_bug664955.html \
 		test_bug667520.html \
+		test_bug645998.html \
+		file_bug645998-1.css \
+		file_bug645998-2.css \
 		test_cascade.html \
 		test_compute_data_with_start_struct.html \
 		test_computed_style.html \
 		test_computed_style_no_pseudo.html \
 		test_css_cross_domain.html \
 		test_css_eof_handling.html \
 		test_descriptor_storage.html \
 		test_descriptor_syntax_errors.html \
new file mode 100644
--- /dev/null
+++ b/layout/style/test/file_bug645998-1.css
@@ -0,0 +1,1 @@
+@import url("file_bug645998-2.css");
new file mode 100644
--- /dev/null
+++ b/layout/style/test/file_bug645998-2.css
@@ -0,0 +1,1 @@
+@import url("file_bug645998-1.css");
new file mode 100644
--- /dev/null
+++ b/layout/style/test/test_bug645998.html
@@ -0,0 +1,30 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=645998
+-->
+<head>
+  <title>Test for Bug 645998</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <!-- This is the real test: will these stylesheets ever finish loading? -->
+  <link rel="stylesheet" href="file_bug645998-1.css">
+  <link rel="stylesheet" href="file_bug645998-2.css">
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=645998">Mozilla Bug 645998</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 645998 **/
+ok(true, "Hey, we got here!  That's a good sign");
+
+</script>
+</pre>
+</body>
+</html>