Bug 836567 - Part 2: Don't switch away from preload browser on loading javascript: URIs. r?bz draft
authorSamael Wang <freesamael@gmail.com>
Tue, 07 Nov 2017 15:27:05 +0800
changeset 704050 284f75f1cb0b2fd41403f78af57ed1649034aee1
parent 704049 ec6b203b3b439d39338d96ea62d7d563e404f7c5
child 704051 183877c9e68798c3143bbd5bb3e1b48cc83d4de7
push id91050
push userbmo:sawang@mozilla.com
push dateTue, 28 Nov 2017 06:08:27 +0000
reviewersbz
bugs836567
milestone58.0a1
Bug 836567 - Part 2: Don't switch away from preload browser on loading javascript: URIs. r?bz When loading happens in a preloaded about:newtab tab, it would trigger RedirectLoad to switch process. This is not appropriate for javascript: URIs, since it's supposed to take side effects on current page. MozReview-Commit-ID: 2vgVr4hN6R7
browser/base/content/browser.js
browser/modules/E10SUtils.jsm
dom/base/test/browser.ini
dom/base/test/browser_aboutnewtab_javascript_uri.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1044,19 +1044,22 @@ function _loadURIWithFlags(browser, uri,
     requiredRemoteType = gMultiProcessBrowser ? E10SUtils.DEFAULT_REMOTE_TYPE
                                               : E10SUtils.NOT_REMOTE;
   }
 
   let mustChangeProcess = requiredRemoteType != currentRemoteType;
   let newFrameloader = false;
   if (browser.getAttribute("isPreloadBrowser") == "true" && uri != "about:newtab") {
     // Leaving about:newtab from a used to be preloaded browser should run the process
-    // selecting algorithm again.
-    mustChangeProcess = true;
-    newFrameloader = true;
+    // selecting algorithm again, unless it's a javascript: URI which should have
+    // side effects on current page.
+    if (!uri.startsWith("javascript:")) {
+      mustChangeProcess = true;
+      newFrameloader = true;
+    }
     browser.removeAttribute("isPreloadBrowser");
   }
 
   // !requiredRemoteType means we're loading in the parent/this process.
   if (!requiredRemoteType) {
     browser.inLoadURI = true;
   }
   try {
--- a/browser/modules/E10SUtils.jsm
+++ b/browser/modules/E10SUtils.jsm
@@ -236,17 +236,19 @@ this.E10SUtils = {
 
       // If not originally loaded in this process allow it if the URI would
       // normally be allowed to load in this process by default.
       let remoteType = Services.appinfo.remoteType;
       return remoteType ==
         this.getRemoteTypeForURIObject(aURI, true, remoteType, webNav.currentURI);
     }
 
-    if (sessionHistory.count == 1 && webNav.currentURI.spec == "about:newtab") {
+    if (sessionHistory.count == 1 &&
+        webNav.currentURI.spec == "about:newtab" &&
+        aURI.scheme != "javascript") {
       // This is possibly a preloaded browser and we're about to navigate away for
       // the first time. On the child side there is no way to tell for sure if that
       // is the case, so let's redirect this request to the parent to decide if a new
       // process is needed.
       return false;
     }
 
     // If the URI can be loaded in the current process then continue
--- a/dom/base/test/browser.ini
+++ b/dom/base/test/browser.ini
@@ -49,8 +49,9 @@ skip-if = !e10s # this only makes sense 
 [browser_pagehide_on_tab_close.js]
 skip-if = e10s # this tests non-e10s behavior. it's not expected to work in e10s.
 [browser_state_notifications.js]
 skip-if = true # Bug 1271028
 [browser_use_counters.js]
 [browser_timeout_throttling_with_audio_playback.js]
 [browser_bug1303838.js]
 [browser_inputStream_structuredClone.js]
+[browser_aboutnewtab_javascript_uri.js]
new file mode 100644
--- /dev/null
+++ b/dom/base/test/browser_aboutnewtab_javascript_uri.js
@@ -0,0 +1,39 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Ensure that the preloaded browser exists, and it's finished loading.
+async function ensurePreloaded(gBrowser) {
+  gBrowser._createPreloadBrowser();
+  // We cannot use the regular BrowserTestUtils helper for waiting here, since that
+  // would try to insert the preloaded browser, which would only break things.
+  await BrowserTestUtils.waitForCondition( () => {
+    return gBrowser._preloadedBrowser.contentDocument.readyState == "complete";
+  });
+}
+
+// The test case verifies that loading javascript: URL in a preloaded browser
+// wouldn't cause process switching (by checking outerWindowID).
+add_task(async function() {
+  await ensurePreloaded(gBrowser);
+  await BrowserTestUtils.withNewTab({gBrowser, opening: BROWSER_NEW_TAB_URL, waitForLoad: false}, async function(browser) {
+
+    // Before loading javascript:
+    let outerWindowID = browser.outerWindowID;
+    await ContentTask.spawn(browser, [BROWSER_NEW_TAB_URL], function([BROWSER_NEW_TAB_URL]) {
+      is(content.document.documentURI, BROWSER_NEW_TAB_URL, "verify documentURI");
+      is(content.document.querySelector("#test"), undefined, "verify that document has not been modified yet");
+    });
+
+    let locationChangePromise = BrowserTestUtils.waitForLocationChange(gBrowser, BROWSER_NEW_TAB_URL);
+    // <div id="test"></div>
+    browser.loadURI("javascript:'%3Cdiv%20id%3D%22test%22%3E%3C%2Fdiv%3E';");
+    await locationChangePromise;
+
+    // After loaded javascript:
+    is(browser.outerWindowID, outerWindowID, "verify that the outerWindow didn't change");
+    await ContentTask.spawn(browser, [BROWSER_NEW_TAB_URL], function([BROWSER_NEW_TAB_URL]) {
+      is(content.document.documentURI, BROWSER_NEW_TAB_URL, "verify documentURI");
+      ok(content.document.querySelector("#test"), "verify that document has been modified")
+    });
+  });
+});