Bug 1258595 - Wait for the Push service to shut down between tests. r?wchen draft
authorKit Cambridge <kcambridge@mozilla.com>
Fri, 08 Apr 2016 11:39:00 -0700
changeset 349912 b51da9e58ce9904acff0202d57e258f1355076a1
parent 349911 a59107cac8bd428ca521271be9045797e927b0cc
child 518229 999cd8785aa7a0f6113607ff4a7f0001e21be4d0
push id15223
push userkcambridge@mozilla.com
push dateTue, 12 Apr 2016 17:02:18 +0000
reviewerswchen
bugs1258595
milestone48.0a1
Bug 1258595 - Wait for the Push service to shut down between tests. r?wchen MozReview-Commit-ID: 1Ujqn1xNsMU
dom/push/PushComponents.js
dom/push/PushService.jsm
dom/push/test/mockpushserviceparent.js
dom/push/test/test_multiple_register_during_service_activation.html
dom/push/test/test_utils.js
--- a/dom/push/PushComponents.js
+++ b/dom/push/PushComponents.js
@@ -266,22 +266,22 @@ Object.assign(PushServiceParent.prototyp
   _getResponseName(requestName, suffix) {
     let name = requestName.slice("Push:".length);
     return "PushService:" + name + ":" + suffix;
   },
 
   // 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);
   },
 });
 
 // Used to replace the implementation with a mock.
 Object.defineProperty(PushServiceParent.prototype, "service", {
   get() {
     return this._service || PushService;
   },
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -126,16 +126,17 @@ this.PushService = {
         console.error(
           "stateChangeProcessEnqueue: Error transitioning state", error);
         return this._shutdownService();
       })
       .catch(error => {
         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
   // not send a new registration request. Therefore we need queue of pending
   // register requests. This is the list of scopes which pending registration.
   _pendingRegisterRequest: {},
   _notifyActivated: null,
   _activated: null,
@@ -177,17 +178,16 @@ this.PushService = {
 
     if (this._state == aNewState) {
       return;
     }
 
     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"));
         } else {
           this._notifyActivated.resolve();
         }
       }
       this._notifyActivated = null;
@@ -246,17 +246,17 @@ this.PushService = {
     this._setState(PUSH_SERVICE_CONNECTION_DISABLE);
     return Promise.resolve();
   },
 
   // Used for testing.
   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();
       }
 
       return this._changeServerURL(url, CHANGING_SERVICE_EVENT, options);
     });
   },
@@ -408,42 +408,39 @@ this.PushService = {
       case STARTING_SERVICE_EVENT:
       {
         let [service, uri] = this._findService(serverURI);
         if (!service) {
           this._setState(PUSH_SERVICE_INIT);
           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:
         let [service, uri] = this._findService(serverURI);
         if (service) {
           if (this._state == PUSH_SERVICE_INIT) {
             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 {
             this._setState(PUSH_SERVICE_ACTIVATING);
             // If we already had running service - stop service, start the new
             // one and check connection.enabled and offline state(offline state
             // check is called in changeStateConnectionEnabledEvent function)
             return this._stopService(CHANGING_SERVICE_EVENT)
               .then(_ =>
                  this._startService(service, uri, options)
               )
-              .then(_ => this._stateChangeProcessEnqueue(_ =>
-                this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled")))
+              .then(_ => this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled"))
               );
 
           }
         } else {
           if (this._state == PUSH_SERVICE_INIT) {
             return Promise.resolve();
 
           } else {
@@ -996,23 +993,28 @@ this.PushService = {
     if (this._state == PUSH_SERVICE_CONNECTION_DISABLE) {
       return Promise.reject(new Error("Push service disabled"));
     } else if (this._state == PUSH_SERVICE_ACTIVE_OFFLINE) {
       if (this._service.serviceType() == "WebSocket" && action == "unregister") {
         return Promise.resolve();
       }
       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));
+    });
   },
 
   /**
    * Called on message from the child process. aPageRecord is an object sent by
    * navigator.push, identifying the sending page and other fields.
    */
   _registerWithServer: function(aPageRecord) {
     console.debug("registerWithServer()", aPageRecord);
@@ -1066,17 +1068,17 @@ this.PushService = {
    * Exceptions thrown in _onRegisterError are caught by the promise obtained
    * from _service.request, causing the promise to be rejected instead.
    */
   _onRegisterError: function(reply) {
     console.debug("_onRegisterError()");
     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;
   },
 
   notificationsCleared() {
     this._visibleNotifications.clear();
   },
--- a/dom/push/test/mockpushserviceparent.js
+++ b/dom/push/test/mockpushserviceparent.js
@@ -76,47 +76,53 @@ MockNetworkInfo.prototype = {
     return 'network:offline-status-changed';
   }
 };
 
 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 = {
   requestID: 1,
   resolvers: new Map(),
 
   sendRequest(name, params) {
     return new Promise((resolve, reject) => {
--- a/dom/push/test/test_multiple_register_during_service_activation.html
+++ b/dom/push/test/test_multiple_register_during_service_activation.html
@@ -52,25 +52,29 @@ http://creativecommons.org/licenses/publ
       }, err => {
         ok(false, "could not register for push notification");
         throw err;
       });
   }
 
   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;
     });
   }
 
   function getEndpointExpectNull(swr) {
     return swr.pushManager.getSubscription()
--- a/dom/push/test/test_utils.js
+++ b/dom/push/test/test_utils.js
@@ -56,19 +56,23 @@
     chromeScript.sendSyncMessage("socket-setup");
     chromeScript.addMessageListener("socket-client-msg", function(msg) {
       mockWebSocket.handleMessage(msg);
     });
   }
 
   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();
   }
 
   /**
    * Minimal implementation of web sockets for use in testing. Forwards
    * messages to a mock web socket in the parent process that is used
    * by the push service.
    */
   function MockWebSocket() {}
@@ -141,23 +145,23 @@
   g.setupMockPushSocket = setupMockPushSocket;
   g.teardownMockPushSocket = teardownMockPushSocket;
   g.replacePushService = replacePushService;
   g.restorePushService = restorePushService;
 }(this));
 
 // 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();
   });
 });
 
 function setPushPermission(allow) {
   return new Promise(resolve => {
     SpecialPowers.pushPermissions([
       { type: "desktop-notification", allow, context: document },
       ], resolve);