Bug 914521 - Hold a stack reference to mScriptGlobal when dispatching sync events. r=bz
☠☠ backed out by 749739c77f73 ☠ ☠
authorBobby Holley <bobbyholley@gmail.com>
Wed, 11 Sep 2013 22:57:24 -0700
changeset 146763 cc9df767c70b7007b0243aaf6fd340d8376e4503
parent 146762 ad386a1ca2d223515ef1c85cdedfadcf77be248f
child 146764 5a4bb20926182672204818cd5e30461a6521bd60
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbz
bugs914521
milestone26.0a1
Bug 914521 - Hold a stack reference to mScriptGlobal when dispatching sync events. r=bz Note also MMAdeathGrip earlier in the function.
docshell/base/crashtests/914521.html
docshell/base/crashtests/crashtests.list
docshell/base/nsDocShell.cpp
new file mode 100644
--- /dev/null
+++ b/docshell/base/crashtests/914521.html
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<meta charset="UTF-8">
+<script>
+
+function f()
+{
+    function spin() {
+        for (var i = 0; i < 8; ++i) {
+            var x = new XMLHttpRequest();
+            x.open('GET', 'data:text/html,' + i, false);
+            x.send();
+        }
+    }
+
+    window.addEventListener("popstate", spin, false);
+    window.close();
+    window.location = "#c";
+    finish();
+}
+
+function start()
+{
+    var html = "<script>" + f + "<\/script><body onload=f()>";
+    var win = window.open("data:text/html," + encodeURIComponent(html), null, "width=300,height=300");
+    win.finish = function() { document.documentElement.removeAttribute("class"); };
+}
+
+</script>
+</head>
+<body onload="start();"></body>
+</html>
--- a/docshell/base/crashtests/crashtests.list
+++ b/docshell/base/crashtests/crashtests.list
@@ -6,8 +6,9 @@ load 430628-1.html
 load 432114-1.html
 load 432114-2.html
 asserts-if(Android,2) load 436900-1.html
 asserts(0-3) load 436900-2.html # bug 566159
 load 500328-1.html
 load 514779-1.xhtml
 load 614499-1.html
 load 678872-1.html
+pref(dom.disable_open_during_load,false) load 914521.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -9209,28 +9209,33 @@ nsDocShell::InternalLoad(nsIURI * aURI,
             nsCOMPtr<nsIDocument> doc =
               do_GetInterface(GetAsSupports(this));
             NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE);
             doc->SetDocumentURI(aURI);
 
             SetDocCurrentStateObj(mOSHE);
 
             // Dispatch the popstate and hashchange events, as appropriate.
-            if (mScriptGlobal) {
+            //
+            // The event dispatch below can cause us to re-enter script and
+            // destroy the docshell, nulling out mScriptGlobal. Hold a stack
+            // reference to avoid null derefs. See bug 914521.
+            nsRefPtr<nsGlobalWindow> win = mScriptGlobal;
+            if (win) {
                 // Fire a hashchange event URIs differ, and only in their hashes.
                 bool doHashchange = sameExceptHashes && !curHash.Equals(newHash);
 
                 if (historyNavBetweenSameDoc || doHashchange) {
-                    mScriptGlobal->DispatchSyncPopState();
+                    win->DispatchSyncPopState();
                 }
 
                 if (doHashchange) {
                     // Make sure to use oldURI here, not mCurrentURI, because by
                     // now, mCurrentURI has changed!
-                    mScriptGlobal->DispatchAsyncHashchange(oldURI, aURI);
+                    win->DispatchAsyncHashchange(oldURI, aURI);
                 }
             }
 
             // Inform the favicon service that the favicon for oldURI also
             // applies to aURI.
             CopyFavicon(oldURI, aURI, mInPrivateBrowsing);
 
             return NS_OK;