Bug 1326779 - [e10s] Fire beforeunload event when navigating to a page in different process. f=samael, r=smaug
authorJessica Jong <jjong@mozilla.com>
Fri, 20 Jan 2017 00:47:00 -0500
changeset 353719 e4f09e1128d9a7748d74cd5d0372ac5e56b66df9
parent 353718 bb61bdb559de94ca42d86b4d619dc038a15e4661
child 353720 614d5cdce0beeea2fab025e3c1e6977c85f25574
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1326779
milestone52.0a2
Bug 1326779 - [e10s] Fire beforeunload event when navigating to a page in different process. f=samael, r=smaug
browser/base/content/browser.js
docshell/base/nsDocShell.cpp
dom/tests/browser/browser.ini
dom/tests/browser/browser_beforeunload_between_chrome_content.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -844,16 +844,22 @@ function _loadURIWithFlags(browser, uri,
       if (params.userContextId) {
         browser.webNavigation.setOriginAttributesBeforeLoading({ userContextId: params.userContextId });
       }
 
       browser.webNavigation.loadURIWithOptions(uri, flags,
                                                referrer, referrerPolicy,
                                                postData, null, null);
     } else {
+      // Check if the current browser is allowed to unload.
+      let {permitUnload, timedOut} = browser.permitUnload();
+      if (!timedOut && !permitUnload) {
+        return;
+      }
+
       if (postData) {
         postData = NetUtil.readInputStreamToString(postData, postData.available());
       }
 
       let loadParams = {
         uri: uri,
         flags: flags,
         referrer: referrer ? referrer.spec : null,
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -10505,35 +10505,16 @@ nsDocShell::InternalLoad(nsIURI* aURI,
           win->DispatchAsyncHashchange(currentURI, aURI);
         }
       }
 
       return NS_OK;
     }
   }
 
-  // Check if the webbrowser chrome wants the load to proceed; this can be
-  // used to cancel attempts to load URIs in the wrong process.
-  nsCOMPtr<nsIWebBrowserChrome3> browserChrome3 = do_GetInterface(mTreeOwner);
-  if (browserChrome3) {
-    // In case this is a remote newtab load, set aURI to aOriginalURI (newtab).
-    // This ensures that the verifySignedContent flag is set on loadInfo in
-    // DoURILoad.
-    nsIURI* uriForShouldLoadCheck = aURI;
-    if (IsAboutNewtab(aOriginalURI)) {
-      uriForShouldLoadCheck = aOriginalURI;
-    }
-    bool shouldLoad;
-    rv = browserChrome3->ShouldLoadURI(this, uriForShouldLoadCheck, aReferrer,
-                                       &shouldLoad);
-    if (NS_SUCCEEDED(rv) && !shouldLoad) {
-      return NS_OK;
-    }
-  }
-
   // mContentViewer->PermitUnload can destroy |this| docShell, which
   // causes the next call of CanSavePresentation to crash.
   // Hold onto |this| until we return, to prevent a crash from happening.
   // (bug#331040)
   nsCOMPtr<nsIDocShell> kungFuDeathGrip(this);
 
   // Don't init timing for javascript:, since it generally doesn't
   // actually start a load or anything.  If it does, we'll init
@@ -10563,16 +10544,35 @@ nsDocShell::InternalLoad(nsIURI* aURI,
       return NS_OK;
     }
   }
 
   if (mTiming && timeBeforeUnload) {
     mTiming->NotifyUnloadAccepted(mCurrentURI);
   }
 
+  // Check if the webbrowser chrome wants the load to proceed; this can be
+  // used to cancel attempts to load URIs in the wrong process.
+  nsCOMPtr<nsIWebBrowserChrome3> browserChrome3 = do_GetInterface(mTreeOwner);
+  if (browserChrome3) {
+    // In case this is a remote newtab load, set aURI to aOriginalURI (newtab).
+    // This ensures that the verifySignedContent flag is set on loadInfo in
+    // DoURILoad.
+    nsIURI* uriForShouldLoadCheck = aURI;
+    if (IsAboutNewtab(aOriginalURI)) {
+      uriForShouldLoadCheck = aOriginalURI;
+    }
+    bool shouldLoad;
+    rv = browserChrome3->ShouldLoadURI(this, uriForShouldLoadCheck, aReferrer,
+                                       &shouldLoad);
+    if (NS_SUCCEEDED(rv) && !shouldLoad) {
+      return NS_OK;
+    }
+  }
+
   // Whenever a top-level browsing context is navigated, the user agent MUST
   // lock the orientation of the document to the document's default
   // orientation. We don't explicitly check for a top-level browsing context
   // here because orientation is only set on top-level browsing contexts.
   // We make an exception for apps because they currently rely on
   // orientation locks persisting across browsing contexts.
   if (OrientationLock() != eScreenOrientation_None && !GetIsApp()) {
 #ifdef DEBUG
--- a/dom/tests/browser/browser.ini
+++ b/dom/tests/browser/browser.ini
@@ -12,16 +12,18 @@ support-files =
   test_largeAllocation.html^headers^
   !/dom/tests/mochitest/geolocation/network_geolocation.sjs
 
 [browser_allocateGigabyte.js]
 disabled = Does not reliably pass on 32-bit systems - bug 1314098
 skip-if = !e10s
 [browser_autofocus_background.js]
 [browser_autofocus_preference.js]
+[browser_beforeunload_between_chrome_content.js]
+skip-if = !e10s
 [browser_bug396843.js]
 [browser_bug1004814.js]
 [browser_bug1008941_dismissGeolocationHanger.js]
 [browser_bug1238427.js]
 [browser_ConsoleAPI_originAttributes.js]
 [browser_ConsoleAPITests.js]
 skip-if = e10s
 [browser_ConsoleStorageAPITests.js]
new file mode 100644
--- /dev/null
+++ b/dom/tests/browser/browser_beforeunload_between_chrome_content.js
@@ -0,0 +1,144 @@
+const TEST_URL = "http://www.example.com/browser/dom/tests/browser/dummy.html";
+
+function pageScript() {
+  window.addEventListener("beforeunload", function (event) {
+    var str = "Leaving?";
+    event.returnValue = str;
+    return str;
+  }, true);
+}
+
+function frameScript() {
+  content.window.addEventListener("beforeunload", function (event) {
+    sendAsyncMessage("Test:OnBeforeUnloadReceived");
+    var str = "Leaving?";
+    event.returnValue = str;
+    return str;
+  }, true);
+}
+
+// Wait for onbeforeunload dialog, and dismiss it immediately.
+function awaitAndCloseBeforeUnloadDialog(doStayOnPage) {
+  return new Promise(resolve => {
+    function onDialogShown(node) {
+      Services.obs.removeObserver(onDialogShown, "tabmodal-dialog-loaded");
+      let button = doStayOnPage ? node.ui.button1 : node.ui.button0;
+      button.click();
+      resolve();
+    }
+
+    Services.obs.addObserver(onDialogShown, "tabmodal-dialog-loaded");
+  });
+}
+
+SpecialPowers.pushPrefEnv(
+  {"set": [["dom.require_user_interaction_for_beforeunload", false]]});
+
+/**
+ * Test navigation from a content page to a chrome page. Also check that only
+ * one beforeunload event is fired.
+ */
+add_task(function* () {
+  let beforeUnloadCount = 0;
+  messageManager.addMessageListener("Test:OnBeforeUnloadReceived", function() {
+    beforeUnloadCount++;
+  });
+
+  // Open a content page.
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
+  let browser = tab.linkedBrowser;
+
+  ok(browser.isRemoteBrowser, "Browser should be remote.");
+  browser.messageManager.loadFrameScript(
+    "data:,(" + frameScript.toString() + ")();", true);
+
+  // Navigate to a chrome page.
+  let dialogShown1 = awaitAndCloseBeforeUnloadDialog(false);
+  yield BrowserTestUtils.loadURI(browser, "about:support");
+  yield Promise.all([
+    dialogShown1,
+    BrowserTestUtils.browserLoaded(browser)
+  ]);
+  is(beforeUnloadCount, 1, "Should have received one beforeunload event.");
+  ok(!browser.isRemoteBrowser, "Browser should not be remote.");
+
+  // Go back to content page.
+  ok(gBrowser.webNavigation.canGoBack, "Should be able to go back.");
+  gBrowser.goBack();
+  yield BrowserTestUtils.browserLoaded(browser);
+  browser.messageManager.loadFrameScript(
+    "data:,(" + frameScript.toString() + ")();", true);
+
+  // Test that going forward triggers beforeunload prompt as well.
+  ok(gBrowser.webNavigation.canGoForward, "Should be able to go forward.");
+  let dialogShown2 = awaitAndCloseBeforeUnloadDialog(false);
+  gBrowser.goForward();
+  yield Promise.all([
+    dialogShown2,
+    BrowserTestUtils.browserLoaded(browser)
+  ]);
+  is(beforeUnloadCount, 2, "Should have received two beforeunload events.");
+
+  yield BrowserTestUtils.removeTab(tab);
+});
+
+/**
+ * Test navigation from a chrome page to a content page. Also check that only
+ * one beforeunload event is fired.
+ */
+add_task(function* () {
+  let beforeUnloadCount = 0;
+  messageManager.addMessageListener("Test:OnBeforeUnloadReceived", function() {
+    beforeUnloadCount++;
+  });
+
+  // Open a chrome page.
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser,
+                                                        "about:support");
+  let browser = tab.linkedBrowser;
+
+  ok(!browser.isRemoteBrowser, "Browser should not be remote.");
+  yield ContentTask.spawn(browser, null, function* () {
+    content.window.addEventListener("beforeunload", function (event) {
+      sendAsyncMessage("Test:OnBeforeUnloadReceived");
+      var str = "Leaving?";
+      event.returnValue = str;
+      return str;
+    }, true);
+  });
+
+  // Navigate to a content page.
+  let dialogShown1 = awaitAndCloseBeforeUnloadDialog(false);
+  yield BrowserTestUtils.loadURI(browser, TEST_URL);
+  yield Promise.all([
+    dialogShown1,
+    BrowserTestUtils.browserLoaded(browser)
+  ]);
+  is(beforeUnloadCount, 1, "Should have received one beforeunload event.");
+  ok(browser.isRemoteBrowser, "Browser should be remote.");
+
+  // Go back to chrome page.
+  ok(gBrowser.webNavigation.canGoBack, "Should be able to go back.");
+  gBrowser.goBack();
+  yield BrowserTestUtils.browserLoaded(browser);
+  yield ContentTask.spawn(browser, null, function* () {
+    content.window.addEventListener("beforeunload", function (event) {
+      sendAsyncMessage("Test:OnBeforeUnloadReceived");
+      var str = "Leaving?";
+      event.returnValue = str;
+      return str;
+    }, true);
+  });
+
+  // Test that going forward triggers beforeunload prompt as well.
+  ok(gBrowser.webNavigation.canGoForward, "Should be able to go forward.");
+  let dialogShown2 = awaitAndCloseBeforeUnloadDialog(false);
+  gBrowser.goForward();
+  yield Promise.all([
+    dialogShown2,
+    BrowserTestUtils.browserLoaded(browser)
+  ]);
+  is(beforeUnloadCount, 2, "Should have received two beforeunload events.");
+
+  yield BrowserTestUtils.removeTab(tab);
+});