Bug 1911977 - Don't abort networking activity on external protocol navigations. r=smaug

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

docshell/base/nsDocShell.cpp

@@ -9271,6 +9271,8 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
   // XXXbz mTiming should know what channel it's for, so we don't
   // need this hackery.
   const bool isJavaScript = SchemeIsJavascript(aLoadState->URI());
+  const bool isExternalProtocol =
+      nsContentUtils::IsExternalProtocol(aLoadState->URI());
   const bool isDownload = !aLoadState->FileName().IsVoid();
   const bool toBeReset = !isJavaScript && MaybeInitTiming();
 
@@ -9281,6 +9283,9 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
   }
   // Check if the page doesn't want to be unloaded. The javascript:
   // protocol handler deals with this for javascript: URLs.
+  // NOTE(emilio): As of this writing, other browsers fire beforeunload for
+  // external protocols, so keep doing that even though they don't return data
+  // and thus we won't really unload this...
   if (!isJavaScript && !isDownload &&
       !aLoadState->NotifiedBeforeUnloadListeners() && mDocumentViewer) {
     // Check if request is exempted from HTTPSOnlyMode and if https-first is
@@ -9380,7 +9385,7 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
   // In the case where they do result in data, the javascript: URL channel takes
   // care of stopping current network activity. Similarly, downloads don't
   // unload this document...
-  if (!isJavaScript && !isDownload) {
+  if (!isJavaScript && !isDownload && !isExternalProtocol) {
     // Stop any current network activity.
     // Also stop content if this is a zombie doc. otherwise
     // the onload will be delayed by other loads initiated in the

dom/base/test/file_location_href_unknown_protocol.html

@@ -3,8 +3,11 @@
 onbeforeunload = function() {
   opener.onChildBeforeUnload();
 };
-onload = function() {
+onload = async function() {
+  let ongoingLoad = fetch(location.href);
   location.href = "this-protocol-is-unlikely-to-exist://foo";
+  // Load shouldn't be canceled.
+  await ongoingLoad;
   setTimeout(function() {
     opener.onChildLoadTimedOut();
   }, 1000);

toolkit/actors/BackgroundThumbnailsChild.sys.mjs

@@ -74,6 +74,9 @@ export class BackgroundThumbnailsChild extends JSWindowActorChild {
           loadFlags: Ci.nsIWebNavigation.LOAD_FLAGS_STOP_CONTENT,
         };
         try {
+          // Some URIs like external protocols don't necessarily stop the
+          // ongoing network activity. See also bug 1917863.
+          docShell.stop(Ci.nsIWebNavigation.STOP_ALL);
           docShell.loadURI(
             Services.io.newURI(message.data.url),
             loadURIOptions

uriloader/base/nsDocLoader.cpp

@@ -454,9 +454,8 @@ nsDocLoader::OnStartRequest(nsIRequest* request) {
       // Make sure that the document channel is null at this point...
       // (unless its been redirected)
       //
-      NS_ASSERTION(
-          (loadFlags & nsIChannel::LOAD_REPLACE) || !(mDocumentRequest.get()),
-          "Overwriting an existing document channel!");
+      NS_ASSERTION((loadFlags & nsIChannel::LOAD_REPLACE) || !mDocumentRequest,
+                   "Overwriting an existing document channel!");
 
       // This request is associated with the entire document...
       mDocumentRequest = request;
@@ -480,7 +479,7 @@ nsDocLoader::OnStartRequest(nsIRequest* request) {
                "mDocumentRequest MUST be set for the duration of a page load!");
 
   // This is the only way to catch document request start event after a redirect
-  // has occured without changing inherited Firefox behaviour significantly.
+  // has occurred without changing inherited Firefox behaviour significantly.
   // Problem description:
   // The combination of |STATE_START + STATE_IS_DOCUMENT| is only sent for
   // initial request (see |doStartDocumentLoad| call above).