Bug 1810141 - make browsingcontext's loadURI actually take a URI rather than a string, r=nika

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

docshell/base/CanonicalBrowsingContext.cpp

@@ -1418,10 +1418,26 @@ void CanonicalBrowsingContext::UpdateMediaControlAction(
   });
 }
 
-void CanonicalBrowsingContext::LoadURI(const nsAString& aURI,
+void CanonicalBrowsingContext::LoadURI(nsIURI* aURI,
                                        const LoadURIOptions& aOptions,
                                        ErrorResult& aError) {
   RefPtr<nsDocShellLoadState> loadState;
+  nsresult rv = nsDocShellLoadState::CreateFromLoadURIOptions(
+      this, aURI, aOptions, getter_AddRefs(loadState));
+  MOZ_ASSERT(rv != NS_ERROR_MALFORMED_URI);
+
+  if (NS_FAILED(rv)) {
+    aError.Throw(rv);
+    return;
+  }
+
+  LoadURI(loadState, true);
+}
+
+void CanonicalBrowsingContext::FixupAndLoadURIString(
+    const nsAString& aURI, const LoadURIOptions& aOptions,
+    ErrorResult& aError) {
+  RefPtr<nsDocShellLoadState> loadState;
   nsresult rv = nsDocShellLoadState::CreateFromLoadURIOptions(
       this, aURI, aOptions, getter_AddRefs(loadState));
 

docshell/base/CanonicalBrowsingContext.h

@@ -206,7 +206,10 @@ class CanonicalBrowsingContext final : public BrowsingContext {
 
   // Triggers a load in the process
   using BrowsingContext::LoadURI;
-  void LoadURI(const nsAString& aURI, const LoadURIOptions& aOptions,
+  void FixupAndLoadURIString(const nsAString& aURI,
+                             const LoadURIOptions& aOptions,
+                             ErrorResult& aError);
+  void LoadURI(nsIURI* aURI, const LoadURIOptions& aOptions,
                ErrorResult& aError);
 
   void GoBack(const Optional<int32_t>& aCancelContentJSEpoch,

docshell/base/nsDocShellLoadState.cpp

@@ -297,7 +297,6 @@ nsresult nsDocShellLoadState::CreateFromLoadURIOptions(
       "Unexpected flags");
 
   nsCOMPtr<nsIURI> uri;
-  nsCOMPtr<nsIInputStream> postData(aLoadURIOptions.mPostData);
   nsresult rv = NS_OK;
 
   NS_ConvertUTF16toUTF8 uriString(aURI);
@@ -326,7 +325,7 @@ nsresult nsDocShellLoadState::CreateFromLoadURIOptions(
   }
 
   nsAutoString searchProvider, keyword;
-  bool didFixup = false;
+  RefPtr<nsIInputStream> fixupStream;
   if (fixup) {
     uint32_t fixupFlags =
         WebNavigationFlagsToFixupFlags(uri, uriString, loadFlags);
@@ -341,7 +340,6 @@ nsresult nsDocShellLoadState::CreateFromLoadURIOptions(
       fixupFlags |= nsIURIFixup::FIXUP_FLAG_PRIVATE_CONTEXT;
     }
 
-    RefPtr<nsIInputStream> fixupStream;
     if (!XRE_IsContentProcess()) {
       nsCOMPtr<nsIURIFixupInfo> fixupInfo;
       sURIFixup->GetFixupURIInfo(uriString, fixupFlags,
@@ -353,8 +351,11 @@ nsresult nsDocShellLoadState::CreateFromLoadURIOptions(
         fixupInfo->SetConsumer(aBrowsingContext);
         fixupInfo->GetKeywordProviderName(searchProvider);
         fixupInfo->GetKeywordAsSent(keyword);
+        // GetFixupURIInfo only returns a post data stream if it succeeded
+        // and changed the URI, in which case we should override the
+        // passed-in post data by passing this as an override arg to
+        // our internal method.
         fixupInfo->GetPostData(getter_AddRefs(fixupStream));
-        didFixup = true;
 
         if (fixupInfo &&
             loadFlags & nsIWebNavigation::LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP) {
@@ -364,15 +365,9 @@ nsresult nsDocShellLoadState::CreateFromLoadURIOptions(
                                   PromiseFlatString(aURI).get());
           }
         }
+        nsDocShell::MaybeNotifyKeywordSearchLoading(searchProvider, keyword);
       }
     }
-
-    if (fixupStream) {
-      // GetFixupURIInfo only returns a post data stream if it succeeded
-      // and changed the URI, in which case we should override the
-      // passed-in post data.
-      postData = fixupStream;
-    }
   }
 
   if (rv == NS_ERROR_MALFORMED_URI) {
@@ -384,6 +379,32 @@ nsresult nsDocShellLoadState::CreateFromLoadURIOptions(
     return NS_ERROR_FAILURE;
   }
 
+  RefPtr<nsDocShellLoadState> loadState;
+  rv = CreateFromLoadURIOptions(
+      aBrowsingContext, uri, aLoadURIOptions, loadFlags,
+      fixupStream ? fixupStream : aLoadURIOptions.mPostData,
+      getter_AddRefs(loadState));
+  NS_ENSURE_SUCCESS(rv, rv);
+  loadState->SetOriginalURIString(uriString);
+  loadState.forget(aResult);
+  return NS_OK;
+}
+
+nsresult nsDocShellLoadState::CreateFromLoadURIOptions(
+    BrowsingContext* aBrowsingContext, nsIURI* aURI,
+    const LoadURIOptions& aLoadURIOptions, nsDocShellLoadState** aResult) {
+  return CreateFromLoadURIOptions(aBrowsingContext, aURI, aLoadURIOptions,
+                                  aLoadURIOptions.mLoadFlags,
+                                  aLoadURIOptions.mPostData, aResult);
+}
+
+nsresult nsDocShellLoadState::CreateFromLoadURIOptions(
+    BrowsingContext* aBrowsingContext, nsIURI* aURI,
+    const LoadURIOptions& aLoadURIOptions, uint32_t aLoadFlagsOverride,
+    nsIInputStream* aPostDataOverride, nsDocShellLoadState** aResult) {
+  nsresult rv = NS_OK;
+  uint32_t loadFlags = aLoadFlagsOverride;
+  RefPtr<nsIInputStream> postData = aPostDataOverride;
   uint64_t available;
   if (postData) {
     rv = postData->Available(&available);
@@ -410,7 +431,7 @@ nsresult nsDocShellLoadState::CreateFromLoadURIOptions(
   uint32_t extraFlags = (loadFlags & EXTRA_LOAD_FLAGS);
   loadFlags &= ~EXTRA_LOAD_FLAGS;
 
-  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(uri);
+  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(aURI);
   loadState->SetReferrerInfo(aLoadURIOptions.mReferrerInfo);
 
   loadState->SetLoadType(MAKE_LOAD_TYPE(LOAD_NORMAL, loadFlags));
@@ -426,15 +447,10 @@ nsresult nsDocShellLoadState::CreateFromLoadURIOptions(
   loadState->SetTriggeringPrincipal(aLoadURIOptions.mTriggeringPrincipal);
   loadState->SetCsp(aLoadURIOptions.mCsp);
   loadState->SetForceAllowDataURI(forceAllowDataURI);
-  loadState->SetOriginalURIString(uriString);
   if (aLoadURIOptions.mCancelContentJSEpoch) {
     loadState->SetCancelContentJSEpoch(aLoadURIOptions.mCancelContentJSEpoch);
   }
 
-  if (didFixup) {
-    nsDocShell::MaybeNotifyKeywordSearchLoading(searchProvider, keyword);
-  }
-
   if (aLoadURIOptions.mTriggeringRemoteType.WasPassed()) {
     if (XRE_IsParentProcess()) {
       loadState->SetTriggeringRemoteType(

docshell/base/nsDocShellLoadState.h

@@ -61,6 +61,10 @@ class nsDocShellLoadState final {
       BrowsingContext* aBrowsingContext, const nsAString& aURI,
       const mozilla::dom::LoadURIOptions& aLoadURIOptions,
       nsDocShellLoadState** aResult);
+  static nsresult CreateFromLoadURIOptions(
+      BrowsingContext* aBrowsingContext, nsIURI* aURI,
+      const mozilla::dom::LoadURIOptions& aLoadURIOptions,
+      nsDocShellLoadState** aResult);
 
   // Getters and Setters
 
@@ -357,7 +361,12 @@ class nsDocShellLoadState final {
   // nullptr if it succeeds.
   const char* ValidateWithOriginalState(nsDocShellLoadState* aOriginalState);
 
- protected:
+  static nsresult CreateFromLoadURIOptions(
+      BrowsingContext* aBrowsingContext, nsIURI* aURI,
+      const mozilla::dom::LoadURIOptions& aLoadURIOptions,
+      uint32_t aLoadFlagsOverride, nsIInputStream* aPostDataOverride,
+      nsDocShellLoadState** aResult);
+
   // This is the referrer for the load.
   nsCOMPtr<nsIReferrerInfo> mReferrerInfo;
 

dom/chrome-webidl/BrowsingContext.webidl

@@ -289,7 +289,22 @@ interface CanonicalBrowsingContext : BrowsingContext {
    * loading.
    *
    * @param aURI
-   *        The URI string to load.  For HTTP and FTP URLs and possibly others,
+   *        The URI to load.  No fixup will be performed on this URI.
+   * @param aLoadURIOptions
+   *        A JSObject defined in LoadURIOptions.webidl holding info like e.g.
+   *        the triggeringPrincipal, the referrer info.
+   */
+  [Throws]
+  undefined loadURI(URI aURI, optional LoadURIOptions aOptions = {});
+
+  /**
+   * Like `loadURI` but takes a DOMString instead. This will use nsIURIFixup
+   * to "fix up" the input if it doesn't parse as a string. If an existing
+   * DOM URL or nsIURI object is available to you, prefer using `loadURI`
+   * directly.
+   *
+   * @param aURI
+   *        The URI to load.  For HTTP and FTP URLs and possibly others,
    *        characters above U+007F will be converted to UTF-8 and then URL-
    *        escaped per the rules of RFC 2396.
    * @param aLoadURIOptions
@@ -297,7 +312,7 @@ interface CanonicalBrowsingContext : BrowsingContext {
    *        the triggeringPrincipal, the referrer info.
    */
   [Throws]
-  undefined loadURI(DOMString aURI, optional LoadURIOptions aOptions = {});
+  undefined fixupAndLoadURIString(DOMString aURI, optional LoadURIOptions aOptions = {});
 
    /**
     * Print the current document.