Backed out changeset 819c1fb3e554 (bug 1635408) for xpcshell perma failures on test_shutdown_handling.js. CLOSED TREE

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);
   },
 
   /**

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() {

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);
 });