Bug 1157712 - Fix multiple OS X notifications when someone joins a Loop room. Ensure we don't attempt to re-initialise twice. r=mikedeboer, a=sledru
authorMark Banner <standard8@mozilla.com>
Fri, 22 May 2015 14:45:13 +0100
changeset 266113 928e51389b65
parent 266112 9d2ba69b0ff3
child 266114 2fe463b29e20
push id4758
push userryanvm@gmail.com
push date2015-05-26 18:35 +0000
treeherdermozilla-beta@928e51389b65 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer, sledru
bugs1157712
milestone39.0
Bug 1157712 - Fix multiple OS X notifications when someone joins a Loop room. Ensure we don't attempt to re-initialise twice. r=mikedeboer, a=sledru
browser/base/content/browser-loop.js
browser/components/loop/MozLoopService.jsm
browser/components/loop/test/xpcshell/test_loopservice_initialize.js
browser/components/loop/test/xpcshell/test_loopservice_restart.js
--- a/browser/base/content/browser-loop.js
+++ b/browser/base/content/browser-loop.js
@@ -218,17 +218,25 @@ XPCOMUtils.defineLazyModuleGetter(this, 
     /**
      * Triggers the initialization of the loop service.  Called by
      * delayedStartup.
      */
     init: function() {
       // Add observer notifications before the service is initialized
       Services.obs.addObserver(this, "loop-status-changed", false);
 
-      MozLoopService.initialize();
+      // This is a promise for test purposes, but we don't want to be logging
+      // expected errors to the console, so we catch them here.
+      MozLoopService.initialize().catch(ex => {
+        if (!ex.message ||
+            (!ex.message.contains("not enabled") &&
+             !ex.message.contains("not needed"))) {
+          console.error(ex);
+        }
+      });
       this.updateToolbarState();
     },
 
     uninit: function() {
       Services.obs.removeObserver(this, "loop-status-changed");
     },
 
     // Implements nsIObserver
--- a/browser/components/loop/MozLoopService.jsm
+++ b/browser/components/loop/MozLoopService.jsm
@@ -1047,16 +1047,17 @@ let gInitializeTimerFunc = (deferredInit
   // Kick off the push notification service into registering after a timeout.
   // This ensures we're not doing too much straight after the browser's finished
   // starting up.
 
   setTimeout(MozLoopService.delayedInitialize.bind(MozLoopService, deferredInitialization),
              MozLoopServiceInternal.initialRegistrationDelayMilliseconds);
 };
 
+let gServiceInitialized = false;
 
 /**
  * Public API
  */
 this.MozLoopService = {
   _DNSService: gDNSService,
   _activeScreenShares: [],
 
@@ -1064,31 +1065,51 @@ this.MozLoopService = {
     // Channel ids that will be registered with the PushServer for notifications
     return {
       callsFxA: "25389583-921f-4169-a426-a4673658944b",
       roomsFxA: "6add272a-d316-477c-8335-f00f73dfde71",
       roomsGuest: "19d3f799-a8f3-4328-9822-b7cd02765832",
     };
   },
 
+  /**
+   * Used to override the initalize timer function for test purposes.
+   */
   set initializeTimerFunc(value) {
     gInitializeTimerFunc = value;
   },
 
+  /**
+   * Used to reset if the service has been initialized or not - for test
+   * purposes.
+   */
+  resetServiceInitialized: function() {
+    gServiceInitialized = false;
+  },
+
   get roomsParticipantsCount() {
     return LoopRooms.participantsCount;
   },
 
   /**
    * Initialized the loop service, and starts registration with the
    * push and loop servers.
    *
+   * Note: this returns a promise for unit test purposes.
+   *
    * @return {Promise}
    */
   initialize: Task.async(function*() {
+    // Ensure we don't setup things like listeners more than once.
+    if (gServiceInitialized) {
+      return Promise.resolve();
+    }
+
+    gServiceInitialized = true;
+
     // Do this here, rather than immediately after definition, so that we can
     // stub out API functions for unit testing
     Object.freeze(this);
 
     // Don't do anything if loop is not enabled.
     if (!Services.prefs.getBoolPref("loop.enabled")) {
       return Promise.reject(new Error("loop is not enabled"));
     }
--- a/browser/components/loop/test/xpcshell/test_loopservice_initialize.js
+++ b/browser/components/loop/test/xpcshell/test_loopservice_initialize.js
@@ -33,21 +33,29 @@ add_task(function test_initialize_no_gue
 
 /**
  * Tests that registration happens when the expiry time is in
  * the future.
  */
 add_task(function test_initialize_with_guest_rooms() {
   Services.prefs.setBoolPref("loop.createdRoom", true);
   startTimerCalled = false;
+  MozLoopService.resetServiceInitialized();
 
   MozLoopService.initialize();
 
   Assert.equal(startTimerCalled, true,
     "should start the timer when guest rooms have been created");
+
+  startTimerCalled = false;
+
+  MozLoopService.initialize();
+
+  Assert.equal(startTimerCalled, false,
+    "should not have initialized a second time");
 });
 
 function run_test() {
   setupFakeLoopServer();
 
   // Override MozLoopService's initializeTimer, so that we can verify the timeout is called
   // correctly.
   MozLoopService.initializeTimerFunc = function() {
--- a/browser/components/loop/test/xpcshell/test_loopservice_restart.js
+++ b/browser/components/loop/test/xpcshell/test_loopservice_restart.js
@@ -32,32 +32,34 @@ add_task(function test_initialize_with_n
   }, (error) => {
     Assert.ok(false, error, "should have resolved the promise that initialize returned");
   });
 });
 
 add_task(function test_initialize_with_created_room_and_no_auth_token() {
   Services.prefs.setBoolPref(LOOP_CREATED_ROOM_PREF, true);
   Services.prefs.clearUserPref(LOOP_FXA_TOKEN_PREF);
+  MozLoopService.resetServiceInitialized();
 
   loopServer.registerPathHandler("/registration", (request, response) => {
     response.setStatusLine(null, 200, "OK");
   });
 
   yield MozLoopService.initialize().then((msg) => {
     Assert.equal(msg, "initialized without FxA status", "Initialize should register as a " +
                                                      "guest when no auth tokens but expired URLs");
   }, (error) => {
     Assert.ok(false, error, "should have resolved the promise that initialize returned");
   });
 });
 
 add_task(function test_initialize_with_invalid_fxa_token() {
   Services.prefs.setCharPref(LOOP_FXA_PROFILE_PREF, FAKE_FXA_PROFILE);
   Services.prefs.setCharPref(LOOP_FXA_TOKEN_PREF, FAKE_FXA_TOKEN_DATA);
+  MozLoopService.resetServiceInitialized();
 
   // Only need to implement the FxA registration because the previous
   // test registered as a guest.
   loopServer.registerPathHandler("/registration", (request, response) => {
     response.setStatusLine(null, 401, "Unauthorized");
     response.write(JSON.stringify({
       code: 401,
       errno: 110,
@@ -81,16 +83,17 @@ add_task(function test_initialize_with_i
     Assert.ok(MozLoopServiceInternal.errors.get("login").friendlyDetailsButtonCallback,
               "Check that there is a retry callback");
   });
 });
 
 add_task(function test_initialize_with_fxa_token() {
   Services.prefs.setCharPref(LOOP_FXA_PROFILE_PREF, FAKE_FXA_PROFILE);
   Services.prefs.setCharPref(LOOP_FXA_TOKEN_PREF, FAKE_FXA_TOKEN_DATA);
+  MozLoopService.resetServiceInitialized();
 
   MozLoopService.errors.clear();
 
   loopServer.registerPathHandler("/registration", (request, response) => {
     response.setStatusLine(null, 200, "OK");
   });
 
   yield MozLoopService.initialize().then(() => {