Bug 1245600 - Implement chrome.downloads.onChanged for state r?kmag
MozReview-Commit-ID: BaAyU1dgMB7
--- a/toolkit/components/extensions/ext-downloads.js
+++ b/toolkit/components/extensions/ext-downloads.js
@@ -9,34 +9,39 @@ XPCOMUtils.defineLazyModuleGetter(this,
XPCOMUtils.defineLazyModuleGetter(this, "DownloadPaths",
"resource://gre/modules/DownloadPaths.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "OS",
"resource://gre/modules/osfile.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
"resource://gre/modules/FileUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
"resource://gre/modules/NetUtil.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "EventEmitter",
+ "resource://devtools/shared/event-emitter.js");
Cu.import("resource://gre/modules/ExtensionUtils.jsm");
const {
ignoreEvent,
+ runSafeSync,
+ SingletonEventManager,
} = ExtensionUtils;
const DOWNLOAD_ITEM_FIELDS = ["id", "url", "referrer", "filename", "incognito",
"danger", "mime", "startTime", "endTime",
"estimatedEndTime", "state", "canResume",
"error", "bytesReceived", "totalBytes",
"fileSize", "exists",
"byExtensionId", "byExtensionName"];
class DownloadItem {
constructor(id, download, extension) {
this.id = id;
this.download = download;
this.extension = extension;
+ this.prechange = {};
}
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; }
@@ -98,16 +103,26 @@ class DownloadItem {
for (let field of DOWNLOAD_ITEM_FIELDS) {
obj[field] = this[field];
}
if (obj.startTime) {
obj.startTime = obj.startTime.toISOString();
}
return obj;
}
+
+ // When a change event fires, handlers can look at how an individual
+ // field changed by comparing item.fieldname with item.prechange.fieldname.
+ // After all handlers have been invoked, this gets called to store the
+ // current values of all fields ahead of the next event.
+ _change() {
+ for (let field of DOWNLOAD_ITEM_FIELDS) {
+ this.prechange[field] = this[field];
+ }
+ }
}
// DownloadMap maps back and forth betwen the numeric identifiers used in
// the downloads WebExtension API and a Download object from the Downloads jsm.
// todo: make id and extension info persistent (bug 1247794)
const DownloadMap = {
currentId: 0,
@@ -116,30 +131,41 @@ const DownloadMap = {
// Maps numeric id -> DownloadItem
byId: new Map(),
// Maps Download object -> DownloadItem
byDownload: new WeakMap(),
lazyInit() {
if (this.loadPromise == null) {
+ EventEmitter.decorate(this);
this.loadPromise = Downloads.getList(Downloads.ALL).then(list => {
let self = this;
return list.addView({
onDownloadAdded(download) {
self.newFromDownload(download, null);
},
onDownloadRemoved(download) {
const item = self.byDownload.get(download);
if (item != null) {
self.byDownload.delete(download);
self.byId.delete(item.id);
}
},
+
+ onDownloadChanged(download) {
+ const item = self.byDownload.get(download);
+ if (item == null) {
+ Cu.reportError("Got onDownloadChanged for unknown download object");
+ } else {
+ self.emit("change", item);
+ item._change();
+ }
+ },
}).then(() => list.getAll())
.then(downloads => {
downloads.forEach(download => {
this.newFromDownload(download, null);
});
})
.then(() => list);
});
@@ -417,15 +443,37 @@ extensions.registerSchemaAPI("downloads"
// open(downloadId) {
// if (!extension.hasPermission("downloads.open")) {
// throw new context.cloneScope.Error("Permission denied because 'downloads.open' permission is missing.");
// }
// ...
// }
// likewise for setShelfEnabled() and the "download.shelf" permission
+ onChanged: new SingletonEventManager(context, "downloads.onChanged", fire => {
+ const handler = (what, item) => {
+ if (item.state != item.prechange.state) {
+ runSafeSync(context, fire, {
+ id: item.id,
+ state: {
+ previous: item.prechange.state || null,
+ current: item.state,
+ },
+ });
+ }
+ };
+
+ let registerPromise = DownloadMap.getDownloadList().then(() => {
+ DownloadMap.on("change", handler);
+ });
+ return () => {
+ registerPromise.then(() => {
+ DownloadMap.off("change", handler);
+ });
+ };
+ }).api(),
+
onCreated: ignoreEvent(context, "downloads.onCreated"),
onErased: ignoreEvent(context, "downloads.onErased"),
- onChanged: ignoreEvent(context, "downloads.onChanged"),
onDeterminingFilename: ignoreEvent(context, "downloads.onDeterminingFilename"),
},
};
});
--- a/toolkit/components/extensions/schemas/downloads.json
+++ b/toolkit/components/extensions/schemas/downloads.json
@@ -684,17 +684,16 @@
"type": "integer"
}
],
"type": "function"
},
{
"name": "onChanged",
"description": "When any of a <a href='#type-DownloadItem'>DownloadItem</a>'s properties except <code>bytesReceived</code> changes, this event fires with the <code>downloadId</code> and an object containing the properties that changed.",
- "unsupported": true,
"parameters": [
{
"name": "downloadDelta",
"type": "object",
"properties": {
"id": {
"description": "The <code>id</code> of the <a href='#type-DownloadItem'>DownloadItem</a> that changed.",
"type": "integer"
--- a/toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_search.html
+++ b/toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_search.html
@@ -27,16 +27,38 @@ const TXT_FILE = "file_download.txt";
const TXT_URL = BASE + "/" + TXT_FILE;
const TXT_LEN = 46;
const HTML_FILE = "file_download.html";
const HTML_URL = BASE + "/" + HTML_FILE;
const HTML_LEN = 117;
const BIG_LEN = 1000; // something bigger both TXT_LEN and HTML_LEN
function backgroundScript() {
+ let complete = new Map();
+
+ function waitForComplete(id) {
+ if (complete.has(id)) {
+ return complete.get(id).promise;
+ }
+
+ let promise = new Promise(resolve => {
+ complete.set(id, {resolve});
+ });
+ complete.get(id).promise = promise;
+ return promise;
+ }
+
+ browser.downloads.onChanged.addListener(change => {
+ if (change.state && change.state.current == "complete") {
+ // Make sure we have a promise.
+ waitForComplete(change.id);
+ complete.get(change.id).resolve();
+ }
+ });
+
browser.test.onMessage.addListener(function(msg) {
// extension functions throw on bad arguments, we can remove the extra
// promise when bug 1250223 is fixed.
if (msg == "download.request") {
Promise.resolve().then(() => browser.downloads.download(arguments[1]))
.then(id => {
browser.test.sendMessage("download.done", {status: "success", id});
})
@@ -46,44 +68,35 @@ function backgroundScript() {
} else if (msg == "search.request") {
Promise.resolve().then(() => browser.downloads.search(arguments[1]))
.then(downloads => {
browser.test.sendMessage("search.done", {status: "success", downloads});
})
.catch(error => {
browser.test.sendMessage("search.done", {status: "error", errmsg: error.message});
});
+ } else if (msg == "waitForComplete.request") {
+ waitForComplete(arguments[1]).then(() => {
+ browser.test.sendMessage("waitForComplete.done");
+ });
}
});
browser.test.sendMessage("ready");
}
function clearDownloads(callback) {
return Downloads.getList(Downloads.ALL).then(list => {
return list.getAll().then(downloads => {
return Promise.all(downloads.map(download => list.remove(download)))
.then(() => downloads);
});
});
}
-// This function is a bit of a sledgehammer, it looks at every download
-// the browser knows about and waits for all active downloads to complete.
-// But we only start one at a time and only do a handful in total.
-// Replace this when we have onChanged (bug 1245600)
-function waitForDownloads() {
- return Downloads.getList(Downloads.ALL)
- .then(list => list.getAll())
- .then(downloads => {
- let inprogress = downloads.filter(dl => !dl.stopped);
- return Promise.all(inprogress.map(dl => dl.whenSucceeded()));
- });
-}
-
add_task(function* test_search() {
const nsIFile = Ci.nsIFile;
let downloadDir = FileUtils.getDir("TmpD", ["downloads"]);
downloadDir.createUnique(nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
info(`downloadDir ${downloadDir.path}`);
function downloadPath(filename) {
let path = downloadDir.clone();
@@ -109,17 +122,27 @@ add_task(function* test_search() {
background: `(${backgroundScript})()`,
manifest: {
permissions: ["downloads"],
},
});
function download(options) {
extension.sendMessage("download.request", options);
- return extension.awaitMessage("download.done");
+ return extension.awaitMessage("download.done").then(result => {
+ let promise;
+ if (result.status == "success") {
+ info(`wait for onChanged event to indicate ${result.id} is complete`);
+ extension.sendMessage("waitForComplete.request", result.id);
+ promise = extension.awaitMessage("waitForComplete.done");
+ } else {
+ promise = Promise.resolve();
+ }
+ return promise.then(() => result);
+ });
}
function search(query) {
extension.sendMessage("search.request", query);
return extension.awaitMessage("search.done");
}
yield extension.startup();
@@ -147,18 +170,16 @@ add_task(function* test_search() {
const HTML_FILE2 = "renamed.html";
msg = yield download({url: HTML_URL, filename: HTML_FILE2});
is(msg.status, "success", "download() succeeded");
downloadIds.html2 = msg.id;
const time3 = new Date();
- yield waitForDownloads();
-
// Search for each individual download and check
// the corresponding DownloadItem.
function* checkDownloadItem(id, expect) {
let msg = yield search({id});
is(msg.status, "success", "search() succeeded");
is(msg.downloads.length, 1, "search() found exactly 1 download");
Object.keys(expect).forEach(function(field) {