Bug 1453519 - Extract process selection logic from _loadURI. r=mconley draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Fri, 04 May 2018 15:27:28 -0500
changeset 792286 c3385a8d1b850aebd6daf77aca541acdc53e3b00
parent 791691 b2fe74b7adf9cf76301b21d4417c70dd8b2579ed
child 792287 2ac33c09f47dd71230aa170db7c44bddc696cae1
push id109067
push userbmo:jryans@gmail.com
push dateTue, 08 May 2018 01:13:33 +0000
reviewersmconley
bugs1453519
milestone61.0a1
Bug 1453519 - Extract process selection logic from _loadURI. r=mconley Extract logic around whether a browser needs to change processes, get a new frameloader because of preloading, etc. from `_loadURI` in `browser.js` to `E10SUtils.jsm` where it can be shared with other code paths. The side effect paths (trying to handle in chrome, removing preloaded state) are left behind in `browser.js` so that the `E10SUtils` version can be a pure function. MozReview-Commit-ID: 6LYB3e3U5o8
browser/base/content/browser.js
toolkit/modules/E10SUtils.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1005,51 +1005,30 @@ function _loadURI(browser, uri, params =
     flags = Ci.nsIWebNavigation.LOAD_FLAGS_NONE,
     referrerURI,
     referrerPolicy = Ci.nsIHttpChannel.REFERRER_POLICY_UNSET,
     triggeringPrincipal,
     postData,
     userContextId,
   } = params || {};
 
-  let currentRemoteType = browser.remoteType;
-  let requiredRemoteType;
-  try {
-    let fixupFlags = Ci.nsIURIFixup.FIXUP_FLAG_NONE;
-    if (flags & Ci.nsIWebNavigation.LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP) {
-      fixupFlags |= Ci.nsIURIFixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
-    }
-    if (flags & Ci.nsIWebNavigation.LOAD_FLAGS_FIXUP_SCHEME_TYPOS) {
-      fixupFlags |= Ci.nsIURIFixup.FIXUP_FLAG_FIX_SCHEME_TYPOS;
-    }
-    let uriObject = Services.uriFixup.createFixupURI(uri, fixupFlags);
-    if (handleUriInChrome(browser, uriObject)) {
-      // If we've handled the URI in Chrome then just return here.
-      return;
-    }
-
-    // Note that I had thought that we could set uri = uriObject.spec here, to
-    // save on fixup later on, but that changes behavior and breaks tests.
-    requiredRemoteType =
-      E10SUtils.getRemoteTypeForURIObject(uriObject, gMultiProcessBrowser,
-                                          currentRemoteType, browser.currentURI);
-  } catch (e) {
-    // createFixupURI throws if it can't create a URI. If that's the case then
-    // we still need to pass down the uri because docshell handles this case.
-    requiredRemoteType = gMultiProcessBrowser ? E10SUtils.DEFAULT_REMOTE_TYPE
-                                              : E10SUtils.NOT_REMOTE;
-  }
-
-  let mustChangeProcess = requiredRemoteType != currentRemoteType;
-  let newFrameloader = false;
-  if (browser.getAttribute("preloadedState") === "consumed" && uri != "about:newtab") {
-    // Leaving about:newtab from a used to be preloaded browser should run the process
-    // selecting algorithm again.
-    mustChangeProcess = true;
-    newFrameloader = true;
+  let {
+    uriObject,
+    requiredRemoteType,
+    mustChangeProcess,
+    newFrameloader,
+  } = E10SUtils.shouldLoadURIInBrowser(browser, uri, gMultiProcessBrowser,
+                                       flags);
+  if (uriObject && handleUriInChrome(browser, uriObject)) {
+    // If we've handled the URI in Chrome then just return here.
+    return;
+  }
+  if (newFrameloader) {
+    // If a new frameloader is needed for process reselection because this used
+    // to be a preloaded browser, clear the preloaded state now.
     browser.removeAttribute("preloadedState");
   }
 
   // !requiredRemoteType means we're loading in the parent/this process.
   if (!requiredRemoteType) {
     browser.inLoadURI = true;
   }
   try {
--- a/toolkit/modules/E10SUtils.jsm
+++ b/toolkit/modules/E10SUtils.jsm
@@ -195,16 +195,59 @@ var E10SUtils = {
                                                 aPreferredRemoteType,
                                                 aCurrentUri);
         }
 
         return validatedWebRemoteType(aPreferredRemoteType, aURI, aCurrentUri);
     }
   },
 
+  shouldLoadURIInBrowser(browser, uri, multiProcess = true,
+                         flags = Ci.nsIWebNavigation.LOAD_FLAGS_NONE) {
+    let currentRemoteType = browser.remoteType;
+    let requiredRemoteType;
+    let uriObject;
+    try {
+      let fixupFlags = Ci.nsIURIFixup.FIXUP_FLAG_NONE;
+      if (flags & Ci.nsIWebNavigation.LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP) {
+        fixupFlags |= Ci.nsIURIFixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
+      }
+      if (flags & Ci.nsIWebNavigation.LOAD_FLAGS_FIXUP_SCHEME_TYPOS) {
+        fixupFlags |= Ci.nsIURIFixup.FIXUP_FLAG_FIX_SCHEME_TYPOS;
+      }
+      uriObject = Services.uriFixup.createFixupURI(uri, fixupFlags);
+      // Note that I had thought that we could set uri = uriObject.spec here, to
+      // save on fixup later on, but that changes behavior and breaks tests.
+      requiredRemoteType =
+        this.getRemoteTypeForURIObject(uriObject, multiProcess,
+                                       currentRemoteType, browser.currentURI);
+    } catch (e) {
+      // createFixupURI throws if it can't create a URI. If that's the case then
+      // we still need to pass down the uri because docshell handles this case.
+      requiredRemoteType = multiProcess ? DEFAULT_REMOTE_TYPE : NOT_REMOTE;
+    }
+
+    let mustChangeProcess = requiredRemoteType != currentRemoteType;
+    let newFrameloader = false;
+    if (browser.getAttribute("preloadedState") === "consumed" &&
+        uri != "about:newtab") {
+      // Leaving about:newtab from a used to be preloaded browser should run the process
+      // selecting algorithm again.
+      mustChangeProcess = true;
+      newFrameloader = true;
+    }
+
+    return {
+      uriObject,
+      requiredRemoteType,
+      mustChangeProcess,
+      newFrameloader,
+    };
+  },
+
   shouldLoadURIInThisProcess(aURI) {
     let remoteType = Services.appinfo.remoteType;
     return remoteType == this.getRemoteTypeForURIObject(aURI, true, remoteType);
   },
 
   shouldLoadURI(aDocShell, aURI, aReferrer, aHasPostData) {
     // Inner frames should always load in the current process
     if (aDocShell.QueryInterface(Ci.nsIDocShellTreeItem).sameTypeParent)