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
--- 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,48 @@ 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.
+ */
+add_task(function* test_channelIsForDownload() {
+ 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);
+});
+
+/**
* 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;