From ee86ece308f0c2ad3d40b900cbdaa700379d267f Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Mon, 9 Mar 2015 23:42:08 +0000 Subject: [PATCH] Bug 1140092 - Remove handling of guest calls from the Loop backend. r=mikedeboer --- browser/components/loop/LoopCalls.jsm | 2 - browser/components/loop/MozLoopService.jsm | 74 +++++------------- browser/components/loop/test/xpcshell/head.js | 14 ++++ .../test/xpcshell/test_loopservice_busy.js | 76 +------------------ .../test/xpcshell/test_loopservice_dnd.js | 8 +- .../xpcshell/test_loopservice_hawk_errors.js | 4 +- .../xpcshell/test_loopservice_initialize.js | 24 +++--- .../xpcshell/test_loopservice_notification.js | 6 +- .../test_loopservice_registration_retry.js | 5 +- .../test/xpcshell/test_loopservice_restart.js | 12 +-- .../test_loopservice_token_invalid.js | 2 +- 11 files changed, 68 insertions(+), 159 deletions(-) diff --git a/browser/components/loop/LoopCalls.jsm b/browser/components/loop/LoopCalls.jsm index b58bb6167773..9574bff83f80 100644 --- a/browser/components/loop/LoopCalls.jsm +++ b/browser/components/loop/LoopCalls.jsm @@ -221,8 +221,6 @@ let LoopCallsInternal = { if (channelID == MozLoopService.channelIDs.callsFxA && MozLoopService.userProfile) { this._getCalls(LOOP_SESSION_TYPE.FXA, version); - } else { - this._getCalls(LOOP_SESSION_TYPE.GUEST, version); } }, diff --git a/browser/components/loop/MozLoopService.jsm b/browser/components/loop/MozLoopService.jsm index df8345e06d80..113526eb68f6 100644 --- a/browser/components/loop/MozLoopService.jsm +++ b/browser/components/loop/MozLoopService.jsm @@ -157,37 +157,6 @@ let MozLoopServiceInternal = { return initialDelay; }, - /** - * Gets the current latest expiry time for urls. - * - * In seconds since epoch. - */ - get expiryTimeSeconds() { - try { - return Services.prefs.getIntPref("loop.urlsExpiryTimeSeconds"); - } catch (x) { - // It is ok for the pref not to exist. - return 0; - } - }, - - /** - * Sets the expiry time to either the specified time, or keeps it the same - * depending on which is latest. - */ - set expiryTimeSeconds(time) { - if (time > this.expiryTimeSeconds) { - Services.prefs.setIntPref("loop.urlsExpiryTimeSeconds", time); - } - }, - - /** - * Returns true if the expiry time is in the future. - */ - urlExpiryTimeIsInFuture: function() { - return this.expiryTimeSeconds * 1000 > Date.now(); - }, - /** * Retrieves MozLoopService Firefox Accounts OAuth token. * @@ -390,17 +359,20 @@ let MozLoopServiceInternal = { let options = this.mocks.webSocket ? { mockWebSocket: this.mocks.webSocket } : {}; this.pushHandler.initialize(options); // This can be called more than once. - 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; - - let regPromise = this.createNotificationChannel( - callsID, sessionType, "calls", LoopCalls.onNotification).then(() => { - return this.createNotificationChannel( - roomsID, sessionType, "rooms", roomsPushNotification)}); + let regPromise; + if (sessionType == LOOP_SESSION_TYPE.GUEST) { + regPromise = this.createNotificationChannel( + MozLoopService.channelIDs.roomsGuest, sessionType, "rooms", + roomsPushNotification); + } else { + regPromise = this.createNotificationChannel( + MozLoopService.channelIDs.callsFxA, sessionType, "calls", + LoopCalls.onNotification).then(() => { + return this.createNotificationChannel( + MozLoopService.channelIDs.roomsFxA, sessionType, "rooms", + roomsPushNotification); + }); + } log.debug("assigning to deferredRegistrations for sessionType:", sessionType); this.deferredRegistrations.set(sessionType, regPromise); @@ -587,11 +559,8 @@ let MozLoopServiceInternal = { 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); } + this.setError("registration", error); throw error; }; @@ -1058,7 +1027,6 @@ this.MozLoopService = { // Channel ids that will be registered with the PushServer for notifications return { callsFxA: "25389583-921f-4169-a426-a4673658944b", - callsGuest: "801f754b-686b-43ec-bd83-1419bbf58388", roomsFxA: "6add272a-d316-477c-8335-f00f73dfde71", roomsGuest: "19d3f799-a8f3-4328-9822-b7cd02765832", }; @@ -1124,10 +1092,9 @@ this.MozLoopService = { LoopRooms.on("joined", this.maybeResumeTourOnRoomJoined.bind(this)); - // If expiresTime is not in the future and the user hasn't + // If there's no guest room created and the user hasn't // previously authenticated then skip registration. - if (!MozLoopServiceInternal.urlExpiryTimeIsInFuture() && - !LoopRooms.getGuestCreatedRoom() && + if (!LoopRooms.getGuestCreatedRoom() && !MozLoopServiceInternal.fxAOAuthTokenData) { return Promise.resolve("registration not needed"); } @@ -1204,11 +1171,10 @@ this.MozLoopService = { }); try { - if (MozLoopServiceInternal.urlExpiryTimeIsInFuture() || - LoopRooms.getGuestCreatedRoom()) { + if (LoopRooms.getGuestCreatedRoom()) { yield this.promiseRegisteredWithServers(LOOP_SESSION_TYPE.GUEST); } else { - log.debug("delayedInitialize: URL expiry time isn't in the future so not registering as a guest"); + log.debug("delayedInitialize: Guest Room hasn't been created so not registering as a guest"); } } catch (ex) { log.debug("MozLoopService: Failure of guest registration", ex); @@ -1461,7 +1427,7 @@ this.MozLoopService = { yield MozLoopServiceInternal.unregisterFromLoopServer(LOOP_SESSION_TYPE.FXA); } catch (err) { - throw err + throw err; } finally { MozLoopServiceInternal.clearSessionToken(LOOP_SESSION_TYPE.FXA); diff --git a/browser/components/loop/test/xpcshell/head.js b/browser/components/loop/test/xpcshell/head.js index 5c51d485d57e..59e90bb29b32 100644 --- a/browser/components/loop/test/xpcshell/head.js +++ b/browser/components/loop/test/xpcshell/head.js @@ -32,6 +32,7 @@ Services.prefs.setBoolPref("loop.enabled", true); // Cleanup function for all tests do_register_cleanup(() => { + Services.prefs.clearUserPref("loop.enabled"); MozLoopService.errors.clear(); }); @@ -50,6 +51,19 @@ function setupFakeLoopServer() { }); } +/** + * Sets up the userProfile to make the service think we're logged into FxA. + */ +function setupFakeFxAUserProfile() { + MozLoopServiceInternal.fxAOAuthTokenData = { token_type: "bearer" }; + MozLoopServiceInternal.fxAOAuthProfile = { email: "fake@invalid.com" }; + + do_register_cleanup(function() { + MozLoopServiceInternal.fxAOAuthTokenData = null; + MozLoopServiceInternal.fxAOAuthProfile = null; + }); +} + function waitForCondition(aConditionFn, aMaxTries=50, aCheckInterval=100) { function tryAgain() { function tryNow() { diff --git a/browser/components/loop/test/xpcshell/test_loopservice_busy.js b/browser/components/loop/test/xpcshell/test_loopservice_busy.js index 178217616030..c3d302f06f2e 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_busy.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_busy.js @@ -22,55 +22,6 @@ let msgHandler = function(msg) { } }; -add_task(function* test_busy_2guest_calls() { - actionReceived = false; - - mockPushHandler.registrationPushURL = kEndPointUrl; - - yield MozLoopService.promiseRegisteredWithServers(LOOP_SESSION_TYPE.GUEST); - - 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); - - 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_task(function* test_busy_1fxa_1guest_calls() { - actionReceived = false; - - 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); - - 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_task(function* test_busy_2fxa_calls() { actionReceived = false; @@ -95,31 +46,6 @@ add_task(function* test_busy_2fxa_calls() { }); }); -add_task(function* test_busy_1guest_1fxa_calls() { - actionReceived = false; - - 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); - - 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(); @@ -134,6 +60,8 @@ function run_test() { Services.io.offline = false; + mockPushHandler.registrationPushURL = kEndPointUrl; + // For each notification received from the PushServer, MozLoopService will first query // for any pending calls on the FxA hawk session and then again using the guest session. // A pair of response objects in the callsResponses array will be consumed for each diff --git a/browser/components/loop/test/xpcshell/test_loopservice_dnd.js b/browser/components/loop/test/xpcshell/test_loopservice_dnd.js index 5e41c9d24b69..93bca0ffc0c5 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_dnd.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_dnd.js @@ -33,13 +33,13 @@ add_test(function test_do_not_disturb_disabled_should_open_chat_window() { mockPushHandler.registrationPushURL = kEndPointUrl; - MozLoopService.promiseRegisteredWithServers().then(() => { + MozLoopService.promiseRegisteredWithServers(LOOP_SESSION_TYPE.FXA).then(() => { let opened = false; Chat.open = function() { opened = true; }; - mockPushHandler.notify(1, MozLoopService.channelIDs.callsGuest); + mockPushHandler.notify(1, MozLoopService.channelIDs.callsFxA); waitForCondition(function() opened).then(() => { run_next_test(); @@ -58,7 +58,7 @@ add_test(function test_do_not_disturb_enabled_shouldnt_open_chat_window() { opened = true; }; - mockPushHandler.notify(1, MozLoopService.channelIDs.callsGuest); + mockPushHandler.notify(1, MozLoopService.channelIDs.callsFxA); do_timeout(500, function() { do_check_false(opened, "should not open a chat window"); @@ -69,6 +69,8 @@ add_test(function test_do_not_disturb_enabled_shouldnt_open_chat_window() { function run_test() { setupFakeLoopServer(); + setupFakeFxAUserProfile(); + loopServer.registerPathHandler("/registration", (request, response) => { response.setStatusLine(null, 200, "OK"); response.processAsync(); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_hawk_errors.js b/browser/components/loop/test/xpcshell/test_loopservice_hawk_errors.js index 1ad6cff88fd2..91b8ff72e79e 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_hawk_errors.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_hawk_errors.js @@ -173,14 +173,14 @@ add_task(cleanup_between_tests); function run_test() { setupFakeLoopServer(); - // Set the expiry time one hour in the future so that an error is shown when the guest session expires. - MozLoopServiceInternal.expiryTimeSeconds = (Date.now() / 1000) + 3600; + Services.prefs.setBoolPref("loop.createdRoom", true); do_register_cleanup(() => { Services.prefs.clearUserPref("loop.hawk-session-token"); Services.prefs.clearUserPref("loop.hawk-session-token.fxa"); Services.prefs.clearUserPref("loop.urlsExpiryTimeSeconds"); Services.prefs.clearUserPref("network.dns.offline-localhost"); + Services.prefs.clearUserPref("loop.createdRoom"); MozLoopService.errors.clear(); }); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_initialize.js b/browser/components/loop/test/xpcshell/test_loopservice_initialize.js index 636d12424e31..e82244af97e8 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_initialize.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_initialize.js @@ -18,35 +18,31 @@ add_task(function test_initialize_no_expiry() { }); /** - * Tests that registration doesn't happen when the expiry time is - * in the past. + * Tests that registration doesn't happen when there has been no + * room created. */ -add_task(function test_initialize_expiry_past() { - // Set time to be 2 seconds in the past. - let nowSeconds = Date.now() / 1000; - Services.prefs.setIntPref("loop.urlsExpiryTimeSeconds", nowSeconds - 2); +add_task(function test_initialize_no_guest_rooms() { + Services.prefs.setBoolPref("loop.createdRoom", false); startTimerCalled = false; MozLoopService.initialize(); Assert.equal(startTimerCalled, false, - "should not register when expiry time is in past"); + "should not register when no guest rooms have been created"); }); /** * Tests that registration happens when the expiry time is in * the future. */ -add_task(function test_initialize_starts_timer() { - // Set time to be 1 minute in the future - let nowSeconds = Date.now() / 1000; - Services.prefs.setIntPref("loop.urlsExpiryTimeSeconds", nowSeconds + 60); +add_task(function test_initialize_with_guest_rooms() { + Services.prefs.setBoolPref("loop.createdRoom", true); startTimerCalled = false; MozLoopService.initialize(); Assert.equal(startTimerCalled, true, - "should start the timer when expiry time is in the future"); + "should start the timer when guest rooms have been created"); }); function run_test() { @@ -58,5 +54,9 @@ function run_test() { startTimerCalled = true; }; + do_register_cleanup(function() { + Services.prefs.clearUserPref("loop.createdRoom"); + }); + run_next_test(); } diff --git a/browser/components/loop/test/xpcshell/test_loopservice_notification.js b/browser/components/loop/test/xpcshell/test_loopservice_notification.js index c27e7b822a6b..81b9fd6146af 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_notification.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_notification.js @@ -12,13 +12,13 @@ add_test(function test_openChatWindow_on_notification() { mockPushHandler.registrationPushURL = kEndPointUrl; - MozLoopService.promiseRegisteredWithServers().then(() => { + MozLoopService.promiseRegisteredWithServers(LOOP_SESSION_TYPE.FXA).then(() => { let opened = false; Chat.open = function() { opened = true; }; - mockPushHandler.notify(1, MozLoopService.channelIDs.callsGuest); + mockPushHandler.notify(1, MozLoopService.channelIDs.callsFxA); waitForCondition(function() opened).then(() => { do_check_true(opened, "should open a chat window"); @@ -37,6 +37,8 @@ add_test(function test_openChatWindow_on_notification() { function run_test() { setupFakeLoopServer(); + setupFakeFxAUserProfile(); + loopServer.registerPathHandler("/registration", (request, response) => { response.setStatusLine(null, 200, "OK"); response.processAsync(); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_registration_retry.js b/browser/components/loop/test/xpcshell/test_loopservice_registration_retry.js index cba0ea105538..c568428dd3bb 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_registration_retry.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_registration_retry.js @@ -59,12 +59,11 @@ function run_test() { response.setStatusLine(null, 200, "OK"); }); - let nowSeconds = Date.now() / 1000; - Services.prefs.setIntPref("loop.urlsExpiryTimeSeconds", nowSeconds + 60); + Services.prefs.setBoolPref("loop.createdRoom", true); do_register_cleanup(function() { Services.prefs.clearUserPref("loop.hawk-session-token"); - Services.prefs.clearUserPref("loop.urlsExpiryTimeSeconds"); + Services.prefs.clearUserPref("loop.createdRoom"); }); run_next_test(); diff --git a/browser/components/loop/test/xpcshell/test_loopservice_restart.js b/browser/components/loop/test/xpcshell/test_loopservice_restart.js index 2058158836a2..cc0b16c8d358 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_restart.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_restart.js @@ -13,17 +13,17 @@ const FAKE_FXA_PROFILE = JSON.stringify({ }); const LOOP_FXA_TOKEN_PREF = "loop.fxa_oauth.tokendata"; const LOOP_FXA_PROFILE_PREF = "loop.fxa_oauth.profile"; -const LOOP_URL_EXPIRY_PREF = "loop.urlsExpiryTimeSeconds"; +const LOOP_CREATED_ROOM_PREF = "loop.createdRoom"; const LOOP_INITIAL_DELAY_PREF = "loop.initialDelay"; /** * This file is to test restart+reauth. */ -add_task(function test_initialize_with_expired_urls_and_no_auth_token() { +add_task(function test_initialize_with_no_guest_rooms_and_no_auth_token() { // Set time to be 2 seconds in the past. var nowSeconds = Date.now() / 1000; - Services.prefs.setIntPref(LOOP_URL_EXPIRY_PREF, nowSeconds - 2); + Services.prefs.setBoolPref(LOOP_CREATED_ROOM_PREF, false); Services.prefs.clearUserPref(LOOP_FXA_TOKEN_PREF); yield MozLoopService.initialize().then((msg) => { @@ -34,8 +34,8 @@ add_task(function test_initialize_with_expired_urls_and_no_auth_token() { }); }); -add_task(function test_initialize_with_urls_and_no_auth_token() { - Services.prefs.setIntPref(LOOP_URL_EXPIRY_PREF, Date.now() / 1000 + 10); +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); loopServer.registerPathHandler("/registration", (request, response) => { @@ -114,7 +114,7 @@ function run_test() { Services.prefs.clearUserPref(LOOP_INITIAL_DELAY_PREF); Services.prefs.clearUserPref(LOOP_FXA_TOKEN_PREF); Services.prefs.clearUserPref(LOOP_FXA_PROFILE_PREF); - Services.prefs.clearUserPref(LOOP_URL_EXPIRY_PREF); + Services.prefs.clearUserPref(LOOP_CREATED_ROOM_PREF); }); run_next_test(); 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 2bcb799df181..d78fc3b24ae4 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_token_invalid.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_token_invalid.js @@ -34,7 +34,7 @@ add_test(function test_registration_invalid_token() { 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, 4); //hawk will repeat each registration attemtp twice: calls and rooms. + Assert.equal(authorizationAttempts, 2); // Hawk will repeat the registration attempt twice. Assert.equal(Services.prefs.getCharPref(LOOP_HAWK_PREF), fakeSessionToken2); run_next_test(); }, err => {