diff --git a/services/settings/RemoteSettingsWorker.js b/services/settings/RemoteSettingsWorker.js index 3085f42f6d8d..b4f316812098 100644 --- a/services/settings/RemoteSettingsWorker.js +++ b/services/settings/RemoteSettingsWorker.js @@ -84,7 +84,8 @@ const Agent = { return false; } const buffer = await resp.arrayBuffer(); - return this.checkContentHash(buffer, size, hash); + const bytes = new Uint8Array(buffer); + return this.checkContentHash(bytes, size, hash); }, /** diff --git a/services/settings/RemoteSettingsWorker.jsm b/services/settings/RemoteSettingsWorker.jsm index 67c319b7fae0..fe8dc6925be7 100644 --- a/services/settings/RemoteSettingsWorker.jsm +++ b/services/settings/RemoteSettingsWorker.jsm @@ -55,19 +55,17 @@ class Worker { this.idleTimeoutId = null; } - async _execute(method, args = [], options = {}) { + async _execute(method, args = []) { if (gShutdown) { throw new RemoteSettingsWorkerError("Remote Settings has shut down."); } - const { mustComplete = false } = options; - // (Re)instantiate the worker if it was terminated. if (!this.worker) { this.worker = new ChromeWorker(this.source); this.worker.onmessage = this._onWorkerMessage.bind(this); this.worker.onerror = error => { // Worker crashed. Reject each pending callback. - for (const { reject } of this.callbacks.values()) { + for (const [, reject] of this.callbacks.values()) { reject(error); } this.callbacks.clear(); @@ -79,21 +77,16 @@ class Worker { if (this.idleTimeoutId) { clearTimeout(this.idleTimeoutId); } - let identifier = method + "-"; - // Include the collection details in the importJSONDump case. - if (identifier == "importJSONDump") { - identifier += `${args[0]}-${args[1]}-`; - } return new Promise((resolve, reject) => { - const callbackId = `${identifier}${++this.lastCallbackId}`; - this.callbacks.set(callbackId, { resolve, reject, mustComplete }); + const callbackId = `${method}-${++this.lastCallbackId}`; + this.callbacks.set(callbackId, [resolve, reject]); this.worker.postMessage({ callbackId, method, args }); }); } _onWorkerMessage(event) { const { callbackId, result, error } = event.data; - const { resolve, reject } = this.callbacks.get(callbackId); + const [resolve, reject] = this.callbacks.get(callbackId); if (error) { reject(new RemoteSettingsWorkerError(error)); } else { @@ -117,31 +110,6 @@ class Worker { } } - /** - * Called at shutdown to abort anything the worker is doing that isn't - * critical. - */ - _abortCancelableRequests() { - // End all tasks that we can. - const callbackCopy = Array.from(this.callbacks.entries()); - const error = new Error("Shutdown, aborting read-only worker requests."); - for (const [id, { reject, mustComplete }] of callbackCopy) { - if (!mustComplete) { - this.callbacks.delete(id); - reject(error); - } - } - // There might be nothing left now: - if (!this.callbacks.size) { - this.stop(); - if (gShutdownResolver) { - gShutdownResolver(); - } - } - // If there was something left, we'll stop as soon as we get messages from - // those tasks, too. - } - stop() { this.worker.terminate(); this.worker = null; @@ -157,9 +125,7 @@ class Worker { } async importJSONDump(bucket, collection) { - return this._execute("importJSONDump", [bucket, collection], { - mustComplete: true, - }); + return this._execute("importJSONDump", [bucket, collection]); } async checkFileHash(filepath, size, hash) { @@ -190,16 +156,10 @@ try { ) { return null; } - // Otherwise, there's something left to do. Set up a promise: - let finishedPromise = new Promise(resolve => { + // Otherwise, return a promise that the worker will resolve. + return new Promise(resolve => { gShutdownResolver = resolve; }); - - // Try to cancel most of the work: - RemoteSettingsWorker._abortCancelableRequests(); - - // Return a promise that the worker will resolve. - return finishedPromise; }, { fetchState() { diff --git a/services/settings/test/unit/test_shutdown_handling.js b/services/settings/test/unit/test_shutdown_handling.js index a933c77ab7ad..fedc987777fc 100644 --- a/services/settings/test/unit/test_shutdown_handling.js +++ b/services/settings/test/unit/test_shutdown_handling.js @@ -6,12 +6,6 @@ http://creativecommons.org/publicdomain/zero/1.0/ */ const { Database } = ChromeUtils.import( "resource://services-settings/Database.jsm" ); -const { RemoteSettingsWorker } = ChromeUtils.import( - "resource://services-settings/RemoteSettingsWorker.jsm" -); -const { RemoteSettingsClient } = ChromeUtils.import( - "resource://services-settings/RemoteSettingsClient.jsm" -); add_task(async function test_shutdown_abort_after_start() { // Start a forever transaction: @@ -88,33 +82,4 @@ add_task(async function test_shutdown_immediate_abort() { rejection = e; }); ok(rejection, "Directly aborted promise should also have rejected."); - // Now clear the shutdown flag and rejection error: - Database._cancelShutdown(); -}); - -add_task(async function test_shutdown_worker() { - const client = new RemoteSettingsClient("language-dictionaries", { - bucketNamePref: "services.settings.default_bucket", - }); - const before = await client.get({ syncIfEmpty: false }); - Assert.equal(before.length, 0); - - let importPromise = RemoteSettingsWorker.importJSONDump( - "main", - "language-dictionaries" - ); - let stringifyPromise = RemoteSettingsWorker.canonicalStringify( - [], - [], - Date.now() - ); - RemoteSettingsWorker._abortCancelableRequests(); - await Assert.rejects( - stringifyPromise, - /Shutdown/, - "Should have aborted the stringify request at shutdown." - ); - await importPromise.catch(e => ok(false, "importing failed!")); - const after = await client.get(); - Assert.ok(after.length > 0); });