Bug 1139913 - Do not delete placeholders when keeping partial downloads. r?paolo
MozReview-Commit-ID: 4rFZ5rP8saJ
--- a/toolkit/components/jsdownloads/src/DownloadCore.jsm
+++ b/toolkit/components/jsdownloads/src/DownloadCore.jsm
@@ -486,26 +486,23 @@ this.Download.prototype = {
// Now that the actual saving finished, read the actual file size on
// disk, that may be different from the amount of data transferred.
yield this.target.refresh();
// Check for the last time if the download has been canceled. This must
// be done right before setting the "stopped" property of the download,
// without any asynchronous operations in the middle, so that another
// cancellation request cannot start in the meantime and stay unhandled.
- if (this._promiseCanceled) {
+ if (this._promiseCanceled && !this.tryToKeepPartialData) {
try {
- yield OS.File.remove(this.target.path);
+ yield this.saver.removePartialData();
} catch (ex) {
Cu.reportError(ex);
}
- this.target.exists = false;
- this.target.size = 0;
-
// Cancellation exceptions will be changed in the catch block below.
throw new DownloadError();
}
// Update the status properties for a successful download.
this.progress = 100;
this.succeeded = true;
this.hasPartialData = false;
@@ -674,17 +671,17 @@ this.Download.prototype = {
if (!this.hasBlockedData) {
return Promise.reject(new Error(
"confirmBlock may only be called on Downloads with blocked data."));
}
this._promiseConfirmBlock = Task.spawn(function* () {
try {
- yield OS.File.remove(this.target.partFilePath);
+ yield this.saver.removePartialData();
} catch (ex) {
yield this.refresh();
this._promiseConfirmBlock = null;
throw ex;
}
this.hasBlockedData = false;
this._notifyChange();
@@ -2138,24 +2135,25 @@ this.DownloadCopySaver.prototype = {
yield deferSaveComplete.promise;
yield this._checkReputationAndMove(aSetPropertiesFn);
} catch (ex) {
// Ensure we always remove the placeholder for the final target file on
// failure, independently of which code path failed. In some cases, the
// background file saver may have already removed the file.
try {
- yield OS.File.remove(targetPath);
+ if (!this.download.tryToKeepPartialData || this.download.currentBytes <= 0) {
+ yield this.removePartialData();
+ }
} catch (e2) {
// If we failed during the operation, we report the error but use the
// original one as the failure reason of the download. Note that on
// Windows we may get an access denied error instead of a no such file
// error if the file existed before, and was recently deleted.
- if (!(e2 instanceof OS.File.Error &&
- (e2.becauseNoSuchFile || e2.becauseAccessDenied))) {
+ if (!(e2 instanceof OS.File.Error && e2.becauseAccessDenied)) {
Cu.reportError(e2);
}
}
throw ex;
}
}.bind(this));
},
@@ -2183,17 +2181,17 @@ this.DownloadCopySaver.prototype = {
let newProperties = { progress: 100, hasPartialData: false };
// We will remove the potentially dangerous file if instructed by
// DownloadIntegration. We will always remove the file when the
// download did not use a partial file path, meaning it
// currently has its final filename.
if (!DownloadIntegration.shouldKeepBlockedData() || !partFilePath) {
try {
- yield OS.File.remove(partFilePath || targetPath);
+ yield this.removePartialData();
} catch (ex) {
Cu.reportError(ex);
}
} else {
newProperties.hasBlockedData = true;
}
aSetPropertiesFn(newProperties);
@@ -2222,25 +2220,30 @@ this.DownloadCopySaver.prototype = {
},
/**
* Implements "DownloadSaver.removePartialData".
*/
removePartialData: function ()
{
return Task.spawn(function* task_DCS_removePartialData() {
- if (this.download.target.partFilePath) {
try {
- yield OS.File.remove(this.download.target.partFilePath);
+ if (this.download.target.path) {
+ yield OS.File.remove(this.download.target.path);
+ this.download.target.exists = false;
+ this.download.target.size = 0;
+ }
+ if (this.download.target.partFilePath) {
+ yield OS.File.remove(this.download.target.partFilePath);
+ }
} catch (ex) {
if (!(ex instanceof OS.File.Error) || !ex.becauseNoSuchFile) {
throw ex;
}
}
- }
}.bind(this));
},
/**
* Implements "DownloadSaver.toSerializable".
*/
toSerializable: function ()
{
@@ -2539,24 +2542,25 @@ this.DownloadLegacySaver.prototype = {
yield this._checkReputationAndMove(aSetPropertiesFn);
} catch (ex) {
// Ensure we always remove the final target file on failure,
// independently of which code path failed. In some cases, the
// component executing the download may have already removed the file.
try {
- yield OS.File.remove(this.download.target.path);
+ if (!this.download.tryToKeepPartialData || this.download.currentBytes <= 0) {
+ yield this.removePartialData();
+ }
} catch (e2) {
// If we failed during the operation, we report the error but use the
// original one as the failure reason of the download. Note that on
// Windows we may get an access denied error instead of a no such file
// error if the file existed before, and was recently deleted.
- if (!(e2 instanceof OS.File.Error &&
- (e2.becauseNoSuchFile || e2.becauseAccessDenied))) {
+ if (!(e2 instanceof OS.File.Error && e2.becauseAccessDenied)) {
Cu.reportError(e2);
}
}
// In case the operation failed, ensure we stop downloading data. Since
// we never re-enter this function, deferCanceled is always available.
this.deferCanceled.resolve();
throw ex;
} finally {
--- a/toolkit/components/jsdownloads/test/unit/common_test_Download.js
+++ b/toolkit/components/jsdownloads/test/unit/common_test_Download.js
@@ -659,16 +659,17 @@ add_task(function* test_cancel_midway()
do_check_true(download.stopped);
do_check_true(download.canceled);
do_check_true(download.error === null);
do_check_false(download.target.exists);
do_check_eq(download.target.size, 0);
do_check_false(yield OS.File.exists(download.target.path));
+ do_check_false(yield OS.File.exists(download.target.partFilePath));
// Progress properties are not reset by canceling.
do_check_eq(download.progress, 50);
do_check_eq(download.totalBytes, TEST_DATA_SHORT.length * 2);
do_check_eq(download.currentBytes, TEST_DATA_SHORT.length);
if (!gUseLegacySaver) {
// The promise returned by "start" should have been rejected meanwhile.
@@ -794,20 +795,19 @@ add_task(function* test_cancel_midway_re
add_task(function* test_cancel_midway_restart_tryToKeepPartialData()
{
let download = yield promiseStartDownload_tryToKeepPartialData();
yield download.cancel();
do_check_true(download.stopped);
do_check_true(download.hasPartialData);
- // The target file should not exist, but we should have kept the partial data.
- do_check_false(yield OS.File.exists(download.target.path));
+ // The target file should exist and we should have kept the partial data.
+ do_check_true(yield OS.File.exists(download.target.path));
yield promiseVerifyContents(download.target.partFilePath, TEST_DATA_SHORT);
- do_check_false(download.target.exists);
do_check_eq(download.target.size, 0);
// Verify that the server sent the response from the start.
do_check_eq(gMostRecentFirstBytePos, 0);
// The second time, we'll request and obtain the second part of the response,
// but we still stop when half of the remaining progress is reached.
let deferMidway = Promise.defer();
@@ -1246,16 +1246,17 @@ add_task(function* test_error_source()
// Check the properties now that the download stopped.
do_check_true(download.stopped);
do_check_false(download.canceled);
do_check_true(download.error !== null);
do_check_true(download.error.becauseSourceFailed);
do_check_false(download.error.becauseTargetFailed);
do_check_false(yield OS.File.exists(download.target.path));
+ do_check_false(yield OS.File.exists(download.target.partFilePath));
do_check_false(download.target.exists);
do_check_eq(download.target.size, 0);
} finally {
serverSocket.close();
}
});
/**
@@ -1676,16 +1677,19 @@ add_task(function* test_blocked_parental
throw ex;
}
do_check_true(ex.becauseBlockedByParentalControls);
do_check_true(download.error.becauseBlockedByParentalControls);
do_check_true(download.stopped);
}
do_check_false(yield OS.File.exists(download.target.path));
+ do_check_false(download.target.exists);
+ do_check_eq(download.target.size, 0);
+ do_check_false(yield OS.File.exists(download.target.partFilePath));
});
/**
* Download with runtime permissions
*/
add_task(function* test_blocked_runtime_permissions()
{
let blockFn = base => ({
@@ -1795,17 +1799,17 @@ var promiseBlockedDownload = Task.async(
Downloads.Error.BLOCK_VERDICT_UNCOMMON);
do_check_true(download.error.becauseBlockedByReputationCheck);
do_check_eq(download.error.reputationCheckVerdict,
Downloads.Error.BLOCK_VERDICT_UNCOMMON);
}
do_check_true(download.stopped);
do_check_false(download.succeeded);
- do_check_false(yield OS.File.exists(download.target.path));
+ do_check_eq(download.target.size, 0);
cleanup();
return download;
});
/**
* Checks that application reputation blocks the download and the target file
* does not exist.
@@ -1906,16 +1910,17 @@ add_task(function* test_blocked_applicat
add_task(function* test_blocked_applicationReputation_confirmBlock()
{
let download = yield promiseBlockedDownload({
keepPartialData: true,
keepBlockedData: true,
});
do_check_true(download.hasBlockedData);
+ do_check_true(yield OS.File.exists(download.target.path));
do_check_true(yield OS.File.exists(download.target.partFilePath));
yield download.confirmBlock();
// After confirming the block the download should be in a failed state and
// have no downloaded data left on disk.
do_check_true(download.stopped);
do_check_false(download.succeeded);
@@ -1933,26 +1938,28 @@ add_task(function* test_blocked_applicat
add_task(function* test_blocked_applicationReputation_unblock()
{
let download = yield promiseBlockedDownload({
keepPartialData: true,
keepBlockedData: true,
});
do_check_true(download.hasBlockedData);
+ do_check_true(yield OS.File.exists(download.target.path));
do_check_true(yield OS.File.exists(download.target.partFilePath));
yield download.unblock();
// After unblocking the download should have succeeded and be
// present at the final path.
do_check_true(download.stopped);
do_check_true(download.succeeded);
do_check_false(download.hasBlockedData);
do_check_false(yield OS.File.exists(download.target.partFilePath));
+ do_check_true(yield OS.File.exists(download.target.path));
yield promiseVerifyTarget(download.target, TEST_DATA_SHORT + TEST_DATA_SHORT);
// The only indication the download was previously blocked is the
// existence of the error, so we make sure it's still set.
do_check_true(download.error instanceof Downloads.Error);
do_check_true(download.error.becauseBlocked);
do_check_true(download.error.becauseBlockedByReputationCheck);
});