Bug 1552017 - Expand the kinds of URLs that can cancel content JS when navigating; r=smaug

This patch makes several changes to the kinds of URLs where we can cancel
content JS when navigating between them:

 1) When navigating directly to a URL (e.g. by typing something into the
    location bar and hitting Enter), we allow canceling content JS if the URLs
    differ in any way *except* their ref ("#"). To help with this, we also
    attempt to fix up the URL (e.g. by prepending "http://" to it).

 2) When navigating through history, we allow canceling content JS if the
    `prePath` part of the URLs differ. Most notably, this allows canceling
    content JS when one of the URLs is an `about:` page (e.g. when hitting the
    Home button).

 3) We explicitly disallow cancelling content JS if the currently-running JS
    is trusted or if the page being navigated away from is anything but
    http(s): or file:.

 4) We also disallow cancelling content JS for windows that are still being
    created (e.g. when creating a new tab or window via `window.open`). For
    more background on this, see the comments about `mCreatingWindow` in
    dom/ipc/BrowserParent.h.

 5) We ensure that, when attempting to cancel JS, the tab ID of the
    currently-running script matches the original tab that requested the
    cancellation. This avoids a race condition in which a particular JSContext
    has already moved on to executing another tab's JS by the time we hit our
    interrupt callback.

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

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

docshell/base/nsDefaultURIFixup.cpp

@@ -14,6 +14,7 @@
 #include "nsISearchService.h"
 #include "nsIURIFixup.h"
 #include "nsIURIMutator.h"
+#include "nsIWebNavigation.h"
 #include "nsDefaultURIFixup.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/dom/ContentChild.h"
@@ -381,6 +382,27 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI,
   return rv;
 }
 
+NS_IMETHODIMP
+nsDefaultURIFixup::WebNavigationFlagsToFixupFlags(const nsACString& aStringURI,
+                                                  uint32_t aDocShellFlags,
+                                                  uint32_t* aFixupFlags) {
+  nsCOMPtr<nsIURI> uri;
+  NS_NewURI(getter_AddRefs(uri), aStringURI);
+  if (uri) {
+    aDocShellFlags &= ~nsIWebNavigation::LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
+  }
+
+  *aFixupFlags = 0;
+  if (aDocShellFlags & nsIWebNavigation::LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP) {
+    *aFixupFlags |= FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
+  }
+  if (aDocShellFlags & nsIWebNavigation::LOAD_FLAGS_FIXUP_SCHEME_TYPOS) {
+    *aFixupFlags |= FIXUP_FLAG_FIX_SCHEME_TYPOS;
+  }
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsDefaultURIFixup::KeywordToURI(const nsACString& aKeyword,
                                 nsIInputStream** aPostData,

docshell/base/nsDocShell.cpp

@@ -3916,10 +3916,6 @@ nsresult nsDocShell::LoadURI(const nsAString& aURI,
   nsCOMPtr<nsIInputStream> postData(aLoadURIOptions.mPostData);
   nsresult rv = NS_OK;
 
-  // Create a URI from our string; if that succeeds, we want to
-  // change loadFlags to not include the ALLOW_THIRD_PARTY_FIXUP
-  // flag.
-
   NS_ConvertUTF16toUTF8 uriString(aURI);
   // Cleanup the empty spaces that might be on each end.
   uriString.Trim(" ");
@@ -3931,24 +3927,19 @@ nsresult nsDocShell::LoadURI(const nsAString& aURI,
     return NS_ERROR_FAILURE;
   }
 
-  rv = NS_NewURI(getter_AddRefs(uri), uriString);
+  nsCOMPtr<nsIURIFixupInfo> fixupInfo;
-  if (uri) {
+  if (sURIFixup) {
+    uint32_t fixupFlags;
+    rv = sURIFixup->WebNavigationFlagsToFixupFlags(uriString, loadFlags,
+                                                   &fixupFlags);
+    NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
+
+    // If we don't allow keyword lookups for this URL string, make sure to
+    // update loadFlags to indicate this as well.
+    if (!(fixupFlags & nsIURIFixup::FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP)) {
       loadFlags &= ~LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
     }
 
-  nsCOMPtr<nsIURIFixupInfo> fixupInfo;
-  if (sURIFixup) {
-    // Call the fixup object.  This will clobber the rv from NS_NewURI
-    // above, but that's fine with us.  Note that we need to do this even
-    // if NS_NewURI returned a URI, because fixup handles nested URIs, etc
-    // (things like view-source:mozilla.org for example).
-    uint32_t fixupFlags = 0;
-    if (loadFlags & LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP) {
-      fixupFlags |= nsIURIFixup::FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
-    }
-    if (loadFlags & LOAD_FLAGS_FIXUP_SCHEME_TYPOS) {
-      fixupFlags |= nsIURIFixup::FIXUP_FLAG_FIX_SCHEME_TYPOS;
-    }
     nsCOMPtr<nsIInputStream> fixupStream;
     rv = sURIFixup->GetFixupURIInfo(uriString, fixupFlags,
                                     getter_AddRefs(fixupStream),
@@ -3973,9 +3964,11 @@ nsresult nsDocShell::LoadURI(const nsAString& aURI,
                               PromiseFlatString(aURI).get());
       }
     }
+  } else {
+    // No fixup service so just create a URI and see what happens...
+    rv = NS_NewURI(getter_AddRefs(uri), uriString);
+    loadFlags &= ~LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
   }
-  // else no fixup service so just use the URI we created and see
-  // what happens
 
   if (NS_ERROR_MALFORMED_URI == rv) {
     if (DisplayLoadError(rv, uri, PromiseFlatString(aURI).get(), nullptr) &&

docshell/base/nsIURIFixup.idl

@@ -135,6 +135,17 @@ interface nsIURIFixup : nsISupports
                                     in unsigned long aFixupFlags,
                                     [optional] out nsIInputStream aPostData);
 
+    /**
+     * Convert load flags from nsIWebNavigation to URI fixup flags for use in
+     * createFixupURI or getFixupURIInfo.
+     *
+     * @param aURIText       Candidate URI; used for determining whether to
+     *                       allow keyword lookups.
+     * @param aDocShellFlags Load flags from nsIDocShell to convert.
+     */
+    unsigned long webNavigationFlagsToFixupFlags(
+        in AUTF8String aURIText, in unsigned long aDocShellFlags);
+
     /**
      * Converts the specified keyword string into a URI.  Note that it's the
      * caller's responsibility to check whether keywords are enabled and

dom/ipc/BrowserChild.cpp

@@ -3401,6 +3401,14 @@ nsresult BrowserChild::CanCancelContentJS(
   rv = history->GetEntryAtIndex(current, getter_AddRefs(entry));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  nsCOMPtr<nsIURI> currentURI = entry->GetURI();
+  if (!currentURI->SchemeIs("http") && !currentURI->SchemeIs("https") &&
+      !currentURI->SchemeIs("file")) {
+    // Only cancel content JS for http(s) and file URIs. Other URIs are probably
+    // internal and we should just let them run to completion.
+    return NS_OK;
+  }
+
   if (aNavigationType == nsIRemoteTab::NAVIGATE_BACK) {
     aNavigationIndex = current - 1;
   } else if (aNavigationType == nsIRemoteTab::NAVIGATE_FORWARD) {
@@ -3410,9 +3418,13 @@ nsresult BrowserChild::CanCancelContentJS(
       return NS_ERROR_FAILURE;
     }
 
-    nsCOMPtr<nsIURI> currentURI = entry->GetURI();
+    // If navigating directly to a URL (e.g. via hitting Enter in the location
-    CanCancelContentJSBetweenURIs(currentURI, aNavigationURI, aCanCancel);
+    // bar), then we can cancel anytime the next URL is different from the
+    // current, *excluding* the ref ("#").
+    bool equals;
+    rv = currentURI->EqualsExceptRef(aNavigationURI, &equals);
     NS_ENSURE_SUCCESS(rv, rv);
+    *aCanCancel = !equals;
     return NS_OK;
   }
   // Note: aNavigationType may also be NAVIGATE_INDEX, in which case we don't
@@ -3427,15 +3439,22 @@ nsresult BrowserChild::CanCancelContentJS(
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsISHEntry> laterEntry = delta == 1 ? nextEntry : entry;
-    nsCOMPtr<nsIURI> uri = entry->GetURI();
+    nsCOMPtr<nsIURI> thisURI = entry->GetURI();
     nsCOMPtr<nsIURI> nextURI = nextEntry->GetURI();
 
     // If we changed origin and the load wasn't in a subframe, we know it was
     // a full document load, so we can cancel the content JS safely.
     if (!laterEntry->GetIsSubFrame()) {
-      CanCancelContentJSBetweenURIs(uri, nextURI, aCanCancel);
+      nsAutoCString thisHost;
+      rv = thisURI->GetPrePath(thisHost);
       NS_ENSURE_SUCCESS(rv, rv);
-      if (*aCanCancel) {
+
+      nsAutoCString nextHost;
+      rv = nextURI->GetPrePath(nextHost);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      if (!thisHost.Equals(nextHost)) {
+        *aCanCancel = true;
         return NS_OK;
       }
     }
@@ -3446,27 +3465,6 @@ nsresult BrowserChild::CanCancelContentJS(
   return NS_OK;
 }
 
-nsresult BrowserChild::CanCancelContentJSBetweenURIs(nsIURI* aFirstURI,
-                                                     nsIURI* aSecondURI,
-                                                     bool* aCanCancel) {
-  nsresult rv;
-  *aCanCancel = false;
-
-  nsAutoCString firstHost;
-  rv = aFirstURI->GetHostPort(firstHost);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsAutoCString secondHost;
-  rv = aSecondURI->GetHostPort(secondHost);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (!firstHost.Equals(secondHost)) {
-    *aCanCancel = true;
-  }
-
-  return NS_OK;
-}
-
 void BrowserChild::BeforeUnloadAdded() {
   // Don't bother notifying the parent if we don't have an IPC link open.
   if (mBeforeUnloadListeners == 0 && IPCOpen()) {

dom/ipc/BrowserChild.h

@@ -827,9 +827,6 @@ class BrowserChild final : public BrowserChildBase,
                                        Maybe<WebProgressData>& aWebProgressData,
                                        RequestData& aRequestData);
 
-  nsresult CanCancelContentJSBetweenURIs(nsIURI* aFirstURI, nsIURI* aSecondURI,
-                                         bool* aCanCancel);
-
   class DelayedDeleteRunnable;
 
   TextureFactoryIdentifier mTextureFactoryIdentifier;

dom/ipc/BrowserHost.cpp

@@ -357,7 +357,10 @@ NS_IMETHODIMP
 BrowserHost::MaybeCancelContentJSExecutionFromScript(
     nsIRemoteTab::NavigationType aNavigationType,
     JS::Handle<JS::Value> aCancelContentJSOptions, JSContext* aCx) {
-  if (!mRoot) {
+  // If we're in the process of creating a new window (via window.open), then
+  // the load that called this function isn't a "normal" load and should be
+  // ignored for the purposes of cancelling content JS.
+  if (!mRoot || mRoot->CreatingWindow()) {
     return NS_OK;
   }
   dom::CancelContentJSOptions cancelContentJSOptions;

dom/ipc/BrowserParent.h

@@ -189,6 +189,13 @@ class BrowserParent final : public PBrowserParent,
    */
   bool IsDestroyed() const { return mIsDestroyed; }
 
+  /**
+   * Returns whether we're in the process of creating a new window (from
+   * window.open). If so, LoadURL calls are being skipped until everything is
+   * set up. For further details, see `mCreatingWindow` below.
+   */
+  bool CreatingWindow() const { return mCreatingWindow; }
+
   /*
    * Visit each BrowserParent in the tree formed by PBrowser and
    * PBrowserBridge, including `this`.

dom/ipc/ProcessHangMonitor.cpp

@@ -381,10 +381,16 @@ bool HangMonitorChild::InterruptCallback() {
     }
   }
 
-  // Only handle the interrupt for cancelling content JS if we have an actual
+  // Only handle the interrupt for cancelling content JS if we have a
-  // window associated with this context...
+  // non-privileged script (i.e. not part of Gecko or an add-on).
   JS::RootedObject global(mContext, JS::CurrentGlobalOrNull(mContext));
-  RefPtr<nsGlobalWindowInner> win = xpc::WindowOrNull(global);
+  nsIPrincipal* principal = xpc::GetObjectPrincipal(global);
+  if (principal && (principal->IsSystemPrincipal() ||
+                    principal->GetIsAddonOrExpandedAddonPrincipal())) {
+    return true;
+  }
+
+  nsCOMPtr<nsPIDOMWindowInner> win = xpc::WindowOrNull(global);
   if (!win) {
     return true;
   }
@@ -409,10 +415,17 @@ bool HangMonitorChild::InterruptCallback() {
   }
 
   if (cancelContentJS) {
+    js::AutoAssertNoContentJS nojs(mContext);
+    TabId currentJSTabId = BrowserChild::GetFrom(win)->GetTabId();
+    if (currentJSTabId != cancelContentJSTab) {
+      // The currently-executing content JS doesn't belong to the tab that
+      // requested cancellation of JS. Just return and let the JS continue.
+      return true;
+    }
+
     RefPtr<BrowserChild> browserChild =
         BrowserChild::FindBrowserChild(cancelContentJSTab);
     if (browserChild) {
-      js::AutoAssertNoContentJS nojs(mContext);
       nsresult rv;
       nsCOMPtr<nsIURI> uri;
 

toolkit/components/remotebrowserutils/RemoteWebNavigation.jsm

@@ -12,10 +12,6 @@ ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils",
 ChromeUtils.defineModuleGetter(this, "E10SUtils",
   "resource://gre/modules/E10SUtils.jsm");
 
-function makeURI(url) {
-  return Services.io.newURI(url);
-}
-
 function RemoteWebNavigation() {
   this.wrappedJSObject = this;
   this._cancelContentJSEpoch = 1;
@@ -77,14 +73,18 @@ RemoteWebNavigation.prototype = {
   },
   loadURI(aURI, aLoadURIOptions) {
     let uri;
+    try {
+      let fixup = Cc["@mozilla.org/docshell/urifixup;1"].getService();
+      let fixupFlags = fixup.webNavigationFlagsToFixupFlags(
+        aURI, aLoadURIOptions.loadFlags);
+      uri = fixup.createFixupURI(aURI, fixupFlags);
 
       // We know the url is going to be loaded, let's start requesting network
       // connection before the content process asks.
-    // Note that we might have already setup the speculative connection in some
+      // Note that we might have already setup the speculative connection in
-    // cases, especially when the url is from location bar or its popup menu.
+      // some cases, especially when the url is from location bar or its popup
-    if (aURI.startsWith("http:") || aURI.startsWith("https:")) {
+      // menu.
-      try {
+      if (uri.schemeIs("http") || uri.schemeIs("https")) {
-        uri = makeURI(aURI);
         let principal = aLoadURIOptions.triggeringPrincipal;
         // We usually have a triggeringPrincipal assigned, but in case we
         // don't have one or if it's a SystemPrincipal, let's create it with OA
@@ -97,11 +97,11 @@ RemoteWebNavigation.prototype = {
           principal = Services.scriptSecurityManager.createCodebasePrincipal(uri, attrs);
         }
         Services.io.speculativeConnect(uri, principal, null);
+      }
     } catch (ex) {
       // Can't setup speculative connection for this uri string for some
       // reason (such as failing to parse the URI), just ignore it.
     }
-    }
 
     let cancelContentJSEpoch = this._cancelContentJSEpoch++;
     this._browser.frameLoader.remoteTab.maybeCancelContentJSExecution(
@@ -138,7 +138,7 @@ RemoteWebNavigation.prototype = {
   _currentURI: null,
   get currentURI() {
     if (!this._currentURI) {
-      this._currentURI = makeURI("about:blank");
+      this._currentURI = Services.io.newURI("about:blank");
     }
 
     return this._currentURI;