diff --git a/mobile/android/base/java/org/mozilla/gecko/AccountsHelper.java b/mobile/android/base/java/org/mozilla/gecko/AccountsHelper.java index 79970c744682..817627b36f55 100644 --- a/mobile/android/base/java/org/mozilla/gecko/AccountsHelper.java +++ b/mobile/android/base/java/org/mozilla/gecko/AccountsHelper.java @@ -215,7 +215,7 @@ public class AccountsHelper implements BundleEventListener { // 'UpdateFirefoxAccountFromJSON' message will be sent during a password change as well // during a reconnect, so we need to ensure we don't fire-off false sign-in telemtry events. - if (action.equals("reconnect")) { + if (!action.equals("passwordChange")) { MmaDelegate.track(MmaDelegate.USER_RECONNECTED_TO_FXA); } diff --git a/mobile/android/modules/FxAccountsWebChannel.jsm b/mobile/android/modules/FxAccountsWebChannel.jsm index f41adfde5c12..66c24957c933 100644 --- a/mobile/android/modules/FxAccountsWebChannel.jsm +++ b/mobile/android/modules/FxAccountsWebChannel.jsm @@ -310,16 +310,25 @@ this.FxAccountsWebChannel.prototype = { // succeed or get an error. Accounts.getFirefoxAccount() .then(account => { - if (!account) { - // This is how way we can determine if we're logging-in or signin-up. - // A choice of what to sync is only presented to the user during signup. - // A more robust approach is tracked in https://github.com/mozilla/fxa/issues/1998 + // "action" will be introduced in https://github.com/mozilla/fxa/issues/1998. + // We're both backwards and forwards compatible here, falling back on heuristics + // if its missing, and passing it along otherwise. + // WebChannel "login data" payload docs at the time this comment was written: + // https://github.com/mozilla/fxa/blob/8701348cdd79dbdc9879b2b4a55a23a135a32bc1/packages/fxa-content-server/docs/relier-communication-protocols/fx-webchannel.md#loginData + if (!data.hasOwnProperty("action")) { + // This is how way we can determine if we're logging-in or signing-up. + // Currently, a choice of what to sync (CWTS) is only presented to the user during signup. + // This is likely to change in the future - we will offer CWTS to first-time Sync users as well. + // Once those changes occur, "action" is expected to be present in the webchannel message data, + // letting us avoid this codepath. if ("offeredSyncEngines" in data) { data.action = "signup"; } else { data.action = "signin"; } + } + if (!account) { return Accounts.createFirefoxAccountFromJSON(data).then( success => { if (!success) { @@ -336,9 +345,12 @@ this.FxAccountsWebChannel.prototype = { ); } - // If we already had an Android Account, which means we're re-connecting to an existing account. - data.action = "reconnect"; - + // At this point, we're reconnectig to an existing account (due to a password change, or a device disconnect). + // As far as `action` parameter is concerned that we pass along in `data`, this is considered a `signin`. + // Receiving native code is expected to differentiate between a "signin" and a "reconnect" by looking + // at account presence. + // We also send `updateFirefoxAccountFromJSON` in case of a password change, and in that case "action" + // is explicitely set to "passwordChange". return Accounts.updateFirefoxAccountFromJSON(data).then( success => { if (!success) { @@ -381,8 +393,16 @@ this.FxAccountsWebChannel.prototype = { ); } - // Make sure to differentiate this action from other actions that send a 'updateFirefoxAccountFromJSON' message. - data.action = "passwordChange"; + // "action" will be introduced in https://github.com/mozilla/fxa/issues/1998. + // In case it's missing, hard-code it below. Once "action" parameter is added + // for COMMAND_CHANGE_PASSWORD payload, it's expected to be the same. + if (!data.hasOwnProperty("action")) { + data.action = "passwordChange"; + } else if (data.action != "passwordChange") { + throw new Error( + "Expected 'action' to be 'passwordChange', but saw " + data.action + ); + } return Accounts.updateFirefoxAccountFromJSON(data); })