From 1307e179d1745ad60599147725228635f92ce20c Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Fri, 22 May 2015 14:45:13 +0100 Subject: [PATCH] Bug 1157712 - Fix multiple OS X notifications when someone joins a Loop room. Ensure we don't attempt to re-initialise twice. r=mikedeboer --- browser/base/content/browser-loop.js | 10 ++++++++- .../loop/modules/MozLoopService.jsm | 21 +++++++++++++++++++ .../xpcshell/test_loopservice_initialize.js | 8 +++++++ .../test/xpcshell/test_loopservice_restart.js | 3 +++ 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/browser/base/content/browser-loop.js b/browser/base/content/browser-loop.js index 8f954b15c532..6f77579ef804 100644 --- a/browser/base/content/browser-loop.js +++ b/browser/base/content/browser-loop.js @@ -218,7 +218,15 @@ let LoopUI; // Add observer notifications before the service is initialized Services.obs.addObserver(this, "loop-status-changed", false); - this.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. + this.MozLoopService.initialize().catch(ex => { + if (!ex.message || + (!ex.message.contains("not enabled") && + !ex.message.contains("not needed"))) { + console.error(ex); + } + }); this.updateToolbarState(); }, diff --git a/browser/components/loop/modules/MozLoopService.jsm b/browser/components/loop/modules/MozLoopService.jsm index 455e251cd76d..096a93f8f0c0 100644 --- a/browser/components/loop/modules/MozLoopService.jsm +++ b/browser/components/loop/modules/MozLoopService.jsm @@ -1072,6 +1072,7 @@ let gInitializeTimerFunc = (deferredInitialization) => { MozLoopServiceInternal.initialRegistrationDelayMilliseconds); }; +let gServiceInitialized = false; /** * Public API @@ -1089,10 +1090,21 @@ this.MozLoopService = { }; }, + /** + * 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; }, @@ -1101,9 +1113,18 @@ this.MozLoopService = { * 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); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_initialize.js b/browser/components/loop/test/xpcshell/test_loopservice_initialize.js index 37a6fd5d688e..5b42df4a8b63 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_initialize.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_initialize.js @@ -40,11 +40,19 @@ add_task(function test_initialize_no_guest_rooms() { 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() { diff --git a/browser/components/loop/test/xpcshell/test_loopservice_restart.js b/browser/components/loop/test/xpcshell/test_loopservice_restart.js index e4001bb5a137..d7f270119855 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_restart.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_restart.js @@ -39,6 +39,7 @@ add_task(function* test_initialize_with_no_guest_rooms_and_no_auth_token() { 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"); @@ -55,6 +56,7 @@ add_task(function* test_initialize_with_created_room_and_no_auth_token() { 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. @@ -88,6 +90,7 @@ add_task(function* test_initialize_with_invalid_fxa_token() { 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();