Bug 1258595 - Wait for the Push service to shut down between tests. r=wchen
authorKit Cambridge <kcambridge@mozilla.com>
Fri, 08 Apr 2016 11:39:00 -0700
changeset 293106 1085ea32229e7ef6df1a36ef98272fd06d5a4925
parent 293105 915a3ac4e5639da7c02b92ef1457711301393417
child 293107 f95bb4a2a46ddc64770060ce2967e7ba7695fd9f
push id30175
push usercbook@mozilla.com
push dateThu, 14 Apr 2016 09:38:40 +0000
treeherdermozilla-central@91115264629d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswchen
bugs1258595
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);