Bug 1847718 - Require nsIChannel instead of nsIRequest in nsIExternalHelperAppService r=necko-reviewers,geckoview-reviewers,valentin,owlish

All callers already pass an nsIChannel and lower in the stack an
nsIChannel is required to serialize its LoadInfoArgs over IPC.

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

mobile/android/components/geckoview/GeckoViewExternalAppService.cpp

@@ -53,7 +53,7 @@ GeckoViewExternalAppService::GeckoViewExternalAppService() {}
 NS_IMPL_ISUPPORTS(GeckoViewExternalAppService, nsIExternalHelperAppService);
 
 NS_IMETHODIMP GeckoViewExternalAppService::DoContent(
-    const nsACString& aMimeContentType, nsIRequest* aRequest,
+    const nsACString& aMimeContentType, nsIChannel* aChannel,
     nsIInterfaceRequestor* aContentContext, bool aForceSave,
     nsIInterfaceRequestor* aWindowContext,
     nsIStreamListener** aStreamListener) {
@@ -61,17 +61,14 @@ NS_IMETHODIMP GeckoViewExternalAppService::DoContent(
 }
 
 NS_IMETHODIMP GeckoViewExternalAppService::CreateListener(
-    const nsACString& aMimeContentType, nsIRequest* aRequest,
+    const nsACString& aMimeContentType, nsIChannel* aChannel,
     mozilla::dom::BrowsingContext* aContentContext, bool aForceSave,
     nsIInterfaceRequestor* aWindowContext,
     nsIStreamListener** aStreamListener) {
   using namespace mozilla;
   using namespace mozilla::dom;
   MOZ_ASSERT(XRE_IsParentProcess());
-
-  nsresult rv;
-  nsCOMPtr<nsIChannel> channel(do_QueryInterface(aRequest, &rv));
-  NS_ENSURE_SUCCESS(rv, rv);
+  NS_ENSURE_ARG_POINTER(aChannel);
 
   nsCOMPtr<nsIWidget> widget =
       aContentContext->Canonical()->GetParentProcessWidgetContaining();
@@ -84,7 +81,8 @@ NS_IMETHODIMP GeckoViewExternalAppService::CreateListener(
 
   RefPtr<StreamListener> listener = new StreamListener(window);
 
-  rv = channel->SetNotificationCallbacks(listener);
+  nsresult rv;
+  rv = aChannel->SetNotificationCallbacks(listener);
   NS_ENSURE_SUCCESS(rv, rv);
 
   listener.forget(aStreamListener);

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -636,10 +636,12 @@ nsresult nsExternalHelperAppService::Init() {
 nsExternalHelperAppService::~nsExternalHelperAppService() {}
 
 nsresult nsExternalHelperAppService::DoContentContentProcessHelper(
-    const nsACString& aMimeContentType, nsIRequest* aRequest,
+    const nsACString& aMimeContentType, nsIChannel* aChannel,
     BrowsingContext* aContentContext, bool aForceSave,
     nsIInterfaceRequestor* aWindowContext,
     nsIStreamListener** aStreamListener) {
+  NS_ENSURE_ARG_POINTER(aChannel);
+
   // We need to get a hold of a ContentChild so that we can begin forwarding
   // this data to the parent.  In the HTTP case, this is unfortunate, since
   // we're actually passing data from parent->child->parent wastefully, but
@@ -660,26 +662,23 @@ nsresult nsExternalHelperAppService::DoContentContentProcessHelper(
   nsAutoString fileName;
   nsCOMPtr<nsILoadInfo> loadInfo;
 
-  nsCOMPtr<nsIChannel> channel = do_QueryInterface(aRequest);
-  if (channel) {
-    channel->GetURI(getter_AddRefs(uri));
-    channel->GetContentLength(&contentLength);
-    channel->GetContentDisposition(&contentDisposition);
-    channel->GetContentDispositionFilename(fileName);
-    channel->GetContentDispositionHeader(disp);
-    loadInfo = channel->LoadInfo();
+  aChannel->GetURI(getter_AddRefs(uri));
+  aChannel->GetContentLength(&contentLength);
+  aChannel->GetContentDisposition(&contentDisposition);
+  aChannel->GetContentDispositionFilename(fileName);
+  aChannel->GetContentDispositionHeader(disp);
+  loadInfo = aChannel->LoadInfo();
 
-    nsCOMPtr<nsIFileChannel> fileChan(do_QueryInterface(aRequest));
-    wasFileChannel = fileChan != nullptr;
-  }
+  nsCOMPtr<nsIFileChannel> fileChan(do_QueryInterface(aChannel));
+  wasFileChannel = fileChan != nullptr;
 
   nsCOMPtr<nsIURI> referrer;
-  NS_GetReferrerFromChannel(channel, getter_AddRefs(referrer));
+  NS_GetReferrerFromChannel(aChannel, getter_AddRefs(referrer));
 
   mozilla::net::LoadInfoArgs loadInfoArgs;
   MOZ_ALWAYS_SUCCEEDS(LoadInfoToLoadInfoArgs(loadInfo, &loadInfoArgs));
 
-  nsCOMPtr<nsIPropertyBag2> props(do_QueryInterface(aRequest));
+  nsCOMPtr<nsIPropertyBag2> props(do_QueryInterface(aChannel));
   // Determine whether a new window was opened specifically for this request
   bool shouldCloseWindow = false;
   if (props) {
@@ -715,23 +714,21 @@ nsresult nsExternalHelperAppService::DoContentContentProcessHelper(
 }
 
 NS_IMETHODIMP nsExternalHelperAppService::CreateListener(
-    const nsACString& aMimeContentType, nsIRequest* aRequest,
+    const nsACString& aMimeContentType, nsIChannel* aChannel,
     BrowsingContext* aContentContext, bool aForceSave,
     nsIInterfaceRequestor* aWindowContext,
     nsIStreamListener** aStreamListener) {
   MOZ_ASSERT(!XRE_IsContentProcess());
+  NS_ENSURE_ARG_POINTER(aChannel);
 
   nsAutoString fileName;
   nsAutoCString fileExtension;
   uint32_t reason = nsIHelperAppLauncherDialog::REASON_CANTHANDLE;
 
-  nsCOMPtr<nsIChannel> channel = do_QueryInterface(aRequest);
-  if (channel) {
-    uint32_t contentDisposition = -1;
-    channel->GetContentDisposition(&contentDisposition);
-    if (contentDisposition == nsIChannel::DISPOSITION_ATTACHMENT) {
-      reason = nsIHelperAppLauncherDialog::REASON_SERVERREQUEST;
-    }
+  uint32_t contentDisposition = -1;
+  aChannel->GetContentDisposition(&contentDisposition);
+  if (contentDisposition == nsIChannel::DISPOSITION_ATTACHMENT) {
+    reason = nsIHelperAppLauncherDialog::REASON_SERVERREQUEST;
   }
 
   *aStreamListener = nullptr;
@@ -739,7 +736,7 @@ NS_IMETHODIMP nsExternalHelperAppService::CreateListener(
   // Get the file extension and name that we will need later
   nsCOMPtr<nsIURI> uri;
   bool allowURLExtension =
-      GetFileNameFromChannel(channel, fileName, getter_AddRefs(uri));
+      GetFileNameFromChannel(aChannel, fileName, getter_AddRefs(uri));
 
   uint32_t flags = VALIDATE_ALLOW_EMPTY;
   if (aMimeContentType.Equals(APPLICATION_GUESS_FROM_EXT,
@@ -758,12 +755,10 @@ NS_IMETHODIMP nsExternalHelperAppService::CreateListener(
   }
 
   if (flags & VALIDATE_GUESS_FROM_EXTENSION) {
-    if (channel) {
-      // Replace the content type with what was guessed.
-      nsAutoCString mimeType;
-      mimeInfo->GetMIMEType(mimeType);
-      channel->SetContentType(mimeType);
-    }
+    // Replace the content type with what was guessed.
+    nsAutoCString mimeType;
+    mimeInfo->GetMIMEType(mimeType);
+    aChannel->SetContentType(mimeType);
 
     if (reason == nsIHelperAppLauncherDialog::REASON_CANTHANDLE) {
       reason = nsIHelperAppLauncherDialog::REASON_TYPESNIFFED;
@@ -790,7 +785,7 @@ NS_IMETHODIMP nsExternalHelperAppService::CreateListener(
 }
 
 NS_IMETHODIMP nsExternalHelperAppService::DoContent(
-    const nsACString& aMimeContentType, nsIRequest* aRequest,
+    const nsACString& aMimeContentType, nsIChannel* aChannel,
     nsIInterfaceRequestor* aContentContext, bool aForceSave,
     nsIInterfaceRequestor* aWindowContext,
     nsIStreamListener** aStreamListener) {
@@ -809,12 +804,12 @@ NS_IMETHODIMP nsExternalHelperAppService::DoContent(
   }
 
   if (XRE_IsContentProcess()) {
-    return DoContentContentProcessHelper(aMimeContentType, aRequest, bc,
+    return DoContentContentProcessHelper(aMimeContentType, aChannel, bc,
                                          aForceSave, aWindowContext,
                                          aStreamListener);
   }
 
-  nsresult rv = CreateListener(aMimeContentType, aRequest, bc, aForceSave,
+  nsresult rv = CreateListener(aMimeContentType, aChannel, bc, aForceSave,
                                aWindowContext, aStreamListener);
   return rv;
 }

uriloader/exthandler/nsExternalHelperAppService.h

@@ -253,7 +253,7 @@ class nsExternalHelperAppService : public nsIExternalHelperAppService,
 
  private:
   nsresult DoContentContentProcessHelper(
-      const nsACString& aMimeContentType, nsIRequest* aRequest,
+      const nsACString& aMimeContentType, nsIChannel* aChannel,
       mozilla::dom::BrowsingContext* aContentContext, bool aForceSave,
       nsIInterfaceRequestor* aWindowContext,
       nsIStreamListener** aStreamListener);

uriloader/exthandler/nsIExternalHelperAppService.idl

@@ -7,7 +7,7 @@
 #include "nsICancelable.idl"
 
 interface nsIURI;
-interface nsIRequest;
+interface nsIChannel;
 interface nsIStreamListener;
 interface nsIFile;
 interface nsIMIMEInfo;
@@ -28,7 +28,7 @@ interface nsIExternalHelperAppService : nsISupports
    * is issued, the stream listener implementation will launch the helper app
    * with this data.
    * @param aMimeContentType The content type of the incoming data
-   * @param aRequest The request corresponding to the incoming data
+   * @param aChannel The channel corresponding to the incoming data
    * @param aContentContext Used in processing content document refresh
    *  headers after target content is downloaded.
    * @param aForceSave True to always save this content to disk, regardless of
@@ -39,7 +39,7 @@ interface nsIExternalHelperAppService : nsISupports
    * @return A nsIStreamListener which the caller should pump the data into.
    */
   nsIStreamListener doContent (in ACString aMimeContentType,
-                               in nsIRequest aRequest,
+                               in nsIChannel aChannel,
                                in nsIInterfaceRequestor aContentContext,
                                in boolean aForceSave,
                                [optional] in nsIInterfaceRequestor aWindowContext);
@@ -52,8 +52,8 @@ interface nsIExternalHelperAppService : nsISupports
    * Replaces doContent for native code, and uses BrowsingContext.
    *
    * @param aMimeContentType The content type of the incoming data
-   * @param aRequest The request corresponding to the incoming data
-   * @param aContentContext The BrowsingContext that the request was initiated
+   * @param aChannel The channel corresponding to the incoming data
+   * @param aContentContext The BrowsingContext that the channel was initiated
    *  by. Used for closing the window if we opened one specifically for this download.
    * @param aForceSave True to always save this content to disk, regardless of
    *  nsIMIMEInfo and other such influences.
@@ -63,7 +63,7 @@ interface nsIExternalHelperAppService : nsISupports
    * @return A nsIStreamListener which the caller should pump the data into.
    */
   nsIStreamListener createListener (in ACString aMimeContentType,
-                                                   in nsIRequest aRequest,
+                                                   in nsIChannel aChannel,
                                                    in BrowsingContext aContentContext,
                                                    in boolean aForceSave,
                                                    [optional] in nsIInterfaceRequestor aWindowContext);