Backed out 4 changesets (bug 1637869) for Geckoview failures in org.mozilla.geckoview.test.NavigationDelegateTest.extensionProcessSwitching. CLOSED TREE

Backed out changeset a0c0203ee8c9 (bug 1637869)
Backed out changeset 75359ba23865 (bug 1637869)
Backed out changeset d300b61ed89f (bug 1637869)
Backed out changeset 34389f9c86e4 (bug 1637869)

docshell/base/nsAboutRedirector.cpp

@@ -75,8 +75,7 @@ static const RedirEntry kRedirMap[] = {
     {"crashes", "chrome://global/content/crashes.html", 0},
 #endif
     {"credits", "https://www.mozilla.org/credits/",
-     nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT |
+     nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT},
-         nsIAboutModule::URI_MUST_LOAD_IN_CHILD},
     {"license", "chrome://global/content/license.html",
      nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT},
     {"logo", "chrome://branding/content/about.png",

docshell/base/nsDocShell.cpp

@@ -8798,12 +8798,48 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
 
   // In e10s, in the parent process, we refuse to load anything other than
   // "safe" resources that we ship or trust enough to give "special" URLs.
-  // Similar check will be performed by the ParentProcessDocumentChannel if in
+  if (XRE_IsE10sParentProcess()) {
-  // use.
+    nsCOMPtr<nsIURI> uri = aLoadState->URI();
-  if (XRE_IsE10sParentProcess() &&
+    do {
-      !DocumentChannel::CanUseDocumentChannel(aLoadState) &&
+      bool canLoadInParent = false;
-      !CanLoadInParentProcess(aLoadState->URI())) {
+      if (NS_SUCCEEDED(NS_URIChainHasFlags(
-    return NS_ERROR_FAILURE;
+              uri, nsIProtocolHandler::URI_IS_UI_RESOURCE, &canLoadInParent)) &&
+          canLoadInParent) {
+        // We allow UI resources.
+        break;
+      }
+      // For about: and extension-based URIs, which don't get
+      // URI_IS_UI_RESOURCE, first remove layers of view-source:, if present.
+      while (uri && uri->SchemeIs("view-source")) {
+        nsCOMPtr<nsINestedURI> nested = do_QueryInterface(uri);
+        if (nested) {
+          nested->GetInnerURI(getter_AddRefs(uri));
+        } else {
+          break;
+        }
+      }
+      // Allow about: URIs, and allow moz-extension ones if we're running
+      // extension content in the parent process.
+      if (!uri || uri->SchemeIs("about") ||
+          (!StaticPrefs::extensions_webextensions_remote() &&
+           uri->SchemeIs("moz-extension"))) {
+        break;
+      }
+      nsAutoCString scheme;
+      uri->GetScheme(scheme);
+      // Allow ext+foo URIs (extension-registered custom protocols). See
+      // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/protocol_handlers
+      if (StringBeginsWith(scheme, NS_LITERAL_CSTRING("ext+")) &&
+          !StaticPrefs::extensions_webextensions_remote()) {
+        break;
+      }
+      // Final exception for some legacy automated tests:
+      if (xpc::IsInAutomation() &&
+          Preferences::GetBool("security.allow_unsafe_parent_loads", false)) {
+        break;
+      }
+      return NS_ERROR_FAILURE;
+    } while (0);
   }
 
   // Whenever a top-level browsing context is navigated, the user agent MUST
@@ -8964,51 +9000,6 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
   return rv;
 }
 
-/* static */
-bool nsDocShell::CanLoadInParentProcess(nsIURI* aURI) {
-  nsCOMPtr<nsIURI> uri = aURI;
-  // In e10s, in the parent process, we refuse to load anything other than
-  // "safe" resources that we ship or trust enough to give "special" URLs.
-  bool canLoadInParent = false;
-  if (NS_SUCCEEDED(NS_URIChainHasFlags(
-          uri, nsIProtocolHandler::URI_IS_UI_RESOURCE, &canLoadInParent)) &&
-      canLoadInParent) {
-    // We allow UI resources.
-    return true;
-  }
-  // For about: and extension-based URIs, which don't get
-  // URI_IS_UI_RESOURCE, first remove layers of view-source:, if present.
-  while (uri && uri->SchemeIs("view-source")) {
-    nsCOMPtr<nsINestedURI> nested = do_QueryInterface(uri);
-    if (nested) {
-      nested->GetInnerURI(getter_AddRefs(uri));
-    } else {
-      break;
-    }
-  }
-  // Allow about: URIs, and allow moz-extension ones if we're running
-  // extension content in the parent process.
-  if (!uri || uri->SchemeIs("about") ||
-      (!StaticPrefs::extensions_webextensions_remote() &&
-       uri->SchemeIs("moz-extension"))) {
-    return true;
-  }
-  nsAutoCString scheme;
-  uri->GetScheme(scheme);
-  // Allow ext+foo URIs (extension-registered custom protocols). See
-  // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/protocol_handlers
-  if (StringBeginsWith(scheme, NS_LITERAL_CSTRING("ext+")) &&
-      !StaticPrefs::extensions_webextensions_remote()) {
-    return true;
-  }
-  // Final exception for some legacy automated tests:
-  if (xpc::IsInAutomation() &&
-      StaticPrefs::security_allow_unsafe_parent_loads()) {
-    return true;
-  }
-  return false;
-}
-
 nsIPrincipal* nsDocShell::GetInheritedPrincipal(
     bool aConsiderCurrentDocument, bool aConsiderStoragePrincipal) {
   RefPtr<Document> document;

docshell/base/nsDocShell.h

@@ -454,8 +454,6 @@ class nsDocShell final : public nsDocLoader,
     return static_cast<nsDocShell*>(aDocShell);
   }
 
-  static bool CanLoadInParentProcess(nsIURI* aURI);
-
   // Returns true if the current load is a force reload (started by holding
   // shift while triggering reload)
   bool IsForceReloading();

modules/libpref/init/StaticPrefList.yaml

@@ -8409,13 +8409,6 @@
   value: true
   mirror: always
 
-# Disallowed by default, ensure not disallowed content is loaded in the parent
-# process.
-- name: security.allow_unsafe_parent_loads
-  type: bool
-  value: false
-  mirror: always
-
 # Pref to block mixed scripts (fonts, plugin content, scripts, stylesheets,
 # iframes, websockets, XHR).
 - name: security.mixed_content.block_active_content

netwerk/ipc/DocumentLoadListener.cpp

@@ -11,24 +11,22 @@
 #include "mozilla/ContentBlockingAllowList.h"
 #include "mozilla/LoadInfo.h"
 #include "mozilla/MozPromiseInlines.h"  // For MozPromise::FromDomPromise
-#include "mozilla/StaticPrefs_extensions.h"
 #include "mozilla/StaticPrefs_fission.h"
 #include "mozilla/StaticPrefs_security.h"
 #include "mozilla/dom/CanonicalBrowsingContext.h"
 #include "mozilla/dom/ClientChannelHelper.h"
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/ContentProcessManager.h"
-#include "mozilla/dom/SessionHistoryEntry.h"
 #include "mozilla/dom/WindowGlobalParent.h"
 #include "mozilla/dom/ipc/IdType.h"
 #include "mozilla/net/CookieJarSettings.h"
+#include "mozilla/dom/SessionHistoryEntry.h"
 #include "mozilla/net/HttpChannelParent.h"
 #include "mozilla/net/RedirectChannelRegistrar.h"
 #include "mozilla/net/UrlClassifierCommon.h"
 #include "nsContentSecurityUtils.h"
 #include "nsDocShell.h"
 #include "nsDocShellLoadState.h"
-#include "nsDocShellLoadTypes.h"
 #include "nsExternalHelperAppService.h"
 #include "nsHttpChannel.h"
 #include "nsIBrowser.h"
@@ -37,10 +35,12 @@
 #include "nsIViewSourceChannel.h"
 #include "nsImportModule.h"
 #include "nsMimeTypes.h"
+#include "mozilla/dom/CanonicalBrowsingContext.h"
 #include "nsRedirectHistoryEntry.h"
-#include "nsSandboxFlags.h"
 #include "nsURILoader.h"
 #include "nsWebNavigationInfo.h"
+#include "nsDocShellLoadTypes.h"
+#include "nsSandboxFlags.h"
 
 #ifdef ANDROID
 #  include "mozilla/widget/nsWindow.h"
@@ -549,7 +549,6 @@ bool DocumentLoadListener::Open(
   return true;
 }
 
-/* static */
 bool DocumentLoadListener::OpenFromParent(
     dom::CanonicalBrowsingContext* aBrowsingContext,
     nsDocShellLoadState* aLoadState, uint64_t aOuterWindowId,
@@ -821,7 +820,6 @@ void DocumentLoadListener::FinishReplacementChannelSetup(bool aSucceeded) {
 
   if (!mRedirectChannelId) {
     if (!aSucceeded) {
-      mChannel->Cancel(NS_BINDING_ABORTED);
       mChannel->Resume();
       return;
     }
@@ -861,7 +859,6 @@ void DocumentLoadListener::FinishReplacementChannelSetup(bool aSucceeded) {
     if (redirectChannel) {
       redirectChannel->Delete();
     }
-    mChannel->Cancel(NS_BINDING_ABORTED);
     mChannel->Resume();
     if (auto* ctx = GetBrowsingContext()) {
       ctx->EndDocumentLoad(this);
@@ -1176,7 +1173,6 @@ void DocumentLoadListener::SerializeRedirectData(
 }
 
 bool DocumentLoadListener::MaybeTriggerProcessSwitch() {
-  MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_DIAGNOSTIC_ASSERT(!mDoingProcessSwitch,
                         "Already in the middle of switching?");
   MOZ_DIAGNOSTIC_ASSERT(mChannel);
@@ -1200,12 +1196,6 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch() {
     return false;
   }
 
-  if (browsingContext->GetParentWindowContext() &&
-      browsingContext->GetParentWindowContext()->IsInProcess()) {
-    LOG(("Process Switch Abort: Subframe with in-process parent"));
-    return false;
-  }
-
   // We currently can't switch processes for toplevel loads unless they're
   // loaded within a browser tab.
   // FIXME: Ideally we won't do this in the future.
@@ -1255,9 +1245,11 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch() {
           browsingContext->GetCurrentWindowGlobal()) {
     currentPrincipal = wgp->DocumentPrincipal();
   }
-  RefPtr<ContentParent> contentParent = browsingContext->GetContentParent();
+  RefPtr<ContentParent> currentProcess = browsingContext->GetContentParent();
-  MOZ_ASSERT(!OtherPid() || contentParent,
+  if (!currentProcess) {
-             "Only PPDC is allowed to not have an existing ContentParent");
+    LOG(("Process Switch Abort: frame currently not remote"));
+    return false;
+  }
 
   // Get the final principal, used to select which process to load into.
   nsCOMPtr<nsIPrincipal> resultPrincipal;
@@ -1268,6 +1260,11 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch() {
     return false;
   }
 
+  if (resultPrincipal->IsSystemPrincipal()) {
+    LOG(("Process Switch Abort: cannot switch process for system principal"));
+    return false;
+  }
+
   // Determine our COOP status, which will be used to determine our preferred
   // remote type.
   bool isCOOPSwitch = HasCrossOriginOpenerPolicyMismatch();
@@ -1279,13 +1276,7 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch() {
     MOZ_ALWAYS_SUCCEEDS(httpChannel->GetCrossOriginOpenerPolicy(&coop));
   }
 
-  nsAutoString currentRemoteType;
+  nsAutoString preferredRemoteType(currentProcess->GetRemoteType());
-  if (contentParent) {
-    currentRemoteType = contentParent->GetRemoteType();
-  } else {
-    currentRemoteType = VoidString();
-  }
-  nsAutoString preferredRemoteType = currentRemoteType;
   if (coop ==
       nsILoadInfo::OPENER_POLICY_SAME_ORIGIN_EMBEDDER_POLICY_REQUIRE_CORP) {
     // We want documents with SAME_ORIGIN_EMBEDDER_POLICY_REQUIRE_CORP COOP
@@ -1301,13 +1292,13 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch() {
     // remote type. Clear it back to the default value.
     preferredRemoteType.Assign(NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE));
   }
-  MOZ_DIAGNOSTIC_ASSERT(!contentParent || !preferredRemoteType.IsEmpty(),
+  MOZ_DIAGNOSTIC_ASSERT(!preferredRemoteType.IsEmpty(),
                         "Unexpected empty remote type!");
 
   LOG(
       ("DocumentLoadListener GetRemoteTypeForPrincipal "
-       "[this=%p, contentParent=%s, preferredRemoteType=%s]",
+       "[this=%p, currentProcess=%s, preferredRemoteType=%s]",
-       this, NS_ConvertUTF16toUTF8(currentRemoteType).get(),
+       this, NS_ConvertUTF16toUTF8(currentProcess->GetRemoteType()).get(),
        NS_ConvertUTF16toUTF8(preferredRemoteType).get()));
 
   nsCOMPtr<nsIE10SUtils> e10sUtils =
@@ -1319,7 +1310,7 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch() {
 
   nsAutoString remoteType;
   rv = e10sUtils->GetRemoteTypeForPrincipal(
-      resultPrincipal, mChannelCreationURI, browsingContext->UseRemoteTabs(),
+      resultPrincipal, browsingContext->UseRemoteTabs(),
       browsingContext->UseRemoteSubframes(), preferredRemoteType,
       currentPrincipal, browsingContext->GetParent(), remoteType);
   if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -1327,12 +1318,9 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch() {
     return false;
   }
 
-  LOG(("GetRemoteTypeForPrincipal -> current:%s remoteType:%s",
-       NS_ConvertUTF16toUTF8(currentRemoteType).get(),
-       NS_ConvertUTF16toUTF8(remoteType).get()));
-
   // Check if a process switch is needed.
-  if (currentRemoteType == remoteType && !isCOOPSwitch && !isPreloadSwitch) {
+  if (currentProcess->GetRemoteType() == remoteType && !isCOOPSwitch &&
+      !isPreloadSwitch) {
     LOG(("Process Switch Abort: type (%s) is compatible",
          NS_ConvertUTF16toUTF8(remoteType).get()));
     return false;
@@ -1343,7 +1331,7 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch() {
   }
 
   LOG(("Process Switch: Changing Remoteness from '%s' to '%s'",
-       NS_ConvertUTF16toUTF8(currentRemoteType).get(),
+       NS_ConvertUTF16toUTF8(currentProcess->GetRemoteType()).get(),
        NS_ConvertUTF16toUTF8(remoteType).get()));
 
   // XXX: This is super hacky, and we should be able to do something better.
@@ -1497,10 +1485,6 @@ DocumentLoadListener::RedirectToRealChannel(
 
 void DocumentLoadListener::TriggerRedirectToRealChannel(
     const Maybe<uint64_t>& aDestinationProcess) {
-  LOG((
-      "DocumentLoadListener::TriggerRedirectToRealChannel [this=%p] "
-      "aDestinationProcess=%" PRId64,
-      this, aDestinationProcess ? int64_t(*aDestinationProcess) : int64_t(-1)));
   // This initiates replacing the current DocumentChannel with a
   // protocol specific 'real' channel, maybe in a different process than
   // the current DocumentChannelChild, if aDestinationProces is set.

netwerk/ipc/ParentProcessDocumentChannel.cpp

@@ -7,8 +7,6 @@
 
 #include "ParentProcessDocumentChannel.h"
 
-#include "mozilla/StaticPrefs_extensions.h"
-#include "nsDocShell.h"
 #include "nsIObserverService.h"
 
 extern mozilla::LazyLogModule gDocumentChannelLog;
@@ -47,19 +45,6 @@ ParentProcessDocumentChannel::RedirectToRealChannel(
     channel->SetLoadGroup(mLoadGroup);
   }
 
-  if (XRE_IsE10sParentProcess()) {
-    nsCOMPtr<nsIURI> uri;
-    MOZ_ALWAYS_SUCCEEDS(NS_GetFinalChannelURI(channel, getter_AddRefs(uri)));
-    if (!nsDocShell::CanLoadInParentProcess(uri)) {
-      nsAutoCString msg;
-      uri->GetSpec(msg);
-      msg.Insert(
-          "Attempt to load a non-authorised load in the parent process: ", 0);
-      NS_ASSERTION(false, msg.get());
-      return PDocumentChannelParent::RedirectToRealChannelPromise::
-          CreateAndResolve(NS_BINDING_ABORTED, __func__);
-    }
-  }
   mStreamFilterEndpoints = std::move(aStreamFilterEndpoints);
 
   RefPtr<PDocumentChannelParent::RedirectToRealChannelPromise> p =

toolkit/modules/E10SUtils.jsm

@@ -567,7 +567,6 @@ var E10SUtils = {
 
   getRemoteTypeForPrincipal(
     aPrincipal,
-    aOriginalURI,
     aMultiProcess,
     aRemoteSubframes,
     aPreferredRemoteType = DEFAULT_REMOTE_TYPE,
@@ -578,31 +577,25 @@ var E10SUtils = {
       return NOT_REMOTE;
     }
 
-    // We want to use the original URI for "about:" and "chrome://" scheme,
+    // We can't pick a process based on a system principal or expanded
-    // so that we can properly determine the remote type.
+    // principal. In fact, we should never end up with one here!
-    let useOriginalURI =
+    if (aPrincipal.isSystemPrincipal || aPrincipal.isExpandedPrincipal) {
-      aOriginalURI.scheme == "about" || aOriginalURI.scheme == "chrome";
+      throw Components.Exception("", Cr.NS_ERROR_UNEXPECTED);
-
-    if (!useOriginalURI) {
-      // We can't pick a process based on a system principal or expanded
-      // principal.
-      if (aPrincipal.isSystemPrincipal || aPrincipal.isExpandedPrincipal) {
-        throw Components.Exception("", Cr.NS_ERROR_UNEXPECTED);
-      }
-
-      // Null principals can be loaded in any remote process, but when
-      // using fission we add the option to force them into the default
-      // web process for better test coverage.
-      if (aPrincipal.isNullPrincipal) {
-        if (
-          (aRemoteSubframes && useSeparateDataUriProcess) ||
-          aPreferredRemoteType == NOT_REMOTE
-        ) {
-          return WEB_REMOTE_TYPE;
-        }
-        return aPreferredRemoteType;
-      }
     }
+
+    // Null principals can be loaded in any remote process, but when
+    // using fission we add the option to force them into the default
+    // web process for better test coverage.
+    if (aPrincipal.isNullPrincipal) {
+      if (
+        (aRemoteSubframes && useSeparateDataUriProcess) ||
+        aPreferredRemoteType == NOT_REMOTE
+      ) {
+        return WEB_REMOTE_TYPE;
+      }
+      return aPreferredRemoteType;
+    }
+
     // We might care about the currently loaded URI. Pull it out of our current
     // principal. We never care about the current URI when working with a
     // non-content principal.
@@ -610,9 +603,8 @@ var E10SUtils = {
       aCurrentPrincipal && aCurrentPrincipal.isContentPrincipal
         ? aCurrentPrincipal.URI
         : null;
-
     return E10SUtils.getRemoteTypeForURIObject(
-      useOriginalURI ? aOriginalURI : aPrincipal.URI,
+      aPrincipal.URI,
       aMultiProcess,
       aRemoteSubframes,
       aPreferredRemoteType,
@@ -820,6 +812,7 @@ var E10SUtils = {
     // handled using DocumentChannel, then we can skip switching
     // for now, and let DocumentChannel do it during the response.
     if (
+      currentRemoteType != NOT_REMOTE &&
       requiredRemoteType != NOT_REMOTE &&
       uriObject &&
       (remoteSubframes || documentChannel) &&
@@ -851,6 +844,7 @@ var E10SUtils = {
 
     if (
       (aRemoteSubframes || documentChannel) &&
+      remoteType != NOT_REMOTE &&
       wantRemoteType != NOT_REMOTE &&
       documentChannelPermittedForURI(aURI)
     ) {
@@ -889,6 +883,7 @@ var E10SUtils = {
     // switch later-on using the nsIProcessSwitchRequestor mechanism.
     if (
       (useRemoteSubframes || documentChannel) &&
+      remoteType != NOT_REMOTE &&
       wantRemoteType != NOT_REMOTE &&
       documentChannelPermittedForURI(aURI)
     ) {

toolkit/modules/nsIE10SUtils.idl

@@ -7,7 +7,6 @@
 #include "nsISupports.idl"
 
 interface nsIPrincipal;
-interface nsIURI;
 
 /**
  * C++ exposed interface for the `E10SUtils` object from the
@@ -20,9 +19,6 @@ interface nsIE10SUtils : nsISupports {
    * principal.
    *
    * @param aPrincipal  The result principal for the document being loaded.
-   * @param aChannelOriginalURI. The original URI being loaded
-   *                             (which isn't always the same as the Principal's
-   *                              URI)
    * @param aMultiProcess  Does the browser have remote tabs enabled.
    * @param aRemoteSubframes  Does the browser have remote subframes enabled.
    * @param aPreferredRemoteType  If multiple remote types are compatible with
@@ -33,7 +29,6 @@ interface nsIE10SUtils : nsISupports {
    * @return  The remote type to complete this load in.
    */
   AString getRemoteTypeForPrincipal(in nsIPrincipal aPrincipal,
-                                    in nsIURI aChannelOriginalURI,
                                     in boolean aMultiProcess,
                                     in boolean aRemoteSubframes,
                                     in AString aPreferredRemoteType,