Bug 1450164 - Don't update URIs on history adds on docshells that are being shut down; r=bzbarsky
authorKyle Machulis <kyle@nonpolynomial.com>
Thu, 02 Aug 2018 00:51:09 +0000
changeset 484776 3317065f14c6837a1560adb69ecb177421c85a7a
parent 484775 fbbb0327f0f1a70336548ca7655b4f60cf847d9d
child 484777 9c9b31d0ed536a7e694c0a5894dd843b605bc9f2
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1450164
milestone63.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 1450164 - Don't update URIs on history adds on docshells that are being shut down; r=bzbarsky If we add to history on a docshell that is being shut down, add history entry but skip trying to load the new URI. MozReview-Commit-ID: JCF9muhxbFd Differential Revision: https://phabricator.services.mozilla.com/D2311
docshell/base/nsDocShell.cpp
docshell/test/file_bug1450164.html
docshell/test/mochitest.ini
docshell/test/test_bug1450164.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -11746,17 +11746,20 @@ nsDocShell::AddState(JS::Handle<JS::Valu
   // explicitly call FireOnLocationChange if we're not calling SetCurrentURI,
   // since SetCurrentURI will call FireOnLocationChange for us.
   //
   // Both SetCurrentURI(...) and FireDummyOnLocationChange() pass
   // nullptr for aRequest param to FireOnLocationChange(...). Such an update
   // notification is allowed only when we know docshell is not loading a new
   // document and it requires LOCATION_CHANGE_SAME_DOCUMENT flag. Otherwise,
   // FireOnLocationChange(...) breaks security UI.
-  if (!equalURIs) {
+  //
+  // If the docshell is shutting down, don't update the document URI, as we
+  // can't load into a docshell that is being destroyed.
+  if (!equalURIs && !mIsBeingDestroyed) {
     document->SetDocumentURI(newURI);
     // We can't trust SetCurrentURI to do always fire locationchange events
     // when we expect it to, so we hack around that by doing it ourselves...
     SetCurrentURI(newURI, nullptr, false, LOCATION_CHANGE_SAME_DOCUMENT);
     if (mLoadType != LOAD_ERROR_PAGE) {
       FireDummyOnLocationChange();
     }
 
new file mode 100644
--- /dev/null
+++ b/docshell/test/file_bug1450164.html
@@ -0,0 +1,16 @@
+<html>
+  <head>
+    <script>
+     function go() {
+       var a = window.history.state;
+       window.history.replaceState(a,"","1");
+       var ok = opener.ok;
+       var SimpleTest = opener.SimpleTest;
+       ok("Addition of history in unload did not crash browser");
+       SimpleTest.finish();
+     }
+    </script>
+  </head>
+  <body onunload="go()">
+  </body>
+</html>
--- a/docshell/test/mochitest.ini
+++ b/docshell/test/mochitest.ini
@@ -38,16 +38,17 @@ support-files =
   file_bug669671.sjs
   file_bug680257.html
   file_bug703855.html
   file_bug728939.html
   file_bug1121701_1.html
   file_bug1121701_2.html
   file_bug1186774.html
   file_bug1151421.html
+  file_bug1450164.html
   file_close_onpagehide1.html
   file_close_onpagehide2.html
   file_pushState_after_document_open.html
   historyframes.html
   start_historyframe.html
   url1_historyframe.html
   url2_historyframe.html
 
@@ -99,16 +100,17 @@ support-files = file_bug675587.html
 [test_bug694612.html]
 [test_bug703855.html]
 [test_bug728939.html]
 [test_bug797909.html]
 [test_bug1045096.html]
 [test_bug1121701.html]
 [test_bug1151421.html]
 [test_bug1186774.html]
+[test_bug1450164.html]
 [test_close_onpagehide_by_history_back.html]
 [test_close_onpagehide_by_window_close.html]
 [test_forceinheritprincipal_overrule_owner.html]
 [test_framedhistoryframes.html]
 skip-if = toolkit == 'android' # bug 784321
 support-files = file_framedhistoryframes.html
 [test_pushState_after_document_open.html]
 [test_windowedhistoryframes.html]
new file mode 100644
--- /dev/null
+++ b/docshell/test/test_bug1450164.html
@@ -0,0 +1,31 @@
+<!DOCTYPE HTML>
+<html>
+  <!--
+       https://bugzilla.mozilla.org/show_bug.cgi?id=1450164
+  -->
+  <head>
+    <meta charset="utf-8">
+    <title>Test for Bug 1450164</title>
+    <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+    <script type="application/javascript">
+
+     /** Test for Bug 1450164 **/
+
+     function runTest() {
+       child = window.open("file_bug1450164.html", "", "width=100,height=100");
+       child.onload = function() {
+         // After the window loads, close it. If we don't crash in debug, consider that a pass.
+         child.close();
+       }
+     }
+
+     SimpleTest.waitForExplicitFinish();
+     addLoadEvent(runTest);
+
+    </script>
+  </head>
+  <body>
+    <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1450164">Mozilla Bug 1450164</a>
+  </body>
+</html>