Bug 633062 p4 - Remove miscellaneous uses of event loop spinning in services/. r=markh

MozReview-Commit-ID: IDGWJevEHLK

--HG--
extra : rebase_source : 8471048a68b9b6204402f3ea9039a21af8c3740c

services/common/tests/unit/test_tokenauthenticatedrequest.js

@@ -11,7 +11,7 @@ function run_test() {
   run_next_test();
 }
 
-add_test(function test_authenticated_request() {
+add_task(async function test_authenticated_request() {
   _("Ensure that sending a MAC authenticated GET request works as expected.");
 
   let message = "Great Success!";
@@ -41,12 +41,10 @@ add_test(function test_authenticated_request() {
   auth = sig.getHeader();
 
   let req = new TokenAuthenticatedRESTRequest(uri, {id, key}, extra);
-  let cb = Async.makeSpinningCallback();
-  req.get(cb);
-  let result = cb.wait();
+  let error = await new Promise(res => req.get(res));
 
-  Assert.equal(null, result);
+  Assert.equal(null, error);
   Assert.equal(message, req.response.body);
 
-  server.stop(run_next_test);
+  await promiseStopServer(server);
 });

services/sync/modules/browserid_identity.js

@@ -728,19 +728,14 @@ this.BrowserIDManager.prototype = {
    * @return a Hawk HTTP Authorization Header, lightly wrapped, for the .uri
    * of a RESTRequest or AsyncResponse object.
    */
-  _getAuthenticationHeader(httpObject, method) {
-    let cb = Async.makeSpinningCallback();
-    this._ensureValidToken().then(cb, cb);
+  async _getAuthenticationHeader(httpObject, method) {
     // Note that in failure states we return null, causing the request to be
     // made without authorization headers, thereby presumably causing a 401,
     // which causes Sync to log out. If we throw, this may not happen as
     // expected.
     try {
-      cb.wait();
+      await this._ensureValidToken();
     } catch (ex) {
-      if (Async.isShutdownException(ex)) {
-        throw ex;
-      }
       this._log.error("Failed to fetch a token for authentication", ex);
       return null;
     }
@@ -796,9 +791,9 @@ BrowserIDClusterManager.prototype = {
   /**
    * Determine the cluster for the current user and update state.
    */
-  setCluster() {
+  async setCluster() {
     // Make sure we didn't get some unexpected response for the cluster.
-    let cluster = this._findCluster();
+    let cluster = await this._findCluster();
     this._log.debug("Cluster value = " + cluster);
     if (cluster == null) {
       return false;
@@ -818,8 +813,20 @@ BrowserIDClusterManager.prototype = {
     return true;
   },
 
-  _findCluster() {
-    let endPointFromIdentityToken = () => {
+  async _findCluster() {
+    try {
+      // Ensure we are ready to authenticate and have a valid token.
+      await this.identity.whenReadyToAuthenticate.promise;
+      // We need to handle node reassignment here.  If we are being asked
+      // for a clusterURL while the service already has a clusterURL, then
+      // it's likely a 401 was received using the existing token - in which
+      // case we just discard the existing token and fetch a new one.
+      if (this.service.clusterURL) {
+        log.debug("_findCluster has a pre-existing clusterURL, so discarding the current token");
+        this.identity._token = null;
+      }
+      await this.identity._ensureValidToken();
+
       // The only reason (in theory ;) that we can end up with a null token
       // is when this._fxaService.canGetKeys() returned false.  In turn, this
       // should only happen if the master-password is locked or the credentials
@@ -840,30 +847,7 @@ BrowserIDClusterManager.prototype = {
       }
       log.debug("_findCluster returning " + endpoint);
       return endpoint;
-    };
-
-    // Spinningly ensure we are ready to authenticate and have a valid token.
-    let promiseClusterURL = () => {
-      return this.identity.whenReadyToAuthenticate.promise.then(
-        () => {
-          // We need to handle node reassignment here.  If we are being asked
-          // for a clusterURL while the service already has a clusterURL, then
-          // it's likely a 401 was received using the existing token - in which
-          // case we just discard the existing token and fetch a new one.
-          if (this.service.clusterURL) {
-            log.debug("_findCluster has a pre-existing clusterURL, so discarding the current token");
-            this.identity._token = null;
-          }
-          return this.identity._ensureValidToken();
-        }
-      ).then(endPointFromIdentityToken
-      );
-    };
-
-    let cb = Async.makeSpinningCallback();
-    promiseClusterURL().then(function(clusterURL) {
-      cb(null, clusterURL);
-    }).catch(err => {
+    } catch (err) {
       log.info("Failed to fetch the cluster URL", err);
       // service.js's verifyLogin() method will attempt to fetch a cluster
       // URL when it sees a 401.  If it gets null, it treats it as a "real"
@@ -878,14 +862,10 @@ BrowserIDClusterManager.prototype = {
       // * On a real 401, we must return null.
       // * On any other problem we must let an exception bubble up.
       if (err instanceof AuthenticationError) {
-        // callback with no error and a null result - cb.wait() returns null.
-        cb(null, null);
-      } else {
-        // callback with an error - cb.wait() completes by raising an exception.
-        cb(err);
+        return null;
       }
-    });
-    return cb.wait();
+      throw err;
+    }
   },
 
   getUserBaseURL() {

services/sync/modules/collection_validator.js

@@ -123,7 +123,7 @@ class CollectionValidator {
 
   // Return whether or not a client item should be present on the server. Expected
   // to be overridden
-  syncedByClient(item) {
+  async syncedByClient(item) {
     return true;
   }
 
@@ -186,7 +186,7 @@ class CollectionValidator {
     let seenClient = new Map();
     for (let record of clientRecords) {
       let id = record[this.idProp];
-      record.shouldSync = this.syncedByClient(record);
+      record.shouldSync = await this.syncedByClient(record);
       let clientHasPossibleDupe = seenClient.has(id);
       if (clientHasPossibleDupe && record.shouldSync) {
         // Only report duplicate client IDs for syncable records.

services/sync/modules/engines/addons.js

@@ -183,7 +183,7 @@ AddonsEngine.prototype = {
           continue;
       }
 
-      if (!this.isAddonSyncable(addons[id])) {
+      if (!(await this.isAddonSyncable(addons[id]))) {
         continue;
       }
 
@@ -239,6 +239,7 @@ AddonsEngine.prototype = {
     return this._reconciler.refreshGlobalState();
   },
 
+  // Returns a promise
   isAddonSyncable(addon, ignoreRepoCheck) {
     return this._store.isAddonSyncable(addon, ignoreRepoCheck);
   }
@@ -291,7 +292,7 @@ AddonsStore.prototype = {
     // Ignore incoming records for which an existing non-syncable addon
     // exists.
     let existingMeta = this.reconciler.addons[record.addonID];
-    if (existingMeta && !this.isAddonSyncable(existingMeta)) {
+    if (existingMeta && !(await this.isAddonSyncable(existingMeta))) {
       this._log.info("Ignoring incoming record for an existing but non-syncable addon", record.addonID);
       return;
     }
@@ -465,7 +466,7 @@ AddonsStore.prototype = {
     let addons = this.reconciler.addons;
     for (let id in addons) {
       let addon = addons[id];
-      if (this.isAddonSyncable(addon)) {
+      if ((await this.isAddonSyncable(addon))) {
         ids[addon.guid] = true;
       }
     }
@@ -536,7 +537,7 @@ AddonsStore.prototype = {
    *         for testing and validation).
    * @return Boolean indicating whether it is appropriate for Sync
    */
-  isAddonSyncable: function isAddonSyncable(addon, ignoreRepoCheck = false) {
+  async isAddonSyncable(addon, ignoreRepoCheck = false) {
     // Currently, we limit syncable add-ons to those that are:
     //   1) In a well-defined set of types
     //   2) Installed in the current profile
@@ -589,9 +590,9 @@ AddonsStore.prototype = {
       return true;
     }
 
-    let cb = Async.makeSyncCallback();
-    AddonRepository.getCachedAddonByID(addon.id, cb);
-    let result = Async.waitForSyncCallback(cb);
+    let result = await new Promise(res => {
+      AddonRepository.getCachedAddonByID(addon.id, res);
+    });
 
     if (!result) {
       this._log.debug(addon.id + " not syncable: add-on not found in add-on " +
@@ -705,7 +706,7 @@ AddonsTracker.prototype = {
       return;
     }
 
-    if (!this.store.isAddonSyncable(addon)) {
+    if (!Async.promiseSpinningly(this.store.isAddonSyncable(addon))) {
       this._log.debug("Ignoring change because add-on isn't syncable: " +
                       addon.id);
       return;
@@ -783,10 +784,12 @@ class AddonValidator extends CollectionValidator {
     return item.applicationID === Services.appinfo.ID;
   }
 
-  syncedByClient(item) {
+  async syncedByClient(item) {
     return !item.original.hidden &&
            !item.original.isSystem &&
            !(item.original.pendingOperations & AddonManager.PENDING_UNINSTALL) &&
+           // No need to await the returned promise explicitely:
+           // |expr1 && expr2| evaluates to expr2 if expr1 is true.
            this.engine.isAddonSyncable(item.original, true);
   }
 }

services/sync/modules/resource.js

@@ -47,6 +47,7 @@ Resource.prototype = {
 
   /**
    * Callback to be invoked at request time to add authentication details.
+   * If the callback returns a promise, it will be awaited upon.
    *
    * By default, a global authenticator is provided. If this is set, it will
    * be used instead of the global one.
@@ -93,7 +94,7 @@ Resource.prototype = {
    * @param {string} method HTTP method
    * @returns {Headers}
    */
-  _buildHeaders(method) {
+  async _buildHeaders(method) {
     const headers = new Headers(this._headers);
 
     if (Resource.SEND_VERSION_INFO) {
@@ -101,7 +102,7 @@ Resource.prototype = {
     }
 
     if (this.authenticator) {
-      const result = this.authenticator(this, method);
+      const result = await this.authenticator(this, method);
       if (result && result.headers) {
         for (const [k, v] of Object.entries(result.headers)) {
           headers.append(k.toLowerCase(), v);
@@ -135,8 +136,8 @@ Resource.prototype = {
    * @param {object} signal AbortSignal instance
    * @returns {Request}
    */
-  _createRequest(method, data, signal) {
-    const headers = this._buildHeaders(method);
+  async _createRequest(method, data, signal) {
+    const headers = await this._buildHeaders(method);
     const init = {
       cache: "no-store", // No cache.
       headers,
@@ -164,7 +165,7 @@ Resource.prototype = {
    */
   async _doRequest(method, data = null) {
     const controller = new AbortController();
-    const request = this._createRequest(method, data, controller.signal);
+    const request = await this._createRequest(method, data, controller.signal);
     const responsePromise = fetch(request); // Rejects on network failure.
     let didTimeout = false;
     const timeoutId = setTimeout(() => {

services/sync/modules/service.js

@@ -441,7 +441,10 @@ Sync11Service.prototype = {
       case "fxaccounts:device_disconnected":
         data = JSON.parse(data);
         if (!data.isLocalDevice) {
-          Async.promiseSpinningly(this.clientsEngine.updateKnownStaleClients());
+          // Refresh the known stale clients list in the background.
+          this.clientsEngine.updateKnownStaleClients().catch(e => {
+            this._log.error(e);
+          });
         }
         break;
       case "weave:service:setup-complete":
@@ -651,7 +654,7 @@ Sync11Service.prototype = {
       // Make sure we have a cluster to verify against.
       // This is a little weird, if we don't get a node we pretend
       // to succeed, since that probably means we just don't have storage.
-      if (this.clusterURL == "" && !this._clusterManager.setCluster()) {
+      if (this.clusterURL == "" && !(await this._clusterManager.setCluster())) {
         this.status.sync = NO_SYNC_NODE_FOUND;
         return true;
       }
@@ -690,7 +693,7 @@ Sync11Service.prototype = {
 
         case 404:
           // Check that we're verifying with the correct cluster
-          if (allow40XRecovery && this._clusterManager.setCluster()) {
+          if (allow40XRecovery && (await this._clusterManager.setCluster())) {
             return await this.verifyLogin(false);
           }
 

services/sync/modules/stages/enginesync.js

@@ -51,7 +51,7 @@ EngineSynchronizer.prototype = {
     }
 
     // If we don't have a node, get one. If that fails, retry in 10 minutes.
-    if (!this.service.clusterURL && !this.service._clusterManager.setCluster()) {
+    if (!this.service.clusterURL && !(await this.service._clusterManager.setCluster())) {
       this.service.status.sync = NO_SYNC_NODE_FOUND;
       this._log.info("No cluster URL found. Cannot sync.");
       return;

services/sync/tests/unit/test_addons_store.js

@@ -250,16 +250,16 @@ add_task(async function test_apply_uninstall() {
   Assert.equal(null, addon);
 });
 
-add_test(function test_addon_syncability() {
+add_task(async function test_addon_syncability() {
   _("Ensure isAddonSyncable functions properly.");
 
   Svc.Prefs.set("addons.trustedSourceHostnames",
                 "addons.mozilla.org,other.example.com");
 
-  Assert.ok(!store.isAddonSyncable(null));
+  Assert.ok(!(await store.isAddonSyncable(null)));
 
   let addon = installAddon("test_bootstrap1_1");
-  Assert.ok(store.isAddonSyncable(addon));
+  Assert.ok((await store.isAddonSyncable(addon)));
 
   let dummy = {};
   const KEYS = ["id", "syncGUID", "type", "scope", "foreignInstall", "isSyncable"];
@@ -267,22 +267,22 @@ add_test(function test_addon_syncability() {
     dummy[k] = addon[k];
   }
 
-  Assert.ok(store.isAddonSyncable(dummy));
+  Assert.ok((await store.isAddonSyncable(dummy)));
 
   dummy.type = "UNSUPPORTED";
-  Assert.ok(!store.isAddonSyncable(dummy));
+  Assert.ok(!(await store.isAddonSyncable(dummy)));
   dummy.type = addon.type;
 
   dummy.scope = 0;
-  Assert.ok(!store.isAddonSyncable(dummy));
+  Assert.ok(!(await store.isAddonSyncable(dummy)));
   dummy.scope = addon.scope;
 
   dummy.isSyncable = false;
-  Assert.ok(!store.isAddonSyncable(dummy));
+  Assert.ok(!(await store.isAddonSyncable(dummy)));
   dummy.isSyncable = addon.isSyncable;
 
   dummy.foreignInstall = true;
-  Assert.ok(!store.isAddonSyncable(dummy));
+  Assert.ok(!(await store.isAddonSyncable(dummy)));
   dummy.foreignInstall = false;
 
   uninstallAddon(addon);
@@ -318,7 +318,6 @@ add_test(function test_addon_syncability() {
 
   Svc.Prefs.reset("addons.trustedSourceHostnames");
 
-  run_next_test();
 });
 
 add_task(async function test_get_all_ids() {
@@ -336,9 +335,9 @@ add_task(async function test_get_all_ids() {
   let addon3 = installAddon("test_install3");
 
   _("Ensure they're syncable.");
-  Assert.ok(store.isAddonSyncable(addon1));
-  Assert.ok(store.isAddonSyncable(addon2));
-  Assert.ok(store.isAddonSyncable(addon3));
+  Assert.ok((await store.isAddonSyncable(addon1)));
+  Assert.ok((await store.isAddonSyncable(addon2)));
+  Assert.ok((await store.isAddonSyncable(addon3)));
 
   let ids = await store.getAllIDs();
 

services/sync/tests/unit/test_addons_tracker.js

@@ -118,26 +118,26 @@ add_task(async function test_track_user_disable() {
   Svc.Obs.notify("weave:engine:start-tracking");
   Assert.equal(0, tracker.score);
 
-  let cb = Async.makeSyncCallback();
-
-  let listener = {
-    onDisabled(disabled) {
-      _("onDisabled");
-      if (disabled.id == addon.id) {
-        AddonManager.removeAddonListener(listener);
-        cb();
+  let disabledPromise = new Promise(res => {
+    let listener = {
+      onDisabled(disabled) {
+        _("onDisabled");
+        if (disabled.id == addon.id) {
+          AddonManager.removeAddonListener(listener);
+          res();
+        }
+      },
+      onDisabling(disabling) {
+        _("onDisabling add-on");
       }
-    },
-    onDisabling(disabling) {
-      _("onDisabling add-on");
-    }
-  };
-  AddonManager.addAddonListener(listener);
+    };
+    AddonManager.addAddonListener(listener);
+  });
 
   _("Disabling add-on");
   addon.userDisabled = true;
   _("Disabling started...");
-  Async.waitForSyncCallback(cb);
+  await disabledPromise;
 
   let changed = tracker.changedIDs;
   Assert.equal(1, Object.keys(changed).length);

services/sync/tests/unit/test_browserid_identity.js

@@ -144,7 +144,7 @@ add_task(async function test_initialializeWithNoKeys() {
     Assert.equal(globalBrowseridManager._token, null, "we don't have a token");
 });
 
-add_test(function test_getResourceAuthenticator() {
+add_task(async function test_getResourceAuthenticator() {
     _("BrowserIDManager supplies a Resource Authenticator callback which returns a Hawk header.");
     configureFxAccountIdentity(globalBrowseridManager);
     let authenticator = globalBrowseridManager.getResourceAuthenticator();
@@ -152,17 +152,16 @@ add_test(function test_getResourceAuthenticator() {
     let req = {uri: CommonUtils.makeURI(
       "https://example.net/somewhere/over/the/rainbow"),
                method: "GET"};
-    let output = authenticator(req, "GET");
+    let output = await authenticator(req, "GET");
     Assert.ok("headers" in output);
     Assert.ok("authorization" in output.headers);
     Assert.ok(output.headers.authorization.startsWith("Hawk"));
     _("Expected internal state after successful call.");
     Assert.equal(globalBrowseridManager._token.uid, globalIdentityConfig.fxaccount.token.uid);
-    run_next_test();
   }
 );
 
-add_test(function test_resourceAuthenticatorSkew() {
+add_task(async function test_resourceAuthenticatorSkew() {
   _("BrowserIDManager Resource Authenticator compensates for clock skew in Hawk header.");
 
   // Clock is skewed 12 hours into the future
@@ -211,7 +210,7 @@ add_test(function test_resourceAuthenticatorSkew() {
 
   let request = new Resource("https://example.net/i/like/pie/");
   let authenticator = browseridManager.getResourceAuthenticator();
-  let output = authenticator(request, "GET");
+  let output = await authenticator(request, "GET");
   dump("output" + JSON.stringify(output));
   let authHeader = output.headers.authorization;
   Assert.ok(authHeader.startsWith("Hawk"));
@@ -221,11 +220,9 @@ add_test(function test_resourceAuthenticatorSkew() {
   Assert.equal(getTimestamp(authHeader), now - 12 * HOUR_MS);
   Assert.ok(
       (getTimestampDelta(authHeader, now) - 12 * HOUR_MS) < 2 * MINUTE_MS);
-
-  run_next_test();
 });
 
-add_test(function test_RESTResourceAuthenticatorSkew() {
+add_task(async function test_RESTResourceAuthenticatorSkew() {
   _("BrowserIDManager REST Resource Authenticator compensates for clock skew in Hawk header.");
 
   // Clock is skewed 12 hours into the future from our arbitary date
@@ -255,7 +252,7 @@ add_test(function test_RESTResourceAuthenticatorSkew() {
 
   let request = new Resource("https://example.net/i/like/pie/");
   let authenticator = browseridManager.getResourceAuthenticator();
-  let output = authenticator(request, "GET");
+  let output = await authenticator(request, "GET");
   dump("output" + JSON.stringify(output));
   let authHeader = output.headers.authorization;
   Assert.ok(authHeader.startsWith("Hawk"));
@@ -265,8 +262,6 @@ add_test(function test_RESTResourceAuthenticatorSkew() {
   Assert.equal(getTimestamp(authHeader), now - 12 * HOUR_MS);
   Assert.ok(
       (getTimestampDelta(authHeader, now) - 12 * HOUR_MS) < 2 * MINUTE_MS);
-
-  run_next_test();
 });
 
 add_task(async function test_ensureLoggedIn() {
@@ -302,7 +297,7 @@ add_task(async function test_ensureLoggedIn() {
   Assert.equal(Status.login, LOGIN_SUCCEEDED, "final ensureLoggedIn worked");
 });
 
-add_test(function test_tokenExpiration() {
+add_task(async function test_tokenExpiration() {
     _("BrowserIDManager notices token expiration:");
     let bimExp = new BrowserIDManager();
     configureFxAccountIdentity(bimExp, globalIdentityConfig);
@@ -312,7 +307,7 @@ add_test(function test_tokenExpiration() {
     let req = {uri: CommonUtils.makeURI(
       "https://example.net/somewhere/over/the/rainbow"),
                method: "GET"};
-    authenticator(req, "GET");
+    await authenticator(req, "GET");
 
     // Mock the clock.
     _("Forcing the token to expire ...");
@@ -325,7 +320,6 @@ add_test(function test_tokenExpiration() {
     Assert.ok(bimExp._token.expiration < bimExp._now());
     _("... means BrowserIDManager knows to re-fetch it on the next call.");
     Assert.ok(!bimExp.hasValidToken());
-    run_next_test();
   }
 );
 

services/sync/tests/unit/test_clients_engine.js

@@ -1745,6 +1745,20 @@ add_task(async function test_other_clients_notified_on_first_sync() {
 
 add_task(async function device_disconnected_notification_updates_known_stale_clients() {
   const spyUpdate = sinon.spy(engine, "updateKnownStaleClients");
+
+  Services.obs.notifyObservers(null, "fxaccounts:device_disconnected",
+                               JSON.stringify({ isLocalDevice: false }));
+  ok(spyUpdate.calledOnce, "updateKnownStaleClients should be called");
+  spyUpdate.reset();
+
+  Services.obs.notifyObservers(null, "fxaccounts:device_disconnected",
+                               JSON.stringify({ isLocalDevice: true }));
+  ok(spyUpdate.notCalled, "updateKnownStaleClients should not be called");
+
+  spyUpdate.restore();
+});
+
+add_task(async function update_known_stale_clients() {
   const makeFakeClient = (id) => ({ id, fxaDeviceId: `fxa-${id}` });
   const clients = [makeFakeClient("one"), makeFakeClient("two"), makeFakeClient("three")];
   const stubRemoteClients = sinon.stub(engine._store, "_remoteClients").get(() => {
@@ -1755,26 +1769,12 @@ add_task(async function device_disconnected_notification_updates_known_stale_cli
   });
 
   engine._knownStaleFxADeviceIds = null;
-  Services.obs.notifyObservers(null, "fxaccounts:device_disconnected",
-                               JSON.stringify({ isLocalDevice: false }));
-  ok(spyUpdate.calledOnce, "updateKnownStaleClients should be called");
+  await engine.updateKnownStaleClients();
   ok(clients[0].stale);
   ok(clients[1].stale);
   ok(!clients[2].stale);
-  spyUpdate.reset();
-
-  ok(engine._knownStaleFxADeviceIds);
-  Services.obs.notifyObservers(null, "fxaccounts:device_disconnected",
-                               JSON.stringify({ isLocalDevice: false }));
-  ok(spyUpdate.calledOnce, "updateKnownStaleClients should be called");
-  spyUpdate.reset();
-
-  Services.obs.notifyObservers(null, "fxaccounts:device_disconnected",
-                               JSON.stringify({ isLocalDevice: true }));
-  ok(spyUpdate.notCalled, "updateKnownStaleClients should not be called");
 
   stubRemoteClients.restore();
-  spyUpdate.restore();
   stubRefresh.restore();
 });
 

services/sync/tests/unit/test_fxa_service_cluster.js

@@ -19,9 +19,7 @@ add_task(async function test_findCluster() {
   await Assert.rejects(Service.identity.whenReadyToAuthenticate.promise,
                        "should reject due to 500");
 
-  Assert.throws(function() {
-    Service._clusterManager._findCluster();
-  });
+  await Assert.rejects(Service._clusterManager._findCluster());
 
   _("_findCluster() returns null on authentication errors.");
   initializeIdentityWithTokenServerResponse({
@@ -34,7 +32,7 @@ add_task(async function test_findCluster() {
   await Assert.rejects(Service.identity.whenReadyToAuthenticate.promise,
                        "should reject due to 401");
 
-  let cluster = Service._clusterManager._findCluster();
+  let cluster = await Service._clusterManager._findCluster();
   Assert.strictEqual(cluster, null);
 
   _("_findCluster() works with correct tokenserver response.");
@@ -54,7 +52,7 @@ add_task(async function test_findCluster() {
 
   await Service.identity.initializeWithCurrentIdentity();
   await Service.identity.whenReadyToAuthenticate.promise;
-  cluster = Service._clusterManager._findCluster();
+  cluster = await Service._clusterManager._findCluster();
   // The cluster manager ensures a trailing "/"
   Assert.strictEqual(cluster, endpoint + "/");
 

services/sync/tests/unit/test_history_store.js

@@ -2,7 +2,6 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
-ChromeUtils.import("resource://services-common/async.js");
 ChromeUtils.import("resource://services-common/utils.js");
 ChromeUtils.import("resource://services-sync/engines/history.js");
 ChromeUtils.import("resource://services-sync/service.js");
@@ -43,14 +42,17 @@ function isDateApproximately(actual, expected, skewMillis = 1000) {
   return actual >= lowerBound && actual <= upperBound;
 }
 
-var engine = new HistoryEngine(Service);
-Async.promiseSpinningly(engine.initialize());
-var store = engine._store;
+let engine, store, fxuri, fxguid, tburi, tbguid;
+
 async function applyEnsureNoFailures(records) {
   Assert.equal((await store.applyIncomingBatch(records)).length, 0);
 }
 
-var fxuri, fxguid, tburi, tbguid;
+add_task(async function setup() {
+  engine = new HistoryEngine(Service);
+  await engine.initialize();
+  store = engine._store;
+});
 
 add_task(async function test_store() {
   _("Verify that we've got an empty store to work with.");

services/sync/tests/unit/test_hmac_error.js

@@ -178,9 +178,9 @@ add_task(async function hmac_error_during_node_reassignment() {
 
   // This kicks off the actual test. Split into a function here to allow this
   // source file to broadly follow actual execution order.
-  function onwards() {
+  async function onwards() {
     _("== Invoking first sync.");
-    Async.promiseSpinningly(Service.sync());
+    await Service.sync();
     _("We should not simultaneously have data but no keys on the server.");
     let hasData = rotaryColl.wbo("flying") ||
                   rotaryColl.wbo("scotsman");
@@ -193,7 +193,7 @@ add_task(async function hmac_error_during_node_reassignment() {
   }
 
   _("Make sure that syncing again causes recovery.");
-  await new Promise(resolve => {
+  let callbacksPromise = new Promise(resolve => {
     onSyncFinished = function() {
       _("== First sync done.");
       _("---------------------------");
@@ -213,7 +213,7 @@ add_task(async function hmac_error_during_node_reassignment() {
           engine.lastSync = 0;
           hmacErrorCount = 0;
 
-          onSyncFinished = function() {
+          onSyncFinished = async function() {
             // Two rotary items, one client record... no errors.
             Assert.equal(hmacErrorCount, 0);
 
@@ -227,12 +227,12 @@ add_task(async function hmac_error_during_node_reassignment() {
             server.stop(resolve);
           };
 
-          Async.promiseSpinningly(Service.sync());
+          Service.sync();
         },
         this);
       };
     };
-
-    onwards();
   });
+  await onwards();
+  await callbacksPromise;
 });

services/sync/tests/unit/test_service_cluster.js

@@ -5,21 +5,10 @@ ChromeUtils.import("resource://gre/modules/PromiseUtils.jsm");
 ChromeUtils.import("resource://services-sync/service.js");
 ChromeUtils.import("resource://services-sync/util.js");
 
-function do_check_throws(func) {
-  var raised = false;
-  try {
-    func();
-  } catch (ex) {
-    raised = true;
-  }
-  Assert.ok(raised);
-}
-
-add_test(function test_findCluster() {
+add_task(async function test_findCluster() {
   syncTestLogging();
   _("Test Service._findCluster()");
   try {
-
     let whenReadyToAuthenticate = PromiseUtils.defer();
     Service.identity.whenReadyToAuthenticate = whenReadyToAuthenticate;
     whenReadyToAuthenticate.resolve(true);
@@ -27,24 +16,21 @@ add_test(function test_findCluster() {
     Service.identity._ensureValidToken = () => Promise.reject(new Error("Connection refused"));
 
     _("_findCluster() throws on network errors (e.g. connection refused).");
-    do_check_throws(function() {
-      Service._clusterManager._findCluster();
-    });
+    await Assert.rejects(Service._clusterManager._findCluster());
 
     Service.identity._ensureValidToken = () => Promise.resolve(true);
     Service.identity._token = { endpoint: "http://weave.user.node" };
 
     _("_findCluster() returns the user's cluster node");
-    let cluster = Service._clusterManager._findCluster();
+    let cluster = await Service._clusterManager._findCluster();
     Assert.equal(cluster, "http://weave.user.node/");
 
   } finally {
     Svc.Prefs.resetBranch("");
-    run_next_test();
   }
 });
 
-add_test(function test_setCluster() {
+add_task(async function test_setCluster() {
   syncTestLogging();
   _("Test Service._setCluster()");
   try {
@@ -54,20 +40,18 @@ add_test(function test_setCluster() {
     Service._clusterManager._findCluster = () => "http://weave.user.node/";
 
     _("Set the cluster URL.");
-    Assert.ok(Service._clusterManager.setCluster());
+    Assert.ok((await Service._clusterManager.setCluster()));
     Assert.equal(Service.clusterURL, "http://weave.user.node/");
 
     _("Setting it again won't make a difference if it's the same one.");
-    Assert.ok(!Service._clusterManager.setCluster());
+    Assert.ok(!(await Service._clusterManager.setCluster()));
     Assert.equal(Service.clusterURL, "http://weave.user.node/");
 
     _("A 'null' response won't make a difference either.");
     Service._clusterManager._findCluster = () => null;
-    Assert.ok(!Service._clusterManager.setCluster());
+    Assert.ok(!(await Service._clusterManager.setCluster()));
     Assert.equal(Service.clusterURL, "http://weave.user.node/");
-
   } finally {
     Svc.Prefs.resetBranch("");
-    run_next_test();
   }
 });

services/sync/tests/unit/test_telemetry.js

@@ -430,7 +430,7 @@ add_task(async function test_engine_fail_ioerror() {
 add_task(async function test_clean_urls() {
   enableValidationPrefs();
 
-  Service.engineManager.register(SteamEngine);
+  await Service.engineManager.register(SteamEngine);
   let engine = Service.engineManager.get("steam");
   engine.enabled = true;
   let server = await serverForFoo(engine);

services/sync/tps/extensions/tps/resource/modules/addons.jsm

@@ -52,15 +52,15 @@ function Addon(TPS, id) {
 Addon.prototype = {
   addon: null,
 
-  uninstall: function uninstall() {
+  async uninstall() {
     // find our addon locally
-    let addon = Async.promiseSpinningly(AddonManager.getAddonByID(this.id));
+    let addon = await AddonManager.getAddonByID(this.id);
     Logger.AssertTrue(!!addon, "could not find addon " + this.id + " to uninstall");
-    Async.promiseSpinningly(AddonUtils.uninstallAddon(addon));
+    await AddonUtils.uninstallAddon(addon);
   },
 
-  find: function find(state) {
-    let addon = Async.promiseSpinningly(AddonManager.getAddonByID(this.id));
+  async find(state) {
+    let addon = await AddonManager.getAddonByID(this.id);
 
     if (!addon) {
       Logger.logInfo("Could not find add-on with ID: " + this.id);
@@ -98,8 +98,8 @@ Addon.prototype = {
                        "Add-on was installed successfully: " + this.id);
   },
 
-  setEnabled: function setEnabled(flag) {
-    Logger.AssertTrue(this.find(), "Add-on is available.");
+  async setEnabled(flag) {
+    Logger.AssertTrue((await this.find()), "Add-on is available.");
 
     let userDisabled;
     if (flag == STATE_ENABLED) {

services/sync/tps/extensions/tps/resource/modules/history.jsm

@@ -15,7 +15,6 @@ ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/PlacesUtils.jsm");
 ChromeUtils.import("resource://gre/modules/PlacesSyncUtils.jsm");
 ChromeUtils.import("resource://tps/logger.jsm");
-ChromeUtils.import("resource://services-common/async.js");
 
 var DumpHistory = async function TPS_History__DumpHistory() {
   let query = PlacesUtils.history.getNewQuery();

services/sync/tps/extensions/tps/resource/tps.jsm

@@ -487,16 +487,16 @@ var TPS = {
           addon.install();
           break;
         case ACTION_DELETE:
-          addon.uninstall();
+          await addon.uninstall();
           break;
         case ACTION_VERIFY:
-          Logger.AssertTrue(addon.find(state), "addon " + addon.id + " not found");
+          Logger.AssertTrue((await addon.find(state)), "addon " + addon.id + " not found");
           break;
         case ACTION_VERIFY_NOT:
-          Logger.AssertFalse(addon.find(state), "addon " + addon.id + " is present, but it shouldn't be");
+          Logger.AssertFalse((await addon.find(state)), "addon " + addon.id + " is present, but it shouldn't be");
           break;
         case ACTION_SET_ENABLED:
-          Logger.AssertTrue(addon.setEnabled(state), "addon " + addon.id + " not found");
+          Logger.AssertTrue((await addon.setEnabled(state)), "addon " + addon.id + " not found");
           break;
         default:
           throw new Error("Unknown action for add-on: " + action);

toolkit/modules/tests/xpcshell/test_sqlite.js

@@ -11,9 +11,7 @@ ChromeUtils.import("resource://gre/modules/FileUtils.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/Sqlite.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
-
-// To spin the event loop in test.
-ChromeUtils.import("resource://services-common/async.js");
+ChromeUtils.import("resource://gre/modules/PromiseUtils.jsm");
 
 function sleep(ms) {
   return new Promise(resolve => {
@@ -645,9 +643,6 @@ add_task(async function test_in_progress_counts() {
   let expectOne;
   let expectTwo;
 
-  // Please forgive me.
-  let inner = Async.makeSpinningCallback();
-  let outer = Async.makeSpinningCallback();
 
   // We want to make sure that two queries executing simultaneously
   // result in `_pendingStatements.size` reaching 2, then dropping back to 0.
@@ -655,6 +650,7 @@ add_task(async function test_in_progress_counts() {
   // To do so, we kick off a second statement within the row handler
   // of the first, then wait for both to finish.
 
+  let inner = PromiseUtils.defer();
   await c.executeCached("SELECT * from dirs", null, function onRow() {
     // In the onRow handler, we're still an outstanding query.
     // Expect a single in-progress entry.
@@ -662,22 +658,11 @@ add_task(async function test_in_progress_counts() {
 
     // Start another query, checking that after its statement has been created
     // there are two statements in progress.
-    let p = c.executeCached("SELECT 10, path from dirs");
+    c.executeCached("SELECT 10, path from dirs").then(inner.resolve);
     expectTwo = c._connectionData._pendingStatements.size;
-
-    // Now wait for it to be done before we return from the row handler …
-    p.then(function onInner() {
-      inner();
-    });
-  }).then(function onOuter() {
-    // … and wait for the inner to be done before we finish …
-    inner.wait();
-    outer();
   });
 
-  // … and wait for both queries to have finished before we go on and
-  // test postconditions.
-  outer.wait();
+  await inner.promise;
 
   Assert.equal(expectOne, 1);
   Assert.equal(expectTwo, 2);