Bug 1348062 - Test that channels are properly marked for downloads. r=mak
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Wed, 19 Apr 2017 10:42:00 +0100
changeset 354530 ffe6a7d2c74c07ac6ecf8a6b890141dacbaa9b9e
parent 354529 e566c50eeb1beb85581e810ca1ba4f621fce4fe2
child 354531 f6f3a2c33c8f0faaff9f06e13bdf99f07e4e0a37
push id89485
push userpaolo.mozmail@amadzone.org
push dateMon, 24 Apr 2017 10:07:37 +0000
treeherdermozilla-inbound@ffe6a7d2c74c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1348062
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1348062 - Test that channels are properly marked for downloads. r=mak Downloads handled by nsIExternalHelperAppService pass a null request to the final onStateChange notification, thus we need to hold a reference to the request earlier. This also allows unit tests to access the request while the download is running. MozReview-Commit-ID: 21rapDmMIZw
toolkit/components/jsdownloads/src/DownloadCore.jsm
toolkit/components/jsdownloads/src/DownloadLegacy.js
toolkit/components/jsdownloads/test/unit/common_test_Download.js
--- a/toolkit/components/jsdownloads/src/DownloadCore.jsm
+++ b/toolkit/components/jsdownloads/src/DownloadCore.jsm
@@ -2406,16 +2406,20 @@ this.DownloadLegacySaver.prototype = {
    * @param aAlreadyAddedToHistory
    *        Indicates that the nsIExternalHelperAppService component already
    *        added the download to the browsing history, unless it was started
    *        from a private browsing window.  When this parameter is false, the
    *        download is added to the browsing history here.  Private downloads
    *        are never added to history even if this parameter is false.
    */
   onTransferStarted(aRequest, aAlreadyAddedToHistory) {
+    // Store a reference to the request, used in some cases when handling
+    // completion, and also checked during the download by unit tests.
+    this.request = aRequest;
+
     // Store the entity ID to use for resuming if required.
     if (this.download.tryToKeepPartialData &&
         aRequest instanceof Ci.nsIResumableChannel) {
       try {
         // If reading the ID succeeds, the source is resumable.
         this.entityID = aRequest.entityID;
       } catch (ex) {
         if (!(ex instanceof Components.Exception) ||
@@ -2433,25 +2437,20 @@ this.DownloadLegacySaver.prototype = {
     if (!aAlreadyAddedToHistory) {
       this.addToHistory();
     }
   },
 
   /**
    * Called by the nsITransfer implementation when the request has finished.
    *
-   * @param aRequest
-   *        nsIRequest associated to the status update.
    * @param aStatus
    *        Status code received by the nsITransfer implementation.
    */
-  onTransferFinished: function DLS_onTransferFinished(aRequest, aStatus) {
-    // Store a reference to the request, used when handling completion.
-    this.request = aRequest;
-
+  onTransferFinished: function DLS_onTransferFinished(aStatus) {
     if (Components.isSuccessCode(aStatus)) {
       this.deferExecuted.resolve();
     } else {
       // Infer the origin of the error from the failure code, because more
       // specific data is not available through the nsITransfer implementation.
       let properties = { result: aStatus, inferCause: true };
       this.deferExecuted.reject(new DownloadError(properties));
     }
--- a/toolkit/components/jsdownloads/src/DownloadLegacy.js
+++ b/toolkit/components/jsdownloads/src/DownloadLegacy.js
@@ -122,17 +122,17 @@ DownloadLegacyTransfer.prototype = {
         // notifying that the download has been canceled.  Since the request has
         // not completed yet, deferCanceled is guaranteed to be set.
         return download.saver.deferCanceled.promise.then(() => {
           // Only cancel if the object executing the download is still running.
           if (this._cancelable && !this._componentFailed) {
             this._cancelable.cancel(Cr.NS_ERROR_ABORT);
             if (this._cancelable instanceof Ci.nsIWebBrowserPersist) {
               // This component will not send the STATE_STOP notification.
-              download.saver.onTransferFinished(aRequest, Cr.NS_ERROR_ABORT);
+              download.saver.onTransferFinished(Cr.NS_ERROR_ABORT);
               this._cancelable = null;
             }
           }
         });
       }).then(null, Cu.reportError);
     } else if ((aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) &&
         (aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK)) {
       // The last file has been received, or the download failed.  Wait for the
@@ -140,17 +140,17 @@ DownloadLegacyTransfer.prototype = {
       this._deferDownload.promise.then(download => {
         // At this point, the hash has been set and we need to copy it to the
         // DownloadSaver.
         if (Components.isSuccessCode(aStatus)) {
           download.saver.setSha256Hash(this._sha256Hash);
           download.saver.setSignatureInfo(this._signatureInfo);
           download.saver.setRedirects(this._redirects);
         }
-        download.saver.onTransferFinished(aRequest, aStatus);
+        download.saver.onTransferFinished(aStatus);
       }).then(null, Cu.reportError);
 
       // Release the reference to the component executing the download.
       this._cancelable = null;
     }
   },
 
   onProgressChange: function DLT_onProgressChange(aWebProgress, aRequest,
@@ -170,17 +170,17 @@ DownloadLegacyTransfer.prototype = {
     // The status change may optionally be received in addition to the state
     // change, but if no network request actually started, it is possible that
     // we only receive a status change with an error status code.
     if (!Components.isSuccessCode(aStatus)) {
       this._componentFailed = true;
 
       // Wait for the associated Download object to be available.
       this._deferDownload.promise.then(function DLT_OSC_onDownload(aDownload) {
-        aDownload.saver.onTransferFinished(aRequest, aStatus);
+        aDownload.saver.onTransferFinished(aStatus);
       }).then(null, Cu.reportError);
     }
   },
 
   onSecurityChange() { },
 
   // nsIWebProgressListener2
 
--- a/toolkit/components/jsdownloads/test/unit/common_test_Download.js
+++ b/toolkit/components/jsdownloads/test/unit/common_test_Download.js
@@ -225,16 +225,53 @@ add_task(function* test_basic_tryToKeepP
 
   // The target file should now have been created, and the ".part" file deleted.
   yield promiseVerifyTarget(download.target, TEST_DATA_SHORT + TEST_DATA_SHORT);
   do_check_false(yield OS.File.exists(download.target.partFilePath));
   do_check_eq(32, download.saver.getSha256Hash().length);
 });
 
 /**
+ * Tests that the channelIsForDownload property is set for the network request,
+ * and that the request is marked as throttleable.
+ */
+add_task(function* test_channelIsForDownload_classFlags() {
+  let downloadChannel = null;
+
+  // We use a different method based on whether we are testing legacy downloads.
+  if (!gUseLegacySaver) {
+    let download = yield Downloads.createDownload({
+      source: {
+        url: httpUrl("interruptible_resumable.txt"),
+        async adjustChannel(channel) {
+          downloadChannel = channel;
+        },
+      },
+      target: getTempFile(TEST_TARGET_FILE_NAME).path,
+    });
+    yield download.start();
+  } else {
+    // Start a download using nsIExternalHelperAppService, but ensure it cannot
+    // finish before we retrieve the "request" property.
+    mustInterruptResponses();
+    let download = yield promiseStartExternalHelperAppServiceDownload();
+    downloadChannel = download.saver.request;
+    continueResponses();
+    yield promiseDownloadStopped(download);
+  }
+
+  do_check_true(downloadChannel.QueryInterface(Ci.nsIHttpChannelInternal)
+                               .channelIsForDownload);
+
+  // Throttleable is the only class flag assigned to downloads.
+  do_check_eq(downloadChannel.QueryInterface(Ci.nsIClassOfService)
+                             .classFlags, Ci.nsIClassOfService.Throttleable);
+});
+
+/**
  * Tests the permissions of the final target file once the download finished.
  */
 add_task(function* test_unix_permissions() {
   // This test is only executed on some Desktop systems.
   if (Services.appinfo.OS != "Darwin" && Services.appinfo.OS != "Linux" &&
       Services.appinfo.OS != "WINNT") {
     do_print("Skipping test.");
     return;