Bug 1258595 - Shut down the Push service if errors occur at startup. r=wchen draft
authorKit Cambridge <kcambridge@mozilla.com>
Mon, 21 Mar 2016 18:07:16 -0700
changeset 349911 a59107cac8bd428ca521271be9045797e927b0cc
parent 348506 b6683e141c47c022598c0caac3ea8ba8c6236d42
child 349912 b51da9e58ce9904acff0202d57e258f1355076a1
push id15223
push userkcambridge@mozilla.com
push dateTue, 12 Apr 2016 17:02:18 +0000
reviewerswchen
bugs1258595
milestone48.0a1
Bug 1258595 - Shut down the Push service if errors occur at startup. r=wchen MozReview-Commit-ID: HMWMJ5qPGwY
dom/push/PushService.jsm
dom/push/test/xpcshell/test_startup_error.js
dom/push/test/xpcshell/xpcshell.ini
modules/libpref/init/all.js
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -116,18 +116,26 @@ this.PushService = {
   // a new db is started. This events must be sequential.
   _stateChangeProcessQueue: null,
   _stateChangeProcessEnqueue: function(op) {
     if (!this._stateChangeProcessQueue) {
       this._stateChangeProcessQueue = Promise.resolve();
     }
 
     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
   // 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,
@@ -189,58 +197,59 @@ this.PushService = {
   },
 
   _changeStateOfflineEvent: function(offline, calledFromConnEnabledEvent) {
     console.debug("changeStateOfflineEvent()", offline);
 
     if (this._state < PUSH_SERVICE_ACTIVE_OFFLINE &&
         this._state != PUSH_SERVICE_ACTIVATING &&
         !calledFromConnEnabledEvent) {
-      return;
+      return Promise.resolve();
     }
 
     if (offline) {
       if (this._state == PUSH_SERVICE_RUNNING) {
         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();
+      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);
       }
-      this._db.getAllUnexpired()
-        .then(records => {
-          if (records.length > 0) {
-            // if there are request waiting
-            this._service.connect(records);
-          }
-        });
-      this._setState(PUSH_SERVICE_RUNNING);
-    }
+    });
   },
 
   _changeStateConnectionEnabledEvent: function(enabled) {
     console.debug("changeStateConnectionEnabledEvent()", enabled);
 
     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.
   changeTestServer(url, options = {}) {
     console.debug("changeTestServer()");
 
     this._stateChangeProcessEnqueue(_ => {
       if (this._state < PUSH_SERVICE_ACTIVATING) {
@@ -398,17 +407,17 @@ 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)
+        return this._startService(service, uri, options)
           .then(_ => this._stateChangeProcessEnqueue(_ =>
             this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled")))
           );
       }
       case CHANGING_SERVICE_EVENT:
         let [service, uri] = this._findService(serverURI);
         if (service) {
           if (this._state == PUSH_SERVICE_INIT) {
@@ -473,28 +482,18 @@ this.PushService = {
 
     this._setState(PUSH_SERVICE_ACTIVATING);
 
     Services.obs.addObserver(this, "quit-application", false);
 
     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
       // slightly different URLs.
       prefs.observe("serverURL", this);
 
       this._stateChangeProcessEnqueue(_ =>
         this._changeServerURL(prefs.get("serverURL"), STARTING_SERVICE_EVENT));
@@ -537,17 +536,17 @@ this.PushService = {
     // Prunes expired registrations and notifies dormant service workers.
     Services.obs.addObserver(this, "idle-daily", false);
 
     // Prunes registrations for sites for which the user revokes push
     // permissions.
     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) {
       return Promise.reject();
     }
 
     this._service = service;
 
@@ -618,33 +617,34 @@ this.PushService = {
     prefs.ignore("connection.enabled", this);
 
     Services.obs.removeObserver(this, this._networkStateChangeEventName);
     Services.obs.removeObserver(this, "clear-origin-data");
     Services.obs.removeObserver(this, "idle-daily");
     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()");
 
     if (this._state == PUSH_SERVICE_UNINIT) {
       return;
     }
 
     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());
   },
 
   /**
    * Drops all active registrations and notifies the associated service
    * workers. This function is called when the user switches Push servers,
    * or when the server invalidates all existing registrations.
    *
    * We ignore expired registrations because they're already handled in other
new file mode 100644
--- /dev/null
+++ b/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'
+  );
+});
--- a/dom/push/test/xpcshell/xpcshell.ini
+++ b/dom/push/test/xpcshell/xpcshell.ini
@@ -43,16 +43,17 @@ run-sequentially = This will delete all 
 [test_unregister_invalid_json.js]
 [test_unregister_not_found.js]
 [test_unregister_success.js]
 [test_updateRecordNoEncryptionKeys_ws.js]
 [test_reconnect_retry.js]
 [test_retry_ws.js]
 [test_service_parent.js]
 [test_service_child.js]
+[test_startup_error.js]
 
 #http2 test
 [test_resubscribe_4xxCode_http2.js]
 [test_resubscribe_5xxCode_http2.js]
 [test_resubscribe_listening_for_msg_error_http2.js]
 [test_register_5xxCode_http2.js]
 [test_updateRecordNoEncryptionKeys_http2.js]
 [test_register_success_http2.js]
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -4603,17 +4603,17 @@ pref("dom.sms.maxReadAheadEntries", 0);
 
 // WebAlarms
 pref("dom.mozAlarms.enabled", false);
 
 // Push
 
 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", "");
 
 // The maximum number of push messages that a service worker can receive
 // without user interaction.
 pref("dom.push.maxQuotaPerSubscription", 16);