diff --git a/browser/components/loop/LoopRooms.jsm b/browser/components/loop/LoopRooms.jsm index 8fdeb51750f..4490041a043 100644 --- a/browser/components/loop/LoopRooms.jsm +++ b/browser/components/loop/LoopRooms.jsm @@ -68,7 +68,7 @@ let LoopRoomsInternal = { } Task.spawn(function* () { - yield MozLoopService.register(); + yield MozLoopService.promiseRegisteredWithServers(); if (!gDirty) { callback(null, [...this.rooms.values()]); diff --git a/browser/components/loop/MozLoopAPI.jsm b/browser/components/loop/MozLoopAPI.jsm index fb386983eff..b64aea507bf 100644 --- a/browser/components/loop/MozLoopAPI.jsm +++ b/browser/components/loop/MozLoopAPI.jsm @@ -369,7 +369,7 @@ function injectLoopAPI(targetWindow) { value: function(callback) { // We translate from a promise to a callback, as we can't pass promises from // Promise.jsm across the priv versus unpriv boundary. - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { callback(null); }, err => { callback(cloneValueInto(err, targetWindow)); diff --git a/browser/components/loop/MozLoopService.jsm b/browser/components/loop/MozLoopService.jsm index bff41176a4d..36990f7ec30 100644 --- a/browser/components/loop/MozLoopService.jsm +++ b/browser/components/loop/MozLoopService.jsm @@ -30,6 +30,7 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm"); Cu.import("resource://gre/modules/Promise.jsm"); Cu.import("resource://gre/modules/osfile.jsm", this); Cu.import("resource://gre/modules/Task.jsm"); +Cu.import("resource://gre/modules/Timer.jsm"); Cu.import("resource://gre/modules/FxAccountsOAuthClient.jsm"); Cu.importGlobalProperties(["URL"]); @@ -111,7 +112,6 @@ function getJSONPref(aName) { let gRegisteredDeferred = null; let gHawkClient = null; let gLocalizedStrings = null; -let gInitializeTimer = null; let gFxAEnabled = true; let gFxAOAuthClientPromise = null; let gFxAOAuthClient = null; @@ -309,7 +309,7 @@ let MozLoopServiceInternal = { /** * Starts registration of Loop with the push server, and then will register - * with the Loop server. It will return early if already registered. + * with the Loop server as a GUEST. It will return early if already registered. * * @returns {Promise} a promise that is resolved with no params on completion, or * rejected with an error code or string. @@ -871,42 +871,17 @@ let MozLoopServiceInternal = { }; Object.freeze(MozLoopServiceInternal); + let gInitializeTimerFunc = (deferredInitialization) => { // 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. - gInitializeTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); - gInitializeTimer.initWithCallback(Task.async(function* initializationCallback() { - yield MozLoopService.register().then(Task.async(function*() { - if (!MozLoopServiceInternal.fxAOAuthTokenData) { - log.debug("MozLoopService: Initialized without an already logged-in account"); - deferredInitialization.resolve("initialized to guest status"); - return; - } - log.debug("MozLoopService: Initializing with already logged-in account"); - let registeredPromise = - MozLoopServiceInternal.registerWithLoopServer( - LOOP_SESSION_TYPE.FXA, { - calls: MozLoopServiceInternal.pushHandler.registeredChannels[MozLoopService.channelIDs.callsFxA], - rooms: MozLoopServiceInternal.pushHandler.registeredChannels[MozLoopService.channelIDs.roomsFxA] - }); - registeredPromise.then(() => { - deferredInitialization.resolve("initialized to logged-in status"); - }, error => { - log.debug("MozLoopService: error logging in using cached auth token"); - MozLoopServiceInternal.setError("login", error); - deferredInitialization.reject("error logging in using cached auth token"); - }); - }), error => { - log.debug("MozLoopService: Failure of initial registration", error); - deferredInitialization.reject(error); - }); - gInitializeTimer = null; - }), - MozLoopServiceInternal.initialRegistrationDelayMilliseconds, Ci.nsITimer.TYPE_ONE_SHOT); + setTimeout(MozLoopService.delayedInitialize.bind(MozLoopService, deferredInitialization), + MozLoopServiceInternal.initialRegistrationDelayMilliseconds); }; + /** * Public API */ @@ -962,13 +937,60 @@ this.MozLoopService = { let deferredInitialization = Promise.defer(); gInitializeTimerFunc(deferredInitialization); - return deferredInitialization.promise.catch(error => { + return deferredInitialization.promise; + }), + + /** + * The core of the initialization work that happens once the browser is ready + * (after a timer when called during startup). + * + * Can be called more than once (e.g. if the initial setup fails at some phase). + * @param {Deferred} deferredInitialization + */ + delayedInitialize: Task.async(function*(deferredInitialization) { + // Set or clear an error depending on how deferredInitialization gets resolved. + // We do this first so that it can handle the early returns below. + let completedPromise = deferredInitialization.promise.then(result => { + MozLoopServiceInternal.clearError("initialization"); + return result; + }, + error => { + // If we get a non-object then setError was already called for a different error type. if (typeof(error) == "object") { - // This never gets cleared since there is no UI to recover. Only restarting will work. MozLoopServiceInternal.setError("initialization", error); } - throw error; }); + + try { + yield this.promiseRegisteredWithServers(); + } catch (ex) { + log.debug("MozLoopService: Failure of initial registration", ex); + deferredInitialization.reject(ex); + yield completedPromise; + return; + } + + if (!MozLoopServiceInternal.fxAOAuthTokenData) { + log.debug("MozLoopService: Initialized without an already logged-in account"); + deferredInitialization.resolve("initialized to guest status"); + yield completedPromise; + return; + } + + log.debug("MozLoopService: Initializing with already logged-in account"); + let pushURLs = { + calls: MozLoopServiceInternal.pushHandler.registeredChannels[this.channelIDs.callsFxA], + rooms: MozLoopServiceInternal.pushHandler.registeredChannels[this.channelIDs.roomsFxA] + }; + + MozLoopServiceInternal.registerWithLoopServer(LOOP_SESSION_TYPE.FXA, pushURLs).then(() => { + deferredInitialization.resolve("initialized to logged-in status"); + }, error => { + log.debug("MozLoopService: error logging in using cached auth token"); + MozLoopServiceInternal.setError("login", error); + deferredInitialization.reject("error logging in using cached auth token"); + }); + yield completedPromise; }), /** @@ -1081,25 +1103,10 @@ this.MozLoopService = { Services.tm.mainThread); }, - /** - * Starts registration of Loop with the push server, and then will register - * with the Loop server. It will return early if already registered. - * - * @returns {Promise} a promise that is resolved with no params on completion, or - * rejected with an error code or string. + * @see MozLoopServiceInternal.promiseRegisteredWithServers */ - register: function() { - log.debug("registering"); - // Don't do anything if loop is not enabled. - if (!Services.prefs.getBoolPref("loop.enabled")) { - throw new Error("Loop is not enabled"); - } - - if (Services.prefs.getBoolPref("loop.throttled")) { - throw new Error("Loop is disabled by the soft-start mechanism"); - } - + promiseRegisteredWithServers: function() { return MozLoopServiceInternal.promiseRegisteredWithServers(); }, diff --git a/browser/components/loop/test/mochitest/browser_fxa_login.js b/browser/components/loop/test/mochitest/browser_fxa_login.js index aa270271c72..088b06b16ce 100644 --- a/browser/components/loop/test/mochitest/browser_fxa_login.js +++ b/browser/components/loop/test/mochitest/browser_fxa_login.js @@ -253,7 +253,7 @@ add_task(function* basicAuthorizationAndRegistration() { mockPushHandler.registrationPushURL = "https://localhost/pushUrl/guest"; // Notification observed due to the error being cleared upon successful registration. let statusChangedPromise = promiseObserverNotified("loop-status-changed"); - yield MozLoopService.register(); + yield MozLoopService.promiseRegisteredWithServers(); yield statusChangedPromise; // Normally the same pushUrl would be registered but we change it in the test @@ -318,7 +318,7 @@ add_task(function* loginWithParams401() { test_error: "params_401", }; yield promiseOAuthParamsSetup(BASE_URL, params); - yield MozLoopService.register(); + yield MozLoopService.promiseRegisteredWithServers(); let loginPromise = MozLoopService.logInToFxA(); yield loginPromise.then(tokenData => { diff --git a/browser/components/loop/test/xpcshell/head.js b/browser/components/loop/test/xpcshell/head.js index 06ec7703109..4dac56080f6 100644 --- a/browser/components/loop/test/xpcshell/head.js +++ b/browser/components/loop/test/xpcshell/head.js @@ -31,6 +31,11 @@ var loopServer; Services.prefs.setBoolPref("loop.enabled", true); Services.prefs.setBoolPref("loop.throttled", false); +// Cleanup function for all tests +do_register_cleanup(() => { + MozLoopService.errors.clear(); +}); + function setupFakeLoopServer() { loopServer = new HttpServer(); loopServer.start(-1); diff --git a/browser/components/loop/test/xpcshell/test_looprooms.js b/browser/components/loop/test/xpcshell/test_looprooms.js index 19c1a0fd7c5..07ba1c42c12 100644 --- a/browser/components/loop/test/xpcshell/test_looprooms.js +++ b/browser/components/loop/test/xpcshell/test_looprooms.js @@ -135,7 +135,7 @@ const compareRooms = function(room1, room2) { }; add_task(function* test_getAllRooms() { - yield MozLoopService.register(mockPushHandler); + yield MozLoopService.promiseRegisteredWithServers(); let rooms = yield LoopRooms.promise("getAll"); Assert.equal(rooms.length, 3); @@ -145,7 +145,7 @@ add_task(function* test_getAllRooms() { }); add_task(function* test_getRoom() { - yield MozLoopService.register(mockPushHandler); + yield MozLoopService.promiseRegisteredWithServers(); let roomToken = "_nxD4V4FflQ"; let room = yield LoopRooms.promise("get", roomToken); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_busy.js b/browser/components/loop/test/xpcshell/test_loopservice_busy.js index 00db71c670a..3b89b8fdb23 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_busy.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_busy.js @@ -24,7 +24,7 @@ let msgHandler = function(msg) { add_test(function test_busy_2guest_calls() { actionReceived = false; - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { let opened = 0; Chat.open = function() { opened++; @@ -47,7 +47,7 @@ add_test(function test_busy_2guest_calls() { add_test(function test_busy_1fxa_1guest_calls() { actionReceived = false; - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { let opened = 0; Chat.open = function() { opened++; @@ -71,7 +71,7 @@ add_test(function test_busy_1fxa_1guest_calls() { add_test(function test_busy_2fxa_calls() { actionReceived = false; - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { let opened = 0; Chat.open = function() { opened++; @@ -94,7 +94,7 @@ add_test(function test_busy_2fxa_calls() { add_test(function test_busy_1guest_1fxa_calls() { actionReceived = false; - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { let opened = 0; Chat.open = function() { opened++; diff --git a/browser/components/loop/test/xpcshell/test_loopservice_dnd.js b/browser/components/loop/test/xpcshell/test_loopservice_dnd.js index 3530dff5032..130af0909c3 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_dnd.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_dnd.js @@ -31,7 +31,7 @@ add_test(function test_set_do_not_disturb() { add_test(function test_do_not_disturb_disabled_should_open_chat_window() { MozLoopService.doNotDisturb = false; - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { let opened = false; Chat.open = function() { opened = true; diff --git a/browser/components/loop/test/xpcshell/test_loopservice_initialize.js b/browser/components/loop/test/xpcshell/test_loopservice_initialize.js index bc4b6c6c2b7..636d12424e3 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_initialize.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_initialize.js @@ -1,7 +1,7 @@ /* Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/ */ -var startTimerCalled = false; +let startTimerCalled = false; /** * Tests that registration doesn't happen when the expiry time is @@ -23,7 +23,7 @@ add_task(function test_initialize_no_expiry() { */ add_task(function test_initialize_expiry_past() { // Set time to be 2 seconds in the past. - var nowSeconds = Date.now() / 1000; + let nowSeconds = Date.now() / 1000; Services.prefs.setIntPref("loop.urlsExpiryTimeSeconds", nowSeconds - 2); startTimerCalled = false; @@ -39,7 +39,7 @@ add_task(function test_initialize_expiry_past() { */ add_task(function test_initialize_starts_timer() { // Set time to be 1 minute in the future - var nowSeconds = Date.now() / 1000; + let nowSeconds = Date.now() / 1000; Services.prefs.setIntPref("loop.urlsExpiryTimeSeconds", nowSeconds + 60); startTimerCalled = false; @@ -49,8 +49,7 @@ add_task(function test_initialize_starts_timer() { "should start the timer when expiry time is in the future"); }); -function run_test() -{ +function run_test() { setupFakeLoopServer(); // Override MozLoopService's initializeTimer, so that we can verify the timeout is called diff --git a/browser/components/loop/test/xpcshell/test_loopservice_notification.js b/browser/components/loop/test/xpcshell/test_loopservice_notification.js index eb92fcdabd3..f4ad6556fde 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_notification.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_notification.js @@ -10,7 +10,7 @@ let openChatOrig = Chat.open; add_test(function test_openChatWindow_on_notification() { Services.prefs.setCharPref("loop.seenToS", "unseen"); - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { let opened = false; Chat.open = function() { opened = true; diff --git a/browser/components/loop/test/xpcshell/test_loopservice_registration.js b/browser/components/loop/test/xpcshell/test_loopservice_registration.js index 960bbc73d92..e4a858f210e 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_registration.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_registration.js @@ -17,7 +17,7 @@ Cu.import("resource://services-common/utils.js"); add_test(function test_register_websocket_success_loop_server_fail() { mockPushHandler.registrationResult = "404"; - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { do_throw("should not succeed when loop server registration fails"); }, (err) => { // 404 is an expected failure indicated by the lack of route being set @@ -49,7 +49,7 @@ add_test(function test_register_success() { response.processAsync(); response.finish(); }); - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { run_next_test(); }, err => { do_throw("shouldn't error on a successful request"); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_restart.js b/browser/components/loop/test/xpcshell/test_loopservice_restart.js index 733b678997f..ee55abcf442 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_restart.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_restart.js @@ -75,12 +75,17 @@ add_task(function test_initialize_with_invalid_fxa_token() { "FXA pref should be cleared if token was invalid"); Assert.equal(Services.prefs.getCharPref(LOOP_FXA_PROFILE_PREF), "", "FXA profile pref should be cleared if token was invalid"); + Assert.ok(MozLoopServiceInternal.errors.has("login"), + "Initialization error should have been reported to UI"); }); }); 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.errors.clear(); + loopServer.registerPathHandler("/registration", (request, response) => { response.setStatusLine(null, 200, "OK"); }); @@ -90,6 +95,7 @@ add_task(function test_initialize_with_fxa_token() { "FXA pref should still be set after initialization"); Assert.equal(Services.prefs.getCharPref(LOOP_FXA_PROFILE_PREF), FAKE_FXA_PROFILE, "FXA profile should still be set after initialization"); + Assert.ok(!MozLoopServiceInternal.errors.has("login"), "Initialization error should not exist"); }); }); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_token_invalid.js b/browser/components/loop/test/xpcshell/test_loopservice_token_invalid.js index 507776f170e..49592446d24 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_token_invalid.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_token_invalid.js @@ -31,7 +31,7 @@ add_test(function test_registration_invalid_token() { response.finish(); }); - MozLoopService.register().then(() => { + 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); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_token_save.js b/browser/components/loop/test/xpcshell/test_loopservice_token_save.js index e9ecfbbbe52..d54ac694217 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_token_save.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_token_save.js @@ -16,7 +16,7 @@ add_test(function test_registration_returns_hawk_session_token() { response.finish(); }); - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { var hawkSessionPref; try { hawkSessionPref = Services.prefs.getCharPref("loop.hawk-session-token"); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_token_send.js b/browser/components/loop/test/xpcshell/test_loopservice_token_send.js index 855ef7c1f8a..eb0881a1aaa 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_token_send.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_token_send.js @@ -24,7 +24,7 @@ add_test(function test_registration_uses_hawk_session_token() { response.finish(); }); - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { run_next_test(); }, err => { do_throw("shouldn't error on a succesful request"); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_token_validation.js b/browser/components/loop/test/xpcshell/test_loopservice_token_validation.js index db1edb69feb..ee121a96852 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_token_validation.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_token_validation.js @@ -16,7 +16,7 @@ add_test(function test_registration_handles_bogus_hawk_token() { response.finish(); }); - MozLoopService.register().then(() => { + MozLoopService.promiseRegisteredWithServers().then(() => { do_throw("should not succeed with a bogus token"); }, err => {