Backed out changeset 388c153da388 (bug 1656296) for causing default process leaks. CLOSED TREE

browser/components/downloads/DownloadsViewUI.jsm

@@ -539,7 +539,6 @@ DownloadsViewUI.DownloadElementShell.prototype = {
               case Downloads.Error.BLOCK_VERDICT_UNCOMMON:
                 this.showButton("askOpenOrRemoveFile");
                 break;
-              case Downloads.Error.BLOCK_VERDICT_INSECURE:
               case Downloads.Error.BLOCK_VERDICT_POTENTIALLY_UNWANTED:
                 this.showButton("askRemoveFileOrAllow");
                 break;
@@ -627,8 +626,6 @@ DownloadsViewUI.DownloadElementShell.prototype = {
     switch (this.download.error.reputationCheckVerdict) {
       case Downloads.Error.BLOCK_VERDICT_UNCOMMON:
         return [s.blockedUncommon2, [s.unblockTypeUncommon2, s.unblockTip2]];
-      case Downloads.Error.BLOCK_VERDICT_INSECURE:
-        return [s.blockedInsecure, [s.blockedInsecure, s.unblockTip2]];
       case Downloads.Error.BLOCK_VERDICT_POTENTIALLY_UNWANTED:
         return [
           s.blockedPotentiallyUnwanted,

browser/components/downloads/test/browser/browser_downloads_panel_block.js

@@ -8,7 +8,6 @@ add_task(async function mainTest() {
     Downloads.Error.BLOCK_VERDICT_UNCOMMON,
     Downloads.Error.BLOCK_VERDICT_MALWARE,
     Downloads.Error.BLOCK_VERDICT_POTENTIALLY_UNWANTED,
-    Downloads.Error.BLOCK_VERDICT_INSECURE,
   ];
   await task_addDownloads(verdicts.map(v => makeDownload(v)));
 

browser/locales/en-US/chrome/browser/downloads/downloads.properties

@@ -32,7 +32,6 @@ stateBlockedParentalControls=Blocked by Parental Controls
 # be longer than the other existing status strings.
 blockedMalware=This file contains a virus or malware.
 blockedPotentiallyUnwanted=This file may harm your computer.
-blockedInsecure = This file could not be downloaded securely.
 blockedUncommon2=This file is not commonly downloaded.
 
 # LOCALIZATION NOTE (fileMovedOrMissing):

dom/security/nsContentSecurityUtils.cpp

@@ -13,7 +13,6 @@
 #include "nsIHttpChannel.h"
 #include "nsIMultiPartChannel.h"
 #include "nsIURI.h"
-#include "nsITransfer.h"
 #if defined(XP_WIN)
 #  include "WinUtils.h"
 #  include <wininet.h>
@@ -1116,7 +1115,7 @@ void nsContentSecurityUtils::LogMessageToConsole(nsIHttpChannel* aChannel,
 }
 
 /* static */
-long nsContentSecurityUtils::ClassifyDownload(
+bool nsContentSecurityUtils::IsDownloadAllowed(
     nsIChannel* aChannel, const nsAutoCString& aMimeTypeGuess) {
   MOZ_ASSERT(aChannel, "IsDownloadAllowed without channel?");
 
@@ -1152,15 +1151,15 @@ long nsContentSecurityUtils::ClassifyDownload(
     if (httpChannel) {
       LogMessageToConsole(httpChannel, "MixedContentBlockedDownload");
     }
-    return nsITransfer::DOWNLOAD_POTENTIALLY_UNSAFE;
+    return false;
   }
 
   if (loadInfo->TriggeringPrincipal()->IsSystemPrincipal()) {
-    return nsITransfer::DOWNLOAD_ACCEPTABLE;
+    return true;
   }
 
   if (!StaticPrefs::dom_block_download_in_sandboxed_iframes()) {
-    return nsITransfer::DOWNLOAD_ACCEPTABLE;
+    return true;
   }
 
   uint32_t triggeringFlags = loadInfo->GetTriggeringSandboxFlags();
@@ -1172,8 +1171,8 @@ long nsContentSecurityUtils::ClassifyDownload(
     if (httpChannel) {
       LogMessageToConsole(httpChannel, "IframeSandboxBlockedDownload");
     }
-    return nsITransfer::DOWNLOAD_FORBIDDEN;
+    return false;
   }
 
-  return nsITransfer::DOWNLOAD_ACCEPTABLE;
+  return true;
-}
+}

dom/security/nsContentSecurityUtils.h

@@ -51,8 +51,8 @@ class nsContentSecurityUtils {
   static void PerformCSPFrameAncestorAndXFOCheck(nsIChannel* aChannel);
 
   // Helper function to Check if a Download is allowed;
-  static long ClassifyDownload(nsIChannel* aChannel,
+  static bool IsDownloadAllowed(nsIChannel* aChannel,
-                               const nsAutoCString& aMimeTypeGuess);
+                                const nsAutoCString& aMimeTypeGuess);
 
 #if defined(DEBUG)
   static void AssertAboutPageHasCSP(mozilla::dom::Document* aDocument);
@@ -60,6 +60,10 @@ class nsContentSecurityUtils {
 
   static bool ValidateScriptFilename(const char* aFilename,
                                      bool aIsSystemRealm);
+  /*
+   * Checks if a Channel should be able to proceed a download.
+   */
+  static bool IsDownloadAllowed(nsIChannel* aChannel);
   // Helper Function to Post a message to the corresponding JS-Console
   static void LogMessageToConsole(nsIHttpChannel* aChannel, const char* aMsg);
 };

dom/security/test/mixedcontentblocker/browser_test_mixed_content_download.js

@@ -1,9 +1,3 @@
-ChromeUtils.defineModuleGetter(
-  this,
-  "Downloads",
-  "resource://gre/modules/Downloads.jsm"
-);
-
 let INSECURE_BASE_URL =
   getRootDirectory(gTestPath).replace(
     "chrome://mochitests/content/",
@@ -55,41 +49,10 @@ function shouldConsoleError() {
   });
 }
 
-async function resetDownloads() {
-  // Removes all downloads from the download List
-  let publicList = await Downloads.getList(Downloads.PUBLIC);
-  let downloads = await publicList.getAll();
-  for (let download of downloads) {
-    publicList.remove(download);
-    await download.finalize(true);
-  }
-}
-
-async function shouldNotifyDownloadUI() {
-  // Waits until a Blocked download was added to the Download List
-  let list = await Downloads.getList(Downloads.ALL);
-  await new Promise(res => {
-    const view = {
-      onDownloadAdded: aDownload => {
-        let { error } = aDownload;
-        if (
-          error.becauseBlockedByReputationCheck &&
-          error.reputationCheckVerdict == Downloads.Error.BLOCK_VERDICT_INSECURE
-        ) {
-          res(true);
-          list.removeView(view);
-        }
-      },
-    };
-    list.addView(view);
-  });
-}
-
 async function runTest(url, link, checkFunction, decscription) {
   await SpecialPowers.pushPrefEnv({
     set: [["dom.block_download_insecure", true]],
   });
-  await resetDownloads();
 
   let tab = BrowserTestUtils.addTab(gBrowser, url);
   gBrowser.selectedTab = tab;
@@ -129,7 +92,7 @@ add_task(async function() {
   await runTest(
     SECURE_BASE_URL,
     "insecure",
-    () => Promise.all([shouldNotifyDownloadUI(), shouldConsoleError()]),
+    shouldConsoleError,
     "Secure -> Insecure should Error"
   );
   await runTest(

toolkit/components/downloads/DownloadCore.jsm

@@ -1690,7 +1690,6 @@ var DownloadError = function(aProperties) {
  */
 DownloadError.BLOCK_VERDICT_MALWARE = "Malware";
 DownloadError.BLOCK_VERDICT_POTENTIALLY_UNWANTED = "PotentiallyUnwanted";
-DownloadError.BLOCK_VERDICT_INSECURE = "Insecure";
 DownloadError.BLOCK_VERDICT_UNCOMMON = "Uncommon";
 
 DownloadError.prototype = {

toolkit/components/downloads/DownloadLegacy.jsm

@@ -19,11 +19,6 @@ ChromeUtils.defineModuleGetter(
   "Downloads",
   "resource://gre/modules/Downloads.jsm"
 );
-ChromeUtils.defineModuleGetter(
-  this,
-  "DownloadError",
-  "resource://gre/modules/DownloadCore.jsm"
-);
 
 /**
  * nsITransfer implementation that provides a bridge to a Download object.
@@ -274,8 +269,7 @@ DownloadLegacyTransfer.prototype = {
     aStartTime,
     aTempFile,
     aCancelable,
-    aIsPrivate,
+    aIsPrivate
-    aDownloadClassification
   ) {
     return this._nsITransferInitInternal(
       aSource,
@@ -285,8 +279,7 @@ DownloadLegacyTransfer.prototype = {
       aStartTime,
       aTempFile,
       aCancelable,
-      aIsPrivate,
+      aIsPrivate
-      aDownloadClassification
     );
   },
 
@@ -300,7 +293,6 @@ DownloadLegacyTransfer.prototype = {
     aTempFile,
     aCancelable,
     aIsPrivate,
-    aDownloadClassification,
     aBrowsingContext,
     aHandleInternally
   ) {
@@ -321,7 +313,6 @@ DownloadLegacyTransfer.prototype = {
       aTempFile,
       aCancelable,
       aIsPrivate,
-      aDownloadClassification,
       userContextId,
       browsingContextId,
       aHandleInternally
@@ -337,7 +328,6 @@ DownloadLegacyTransfer.prototype = {
     aTempFile,
     aCancelable,
     isPrivate,
-    aDownloadClassification,
     userContextId = 0,
     browsingContextId = 0,
     handleInternally = false
@@ -361,10 +351,11 @@ DownloadLegacyTransfer.prototype = {
         launcherPath = appHandler.executable.path;
       }
     }
+
     // Create a new Download object associated to a DownloadLegacySaver, and
     // wait for it to be available.  This operation may cause the entire
     // download system to initialize before the object is created.
-    let serialisedDownload = {
+    Downloads.createDownload({
       source: {
         url: aSource.spec,
         isPrivate,
@@ -380,20 +371,8 @@ DownloadLegacyTransfer.prototype = {
       contentType,
       launcherPath,
       handleInternally,
-    };
+    })
-
+      .then(aDownload => {
-    // In case the Download was classified as insecure/dangerous
-    // it is already canceled, so we need to generate and attach the
-    // corresponding error to the download.
-    if (aDownloadClassification == Ci.nsITransfer.DOWNLOAD_POTENTIALLY_UNSAFE) {
-      serialisedDownload.errorObj = {
-        becauseBlockedByReputationCheck: true,
-        reputationCheckVerdict: DownloadError.BLOCK_VERDICT_INSECURE,
-      };
-    }
-
-    Downloads.createDownload(serialisedDownload)
-      .then(async aDownload => {
         // Legacy components keep partial data when they use a ".part" file.
         if (aTempFile) {
           aDownload.tryToKeepPartialData = true;
@@ -407,14 +386,9 @@ DownloadLegacyTransfer.prototype = {
         this._resolveDownload(aDownload);
 
         // Add the download to the list, allowing it to be seen and canceled.
-        await (await Downloads.getList(Downloads.ALL)).add(aDownload);
+        return Downloads.getList(Downloads.ALL).then(list =>
-        if (serialisedDownload.errorObj) {
+          list.add(aDownload)
-          // In case we added an already canceled dummy download
+        );
-          // we need to manually trigger a change event
-          // as all the animations for finishing downloads are
-          // listening on onChange.
-          aDownload._notifyChange();
-        }
       })
       .catch(Cu.reportError);
   },

toolkit/components/downloads/test/unit/head.js

@@ -407,8 +407,7 @@ function promiseStartLegacyDownload(aSourceUrl, aOptions) {
           null,
           null,
           persist,
-          isPrivate,
+          isPrivate
-          Ci.nsITransfer.DOWNLOAD_ACCEPTABLE
         );
         persist.progressListener = transfer;
 

toolkit/content/contentAreaUtils.js

@@ -447,8 +447,7 @@ function internalPersist(persistArgs) {
     null,
     null,
     persist,
-    persistArgs.isPrivate,
+    persistArgs.isPrivate
-    Ci.nsITransfer.DOWNLOAD_ACCEPTABLE
   );
   persist.progressListener = new DownloadListener(window, tr);
 

uriloader/base/nsITransfer.idl

@@ -15,10 +15,6 @@ webidl BrowsingContext;
 [scriptable, uuid(37ec75d3-97ad-4da8-afaa-eabe5b4afd73)]
 interface nsITransfer : nsIWebProgressListener2 {
 
-    const unsigned long DOWNLOAD_ACCEPTABLE = 0;
-    const unsigned long DOWNLOAD_FORBIDDEN = 1;
-    const unsigned long DOWNLOAD_POTENTIALLY_UNSAFE = 2;
-
     /**
      * Initializes the transfer with certain properties.  This function must
      * be called prior to accessing any properties on this interface.
@@ -55,8 +51,6 @@ interface nsITransfer : nsIWebProgressListener2 {
      *                   If true, indicates that the transfer was initiated from
      *                   a source that desires privacy.
      *
-     * @param aDownloadClassification Indicates wheter the dowload is unwanted,
-     *                                should be considered dangerous or insecure. 
      */
     void init(in nsIURI aSource,
               in nsIURI aTarget,
@@ -65,8 +59,7 @@ interface nsITransfer : nsIWebProgressListener2 {
               in PRTime startTime,
               in nsIFile aTempFile,
               in nsICancelable aCancelable,
-              in boolean aIsPrivate,
+              in boolean aIsPrivate);
-              in long aDownloadClassification);
 
     /**
      * Same as init, but allows for passing the browsingContext
@@ -86,7 +79,6 @@ interface nsITransfer : nsIWebProgressListener2 {
                                  in nsIFile aTempFile,
                                  in nsICancelable aCancelable,
                                  in boolean aIsPrivate,
-                                 in long aDownloadClassification,
                                  in BrowsingContext aBrowsingContext,
                                  in boolean aHandleInternally);
 

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -1578,22 +1578,10 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
   if (mMimeInfo) {
     mMimeInfo->GetMIMEType(MIMEType);
   }
-  // Now get the URI
-  if (aChannel) {
-    aChannel->GetURI(getter_AddRefs(mSourceUrl));
-  }
 
-  mDownloadClassification =
+  if (!nsContentSecurityUtils::IsDownloadAllowed(aChannel, MIMEType)) {
-      nsContentSecurityUtils::ClassifyDownload(aChannel, MIMEType);
-  if (mDownloadClassification != nsITransfer::DOWNLOAD_ACCEPTABLE) {
-    // If the download is rated as forbidden,
-    // we need to silently cancel the request to make sure
-    // it wont show up in the download ui.
     mCanceled = true;
     request->Cancel(NS_ERROR_ABORT);
-    if (mDownloadClassification != nsITransfer::DOWNLOAD_FORBIDDEN) {
-      CreateFailedTransfer();
-    }
     return NS_OK;
   }
 
@@ -1627,6 +1615,11 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
     }
   }
 
+  // Now get the URI
+  if (aChannel) {
+    aChannel->GetURI(getter_AddRefs(mSourceUrl));
+  }
+
   // retarget all load notifications to our docloader instead of the original
   // window's docloader...
   RetargetLoadNotifications(request);
@@ -2187,12 +2180,11 @@ nsresult nsExternalAppHandler::CreateTransfer() {
     rv = transfer->InitWithBrowsingContext(
         mSourceUrl, target, EmptyString(), mMimeInfo, mTimeDownloadStarted,
         mTempFile, this, channel && NS_UsePrivateBrowsing(channel),
-        mDownloadClassification, mBrowsingContext, mHandleInternally);
+        mBrowsingContext, mHandleInternally);
   } else {
     rv = transfer->Init(mSourceUrl, target, EmptyString(), mMimeInfo,
                         mTimeDownloadStarted, mTempFile, this,
-                        channel && NS_UsePrivateBrowsing(channel),
+                        channel && NS_UsePrivateBrowsing(channel));
-                        mDownloadClassification);
   }
 
   NS_ENSURE_SUCCESS(rv, rv);
@@ -2256,13 +2248,12 @@ nsresult nsExternalAppHandler::CreateFailedTransfer() {
     rv = transfer->InitWithBrowsingContext(
         mSourceUrl, pseudoTarget, EmptyString(), mMimeInfo,
         mTimeDownloadStarted, nullptr, this,
-        channel && NS_UsePrivateBrowsing(channel), mDownloadClassification,
+        channel && NS_UsePrivateBrowsing(channel), mBrowsingContext,
-        mBrowsingContext, mHandleInternally);
+        mHandleInternally);
   } else {
     rv = transfer->Init(mSourceUrl, pseudoTarget, EmptyString(), mMimeInfo,
                         mTimeDownloadStarted, nullptr, this,
-                        channel && NS_UsePrivateBrowsing(channel),
+                        channel && NS_UsePrivateBrowsing(channel));
-                        mDownloadClassification);
   }
   NS_ENSURE_SUCCESS(rv, rv);
 

uriloader/exthandler/nsExternalHelperAppService.h

@@ -333,13 +333,6 @@ class nsExternalAppHandler final : public nsIStreamListener,
    */
   uint32_t mReason;
 
-  /**
-   * Indicates if the nsContentSecurityUtils rate this download as
-   * acceptable, potentialy unwanted or illigal request.
-   *
-   */
-  int32_t mDownloadClassification;
-
   /**
    * Track the executable-ness of the temporary file.
    */