Bug 1258595 - Shut down the Push service if errors occur at startup. r=wchen

MozReview-Commit-ID: HMWMJ5qPGwY

--HG--
extra : rebase_source : a59107cac8bd428ca521271be9045797e927b0cc
extra : source : 6733c0861332ad11ae84b9aedb0bc5ab642fa085

dom/push/PushService.jsm

@@ -121,8 +121,16 @@ this.PushService = {
     }
 
     this._stateChangeProcessQueue = this._stateChangeProcessQueue
-                                    .then(op)
-                                    .catch(_ => {});
+      .then(op)
+      .catch(error => {
+        console.error(
+          "stateChangeProcessEnqueue: Error transitioning state", error);
+        return this._shutdownService();
+      })
+      .catch(error => {
+        console.error(
+          "stateChangeProcessEnqueue: Error shutting down service", error);
+      });
   },
 
   // Pending request. If a worker try to register for the same scope again, do
@@ -194,7 +202,7 @@ this.PushService = {
     if (this._state < PUSH_SERVICE_ACTIVE_OFFLINE &&
         this._state != PUSH_SERVICE_ACTIVATING &&
         !calledFromConnEnabledEvent) {
-      return;
+      return Promise.resolve();
     }
 
     if (offline) {
@@ -202,22 +210,22 @@ this.PushService = {
         this._service.disconnect();
       }
       this._setState(PUSH_SERVICE_ACTIVE_OFFLINE);
-    } else {
-      if (this._state == PUSH_SERVICE_RUNNING) {
-        // PushService was not in the offline state, but got notification to
-        // go online (a offline notification has not been sent).
-        // Disconnect first.
-        this._service.disconnect();
-      }
-      this._db.getAllUnexpired()
-        .then(records => {
-          if (records.length > 0) {
-            // if there are request waiting
-            this._service.connect(records);
-          }
-        });
-      this._setState(PUSH_SERVICE_RUNNING);
+      return Promise.resolve();
     }
+
+    if (this._state == PUSH_SERVICE_RUNNING) {
+      // PushService was not in the offline state, but got notification to
+      // go online (a offline notification has not been sent).
+      // Disconnect first.
+      this._service.disconnect();
+    }
+    return this.getAllUnexpired().then(records => {
+      this._setState(PUSH_SERVICE_RUNNING);
+      if (records.length > 0) {
+        // if there are request waiting
+        this._service.connect(records);
+      }
+    });
   },
 
   _changeStateConnectionEnabledEvent: function(enabled) {
@@ -225,17 +233,18 @@ this.PushService = {
 
     if (this._state < PUSH_SERVICE_CONNECTION_DISABLE &&
         this._state != PUSH_SERVICE_ACTIVATING) {
-      return;
+      return Promise.resolve();
     }
 
     if (enabled) {
-      this._changeStateOfflineEvent(Services.io.offline, true);
-    } else {
-      if (this._state == PUSH_SERVICE_RUNNING) {
-        this._service.disconnect();
-      }
-      this._setState(PUSH_SERVICE_CONNECTION_DISABLE);
+      return this._changeStateOfflineEvent(Services.io.offline, true);
     }
+
+    if (this._state == PUSH_SERVICE_RUNNING) {
+      this._service.disconnect();
+    }
+    this._setState(PUSH_SERVICE_CONNECTION_DISABLE);
+    return Promise.resolve();
   },
 
   // Used for testing.
@@ -403,7 +412,7 @@ this.PushService = {
           this._setState(PUSH_SERVICE_INIT);
           return Promise.resolve();
         }
-        return this._startService(service, uri)
+        return this._startService(service, uri, options)
           .then(_ => this._stateChangeProcessEnqueue(_ =>
             this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled")))
           );
@@ -478,18 +487,8 @@ this.PushService = {
     if (options.serverURI) {
       // this is use for xpcshell test.
 
-      let [service, uri] = this._findService(options.serverURI);
-      if (!service) {
-        this._setState(PUSH_SERVICE_INIT);
-        return;
-      }
-
-      // Start service.
-      this._startService(service, uri, options).then(_ => {
-        // Before completing the activation check prefs. This will first check
-        // connection.enabled pref and then check offline state.
-        this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled"));
-      }).catch(Cu.reportError);
+      this._stateChangeProcessEnqueue(_ =>
+        this._changeServerURL(options.serverURI, STARTING_SERVICE_EVENT, options));
 
     } else {
       // This is only used for testing. Different tests require connecting to
@@ -542,7 +541,7 @@ this.PushService = {
     Services.obs.addObserver(this, "perm-changed", false);
   },
 
-  _startService: function(service, serverURI, options = {}) {
+  _startService(service, serverURI, options) {
     console.debug("startService()");
 
     if (this._state != PUSH_SERVICE_ACTIVATING) {
@@ -623,6 +622,13 @@ this.PushService = {
     Services.obs.removeObserver(this, "perm-changed");
   },
 
+  _shutdownService() {
+    let promiseChangeURL = this._changeServerURL("", UNINIT_EVENT);
+    this._setState(PUSH_SERVICE_UNINIT);
+    console.debug("shutdownService: shutdown complete!");
+    return promiseChangeURL;
+  },
+
   uninit: function() {
     console.debug("uninit()");
 
@@ -633,13 +639,7 @@ this.PushService = {
     prefs.ignore("serverURL", this);
     Services.obs.removeObserver(this, "quit-application");
 
-    this._stateChangeProcessEnqueue(_ =>
-      {
-        var p = this._changeServerURL("", UNINIT_EVENT);
-        this._setState(PUSH_SERVICE_UNINIT);
-        console.debug("uninit: shutdown complete!");
-        return p;
-      });
+    this._stateChangeProcessEnqueue(_ => this._shutdownService());
   },
 
   /**

dom/push/test/xpcshell/test_startup_error.js

@@ -0,0 +1,73 @@
+'use strict';
+
+const {PushService, PushServiceWebSocket} = serviceExports;
+
+function run_test() {
+  setPrefs();
+  do_get_profile();
+  run_next_test();
+}
+
+add_task(function* test_startup_error() {
+  let db = PushServiceWebSocket.newPushDB();
+  do_register_cleanup(() => {return db.drop().then(_ => db.close());});
+
+  PushService.init({
+    serverURI: 'wss://push.example.org/',
+    networkInfo: new MockDesktopNetworkInfo(),
+    db: makeStub(db, {
+      getAllExpired(prev) {
+        return Promise.reject('database corruption on startup');
+      },
+    }),
+    makeWebSocket(uri) {
+      return new MockWebSocket(uri, {
+        onHello(request) {
+          ok(false, 'Unexpected handshake');
+        },
+        onRegister(request) {
+          ok(false, 'Unexpected register request');
+        },
+      });
+    },
+  });
+
+  yield rejects(
+    PushService.register({
+      scope: `https://example.net/1`,
+      originAttributes: ChromeUtils.originAttributesToSuffix(
+        { appId: Ci.nsIScriptSecurityManager.NO_APP_ID, inIsolatedMozBrowser: false }),
+    }),
+    'Should not register if startup failed'
+  );
+
+  PushService.uninit();
+
+  PushService.init({
+    serverURI: 'wss://push.example.org/',
+    networkInfo: new MockDesktopNetworkInfo(),
+    db: makeStub(db, {
+      getAllUnexpired(prev) {
+        return Promise.reject('database corruption on connect');
+      },
+    }),
+    makeWebSocket(uri) {
+      return new MockWebSocket(uri, {
+        onHello(request) {
+          ok(false, 'Unexpected handshake');
+        },
+        onRegister(request) {
+          ok(false, 'Unexpected register request');
+        },
+      });
+    },
+  });
+  yield rejects(
+    PushService.registration({
+      scope: `https://example.net/1`,
+      originAttributes: ChromeUtils.originAttributesToSuffix(
+        { appId: Ci.nsIScriptSecurityManager.NO_APP_ID, inIsolatedMozBrowser: false }),
+    }),
+    'Should not return registration if connection failed'
+  );
+});

dom/push/test/xpcshell/xpcshell.ini

@@ -48,6 +48,7 @@ run-sequentially = This will delete all existing push subscriptions.
 [test_retry_ws.js]
 [test_service_parent.js]
 [test_service_child.js]
+[test_startup_error.js]
 
 #http2 test
 [test_resubscribe_4xxCode_http2.js]

modules/libpref/init/all.js

@@ -4612,7 +4612,7 @@ pref("dom.mozAlarms.enabled", false);
 
 pref("dom.push.enabled", false);
 
-pref("dom.push.loglevel", "off");
+pref("dom.push.loglevel", "error");
 
 pref("dom.push.serverURL", "wss://push.services.mozilla.com/");
 pref("dom.push.userAgentID", "");