Bug 1367572 - Remove uniquified file when used in combination with saveAs option
MozReview-Commit-ID: 4atukwcsZgA
--- a/toolkit/components/extensions/ext-downloads.js
+++ b/toolkit/components/extensions/ext-downloads.js
@@ -444,72 +444,75 @@ this.downloads = class extends Extension
channel.QueryInterface(Ci.nsIUploadChannel2);
channel.explicitSetUploadStream(stream, null, -1, method, false);
}
}
return Promise.resolve();
}
- function createTarget(downloadsDir) {
+ async function createTarget(downloadsDir) {
let target;
if (filename) {
target = OS.Path.join(downloadsDir, filename);
} else {
let uri = NetUtil.newURI(options.url);
let remote = "download";
if (uri instanceof Ci.nsIURL) {
remote = uri.fileName;
}
target = OS.Path.join(downloadsDir, remote);
}
// Create any needed subdirectories if required by filename.
const dir = OS.Path.dirname(target);
- return OS.File.makeDir(dir, {from: downloadsDir}).then(() => {
- return OS.File.exists(target);
- }).then(exists => {
+ await OS.File.makeDir(dir, {from: downloadsDir});
+
+ if (await OS.File.exists(target)) {
// This has a race, something else could come along and create
// the file between this test and them time the download code
// creates the target file. But we can't easily fix it without
// modifying DownloadCore so we live with it for now.
- if (exists) {
- switch (options.conflictAction) {
- case "uniquify":
- default:
- target = DownloadPaths.createNiceUniqueFile(new FileUtils.File(target)).path;
- break;
+ switch (options.conflictAction) {
+ case "uniquify":
+ default:
+ target = DownloadPaths.createNiceUniqueFile(new FileUtils.File(target)).path;
+ if (options.saveAs) {
+ // createNiceUniqueFile actually creates the file, which
+ // is premature if we need to show a SaveAs dialog.
+ await OS.File.remove(target);
+ }
+ break;
- case "overwrite":
- break;
- }
+ case "overwrite":
+ break;
}
- }).then(() => {
- if (!options.saveAs) {
- return Promise.resolve(target);
- }
+ }
- // Setup the file picker Save As dialog.
- const picker = Cc["@mozilla.org/filepicker;1"].createInstance(Ci.nsIFilePicker);
- const window = Services.wm.getMostRecentWindow("navigator:browser");
- picker.init(window, null, Ci.nsIFilePicker.modeSave);
- picker.displayDirectory = new FileUtils.File(dir);
- picker.appendFilters(Ci.nsIFilePicker.filterAll);
- picker.defaultString = OS.Path.basename(target);
+ if (!options.saveAs) {
+ return target;
+ }
- // Open the dialog and resolve/reject with the result.
- return new Promise((resolve, reject) => {
- picker.open(result => {
- if (result === Ci.nsIFilePicker.returnCancel) {
- reject({message: "Download canceled by the user"});
- } else {
- resolve(picker.file.path);
- }
- });
+ // Setup the file picker Save As dialog.
+ const picker = Cc["@mozilla.org/filepicker;1"].createInstance(Ci.nsIFilePicker);
+ const window = Services.wm.getMostRecentWindow("navigator:browser");
+ picker.init(window, null, Ci.nsIFilePicker.modeSave);
+ picker.displayDirectory = new FileUtils.File(dir);
+ picker.appendFilters(Ci.nsIFilePicker.filterAll);
+ picker.defaultString = OS.Path.basename(target);
+
+ // Open the dialog and resolve/reject with the result.
+ return new Promise((resolve, reject) => {
+ picker.open(result => {
+ if (result === Ci.nsIFilePicker.returnCancel) {
+ reject({message: "Download canceled by the user"});
+ } else {
+ resolve(picker.file.path);
+ }
});
});
}
let download;
return Downloads.getPreferredDownloadsDirectory()
.then(downloadsDir => createTarget(downloadsDir))
.then(target => {
--- a/toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_saveAs.html
+++ b/toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_saveAs.html
@@ -1,63 +1,125 @@
<!doctype html>
<html>
<head>
<title>Test downloads.download() saveAs option</title>
<script src="chrome://mochikit/content/tests/SimpleTest/SpawnTask.js"></script>
<script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
<script src="chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js"></script>
+ <script src="chrome_head.js"></script>
+ <script src="head.js"></script>
<link rel="stylesheet" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
</head>
<body>
<script type="text/javascript">
"use strict";
+Cu.import("resource://gre/modules/FileUtils.jsm");
+
+let directory;
+
+add_task(async function setup() {
+ directory = FileUtils.getDir("TmpD", ["downloads"]);
+ directory.createUnique(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
+ info(`Using download directory ${directory.path}`);
+
+ await SpecialPowers.pushPrefEnv({"set": [
+ ["browser.download.folderList", 2],
+ ["browser.download.dir", directory.path],
+ ]});
+
+ SimpleTest.registerCleanupFunction(async () => {
+ await SpecialPowers.popPrefEnv();
+ directory.remove(true);
+ });
+});
+
add_task(async function test_downloads_saveAs() {
+ const file = directory.clone();
+ file.append("file_download.txt");
+
+ const unique = directory.clone();
+ unique.append("file_download(1).txt");
+
+ const {MockFilePicker} = SpecialPowers;
+ MockFilePicker.init(window);
+
+ MockFilePicker.showCallback = fp => {
+ let file = directory.clone();
+ file.append(fp.defaultString);
+ MockFilePicker.setFiles([file]);
+ };
+
function background() {
const url = URL.createObjectURL(new Blob(["file content"]));
- browser.test.onMessage.addListener(async () => {
+ browser.test.onMessage.addListener(async filename => {
try {
- let id = await browser.downloads.download({url, saveAs: true});
+ let id = await browser.downloads.download({
+ url,
+ filename,
+ saveAs: true,
+ conflictAction: "uniquify",
+ });
browser.downloads.onChanged.addListener(delta => {
if (delta.id == id && delta.state.current === "complete") {
browser.test.sendMessage("done", {ok: true, id});
}
});
} catch ({message}) {
browser.test.sendMessage("done", {ok: false, message});
}
});
browser.test.sendMessage("ready");
}
- const {MockFilePicker} = SpecialPowers;
const manifest = {background, manifest: {permissions: ["downloads"]}};
const extension = ExtensionTestUtils.loadExtension(manifest);
- MockFilePicker.init(window);
- const file = await MockFilePicker.useAnyFile();
-
await extension.startup();
await extension.awaitMessage("ready");
- extension.sendMessage("download");
+ // Test basic saveAs functionality.
+ MockFilePicker.returnValue = MockFilePicker.returnOK;
+
+ extension.sendMessage("file_download.txt");
let result = await extension.awaitMessage("done");
-
ok(result.ok, "downloads.download() works with saveAs");
ok(file.exists(), "the file exists.");
is(file.fileSize, 12, "downloaded file is the correct size");
+
+ // Test exisisting file with uniquify.
+ MockFilePicker.returnValue = MockFilePicker.returnOK;
+
+ extension.sendMessage("file_download.txt");
+ result = await extension.awaitMessage("done");
+ ok(result.ok, "downloads.download() works with saveAs and uniquify");
+
+ ok(unique.exists(), "the file exists.");
+ is(unique.fileSize, 12, "downloaded file is the correct size");
+ unique.remove(false);
+
+ // Test canceled saveAs for an existing file and uniquify.
+ MockFilePicker.returnValue = MockFilePicker.returnCancel;
+
+ extension.sendMessage("file_download.txt");
+ result = await extension.awaitMessage("done");
+
+ ok(!result.ok, "download rejected if the user cancels the dialog");
+ is(result.message, "Download canceled by the user", "with the correct message");
+
+ ok(!unique.exists(), "unique file not left after SaveAs canceled.");
file.remove(false);
// Test the user canceling the save dialog.
MockFilePicker.returnValue = MockFilePicker.returnCancel;
- extension.sendMessage("download");
+ extension.sendMessage("file_download.txt");
result = await extension.awaitMessage("done");
ok(!result.ok, "download rejected if the user cancels the dialog");
is(result.message, "Download canceled by the user", "with the correct message");
ok(!file.exists(), "file was not downloaded");
await extension.unload();
MockFilePicker.cleanup();