Backed out 5 changesets (bug 1597154) for causing browser-chrome fail…

…ures in browser_UITour_showNewTab.js CLOSED TREE Backed out changeset dc78c6d3d737 (bug 1597154) Backed out changeset 6e82c600d52f (bug 1597154) Backed out changeset 9857504c26e6 (bug 1597154) Backed out changeset e8dccb59bf2a (bug 1597154) Backed out changeset 3c34ca1e2079 (bug 1597154)
dothq · Feb 21, 2020 · f6ff6b7 · f6ff6b7
1 parent 76889b2
commit f6ff6b7
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 51 deletions.
diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
@@ -8981,10 +8981,9 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
   }
 
   // Check if the webbrowser chrome wants the load to proceed; this can be
-  // used to cancel attempts to load URIs in the wrong process. use
-  // GetPendingRedirectedChannel() to avoid revisiting a redirect decision.
+  // used to cancel attempts to load URIs in the wrong process.
   nsCOMPtr<nsIWebBrowserChrome3> browserChrome3 = do_GetInterface(mTreeOwner);
-  if (browserChrome3 && !aLoadState->GetPendingRedirectedChannel()) {
+  if (browserChrome3) {
     bool shouldLoad;
     rv = browserChrome3->ShouldLoadURI(
         this, aLoadState->URI(), aLoadState->GetReferrerInfo(),
@@ -9361,9 +9360,7 @@ static bool IsConsideredSameOriginForUIR(nsIPrincipal* aTriggeringPrincipal,
   return NS_OK;
 }
 
-// Changes here should also be made in
-// E10SUtils.documentChannelPermittedForURI().
-static bool URIUsesDocChannel(nsIURI* aURI) {
+static bool SchemeUsesDocChannel(nsIURI* aURI) {
   if (SchemeIsJavascript(aURI) || NS_IsAboutBlank(aURI)) {
     return false;
   }
@@ -9895,7 +9892,7 @@ nsresult nsDocShell::DoURILoad(nsDocShellLoadState* aLoadState,
   bool canUseDocumentChannel =
       aLoadState->HasLoadFlags(INTERNAL_LOAD_FLAGS_IS_SRCDOC)
           ? (sandboxFlags & SANDBOXED_ORIGIN)
-          : URIUsesDocChannel(aLoadState->URI());
+          : SchemeUsesDocChannel(aLoadState->URI());
 
   if (StaticPrefs::browser_tabs_documentchannel() && XRE_IsContentProcess() &&
       canUseDocumentChannel) {

diff --git a/toolkit/modules/E10SUtils.jsm b/toolkit/modules/E10SUtils.jsm
@@ -137,21 +137,7 @@ const kSafeSchemes = [
   "xmpp",
 ];
 
-const kDocumentChannelDeniedSchemes = ["javascript"];
-const kDocumentChannelDeniedURIs = [
-  "about:blank",
-  "about:printpreview",
-  "about:privatebrowsing",
-  "about:crashcontent",
-];
-
-// Changes here should also be made in URIUsesDocChannel in nsDocShell.cpp.
-function documentChannelPermittedForURI(aURI) {
-  return (
-    !kDocumentChannelDeniedSchemes.includes(aURI.scheme) &&
-    !kDocumentChannelDeniedURIs.includes(aURI.spec)
-  );
-}
+const kDocumentChannelAllowedSchemes = ["http", "https", "ftp", "data"];
 
 // Note that even if the scheme fits the criteria for a web-handled scheme
 // (ie it is compatible with the checks registerProtocolHandler uses), it may
@@ -270,18 +256,14 @@ function validatedWebRemoteType(
 
   if (
     allowLinkedWebInFileUriProcess &&
-    // This is not supported with documentchannel and will go away in
-    // Bug 1603007
+    // This is not supported with documentchannel
     !documentChannel &&
     aPreferredRemoteType == FILE_REMOTE_TYPE
   ) {
     // If aCurrentUri is passed then we should only allow FILE_REMOTE_TYPE
     // when it is same origin as target or the current URI is already a
     // file:// URI.
     if (aCurrentUri) {
-      if (aCurrentUri.scheme == "file" || aCurrentUri.spec == "about:blank") {
-        return FILE_REMOTE_TYPE;
-      }
       try {
         // checkSameOriginURI throws when not same origin.
         // todo: if you intend to update CheckSameOriginURI to log the error to the
@@ -740,10 +722,9 @@ var E10SUtils = {
     // for now, and let DocumentChannel do it during the response.
     if (
       currentRemoteType != NOT_REMOTE &&
-      requiredRemoteType != NOT_REMOTE &&
       uriObject &&
       (remoteSubframes || documentChannel) &&
-      documentChannelPermittedForURI(uriObject)
+      kDocumentChannelAllowedSchemes.includes(uriObject.scheme)
     ) {
       mustChangeProcess = false;
     }
@@ -805,6 +786,20 @@ var E10SUtils = {
       return false;
     }
 
+    // If we are performing HTTP response process selection, and are loading an
+    // HTTP URI, we can start the load in the current process, and then perform
+    // the switch later-on using the RedirectProcessChooser mechanism.
+    //
+    // We should never be sending a POST request from the parent process to a
+    // http(s) uri, so make sure we switch if we're currently in that process.
+    if (
+      Services.appinfo.remoteType != NOT_REMOTE &&
+      (useRemoteSubframes || documentChannel) &&
+      kDocumentChannelAllowedSchemes.includes(aURI.scheme)
+    ) {
+      return true;
+    }
+
     // If we are in a Large-Allocation process, and it wouldn't be content visible
     // to change processes, we want to load into a new process so that we can throw
     // this one out. We don't want to move into a new process if we have post data,
@@ -821,14 +816,6 @@ var E10SUtils = {
       return false;
     }
 
-    let wantRemoteType = this.getRemoteTypeForURIObject(
-      aURI,
-      true,
-      useRemoteSubframes,
-      Services.appinfo.remoteType,
-      webNav.currentURI
-    );
-
     // Allow history load if loaded in this process before.
     let requestedIndex = sessionHistory.legacySHistory.requestedIndex;
     if (requestedIndex >= 0) {
@@ -839,23 +826,23 @@ var E10SUtils = {
         return true;
       }
 
-      return Services.appinfo.remoteType == wantRemoteType;
-    }
-
-    // If we are using DocumentChannel or remote subframes (fission), we
-    // can start the load in the current process, and then perform the
-    // switch later-on using the nsIProcessSwitchRequestor mechanism.
-    if (
-      (useRemoteSubframes || documentChannel) &&
-      Services.appinfo.remoteType != NOT_REMOTE &&
-      wantRemoteType != NOT_REMOTE &&
-      documentChannelPermittedForURI(aURI)
-    ) {
-      return true;
+      // 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,
+          useRemoteSubframes,
+          remoteType,
+          webNav.currentURI
+        )
+      );
     }
 
     // If the URI can be loaded in the current process then continue
-    return Services.appinfo.remoteType == wantRemoteType;
+    return this.shouldLoadURIInThisProcess(aURI, useRemoteSubframes);
   },
 
   redirectLoad(

diff --git a/uriloader/exthandler/tests/mochitest/browser.ini b/uriloader/exthandler/tests/mochitest/browser.ini
@@ -18,5 +18,6 @@ support-files =
   file_with[funny_name.webm
   file_with[funny_name.webm^headers^
 [browser_protocolhandler_loop.js]
+skip-if = fission # Bug 1597154
 [browser_remember_download_option.js]
 [browser_web_protocol_handlers.js]