Bug 1095379 - Separate push registrations by sessionType and prevent calling promiseRegisteredWithPushServer from outside of promiseRegisteredWithServers. r=Standard8 a=loop-only
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Thu, 13 Nov 2014 18:59:04 +0100
changeset 233928 37a5d049e06984ebc3a1b8a7288f803c545350e8
parent 233927 a71102abcd349230e76b8b6d7ac61394e6900989
child 233929 b92ec32f2344acd12bb9ec4d2580ff5251599a4a
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8, loop-only
bugs1095379
milestone35.0a2
Bug 1095379 - Separate push registrations by sessionType and prevent calling promiseRegisteredWithPushServer from outside of promiseRegisteredWithServers. r=Standard8 a=loop-only
browser/components/loop/LoopRooms.jsm
browser/components/loop/MozLoopService.jsm
browser/components/loop/test/xpcshell/test_loopservice_busy.js
--- a/browser/components/loop/LoopRooms.jsm
+++ b/browser/components/loop/LoopRooms.jsm
@@ -126,17 +126,19 @@ let LoopRoomsInternal = {
    */
   getAll: function(version = null, callback) {
     if (!callback) {
       callback = version;
       version = null;
     }
 
     Task.spawn(function* () {
-      yield MozLoopService.promiseRegisteredWithServers();
+      let deferredInitialization = Promise.defer();
+      MozLoopService.delayedInitialize(deferredInitialization);
+      yield deferredInitialization.promise;
 
       if (!gDirty) {
         callback(null, [...this.rooms.values()]);
         return;
       }
 
       // Fetch the rooms from the server.
       let url = "/rooms" + (version ? "?version=" + encodeURIComponent(version) : "");
--- a/browser/components/loop/MozLoopService.jsm
+++ b/browser/components/loop/MozLoopService.jsm
@@ -318,22 +318,27 @@ let MozLoopServiceInternal = {
   },
 
   get errors() {
     return gErrors;
   },
 
   /**
    * Get endpoints with the push server and register for notifications.
-   * For now we register as both a Guest and FxA user and all must succeed.
+   * This should only be called from promiseRegisteredWithServers to prevent reentrancy.
    *
+   * @param {LOOP_SESSION_TYPE} sessionType
    * @return {Promise} resolves with all push endpoints
    *                   rejects if any of the push registrations failed
    */
-  promiseRegisteredWithPushServer: function() {
+  promiseRegisteredWithPushServer: function(sessionType) {
+    if (!this.deferredRegistrations.has(sessionType)) {
+      return Promise.reject("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));
@@ -352,29 +357,33 @@ let MozLoopServiceInternal = {
 
         MozLoopServiceInternal.pushHandler.register(channelID, onRegistered, onNotification);
       });
     }
 
     let options = this.mocks.webSocket ? { mockWebSocket: this.mocks.webSocket } : {};
     this.pushHandler.initialize(options);
 
-    let callsRegGuest = registerForNotification(MozLoopService.channelIDs.callsGuest,
+    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 roomsRegGuest = registerForNotification(MozLoopService.channelIDs.roomsGuest,
+      let roomsRegFxA = registerForNotification(MozLoopService.channelIDs.roomsFxA,
                                                 roomsPushNotification);
+      return Promise.all([callsRegFxA, roomsRegFxA]);
+    }
 
-    let callsRegFxA = registerForNotification(MozLoopService.channelIDs.callsFxA,
-                                              LoopCalls.onNotification);
-
-    let roomsRegFxA = registerForNotification(MozLoopService.channelIDs.roomsFxA,
-                                              roomsPushNotification);
-
-    return Promise.all([callsRegGuest, roomsRegGuest, callsRegFxA, roomsRegFxA]);
+    return Promise.reject("promiseRegisteredWithPushServer: Invalid sessionType");
   },
 
   /**
    * Starts registration of Loop with the push server, and then will register
    * with the Loop server. It will return early if already registered.
    *
    * @param {LOOP_SESSION_TYPE} sessionType
    * @returns {Promise} a promise that is resolved with no params on completion, or
@@ -389,17 +398,17 @@ let MozLoopServiceInternal = {
     let result = null;
     let deferred = Promise.defer();
     log.debug("assigning to deferredRegistrations for sessionType:", sessionType);
     this.deferredRegistrations.set(sessionType, deferred);
 
     // We grab the promise early in case one of the callers below delete it from the map.
     result = deferred.promise;
 
-    this.promiseRegisteredWithPushServer().then(() => {
+    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);
--- a/browser/components/loop/test/xpcshell/test_loopservice_busy.js
+++ b/browser/components/loop/test/xpcshell/test_loopservice_busy.js
@@ -2,131 +2,126 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const { LoopCallsInternal } = Cu.import("resource:///modules/loop/LoopCalls.jsm", {});
 
 XPCOMUtils.defineLazyModuleGetter(this, "Chat",
                                   "resource:///modules/Chat.jsm");
 
+let actionReceived = false;
 let openChatOrig = Chat.open;
 
 const firstCallId = 4444333221;
 const secondCallId = 1001100101;
 
 let msgHandler = function(msg) {
   if (msg.messageType &&
       msg.messageType === "action" &&
       msg.event === "terminate" &&
       msg.reason === "busy") {
     actionReceived = true;
   }
 };
 
-add_test(function test_busy_2guest_calls() {
+add_task(function* test_busy_2guest_calls() {
   actionReceived = false;
 
   mockPushHandler.registrationPushURL = kEndPointUrl;
 
-  MozLoopService.promiseRegisteredWithServers().then(() => {
-    let opened = 0;
-    let windowId;
-    Chat.open = function(contentWindow, origin, title, url) {
-      opened++;
-      windowId = url.match(/about:loopconversation\#(\d+)$/)[1];
-    };
+  yield MozLoopService.promiseRegisteredWithServers(LOOP_SESSION_TYPE.GUEST);
 
-    mockPushHandler.notify(1, MozLoopService.channelIDs.callsGuest);
+  let opened = 0;
+  let windowId;
+  Chat.open = function(contentWindow, origin, title, url) {
+    opened++;
+    windowId = url.match(/about:loopconversation\#(\d+)$/)[1];
+  };
 
-    waitForCondition(() => {return actionReceived && opened > 0}).then(() => {
-      do_check_true(opened === 1, "should open only one chat window");
-      do_check_true(actionReceived, "should respond with busy/reject to second call");
-      LoopCalls.clearCallInProgress(windowId);
-      run_next_test();
-    }, () => {
-      do_throw("should have opened a chat window for first call and rejected second call");
-    });
+  mockPushHandler.notify(1, MozLoopService.channelIDs.callsGuest);
 
+  yield waitForCondition(() => { return actionReceived && opened > 0; }).then(() => {
+    do_check_true(opened === 1, "should open only one chat window");
+    do_check_true(actionReceived, "should respond with busy/reject to second call");
+    LoopCalls.clearCallInProgress(windowId);
+  }, () => {
+    do_throw("should have opened a chat window for first call and rejected second call");
   });
 });
 
-add_test(function test_busy_1fxa_1guest_calls() {
+add_task(function* test_busy_1fxa_1guest_calls() {
   actionReceived = false;
 
-  MozLoopService.promiseRegisteredWithServers().then(() => {
-    let opened = 0;
-    let windowId;
-    Chat.open = function(contentWindow, origin, title, url) {
-      opened++;
-      windowId = url.match(/about:loopconversation\#(\d+)$/)[1];
-    };
+  yield MozLoopService.promiseRegisteredWithServers(LOOP_SESSION_TYPE.GUEST);
+  yield MozLoopService.promiseRegisteredWithServers(LOOP_SESSION_TYPE.FXA);
+
+  let opened = 0;
+  let windowId;
+  Chat.open = function(contentWindow, origin, title, url) {
+    opened++;
+    windowId = url.match(/about:loopconversation\#(\d+)$/)[1];
+  };
 
-    mockPushHandler.notify(1, MozLoopService.channelIDs.callsFxA);
-    mockPushHandler.notify(1, MozLoopService.channelIDs.callsGuest);
+  mockPushHandler.notify(1, MozLoopService.channelIDs.callsFxA);
+  mockPushHandler.notify(1, MozLoopService.channelIDs.callsGuest);
 
-    waitForCondition(() => {return actionReceived && opened > 0}).then(() => {
-      do_check_true(opened === 1, "should open only one chat window");
-      do_check_true(actionReceived, "should respond with busy/reject to second call");
-      LoopCalls.clearCallInProgress(windowId);
-      run_next_test();
-    }, () => {
-      do_throw("should have opened a chat window for first call and rejected second call");
-    });
-
+  yield waitForCondition(() => { return actionReceived && opened > 0; }).then(() => {
+    do_check_true(opened === 1, "should open only one chat window");
+    do_check_true(actionReceived, "should respond with busy/reject to second call");
+    LoopCalls.clearCallInProgress(windowId);
+  }, () => {
+    do_throw("should have opened a chat window for first call and rejected second call");
   });
 });
 
-add_test(function test_busy_2fxa_calls() {
+add_task(function* test_busy_2fxa_calls() {
   actionReceived = false;
 
-  MozLoopService.promiseRegisteredWithServers().then(() => {
-    let opened = 0;
-    let windowId;
-    Chat.open = function(contentWindow, origin, title, url) {
-      opened++;
-      windowId = url.match(/about:loopconversation\#(\d+)$/)[1];
-    };
+  yield MozLoopService.promiseRegisteredWithServers(LOOP_SESSION_TYPE.FXA);
 
-    mockPushHandler.notify(1, MozLoopService.channelIDs.callsFxA);
+  let opened = 0;
+  let windowId;
+  Chat.open = function(contentWindow, origin, title, url) {
+    opened++;
+    windowId = url.match(/about:loopconversation\#(\d+)$/)[1];
+  };
 
-    waitForCondition(() => {return actionReceived && opened > 0}).then(() => {
-      do_check_true(opened === 1, "should open only one chat window");
-      do_check_true(actionReceived, "should respond with busy/reject to second call");
-      LoopCalls.clearCallInProgress(windowId);
-      run_next_test();
-    }, () => {
-      do_throw("should have opened a chat window for first call and rejected second call");
-    });
+  mockPushHandler.notify(1, MozLoopService.channelIDs.callsFxA);
 
+  yield waitForCondition(() => { return actionReceived && opened > 0; }).then(() => {
+    do_check_true(opened === 1, "should open only one chat window");
+    do_check_true(actionReceived, "should respond with busy/reject to second call");
+    LoopCalls.clearCallInProgress(windowId);
+  }, () => {
+    do_throw("should have opened a chat window for first call and rejected second call");
   });
 });
 
-add_test(function test_busy_1guest_1fxa_calls() {
+add_task(function* test_busy_1guest_1fxa_calls() {
   actionReceived = false;
 
-  MozLoopService.promiseRegisteredWithServers().then(() => {
-    let opened = 0;
-    let windowId;
-    Chat.open = function(contentWindow, origin, title, url) {
-      opened++;
-      windowId = url.match(/about:loopconversation\#(\d+)$/)[1];
-    };
+  yield MozLoopService.promiseRegisteredWithServers(LOOP_SESSION_TYPE.GUEST);
+  yield MozLoopService.promiseRegisteredWithServers(LOOP_SESSION_TYPE.FXA);
+
+  let opened = 0;
+  let windowId;
+  Chat.open = function(contentWindow, origin, title, url) {
+    opened++;
+    windowId = url.match(/about:loopconversation\#(\d+)$/)[1];
+  };
 
-    mockPushHandler.notify(1, MozLoopService.channelIDs.callsGuest);
-    mockPushHandler.notify(1, MozLoopService.channelIDs.callsFxA);
+  mockPushHandler.notify(1, MozLoopService.channelIDs.callsGuest);
+  mockPushHandler.notify(1, MozLoopService.channelIDs.callsFxA);
 
-    waitForCondition(() => {return actionReceived && opened > 0}).then(() => {
-      do_check_true(opened === 1, "should open only one chat window");
-      do_check_true(actionReceived, "should respond with busy/reject to second call");
-      LoopCalls.clearCallInProgress(windowId);
-      run_next_test();
-    }, () => {
-      do_throw("should have opened a chat window for first call and rejected second call");
-    });
-
+  yield waitForCondition(() => { return actionReceived && opened > 0; }).then(() => {
+    do_check_true(opened === 1, "should open only one chat window");
+    do_check_true(actionReceived, "should respond with busy/reject to second call");
+    LoopCalls.clearCallInProgress(windowId);
+  }, () => {
+    do_throw("should have opened a chat window for first call and rejected second call");
   });
 });
 
 function run_test() {
   setupFakeLoopServer();
 
   // Setup fake login state so we get FxA requests.
   const MozLoopServiceInternal = Cu.import("resource:///modules/loop/MozLoopService.jsm", {}).MozLoopServiceInternal;