Bug 1254327 (Part 2) Use filename callback from chrome.downloads.download() r?kmag
MozReview-Commit-ID: 6W4U2Uy1IVD
--- a/toolkit/components/extensions/ext-downloads.js
+++ b/toolkit/components/extensions/ext-downloads.js
@@ -37,17 +37,23 @@ const DOWNLOAD_ITEM_CHANGE_FIELDS = ["en
"error", "exists"];
class DownloadItem {
constructor(id, download, extension) {
this.id = id;
this.download = download;
this.extension = extension;
- this.prechange = {};
+ this.prechange = {
+ state: null,
+ paused: false,
+ canResume: false,
+ error: null,
+ exists: false,
+ };
}
get url() { return this.download.source.url; }
get referrer() { return this.download.source.referrer; }
get filename() { return this.download.target.path; }
get incognito() { return this.download.source.isPrivate; }
get danger() { return "safe"; } // TODO
get mime() { return this.download.contentType; }
@@ -164,21 +170,17 @@ const DownloadMap = {
}
},
onDownloadChanged(download) {
const item = self.byDownload.get(download);
if (item == null) {
Cu.reportError("Got onDownloadChanged for unknown download object");
} else {
- // We get the first one of these when the download is started.
- // In this case, don't emit anything, just initialize prechange.
- if (Object.keys(item.prechange).length > 0) {
- self.emit("change", item);
- }
+ self.emit("change", item);
item._change();
}
},
}).then(() => list.getAll())
.then(downloads => {
downloads.forEach(download => {
this.newFromDownload(download, null);
});
@@ -412,67 +414,78 @@ extensions.registerSchemaAPI("downloads"
}
}
if (options.conflictAction == "prompt") {
// TODO
return Promise.reject({message: "conflictAction prompt not yet implemented"});
}
- function createTarget(downloadsDir) {
- // TODO
- // if (options.saveAs) { }
-
- let target;
- if (options.filename) {
- target = OS.Path.join(downloadsDir, options.filename);
- } else {
- let uri = NetUtil.newURI(options.url).QueryInterface(Ci.nsIURL);
- target = OS.Path.join(downloadsDir, uri.fileName);
- }
+ // The flow of control here is a little messy... Creating the
+ // download is an asynchronous operation which calls createTarget()
+ // asynchronously. We need to wait for createTarget() to complete
+ // before adding the new download object to a DownloadList (this is
+ // a constraint of DownloadList). But since we don't directly call
+ // createTarget(), we have to wrap it with targetPromise to find out
+ // when it is done.
+ let createTarget;
+ let targetPromise = new Promise(resolve => {
+ createTarget = function(downloadsDir, request) {
+ // TODO
+ // if (options.saveAs) { }
- // 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.
- return OS.File.exists(target).then(exists => {
- if (exists) {
- switch (options.conflictAction) {
- case "uniquify":
- default:
- target = DownloadPaths.createNiceUniqueFile(new FileUtils.File(target)).path;
- break;
+ let target;
+ if (options.filename) {
+ target = OS.Path.join(downloadsDir, options.filename);
+ } else {
+ let uri = request.URI;
+ uri.QueryInterface(Ci.nsIURL);
+ let filename = uri.fileName;
+ if (filename == "") {
+ filename = "download";
+ }
+ target = OS.Path.join(downloadsDir, filename);
+ }
- case "overwrite":
- break;
- }
- }
- return target;
- });
- }
+ let promise = OS.File.exists(target).then(exists => {
+ if (exists) {
+ switch (options.conflictAction) {
+ case "uniquify":
+ default:
+ target = DownloadPaths.createNiceUniqueFile(new FileUtils.File(target)).path;
+ break;
- let download;
- return Downloads.getPreferredDownloadsDirectory()
- .then(downloadsDir => createTarget(downloadsDir))
- .then(target => Downloads.createDownload({
- source: options.url,
- target: {
- path: target,
- partFilePath: target + ".part",
- },
- })).then(dl => {
- download = dl;
- return DownloadMap.getDownloadList();
- }).then(list => {
+ case "overwrite":
+ break;
+ }
+ }
+ return target;
+ });
+ promise.then(resolve);
+ return promise;
+ };
+ });
+
+ let downloadPromise = Downloads.getPreferredDownloadsDirectory()
+ .then(downloadsDir => Downloads.createDownload({
+ source: options.url,
+ target: {
+ path: downloadsDir,
+ filenameCallback: req => createTarget(downloadsDir, req),
+ },
+ })).then(download => {
+ // This is necessary to make pause/resume work.
+ download.tryToKeepPartialData = true;
+ download.start();
+ return download;
+ });
+
+ return Promise.all([downloadPromise, targetPromise, DownloadMap.getDownloadList()])
+ .then(([download, unused, list]) => {
list.add(download);
-
- // This is necessary to make pause/resume work.
- download.tryToKeepPartialData = true;
- download.start();
-
const item = DownloadMap.newFromDownload(download, extension);
return item.id;
});
},
removeFile(id) {
return DownloadMap.lazyInit().then(() => {
let item;
--- a/toolkit/components/extensions/test/mochitest/chrome.ini
+++ b/toolkit/components/extensions/test/mochitest/chrome.ini
@@ -1,13 +1,14 @@
[DEFAULT]
support-files =
file_download.html
file_download.txt
interruptible.sjs
+ redirect.sjs
file_sample.html
[test_chrome_ext_downloads_download.html]
[test_chrome_ext_downloads_misc.html]
skip-if = 1 # Currently causes too many intermittent failures.
[test_chrome_ext_downloads_search.html]
[test_chrome_ext_eventpage_warning.html]
[test_chrome_ext_contentscript_unrecognizedprop_warning.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/redirect.sjs
@@ -0,0 +1,10 @@
+const TARGET = "file_download.txt";
+
+// A handler that generates an HTTP redirect to TARGET
+function handleRequest(request, response) {
+ response.setStatusLine(request.httpVersion, 301, "Moved Permanently");
+
+ let newpath = request.path.slice(0, request.path.lastIndexOf('/'));
+ let target = `${request.scheme}://${request.host}:${request.port}${newpath}/${TARGET}`;
+ response.setHeader("Location", target, false);
+}
--- a/toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_download.html
+++ b/toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_download.html
@@ -126,16 +126,19 @@ add_task(function* test_downloads() {
yield testDownload({url: FILE_URL}, FILE_NAME, FILE_LEN, "just source");
// Call download() with a filename property.
yield testDownload({
url: FILE_URL,
filename: "newpath.txt",
}, "newpath.txt", FILE_LEN, "source and filename");
+ // Call download() without a filename on a URL that is redirected
+ yield testDownload({url: BASE + "/redirect.sjs"}, FILE_NAME, FILE_LEN, "redirected source");
+
// Check conflictAction of "uniquify".
touch(FILE_NAME);
yield testDownload({
url: FILE_URL,
conflictAction: "uniquify",
}, FILE_NAME_UNIQUE, FILE_LEN, "conflictAction=uniquify");
// todo check that preexisting file was not modified?
remove(FILE_NAME);