Bug 1586148 - fix web protocol handler behaviour under fission, r=mccr8

Differential Revision: https://phabricator.services.mozilla.com/D48238

--HG--
extra : moz-landing-system : lando

dom/base/Navigator.cpp

@@ -833,6 +833,7 @@ void Navigator::RegisterContentHandler(const nsAString& aMIMEType,
 
 // This list should be kept up-to-date with the spec:
 // https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
+// If you change this list, please also update the copy in E10SUtils.jsm.
 static const char* const kSafeSchemes[] = {
     "bitcoin", "geo",  "im",   "irc",         "ircs", "magnet", "mailto",
     "mms",     "news", "nntp", "openpgp4fpr", "sip",  "sms",    "smsto",

toolkit/modules/E10SUtils.jsm

@@ -67,6 +67,12 @@ XPCOMUtils.defineLazyServiceGetter(
   "@mozilla.org/network/serialization-helper;1",
   "nsISerializationHelper"
 );
+XPCOMUtils.defineLazyServiceGetter(
+  this,
+  "extProtService",
+  "@mozilla.org/uriloader/external-protocol-service;1",
+  "nsIExternalProtocolService"
+);
 
 function debug(msg) {
   Cu.reportError(new Error("E10SUtils: " + msg));
@@ -99,6 +105,46 @@ const PRIVILEGEDMOZILLA_REMOTE_TYPE = "privilegedmozilla";
 const LARGE_ALLOCATION_REMOTE_TYPE = "webLargeAllocation";
 const DEFAULT_REMOTE_TYPE = WEB_REMOTE_TYPE;
 
+// This list is duplicated between Navigator.cpp and here because navigator
+// is not accessible in this context. Please update both if the list changes.
+const kSafeSchemes = [
+  "bitcoin",
+  "geo",
+  "im",
+  "irc",
+  "ircs",
+  "magnet",
+  "mailto",
+  "mms",
+  "news",
+  "nntp",
+  "openpgp4fpr",
+  "sip",
+  "sms",
+  "smsto",
+  "ssh",
+  "tel",
+  "urn",
+  "webcal",
+  "wtai",
+  "xmpp",
+];
+
+// 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
+// not be web-handled - it could still be handled via the OS by another app.
+function hasPotentiallyWebHandledScheme({ scheme }) {
+  // Note that `scheme` comes from a URI object so is already lowercase.
+  if (kSafeSchemes.includes(scheme)) {
+    return true;
+  }
+  if (!scheme.startsWith("web+") || scheme.length < 5) {
+    return false;
+  }
+  // Check the rest of the scheme only consists of ascii a-z chars
+  return /^[a-z]+$/.test(scheme.substr("web+".length));
+}
+
 function validatedWebRemoteType(
   aPreferredRemoteType,
   aTargetUri,
@@ -120,6 +166,39 @@ function validatedWebRemoteType(
     return PRIVILEGEDMOZILLA_REMOTE_TYPE;
   }
 
+  // If we're in the parent and we were passed a web-handled scheme,
+  // transform it now to avoid trying to load it in the wrong process.
+  if (aRemoteSubframes && hasPotentiallyWebHandledScheme(aTargetUri)) {
+    if (
+      Services.appinfo.processType != Services.appinfo.PROCESS_TYPE_DEFAULT &&
+      Services.appinfo.remoteType.startsWith(FISSION_WEB_REMOTE_TYPE_PREFIX)
+    ) {
+      // If we're in a child process, assume we're OK to load this non-web
+      // URL for now. We'll either load it externally or re-evaluate once
+      // we know the "real" URL to which we'll redirect.
+      return Services.appinfo.remoteType;
+    }
+    // This doesn't work (throws) in the child - see
+    // https://bugzilla.mozilla.org/show_bug.cgi?id=1589082
+    // Even if it did, it'd cause sync IPC
+    // ( https://bugzilla.mozilla.org/show_bug.cgi?id=1589085 ), and this code
+    // can get called several times per page load so that seems like something
+    // we'd want to avoid.
+    let handlerInfo = extProtService.getProtocolHandlerInfo(aTargetUri.scheme);
+    try {
+      if (!handlerInfo.alwaysAskBeforeHandling) {
+        let app = handlerInfo.preferredApplicationHandler;
+        app.QueryInterface(Ci.nsIWebHandlerApp);
+        // If we get here, the default handler is a web app.
+        // Target to the origin of that web app:
+        let uriStr = app.uriTemplate.replace(/%s/, aTargetUri.spec);
+        aTargetUri = Services.io.newURI(uriStr);
+      }
+    } catch (ex) {
+      // It's not strange for this to throw, we just ignore it and fall through.
+    }
+  }
+
   // If the domain is whitelisted to allow it to use file:// URIs, then we have
   // to run it in a file content process, in case it uses file:// sub-resources.
   const sm = Services.scriptSecurityManager;

uriloader/exthandler/tests/mochitest/browser.ini

@@ -13,4 +13,3 @@ support-files =
 [browser_download_privatebrowsing.js]
 [browser_remember_download_option.js]
 [browser_web_protocol_handlers.js]
-skip-if = fission