Bug 1432396 - Do not process nsDocShell::Destroy() if the docshell is already being destroyed. r?bz draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Fri, 16 Feb 2018 06:15:04 +0900
changeset 755927 1b8033cc9c3e2e32b3c1d0cafef28a56edd7cb84
parent 755926 3e5075e84c0d6aa06817910b2a693513b81dedb1
child 755928 b10b6eaad50626fd5c5aaeddb5df443803a038b5
push id99325
push userhikezoe@mozilla.com
push dateThu, 15 Feb 2018 23:56:43 +0000
reviewersbz
bugs1432396
milestone60.0a1
Bug 1432396 - Do not process nsDocShell::Destroy() if the docshell is already being destroyed. r?bz The test case in this patch is harmless without this fix, no assertion happens, no failure happens in the test, but nsDocShell::Destroy() is processed twice. MozReview-Commit-ID: 2g949emc7at
docshell/base/nsDocShell.cpp
docshell/test/mochitest.ini
docshell/test/test_close_onpagehide_by_window_close.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -5418,28 +5418,30 @@ nsDocShell::Create()
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocShell::Destroy()
 {
+  if (mIsBeingDestroyed) {
+    return NS_ERROR_DOCSHELL_DYING;
+  }
+
   NS_ASSERTION(mItemType == typeContent || mItemType == typeChrome,
                "Unexpected item type in docshell");
 
   AssertOriginAttributesMatchPrivateBrowsing();
 
-  if (!mIsBeingDestroyed) {
-    nsCOMPtr<nsIObserverService> serv = services::GetObserverService();
-    if (serv) {
-      const char* msg = mItemType == typeContent ?
-        NS_WEBNAVIGATION_DESTROY : NS_CHROME_WEBNAVIGATION_DESTROY;
-      serv->NotifyObservers(GetAsSupports(this), msg, nullptr);
-    }
+  nsCOMPtr<nsIObserverService> serv = services::GetObserverService();
+  if (serv) {
+    const char* msg = mItemType == typeContent ?
+      NS_WEBNAVIGATION_DESTROY : NS_CHROME_WEBNAVIGATION_DESTROY;
+    serv->NotifyObservers(GetAsSupports(this), msg, nullptr);
   }
 
   mIsBeingDestroyed = true;
 
   // Brak the cycle with the initial client, if present.
   mInitialClientSource.reset();
 
   // Make sure we don't record profile timeline markers anymore
--- a/docshell/test/mochitest.ini
+++ b/docshell/test/mochitest.ini
@@ -102,15 +102,16 @@ support-files = file_bug675587.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_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]
 [test_triggeringprincipal_location_seturi.html]
new file mode 100644
--- /dev/null
+++ b/docshell/test/test_close_onpagehide_by_window_close.html
@@ -0,0 +1,20 @@
+<!doctype html>
+<title>Test for closing window in pagehide event callback caused by window.close()</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");
+  w.onpagehide = () => {
+    w.close();
+    info("try to close the popped up window in onpagehide");
+    SimpleTest.finish();
+  };
+  w.close();
+}, { once: true });
+</script>