Bug 1432396 - Bail out if the docshell is being destroyed due to pagehide notification. r?bz
There are four call sites of FirePageHideNotification(). Two of them are
handled in this patch. The other two will be taken care of in the subsequent
patches in this patch series respectively.
Without this fix, the test case in this patch causes assertions when
cycle collection happens.
MozReview-Commit-ID: 6GSxjdfXGcY
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -7710,16 +7710,20 @@ nsDocShell::CreateAboutBlankContentViewe
// Notify the current document that it is about to be unloaded!!
//
// It is important to fire the unload() notification *before* any state
// is changed within the DocShell - otherwise, javascript will get the
// wrong information :-(
//
(void)FirePageHideNotification(!mSavingOldViewer);
+ // pagehide notification might destroy this docshell.
+ if (mIsBeingDestroyed) {
+ return NS_ERROR_DOCSHELL_DYING;
+ }
}
// Now make sure we don't think we're in the middle of firing unload after
// this point. This will make us fire unload when the about:blank document
// unloads... but that's ok, more or less. Would be nice if it fired load
// too, of course.
mFiredUnloadEvent = false;
@@ -8241,16 +8245,20 @@ nsDocShell::RestoreFromHistory()
nsCOMPtr<nsISHEntry> origLSHE = mLSHE;
// Make sure to blow away our mLoadingURI just in case. No loads
// from inside this pagehide.
mLoadingURI = nullptr;
// Notify the old content viewer that it's being hidden.
FirePageHideNotification(!mSavingOldViewer);
+ // pagehide notification might destroy this docshell.
+ if (mIsBeingDestroyed) {
+ return NS_ERROR_DOCSHELL_DYING;
+ }
// If mLSHE was changed as a result of the pagehide event, then
// something else was loaded. Don't finish restoring.
if (mLSHE != origLSHE) {
return NS_OK;
}
// Add the request to our load group. We do this before swapping out
new file mode 100644
--- /dev/null
+++ b/docshell/test/file_close_onpagehide1.html
@@ -0,0 +1,5 @@
+<script>
+ window.onload = () => {
+ opener.postMessage("initial", "*");
+ }
+</script>
new file mode 100644
--- /dev/null
+++ b/docshell/test/file_close_onpagehide2.html
@@ -0,0 +1,5 @@
+<script>
+ window.onload = () => {
+ opener.postMessage("second", "*");
+ }
+</script>;
--- a/docshell/test/mochitest.ini
+++ b/docshell/test/mochitest.ini
@@ -41,16 +41,18 @@ 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_close_onpagehide1.html
+ file_close_onpagehide2.html
file_pushState_after_document_open.html
historyframes.html
start_historyframe.html
url1_historyframe.html
url2_historyframe.html
[test_anchor_scroll_after_document_open.html]
[test_bfcache_plus_hash.html]
@@ -99,15 +101,16 @@ 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_close_onpagehide_by_history_back.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]
[test_triggeringprincipal_location_seturi.html]
new file mode 100644
--- /dev/null
+++ b/docshell/test/test_close_onpagehide_by_history_back.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<title>Test for closing window in pagehide event callback caused by history.back()</title>
+<script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1432396">Mozilla Bug 1432396</a>
+<p id="display"></p>
+<script>
+SimpleTest.waitForExplicitFinish();
+
+const w = window.open("file_close_onpagehide1.html");
+window.addEventListener("message", e => {
+ is(e.data, "initial", "The initial page loaded");
+ window.addEventListener("message", e => {
+ is(e.data, "second", "The second page loaded");
+ w.onpagehide = () => {
+ w.close();
+ info("try to close the popped up window in onpagehide");
+ SimpleTest.finish();
+ };
+ w.history.back();
+ }, { once: true });
+ w.location = "file_close_onpagehide2.html";
+}, { once: true });
+</script>