Bug 1108028: replace pushURL registered with LoopServer whenever PushServer does a re-assignment. r=dmose
authorPaul Kerr [:pkerr] <pkerr@mozilla.com>
Tue, 16 Dec 2014 14:14:06 -0800
changeset 223861 9496d6ec12c3aba63adc3a5c79852f252442c7d5
parent 223860 a3f89050d1f66522cf1b93e52a6d5ccd2e0a5347
child 223862 f3285709bf99f69941b63106f94513d166dc8ef6
push id28111
push usercbook@mozilla.com
push dateThu, 15 Jan 2015 13:10:01 +0000
treeherdermozilla-central@8a0c6e8d2083 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdmose
bugs1108028
milestone38.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 1108028: replace pushURL registered with LoopServer whenever PushServer does a re-assignment. r=dmose
browser/components/loop/MozLoopPushHandler.jsm
browser/components/loop/MozLoopService.jsm
browser/components/loop/test/mochitest/browser_fxa_login.js
browser/components/loop/test/mochitest/head.js
browser/components/loop/test/xpcshell/head.js
browser/components/loop/test/xpcshell/test_looppush_initialize.js
browser/components/loop/test/xpcshell/test_loopservice_registration.js
browser/components/loop/test/xpcshell/test_loopservice_registration_retry.js
browser/components/loop/test/xpcshell/test_loopservice_token_invalid.js
--- a/browser/components/loop/MozLoopPushHandler.jsm
+++ b/browser/components/loop/MozLoopPushHandler.jsm
@@ -439,23 +439,26 @@ let MozLoopPushHandler = {
     // the websocket referencing it as an nsIWebSocketListener is released.
     this.channels.clear();
     this.uaID = undefined;
     this.pushUrl = undefined;
     this.pushServerUri = undefined;
   },
 
    /**
-    * Start registration of a PushServer notification channel.
-    * connection, it will automatically say hello and register the channel
-    * id with the server.
+    * Assign a channel to be registered with the PushServer
+    * This channel will be registered when a connection to the PushServer
+    * has been established or re-registered after a connection has been lost
+    * and re-established. Calling this more than once for the same channel
+    * has no additional effect.
     *
     * onRegistered callback parameters:
     * - {String|null} err: Encountered error, if any
     * - {String} url: The push url obtained from the server
+    * - {String} channelID The channelID on which the notification was sent.
     *
     * onNotification parameters:
     * - {String} version The version string received from the push server for
     *                    the notification.
     * - {String} channelID The channelID on which the notification was sent.
     *
     * @param {String} channelID Channel ID to use in registration.
     *
@@ -473,30 +476,55 @@ let MozLoopPushHandler = {
     if (!channelID || !onRegistered || !onNotification) {
       throw new Error("missing required parameter(s):" +
                       (channelID ? "" : " channelID") +
                       (onRegistered ? "" : " onRegistered") +
                       (onNotification ? "" : " onNotification"));
     }
 
     consoleLog.info("PushHandler: channel registration: ", channelID);
-    // If the channel is already registered, callback with an error immediately
-    // so we don't leave code hanging waiting for an onRegistered callback.
     if (this.channels.has(channelID)) {
-      consoleLog.error("PushHandler: channel already registered");
-      onRegistered("error: channel already registered: " + channelID);
+      // If this channel has an active registration with the PushServer
+      // call the onRegister callback with the URL.
+      if (this.registeredChannels[channelID]) {
+        onRegistered(null, this.registeredChannels[channelID], channelID);
+      }
+      // Update the channel record.
+      this.channels.set(channelID, {onRegistered: onRegistered,
+                        onNotification: onNotification});
       return;
     }
 
     this.channels.set(channelID, {onRegistered: onRegistered,
                                   onNotification: onNotification});
-
     this._channelsToRegister.push(channelID);
     this._registerChannels();
   },
+  
+  /**
+   * Un-register a notification channel.
+   *
+   * @param {String} channelID Notification channel ID.
+   */
+  unregister: function(channelID) {
+    consoleLog.info("MozLoopPushHandler: un-register channel ", channelID);
+    if (!this.channels.has(channelID)) {
+      return;
+    }
+
+    this.channels.delete(channelID);
+
+    if (this.registeredChannels[channelID]) {
+      delete this.registeredChannels[channelID];
+      if (this.connectionState === CONNECTION_STATE_OPEN) {
+        this._pushSocket.send({messageType: "unregister",
+                               channelID: channelID});
+      }
+    }
+  },
 
   /**
    * Handles the start of the websocket stream.
    * Sends a hello message to the server.
    *
    */
   _onStart: function() {
     consoleLog.info("PushHandler: websocket open, sending 'hello' to PushServer");
@@ -613,17 +641,17 @@ let MozLoopPushHandler = {
     // If a new uaID is received, then any previous channel registrations
     // are no longer valid and a Registration request is generated.
     if (this.uaID !== aMsg.uaid) {
       consoleLog.log("PushHandler: registering all channels");
       this.uaID = aMsg.uaid;
       // Re-register all channels.
       this._channelsToRegister = [...this.channels.keys()];
       this.registeredChannels = {};
-     }
+    }
     // Allow queued registrations to start (or all if cleared above).
     this._registerChannels();
   },
 
   /**
    * Handles notification message.
    *
    * This method will parse the Array of updates and trigger
@@ -698,28 +726,26 @@ let MozLoopPushHandler = {
                         msg.channelID);
         // retry the registration request after a suitable delay
         this._retryManager.retry(() => this._sendRegistration(msg.channelID));
         break;
 
       case 409:
         consoleLog.error("PushHandler: received a 409 response from the PushServer: ",
                          msg.channelID);
-        this.channels.get(this._pendingChannelID).onRegistered(
-          "error: PushServer ChannelID already in use: " + msg.channelID);
+        this.channels.get(this._pendingChannelID).onRegistered("409");
         // Remove this channel from the channel list.
         this.channels.delete(this._pendingChannelID);
         this._registerNext();
         break;
 
       default:
         consoleLog.error("PushHandler: received error ", msg.status,
                          " from the PushServer: ", msg.channelID);
-        this.channels.get(this._pendingChannelID).onRegistered(
-          "error: PushServer registration failure, status = " + msg.status);
+        this.channels.get(this._pendingChannelID).onRegistered(msg.status);
         this.channels.delete(this._pendingChannelID);
         this._registerNext();
         break;
     }
   },
 
   /**
    * Attempts to open a websocket.
--- a/browser/components/loop/MozLoopService.jsm
+++ b/browser/components/loop/MozLoopService.jsm
@@ -119,20 +119,20 @@ let gConversationWindowData = new Map();
  * Internal helper methods and state
  *
  * The registration is a two-part process. First we need to connect to
  * and register with the push server. Then we need to take the result of that
  * and register with the Loop server.
  */
 let MozLoopServiceInternal = {
   conversationContexts: new Map(),
+  pushURLs: new Map(),
 
   mocks: {
     pushHandler: undefined,
-    webSocket: undefined,
   },
 
   /**
    * The current deferreds for the registration processes. This is set if in progress
    * or the registration was successful. This is null if a registration attempt was
    * unsuccessful.
    */
   deferredRegistrations: new Map(),
@@ -320,111 +320,209 @@ let MozLoopServiceInternal = {
     }
   },
 
   get errors() {
     return gErrors;
   },
 
   /**
-   * Get endpoints with the push server and register for notifications.
-   * This should only be called from promiseRegisteredWithServers to prevent reentrancy.
+   * Create a notification channel between the LoopServer and this client
+   * via a PushServer. Once created, any subsequent changes in the pushURL
+   * assigned by the PushServer will be communicated to the LoopServer.
+   * with the Loop server. It will return early if already registered.
    *
+   * @param {String} channelID Unique identifier for the notification channel
+   *                 registered with the PushServer.
    * @param {LOOP_SESSION_TYPE} sessionType
-   * @return {Promise} resolves with all push endpoints
-   *                   rejects if any of the push registrations failed
+   * @param {String} serviceType Either 'calls' or 'rooms'.
+   * @param {Function} onNotification Callback function that will be associated
+   *                   with this channel from the PushServer.
+   * @returns {Promise} A promise that is resolved with no params on completion, or
+   *                    rejected with an error code or string.
    */
-  promiseRegisteredWithPushServer: function(sessionType) {
-    if (!this.deferredRegistrations.has(sessionType)) {
-      return Promise.reject(new Error("promiseRegisteredWithPushServer must be called while there is a " +
-                            "deferred in deferredRegistrations in order to prevent reentrancy"));
-    }
-    // Wrap push notification registration call-back in a Promise.
-    function registerForNotification(channelID, onNotification) {
-      log.debug("registerForNotification", channelID);
-      return new Promise((resolve, reject) => {
-        function onRegistered(error, pushUrl) {
-          log.debug("registerForNotification onRegistered:", error, pushUrl);
-          if (error) {
-            reject(Error(error));
-          } else {
-            resolve(pushUrl);
-          }
+  createNotificationChannel: function(channelID, sessionType, serviceType, onNotification) {
+    log.debug("createNotificationChannel", channelID, sessionType, serviceType);
+    // Wrap the push notification registration callback in a Promise.
+    return new Promise((resolve, reject) => {
+      let onRegistered = (error, pushURL, channelID) => {
+        log.debug("createNotificationChannel onRegistered:", error, pushURL, channelID);
+        if (error) {
+          reject(Error(error));
+        } else {
+          resolve(this.registerWithLoopServer(sessionType, serviceType, pushURL));
         }
+      }
 
-        // If we're already registered, resolve with the existing push URL
-        let pushURL = MozLoopServiceInternal.pushHandler.registeredChannels[channelID];
-        if (pushURL) {
-          log.debug("Using the existing push endpoint for channelID:", channelID);
-          resolve(pushURL);
-          return;
-        }
-
-        MozLoopServiceInternal.pushHandler.register(channelID, onRegistered, onNotification);
-      });
-    }
-
-    let options = this.mocks.webSocket ? { mockWebSocket: this.mocks.webSocket } : {};
-    this.pushHandler.initialize(options);
-
-    if (sessionType == LOOP_SESSION_TYPE.GUEST) {
-      let callsRegGuest = registerForNotification(MozLoopService.channelIDs.callsGuest,
-                                                  LoopCalls.onNotification);
-
-      let roomsRegGuest = registerForNotification(MozLoopService.channelIDs.roomsGuest,
-                                                  roomsPushNotification);
-      return Promise.all([callsRegGuest, roomsRegGuest]);
-    } else if (sessionType == LOOP_SESSION_TYPE.FXA) {
-      let callsRegFxA = registerForNotification(MozLoopService.channelIDs.callsFxA,
-                                                LoopCalls.onNotification);
-
-      let roomsRegFxA = registerForNotification(MozLoopService.channelIDs.roomsFxA,
-                                                roomsPushNotification);
-      return Promise.all([callsRegFxA, roomsRegFxA]);
-    }
-
-    return Promise.reject(new Error("promiseRegisteredWithPushServer: Invalid sessionType"));
+      this.pushHandler.register(channelID, onRegistered, onNotification);
+    });
   },
 
   /**
-   * Starts registration of Loop with the push server, and then will register
-   * with the Loop server. It will return early if already registered.
+   * Starts registration of Loop with the PushServer and the LoopServer.
+   * Successful PushServer registration will automatically trigger the registration
+   * of the PushURL returned by the PushServer with the LoopServer. If the registration
+   * chain has already been set up, this function will simply resolve.
    *
    * @param {LOOP_SESSION_TYPE} sessionType
    * @returns {Promise} a promise that is resolved with no params on completion, or
    *          rejected with an error code or string.
    */
   promiseRegisteredWithServers: function(sessionType = LOOP_SESSION_TYPE.GUEST) {
+    if (sessionType !== LOOP_SESSION_TYPE.GUEST && sessionType !== LOOP_SESSION_TYPE.FXA) {
+      return Promise.reject(new Error("promiseRegisteredWithServers: Invalid sessionType"));
+    }
+
     if (this.deferredRegistrations.has(sessionType)) {
-      log.debug("promiseRegisteredWithServers: registration already completed or in progress:", sessionType);
-      return this.deferredRegistrations.get(sessionType).promise;
+      log.debug("promiseRegisteredWithServers: registration already completed or in progress:",
+                sessionType);
+      return this.deferredRegistrations.get(sessionType);
     }
 
-    let result = null;
-    let deferred = Promise.defer();
-    log.debug("assigning to deferredRegistrations for sessionType:", sessionType);
-    this.deferredRegistrations.set(sessionType, deferred);
+    let options = this.mocks.webSocket ? { mockWebSocket: this.mocks.webSocket } : {};
+    this.pushHandler.initialize(options); // This can be called more than once.
 
-    // We grab the promise early in case one of the callers below delete it from the map.
-    result = deferred.promise;
+    let callsID = sessionType == LOOP_SESSION_TYPE.GUEST ?
+          MozLoopService.channelIDs.callsGuest :
+          MozLoopService.channelIDs.callsFxA,
+        roomsID = sessionType == LOOP_SESSION_TYPE.GUEST ?
+          MozLoopService.channelIDs.roomsGuest :
+          MozLoopService.channelIDs.roomsFxA;
 
-    this.promiseRegisteredWithPushServer(sessionType).then(() => {
-      return this.registerWithLoopServer(sessionType);
-    }).then(() => {
-      deferred.resolve("registered to status:" + sessionType);
-      // No need to clear the promise here, everything was good, so we don't need
-      // to re-register.
-    }, error => {
-      log.error("Failed to register with Loop server with sessionType " + sessionType, error);
-      deferred.reject(error);
+    let regPromise = this.createNotificationChannel(
+      callsID, sessionType, "calls", LoopCalls.onNotification).then(() => {
+        return this.createNotificationChannel(
+          roomsID, sessionType, "rooms", roomsPushNotification)});
+
+    log.debug("assigning to deferredRegistrations for sessionType:", sessionType);
+    this.deferredRegistrations.set(sessionType, regPromise);
+
+    // Do not return the new Promise generated by this catch() invocation.
+    // This will be called along with any other onReject function attached to regPromise.
+    regPromise.catch((error) => {
+      log.error("Failed to register with Loop server with sessionType ", sessionType, error);
       this.deferredRegistrations.delete(sessionType);
       log.debug("Cleared deferredRegistration for sessionType:", sessionType);
     });
 
-    return result;
+    return regPromise;
+  },
+
+  /**
+   * Registers with the Loop server either as a guest or a FxA user.
+   *
+   * @private
+   * @param {LOOP_SESSION_TYPE} sessionType The type of session e.g. guest or FxA
+   * @param {String} serviceType: "rooms" or "calls"
+   * @param {Boolean} [retry=true] Whether to retry if authentication fails.
+   * @return {Promise} resolves to pushURL or rejects with an Error
+   */
+  registerWithLoopServer: function(sessionType, serviceType, pushURL, retry = true) {
+    log.debug("registerWithLoopServer with sessionType:", sessionType, serviceType, retry);
+    if (!pushURL || !sessionType || !serviceType) {
+      return Promise.reject(new Error("Invalid or missing parameters for registerWithLoopServer"));
+    }
+
+    let pushURLs = this.pushURLs.get(sessionType);
+
+    // Create a blank URL record set if none exists for this sessionType.
+    if (!pushURLs) {
+      pushURLs = {calls: undefined, rooms: undefined};
+      this.pushURLs.set(sessionType, pushURLs);
+    }
+
+    if (pushURLs[serviceType] == pushURL) {
+      return Promise.resolve(pushURL);
+    }
+
+    let newURLs = {calls: pushURLs.calls,
+                   rooms: pushURLs.rooms};
+    newURLs[serviceType] = pushURL;
+
+    return this.hawkRequestInternal(sessionType, "/registration", "POST",
+                                    {simplePushURLs: newURLs}).then(
+      (response) => {
+        // If this failed we got an invalid token.
+        if (!this.storeSessionToken(sessionType, response.headers)) {
+          throw new Error("session-token-wrong-size");
+        }
+
+        // Record the new push URL
+        pushURLs[serviceType] = pushURL;
+        log.debug("Successfully registered with server for sessionType", sessionType);
+        this.clearError("registration");
+        return pushURL;
+      }, (error) => {
+        // There's other errors than invalid auth token, but we should only do the reset
+        // as a last resort.
+        if (error.code === 401) {
+          // Authorization failed, invalid token, we need to try again with a new token.
+          // XXX (pkerr) - Why is there a retry here? This will not clear up a hawk session
+          // token problem at this level.
+          if (retry) {
+            return this.registerWithLoopServer(sessionType, serviceType, pushURL, false);
+          }
+        }
+
+        log.error("Failed to register with the loop server. Error: ", error);
+        throw error;
+      }
+    );
+  },
+
+  /**
+   * Unregisters from the Loop server either as a guest or a FxA user.
+   *
+   * This is normally only wanted for FxA users as we normally want to keep the
+   * guest session with the device.
+   *
+   * NOTE: It is the responsibiliy of the caller the clear the session token
+   * after all of the notification classes: calls and rooms, for either
+   * Guest or FxA have been unregistered with the LoopServer.
+   *
+   * @param {LOOP_SESSION_TYPE} sessionType The type of session e.g. guest or FxA
+   * @return {Promise} resolving when the unregistration request finishes
+   */
+  unregisterFromLoopServer: function(sessionType) {
+    let prefType = Services.prefs.getPrefType(this.getSessionTokenPrefName(sessionType));
+    if (prefType == Services.prefs.PREF_INVALID) {
+      log.debug("already unregistered from LoopServer", sessionType);
+      return Promise.resolve("already unregistered");
+    }
+
+    let error,
+        pushURLs = this.pushURLs.get(sessionType),
+        callsPushURL = pushURLs ? pushURLs.calls : null,
+        roomsPushURL = pushURLs ? pushURLs.rooms : null;
+    this.pushURLs.delete(sessionType);
+
+    let unregister = (sessionType, pushURL) => {
+      if (!pushURL) {
+        return Promise.resolve("no pushURL of this type to unregister");
+      }
+
+      let unregisterURL = "/registration?simplePushURL=" + encodeURIComponent(pushURL);
+      return this.hawkRequestInternal(sessionType, unregisterURL, "DELETE").then(
+        () => {
+          log.debug("Successfully unregistered from server for sessionType = ", sessionType);
+          return "unregistered sessionType " + sessionType;
+        },
+        error => {
+          if (error.code === 401) {
+            // Authorization failed, invalid token. This is fine since it may mean we already logged out.
+            log.debug("already unregistered - invalid token", sessionType);
+            return "already unregistered, sessionType = " + sessionType;
+          }
+
+          log.error("Failed to unregister with the loop server. Error: ", error);
+          throw error;
+        });
+    }
+
+    return Promise.all([unregister(sessionType, callsPushURL), unregister(sessionType, roomsPushURL)]);
   },
 
   /**
    * Performs a hawk based request to the loop server - there is no pre-registration
    * for this request, if this is required, use hawkRequest.
    *
    * @param {LOOP_SESSION_TYPE} sessionType The type of session to use for the request.
    *                                        This is one of the LOOP_SESSION_TYPE members.
@@ -435,16 +533,17 @@ let MozLoopServiceInternal = {
    * @param {Boolean} [retryOn401=true] Whether to retry if authentication fails.
    * @returns {Promise}
    *        Returns a promise that resolves to the response of the API call,
    *        or is rejected with an error.  If the server response can be parsed
    *        as JSON and contains an 'error' property, the promise will be
    *        rejected with this JSON-parsed response.
    */
   hawkRequestInternal: function(sessionType, path, method, payloadObj, retryOn401 = true) {
+    log.debug("hawkRequestInternal: ", sessionType, path, method);
     if (!gHawkClient) {
       gHawkClient = new HawkClient(this.loopServerUri);
     }
 
     let sessionToken, credentials;
     try {
       sessionToken = Services.prefs.getCharPref(this.getSessionTokenPrefName(sessionType));
     } catch (x) {
@@ -468,25 +567,27 @@ let MozLoopServiceInternal = {
           newPayloadObj[property] = payloadObj[property];
         }
       };
       payloadObj = newPayloadObj;
     }
 
     let handle401Error = (error) => {
       if (sessionType === LOOP_SESSION_TYPE.FXA) {
-        MozLoopService.logOutFromFxA().then(() => {
+        return MozLoopService.logOutFromFxA().then(() => {
           // Set a user-visible error after logOutFromFxA clears existing ones.
           this.setError("login", error);
+          throw error;
         });
       } else if (this.urlExpiryTimeIsInFuture()) {
         // If there are no Guest URLs in the future, don't use setError to notify the user since
         // there isn't a need for a Guest registration at this time.
         this.setError("registration", error);
       }
+      throw error;
     };
 
     return gHawkClient.request(path, method, credentials, payloadObj).then(
       (result) => {
         this.clearError("network");
         return result;
       },
       (error) => {
@@ -494,22 +595,21 @@ let MozLoopServiceInternal = {
         this.clearSessionToken(sessionType);
         if (retryOn401 && sessionType === LOOP_SESSION_TYPE.GUEST) {
           log.info("401 and INVALID_AUTH_TOKEN - retry registration");
           return this.registerWithLoopServer(sessionType, false).then(
             () => {
               return this.hawkRequestInternal(sessionType, path, method, payloadObj, false);
             },
             () => {
-              handle401Error(error); //Process the original error that triggered the retry.
-              throw error;
+              return handle401Error(error); //Process the original error that triggered the retry.
             }
           );
         }
-        handle401Error(error);
+        return handle401Error(error);
       }
       throw error;
     });
   },
 
   /**
    * Performs a hawk based request to the loop server, registering if necessary.
    *
@@ -554,17 +654,16 @@ let MozLoopServiceInternal = {
       case LOOP_SESSION_TYPE.GUEST:
         suffix = "";
         break;
       case LOOP_SESSION_TYPE.FXA:
         suffix = ".fxa";
         break;
       default:
         throw new Error("Unknown LOOP_SESSION_TYPE");
-        break;
     }
     return "loop.hawk-session-token" + suffix;
   },
 
   /**
    * Used to store a session token from a request if it exists in the headers.
    *
    * @param {LOOP_SESSION_TYPE} sessionType The type of session to use for the request.
@@ -599,122 +698,16 @@ let MozLoopServiceInternal = {
    *                                        One of the LOOP_SESSION_TYPE members.
    */
   clearSessionToken: function(sessionType) {
     Services.prefs.clearUserPref(this.getSessionTokenPrefName(sessionType));
     log.debug("Cleared hawk session token for sessionType", sessionType);
   },
 
   /**
-   * Registers with the Loop server either as a guest or a FxA user. This method should only be
-   * called by promiseRegisteredWithServers since it prevents calling this while a registration is
-   * already in progress.
-   *
-   * @private
-   * @param {LOOP_SESSION_TYPE} sessionType The type of session e.g. guest or FxA
-   * @param {Boolean} [retry=true] Whether to retry if authentication fails.
-   * @return {Promise}
-   */
-  registerWithLoopServer: function(sessionType, retry = true) {
-    log.debug("registerWithLoopServer with sessionType:", sessionType);
-
-    let callsPushURL, roomsPushURL;
-    if (sessionType == LOOP_SESSION_TYPE.FXA) {
-      callsPushURL = this.pushHandler.registeredChannels[MozLoopService.channelIDs.callsFxA];
-      roomsPushURL = this.pushHandler.registeredChannels[MozLoopService.channelIDs.roomsFxA];
-    } else if (sessionType == LOOP_SESSION_TYPE.GUEST) {
-      callsPushURL = this.pushHandler.registeredChannels[MozLoopService.channelIDs.callsGuest];
-      roomsPushURL = this.pushHandler.registeredChannels[MozLoopService.channelIDs.roomsGuest];
-    }
-
-    if (!callsPushURL || !roomsPushURL) {
-      return Promise.reject(new Error("Invalid sessionType or missing push URLs for registerWithLoopServer: " + sessionType));
-    }
-
-    // create a registration payload with a backwards compatible attribute (simplePushURL)
-    // that will register only the calls notification.
-    let msg = {
-        simplePushURL: callsPushURL,
-        simplePushURLs: {
-          calls: callsPushURL,
-          rooms: roomsPushURL,
-        },
-    };
-    return this.hawkRequestInternal(sessionType, "/registration", "POST", msg, false)
-      .then((response) => {
-        // If this failed we got an invalid token.
-        if (!this.storeSessionToken(sessionType, response.headers)) {
-          return Promise.reject(new Error("session-token-wrong-size"));
-        }
-
-        log.debug("Successfully registered with server for sessionType", sessionType);
-        this.clearError("registration");
-        return undefined;
-      }, (error) => {
-        // There's other errors than invalid auth token, but we should only do the reset
-        // as a last resort.
-        if (error.code === 401) {
-          // Authorization failed, invalid token, we need to try again with a new token.
-          if (retry) {
-            return this.registerWithLoopServer(sessionType, false);
-          }
-        }
-
-        log.error("Failed to register with the loop server. Error: ", error);
-        let deferred = Promise.defer();
-        deferred.promise.then(() => {
-          log.debug("registration retry succeeded");
-        },
-        error => {
-          log.debug("registration retry failed");
-        });
-        this.setError("registration", error, () => MozLoopService.delayedInitialize(deferred));
-        throw error;
-      }
-    );
-  },
-
-  /**
-   * Unregisters from the Loop server either as a guest or a FxA user.
-   *
-   * This is normally only wanted for FxA users as we normally want to keep the
-   * guest session with the device.
-   *
-   * NOTE: It is the responsibiliy of the caller the clear the session token
-   * after all of the notification classes: calls and rooms, for either
-   * Guest or FxA have been unregistered with the LoopServer.
-   *
-   * @param {LOOP_SESSION_TYPE} sessionType The type of session e.g. guest or FxA
-   * @param {String} pushURL The push URL previously given by the push server.
-   *                         This may not be necessary to unregister in the future.
-   * @return {Promise} resolving when the unregistration request finishes
-   */
-  unregisterFromLoopServer: function(sessionType, pushURL) {
-    let prefType = Services.prefs.getPrefType(this.getSessionTokenPrefName(sessionType));
-    if (prefType == Services.prefs.PREF_INVALID) {
-      return Promise.resolve("already unregistered");
-    }
-
-    let unregisterURL = "/registration?simplePushURL=" + encodeURIComponent(pushURL);
-    return this.hawkRequestInternal(sessionType, unregisterURL, "DELETE")
-      .then(() => {
-        log.debug("Successfully unregistered from server for sessionType", sessionType);
-      },
-      error => {
-        if (error.code === 401) {
-          // Authorization failed, invalid token. This is fine since it may mean we already logged out.
-          return;
-        }
-
-        log.error("Failed to unregister with the loop server. Error: ", error);
-        throw error;
-      });
-  },
-
-  /**
    * A getter to obtain and store the strings for loop. This is structured
    * for use by l10n.js.
    *
    * @returns {Map} a map of element ids with localized string values
    */
   get localizedStrings() {
     if (gLocalizedStrings.size)
       return gLocalizedStrings;
@@ -1286,31 +1279,16 @@ this.MozLoopService = {
   /**
    * Returns a new GUID (UUID) in curly braces format.
    */
   generateUUID: function() {
     return uuidgen.generateUUID().toString();
   },
 
   /**
-   * Returns a new non-global id
-   *
-   * @param {Function} notUnique [optional] This function will be
-   *                   applied to test the generated id for uniqueness
-   *                   in the callers domain.
-   */
-  generateLocalID: function(notUnique = ((id) => {return false})) {
-    do {
-      var id = Date.now().toString(36) + Math.floor((Math.random() * 4096)).toString(16);
-    }
-    while (notUnique(id));
-    return id;
-  },
-
-  /**
    * Retrieves MozLoopService "do not disturb" value.
    *
    * @return {Boolean}
    */
   get doNotDisturb() {
     return MozLoopServiceInternal.doNotDisturb;
   },
 
@@ -1489,34 +1467,31 @@ this.MozLoopService = {
    * Logs the user out from FxA.
    *
    * Gracefully handles if the user is already logged out.
    *
    * @return {Promise} that resolves when the FxA logout flow is complete.
    */
   logOutFromFxA: Task.async(function*() {
     log.debug("logOutFromFxA");
-    let pushHandler = MozLoopServiceInternal.pushHandler;
-    let callsPushUrl = pushHandler.registeredChannels[MozLoopService.channelIDs.callsFxA];
-    let roomsPushUrl = pushHandler.registeredChannels[MozLoopService.channelIDs.roomsFxA];
     try {
-      if (callsPushUrl) {
-        yield MozLoopServiceInternal.unregisterFromLoopServer(LOOP_SESSION_TYPE.FXA, callsPushUrl);
-      }
-      if (roomsPushUrl) {
-        yield MozLoopServiceInternal.unregisterFromLoopServer(LOOP_SESSION_TYPE.FXA, roomsPushUrl);
-      }
-    } catch (error) {
-      throw error;
-    } finally {
+      yield MozLoopServiceInternal.unregisterFromLoopServer(LOOP_SESSION_TYPE.FXA);
+    }
+    catch (err) {
+      throw err
+    }
+    finally {
       MozLoopServiceInternal.clearSessionToken(LOOP_SESSION_TYPE.FXA);
-
       MozLoopServiceInternal.fxAOAuthTokenData = null;
       MozLoopServiceInternal.fxAOAuthProfile = null;
       MozLoopServiceInternal.deferredRegistrations.delete(LOOP_SESSION_TYPE.FXA);
+      // Unregister with PushHandler so these push channels will not get re-registered
+      // if the connection is re-established by the PushHandler.
+      MozLoopServiceInternal.pushHandler.unregister(MozLoopService.channelIDs.callsFxA);
+      MozLoopServiceInternal.pushHandler.unregister(MozLoopService.channelIDs.roomsFxA);
 
       // Reset the client since the initial promiseFxAOAuthParameters() call is
       // what creates a new session.
       gFxAOAuthClient = null;
       gFxAOAuthClientPromise = null;
 
       // clearError calls notifyStatusChanged so should be done last when the
       // state is clean.
--- a/browser/components/loop/test/mochitest/browser_fxa_login.js
+++ b/browser/components/loop/test/mochitest/browser_fxa_login.js
@@ -37,26 +37,22 @@ function* checkFxA401() {
   loopPanel.hidePopup();
 }
 
 add_task(function* setup() {
   Services.prefs.setBoolPref("loop.gettingStarted.seen", true);
   MozLoopServiceInternal.mocks.pushHandler = mockPushHandler;
   // Normally the same pushUrl would be registered but we change it in the test
   // to be able to check for success on the second registration.
-  mockPushHandler.registeredChannels[MozLoopService.channelIDs.callsFxA] = "https://localhost/pushUrl/fxa-calls";
-  mockPushHandler.registeredChannels[MozLoopService.channelIDs.roomsFxA] = "https://localhost/pushUrl/fxa-rooms";
 
   registerCleanupFunction(function* () {
     info("cleanup time");
     yield promiseDeletedOAuthParams(BASE_URL);
     Services.prefs.clearUserPref("loop.gettingStarted.seen");
     MozLoopServiceInternal.mocks.pushHandler = undefined;
-    delete mockPushHandler.registeredChannels[MozLoopService.channelIDs.callsFxA];
-    delete mockPushHandler.registeredChannels[MozLoopService.channelIDs.roomsFxA];
 
     yield resetFxA();
     Services.prefs.clearUserPref(MozLoopServiceInternal.getSessionTokenPrefName(LOOP_SESSION_TYPE.GUEST));
   });
 });
 
 add_task(function* checkOAuthParams() {
   let params = {
@@ -164,17 +160,16 @@ add_task(function* invalidState() {
   yield loginPromise.catch((error) => {
     ok(error, "The login promise should be rejected due to invalid state");
   });
 });
 
 add_task(function* basicRegistrationWithoutSession() {
   yield resetFxA();
   yield promiseDeletedOAuthParams(BASE_URL);
-
   let caught = false;
   yield MozLoopServiceInternal.promiseFxAOAuthToken("code1", "state").catch((error) => {
     caught = true;
     is(error.code, 401, "Should have returned a 401");
   });
   ok(caught, "Should have caught the error requesting /token without a hawk session");
   yield checkFxA401();
 });
@@ -359,51 +354,44 @@ add_task(function* loginWithParams401() 
   });
 
   yield checkFxA401();
 });
 
 add_task(function* logoutWithIncorrectPushURL() {
   yield resetFxA();
   let pushURL = "http://www.example.com/";
-  mockPushHandler.registeredChannels[MozLoopService.channelIDs.callsFxA] = pushURL;
-  mockPushHandler.registeredChannels[MozLoopService.channelIDs.roomsFxA] = pushURL;
-
   // Create a fake FxA hawk session token
   const fxASessionPref = MozLoopServiceInternal.getSessionTokenPrefName(LOOP_SESSION_TYPE.FXA);
   Services.prefs.setCharPref(fxASessionPref, "X".repeat(HAWK_TOKEN_LENGTH));
-
-  yield MozLoopServiceInternal.registerWithLoopServer(LOOP_SESSION_TYPE.FXA);
+  yield MozLoopServiceInternal.registerWithLoopServer(LOOP_SESSION_TYPE.FXA, "calls", pushURL);
   let registrationResponse = yield promiseOAuthGetRegistration(BASE_URL);
   ise(registrationResponse.response.simplePushURLs.calls, pushURL, "Check registered push URL");
-  mockPushHandler.registeredChannels[MozLoopService.channelIDs.callsFxA] = "http://www.example.com/invalid";
+  MozLoopServiceInternal.pushURLs.get(LOOP_SESSION_TYPE.FXA).calls = "http://www.example.com/invalid";
   let caught = false;
   yield MozLoopService.logOutFromFxA().catch((error) => {
     caught = true;
   });
   ok(caught, "Should have caught an error logging out with a mismatched push URL");
   checkLoggedOutState();
   registrationResponse = yield promiseOAuthGetRegistration(BASE_URL);
   ise(registrationResponse.response.simplePushURLs.calls, pushURL, "Check registered push URL wasn't deleted");
 });
 
 add_task(function* logoutWithNoPushURL() {
   yield resetFxA();
   let pushURL = "http://www.example.com/";
-  mockPushHandler.registeredChannels[MozLoopService.channelIDs.callsFxA] = pushURL;
-
   // Create a fake FxA hawk session token
   const fxASessionPref = MozLoopServiceInternal.getSessionTokenPrefName(LOOP_SESSION_TYPE.FXA);
   Services.prefs.setCharPref(fxASessionPref, "X".repeat(HAWK_TOKEN_LENGTH));
 
-  yield MozLoopServiceInternal.registerWithLoopServer(LOOP_SESSION_TYPE.FXA);
+  yield MozLoopServiceInternal.registerWithLoopServer(LOOP_SESSION_TYPE.FXA, "calls", pushURL);
   let registrationResponse = yield promiseOAuthGetRegistration(BASE_URL);
   ise(registrationResponse.response.simplePushURLs.calls, pushURL, "Check registered push URL");
-  mockPushHandler.registeredChannels[MozLoopService.channelIDs.callsFxA] = null;
-  mockPushHandler.registeredChannels[MozLoopService.channelIDs.roomsFxA] = null;
+  MozLoopServiceInternal.pushURLs.delete(LOOP_SESSION_TYPE.FXA);
   yield MozLoopService.logOutFromFxA();
   checkLoggedOutState();
   registrationResponse = yield promiseOAuthGetRegistration(BASE_URL);
   ise(registrationResponse.response.simplePushURLs.calls, pushURL, "Check registered push URL wasn't deleted");
 });
 
 add_task(function* loginWithRegistration401() {
   yield resetFxA();
--- a/browser/components/loop/test/mochitest/head.js
+++ b/browser/components/loop/test/mochitest/head.js
@@ -9,16 +9,17 @@ const {
 const {LoopCalls} = Cu.import("resource:///modules/loop/LoopCalls.jsm", {});
 const {LoopRooms} = Cu.import("resource:///modules/loop/LoopRooms.jsm", {});
 
 // Cache this value only once, at the beginning of a
 // test run, so that it doesn't pick up the offline=true
 // if offline mode is requested multiple times in a test run.
 const WAS_OFFLINE = Services.io.offline;
 
+
 var gMozLoopAPI;
 
 function promiseGetMozLoopAPI() {
   return new Promise((resolve, reject) => {
     let loopPanel = document.getElementById("loop-notification-panel");
     let btn = document.getElementById("loop-button");
 
     // Wait for the popup to be shown if it's not already, then we can get the iframe and
@@ -109,17 +110,16 @@ function promiseOAuthParamsSetup(baseURL
   });
 }
 
 function* resetFxA() {
   let global = Cu.import("resource:///modules/loop/MozLoopService.jsm", {});
   global.gHawkClient = null;
   global.gFxAOAuthClientPromise = null;
   global.gFxAOAuthClient = null;
-  MozLoopServiceInternal.deferredRegistrations.delete(LOOP_SESSION_TYPE.FXA);
   MozLoopServiceInternal.fxAOAuthProfile = null;
   MozLoopServiceInternal.fxAOAuthTokenData = null;
   const fxASessionPref = MozLoopServiceInternal.getSessionTokenPrefName(LOOP_SESSION_TYPE.FXA);
   Services.prefs.clearUserPref(fxASessionPref);
   MozLoopService.errors.clear();
   let notified = promiseObserverNotified("loop-status-changed");
   MozLoopServiceInternal.notifyStatusChanged();
   yield notified;
@@ -184,33 +184,45 @@ function getLoopString(stringID) {
  * This is used to fake push registration and notifications for
  * MozLoopService tests. There is only one object created per test instance, as
  * once registration has taken place, the object cannot currently be changed.
  */
 let mockPushHandler = {
   // This sets the registration result to be returned when initialize
   // is called. By default, it is equivalent to success.
   registrationResult: null,
-  registrationPushURL: null,
+  registrationPushURLs: {},
   notificationCallback: {},
   registeredChannels: {},
 
   /**
    * MozLoopPushHandler API
    */
   initialize: function(options = {}) {
     if ("mockWebSocket" in options) {
       this._mockWebSocket = options.mockWebSocket;
     }
+    this.registrationPushURLs[MozLoopService.channelIDs.callsGuest] =
+      "https://localhost/pushUrl/guest-calls";
+    this.registrationPushURLs[MozLoopService.channelIDs.roomsGuest] =
+      "https://localhost/pushUrl/guest-rooms";
+    this.registrationPushURLs[MozLoopService.channelIDs.callsFxA] =
+      "https://localhost/pushUrl/fxa-calls";
+    this.registrationPushURLs[MozLoopService.channelIDs.roomsFxA] =
+      "https://localhost/pushUrl/fxa-rooms";
   },
 
   register: function(channelId, registerCallback, notificationCallback) {
     this.notificationCallback[channelId] = notificationCallback;
-    this.registeredChannels[channelId] = this.registrationPushURL;
-    setTimeout(registerCallback(this.registrationResult, this.registrationPushURL, channelId), 0);
+    this.registeredChannels[channelId] = this.registrationPushURLs[channelId];
+    setTimeout(registerCallback(this.registrationResult, this.registeredChannels[channelId], channelId), 0);
+  },
+
+  unregister: function(channelID) {
+    return;
   },
 
   /**
    * Test-only API to simplify notifying a push notification result.
    */
   notify: function(version, chanId) {
     this.notificationCallback[chanId](version, chanId);
   }
--- a/browser/components/loop/test/xpcshell/head.js
+++ b/browser/components/loop/test/xpcshell/head.js
@@ -97,16 +97,20 @@ let mockPushHandler = {
   },
 
   register: function(channelId, registerCallback, notificationCallback) {
     this.notificationCallback[channelId] = notificationCallback;
     this.registeredChannels[channelId] = this.registrationPushURL;
     registerCallback(this.registrationResult, this.registrationPushURL, channelId);
   },
 
+  unregister: function(channelID) {
+    return;
+  },
+
   /**
    * Test-only API to simplify notifying a push notification result.
    */
   notify: function(version, chanId) {
     this.notificationCallback[chanId](version, chanId);
   }
 };
 
--- a/browser/components/loop/test/xpcshell/test_looppush_initialize.js
+++ b/browser/components/loop/test/xpcshell/test_looppush_initialize.js
@@ -63,29 +63,25 @@
                      "Should have the origin url from preferences");
         Assert.equal(mockWebSocket.protocol, "push-notification",
                      "Should have the protocol set to push-notifications");
 
         // Register again for the same channel
         MozLoopPushHandler.register(
           "chan-2",
           function(err, url, id) {
-            Assert.notEqual(err, null, "Should have returned an error");
-            // Notify the first registration to make sure that still works.
-            mockWebSocket.notify(16);
+            Assert.equal(err, null, "Should return null for success");
+            Assert.equal(id, "chan-2", "Should have channel id = chan-2");
+            run_next_test();
           },
-          function(version, id) {
-            Assert.ok(false, "The 2nd onNotification callback shouldn't be called");
-        });
+          dummyCallback
+        );
       },
-      function(version, id) {
-        Assert.equal(version, 16, "Should have version number 16");
-        Assert.equal(id, "chan-2", "Should have channel id = chan-2");
-        run_next_test();
-      });
+      dummyCallback
+    );
   });
 
   // Test that the PushHander will re-connect after the near-end disconnect.
   // The uaID is cleared to force re-registration of all notification channels.
   add_test(function test_reconnect_websocket() {
     MozLoopPushHandler.uaID = undefined;
     mockWebSocket.stop();
     // Previously registered onRegistration callbacks will fire and be checked (see above).
@@ -117,19 +113,18 @@
       function(err, url, id) {
         Assert.equal(++regCnt, 1, "onRegistered should only be called once");
         Assert.equal(err, null, "err should be null to indicate success");
         Assert.equal(url, kEndPointUrl, "Should return push server application URL");
         Assert.equal(id, "test-chan", "Should have channel id = test-chan");
         mockWebSocket.stop();
         setTimeout(run_next_test(), 0);
       },
-      function(version, id) {
-        return;
-      });
+      dummyCallback
+    );
   });
 
   add_test(function test_ping_websocket() {
     let pingReceived = false,
         socketClosed = false;
     mockWebSocket.defaultMsgHandler = (msg) => {
       pingReceived = true;
       // Do not send a ping response.
@@ -149,19 +144,18 @@
             run_next_test();
           }, () => {
             do_throw("should have closed the websocket");
           });
         }, () => {
           do_throw("should have sent ping");
         });
       },
-      function(version) {
-        return;
-      });
+      dummyCallback
+    );
   });
 
   add_test(function test_retry_pushurl() {
     MozLoopPushHandler.shutdown();
     loopServer.registerPathHandler("/push-server-config", (request, response) => {
       // The PushHandler should retry the request for the push-server-config for
       // each of these cases without throwing an error.
       let n = 0;
--- a/browser/components/loop/test/xpcshell/test_loopservice_registration.js
+++ b/browser/components/loop/test/xpcshell/test_loopservice_registration.js
@@ -35,20 +35,24 @@ add_test(function test_register_websocke
 
 add_test(function test_register_success() {
   mockPushHandler.registrationPushURL = kEndPointUrl;
   mockPushHandler.registrationResult = null;
 
   loopServer.registerPathHandler("/registration", (request, response) => {
     let body = CommonUtils.readBytesFromInputStream(request.bodyInputStream);
     let data = JSON.parse(body);
-    Assert.equal(data.simplePushURLs.calls, kEndPointUrl,
-                 "Should send correct calls push url");
-    Assert.equal(data.simplePushURLs.rooms, kEndPointUrl,
-                 "Should send correct rooms push url");
+    if (data.simplePushURLs.calls) {
+      Assert.equal(data.simplePushURLs.calls, kEndPointUrl,
+                   "Should send correct calls push url");
+    }
+    if (data.simplePushURLs.rooms) {
+      Assert.equal(data.simplePushURLs.rooms, kEndPointUrl,
+                   "Should send correct rooms push url");
+    }
 
     response.setStatusLine(null, 200, "OK");
     response.processAsync();
     response.finish();
   });
   MozLoopService.promiseRegisteredWithServers().then(() => {
     run_next_test();
   }, err => {
--- a/browser/components/loop/test/xpcshell/test_loopservice_registration_retry.js
+++ b/browser/components/loop/test/xpcshell/test_loopservice_registration_retry.js
@@ -25,37 +25,41 @@ add_test(function test_retry_after_faile
 
     // Remove the error
     mockPushHandler.registrationResult = null;
     mockPushHandler.registrationPushURL = kEndPointUrl;
 
     yield regError.friendlyDetailsButtonCallback();
     Assert.strictEqual(MozLoopService.errors.size, 0, "Check that the errors are gone");
     let deferredRegistrations = MozLoopServiceInternal.deferredRegistrations;
-    yield deferredRegistrations.get(LOOP_SESSION_TYPE.GUEST).promise.then(() => {
+    yield deferredRegistrations.get(LOOP_SESSION_TYPE.GUEST).then(() => {
       Assert.ok(true, "The retry of registration succeeded");
     },
     (error) => {
       Assert.ok(false, "The retry of registration should have succeeded");
     });
 
     run_next_test();
   }));
 });
 
 function run_test() {
   setupFakeLoopServer();
 
   loopServer.registerPathHandler("/registration", (request, response) => {
     let body = CommonUtils.readBytesFromInputStream(request.bodyInputStream);
     let data = JSON.parse(body);
-    Assert.equal(data.simplePushURLs.calls, kEndPointUrl,
-                 "Should send correct calls push url");
-    Assert.equal(data.simplePushURLs.rooms, kEndPointUrl,
-                 "Should send correct rooms push url");
+    if (data.simplePushURLs.calls) {
+      Assert.equal(data.simplePushURLs.calls, kEndPointUrl,
+                   "Should send correct calls push url");
+    }
+    if (data.simplePushURLs.rooms) {
+      Assert.equal(data.simplePushURLs.rooms, kEndPointUrl,
+                   "Should send correct rooms push url");
+    }
 
     response.setStatusLine(null, 200, "OK");
   });
 
   let nowSeconds = Date.now() / 1000;
   Services.prefs.setIntPref("loop.urlsExpiryTimeSeconds", nowSeconds + 60);
 
   do_register_cleanup(function() {
--- a/browser/components/loop/test/xpcshell/test_loopservice_token_invalid.js
+++ b/browser/components/loop/test/xpcshell/test_loopservice_token_invalid.js
@@ -29,17 +29,17 @@ add_test(function test_registration_inva
     }
     response.processAsync();
     response.finish();
   });
 
   MozLoopService.promiseRegisteredWithServers().then(() => {
     // Due to the way the time stamp checking code works in hawkclient, we expect a couple
     // of authorization requests before we reset the token.
-    Assert.equal(authorizationAttempts, 2);
+    Assert.equal(authorizationAttempts, 4); //hawk will repeat each registration attemtp twice: calls and rooms.
     Assert.equal(Services.prefs.getCharPref(LOOP_HAWK_PREF), fakeSessionToken2);
     run_next_test();
   }, err => {
     do_throw("shouldn't be a failure result: " + err);
   });
 });