Bug 1258595 - Wait for the Push service to shut down between tests. r=wchen

MozReview-Commit-ID: 1Ujqn1xNsMU

--HG--
extra : rebase_source : b51da9e58ce9904acff0202d57e258f1355076a1
extra : amend_source : 2ab09ebeb828a4ced87e88a0ced0447d2dfb8480

dom/push/PushComponents.js

@@ -271,12 +271,12 @@ Object.assign(PushServiceParent.prototype, {
   // Methods used for mocking in tests.
 
   replaceServiceBackend(options) {
-    this.service.changeTestServer(options.serverURI, options);
+    return this.service.changeTestServer(options.serverURI, options);
   },
 
   restoreServiceBackend() {
     var defaultServerURL = Services.prefs.getCharPref("dom.push.serverURL");
-    this.service.changeTestServer(defaultServerURL);
+    return this.service.changeTestServer(defaultServerURL);
   },
 });
 

dom/push/PushService.jsm

@@ -131,6 +131,7 @@ this.PushService = {
         console.error(
           "stateChangeProcessEnqueue: Error shutting down service", error);
       });
+    return this._stateChangeProcessQueue;
   },
 
   // Pending request. If a worker try to register for the same scope again, do
@@ -182,7 +183,6 @@ this.PushService = {
     if (this._state == PUSH_SERVICE_ACTIVATING) {
       // It is not important what is the new state as soon as we leave
       // PUSH_SERVICE_ACTIVATING
-      this._state = aNewState;
       if (this._notifyActivated) {
         if (aNewState < PUSH_SERVICE_ACTIVATING) {
           this._notifyActivated.reject(new Error("Push service not active"));
@@ -251,7 +251,7 @@ this.PushService = {
   changeTestServer(url, options = {}) {
     console.debug("changeTestServer()");
 
-    this._stateChangeProcessEnqueue(_ => {
+    return this._stateChangeProcessEnqueue(_ => {
       if (this._state < PUSH_SERVICE_ACTIVATING) {
         console.debug("changeTestServer: PushService not activated?");
         return Promise.resolve();
@@ -413,8 +413,7 @@ this.PushService = {
           return Promise.resolve();
         }
         return this._startService(service, uri, options)
-          .then(_ => this._stateChangeProcessEnqueue(_ =>
-            this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled")))
+          .then(_ => this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled"))
           );
       }
       case CHANGING_SERVICE_EVENT:
@@ -424,8 +423,7 @@ this.PushService = {
             this._setState(PUSH_SERVICE_ACTIVATING);
             // The service has not been running - start it.
             return this._startService(service, uri, options)
-              .then(_ => this._stateChangeProcessEnqueue(_ =>
-                this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled")))
+              .then(_ => this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled"))
               );
 
           } else {
@@ -437,8 +435,7 @@ this.PushService = {
               .then(_ =>
                  this._startService(service, uri, options)
               )
-              .then(_ => this._stateChangeProcessEnqueue(_ =>
-                this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled")))
+              .then(_ => this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled"))
               );
 
           }
@@ -1001,13 +998,18 @@ this.PushService = {
       }
       return Promise.reject(new Error("Push service offline"));
     }
-    switch (action) {
-      case "register":
-        return this._service.register(...params);
-      case "unregister":
-        return this._service.unregister(...params);
-    }
-    return Promise.reject(new Error("Unknown request type: " + action));
+    // Ensure the backend is ready. `getByPageRecord` already checks this, but
+    // we need to check again here in case the service was restarted in the
+    // meantime.
+    return this._checkActivated().then(_ => {
+      switch (action) {
+        case "register":
+          return this._service.register(...params);
+        case "unregister":
+          return this._service.unregister(...params);
+      }
+      return Promise.reject(new Error("Unknown request type: " + action));
+    });
   },
 
   /**
@@ -1071,7 +1073,7 @@ this.PushService = {
     Services.telemetry.getHistogramById("PUSH_API_SUBSCRIBE_FAILED").add()
     if (!reply.error) {
       console.warn("onRegisterError: Called without valid error message!",
-        reply);
+        reply, String(reply));
       throw new Error("Registration error");
     }
     throw reply.error;

dom/push/test/mockpushserviceparent.js

@@ -81,37 +81,43 @@ var pushService = Cc["@mozilla.org/push/Service;1"].
                   getService(Ci.nsIPushService).
                   wrappedJSObject;
 
-var mockWebSocket;
+var mockSocket;
+var serverMsgs = [];
 
 addMessageListener("socket-setup", function () {
-  mockWebSocket = new Promise((resolve, reject) => {
-    var mockSocket = null;
-    pushService.replaceServiceBackend({
-      serverURI: "wss://push.example.org/",
-      networkInfo: new MockNetworkInfo(),
-      makeWebSocket(uri) {
-        if (!mockSocket) {
-          mockSocket = new MockWebSocketParent(uri);
-          resolve(mockSocket);
-        }
-
-        return mockSocket;
+  pushService.replaceServiceBackend({
+    serverURI: "wss://push.example.org/",
+    networkInfo: new MockNetworkInfo(),
+    makeWebSocket(uri) {
+      mockSocket = new MockWebSocketParent(uri);
+      while (serverMsgs.length > 0) {
+        let msg = serverMsgs.shift();
+        mockSocket.serverSendMsg(msg);
       }
-    });
+      return mockSocket;
+    }
   });
 });
 
-addMessageListener("socket-teardown", function () {
-  mockWebSocket.then(socket => {
-    socket.close();
-    pushService.restoreServiceBackend();
-  });
+addMessageListener("socket-teardown", function (msg) {
+  pushService.restoreServiceBackend().then(_ => {
+    serverMsgs.length = 0;
+    if (mockSocket) {
+      mockSocket.close();
+      mockSocket = null;
+    }
+    sendAsyncMessage("socket-server-teardown");
+  }).catch(error => {
+    Cu.reportError(`Error restoring service backend: ${error}`);
+  })
 });
 
 addMessageListener("socket-server-msg", function (msg) {
-  mockWebSocket.then(socket => {
-    socket.serverSendMsg(msg);
-  });
+  if (mockSocket) {
+    mockSocket.serverSendMsg(msg);
+  } else {
+    serverMsgs.push(msg);
+  }
 });
 
 var MockService = {

dom/push/test/test_multiple_register_during_service_activation.html

@@ -57,15 +57,19 @@ http://creativecommons.org/licenses/publicdomain/
 
   function setupMultipleSubscriptions(swr) {
     // We need to do this to restart service so that a queue will be formed.
-    teardownMockPushSocket();
+    let promiseTeardown = teardownMockPushSocket();
     setupMockPushSocket(new MockWebSocket());
 
+    var pushSubscription;
     return Promise.all([
       subscribe(swr),
       subscribe(swr)
     ]).then(a => {
       ok(a[0].endpoint == a[1].endpoint, "setupMultipleSubscriptions - Got the same endpoint back.");
-      return a[0];
+      pushSubscription = a[0];
+      return promiseTeardown;
+    }).then(_ => {
+      return pushSubscription;
     }, err => {
       ok(false, "could not register for push notification");
       throw err;

dom/push/test/test_utils.js

@@ -61,9 +61,13 @@
 
   function teardownMockPushSocket() {
     if (currentMockSocket) {
-      currentMockSocket._isActive = false;
-      chromeScript.sendSyncMessage("socket-teardown");
+      return new Promise(resolve => {
+        currentMockSocket._isActive = false;
+        chromeScript.addMessageListener("socket-server-teardown", resolve);
+        chromeScript.sendSyncMessage("socket-teardown");
+      });
     }
+    return Promise.resolve();
   }
 
   /**
@@ -146,13 +150,13 @@
 
 // Remove permissions and prefs when the test finishes.
 SimpleTest.registerCleanupFunction(() => {
-  new Promise(resolve => {
+  return new Promise(resolve => {
     SpecialPowers.flushPermissions(_ => {
       SpecialPowers.flushPrefEnv(resolve);
     });
   }).then(_ => {
-    teardownMockPushSocket();
     restorePushService();
+    return teardownMockPushSocket();
   });
 });