From 612da4f9b9781ff23010987e1ba5e85c9a8a6cb0 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Mon, 24 Feb 2020 14:37:21 -0800 Subject: [PATCH] [components] Closes https://github.com/mozilla-mobile/android-components/issues/6053: Register an account state persistence callback for offline migration recovery This fixes a problem where an account state persistence callback was not registered during the recovery part of the offline fxa migration flow. This caused us to successfully migrate, but then migrate again and again (always successfully) upon application restart, since we wouldn't remember our own success. --- .../mozilla/components/service/fxa/manager/FxaAccountManager.kt | 8 +++++++- .../java/mozilla/components/service/fxa/FxaAccountManagerTest.kt | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt b/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt index c53e080022a2..2e3081f4e50b 100644 --- a/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt +++ b/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt @@ -711,6 +711,8 @@ open class FxaAccountManager( when (via) { Event.RetryMigration, Event.InFlightReuseMigration -> { + logger.info("Registering persistence callback") + account.registerPersistenceCallback(statePersistenceCallback) return retryMigration(reuseAccount = true) } else -> null @@ -720,6 +722,8 @@ open class FxaAccountManager( when (via) { Event.RetryMigration, Event.InFlightCopyMigration -> { + logger.info("Registering persistence callback") + account.registerPersistenceCallback(statePersistenceCallback) return retryMigration(reuseAccount = false) } else -> null @@ -774,7 +778,9 @@ open class FxaAccountManager( is Event.SignedInShareableAccount -> { // Note that we are not registering an account persistence callback here like // we do in other `AuthenticatedNoProfile` methods, because it would have been - // already registered while handling `Event.SignInShareableAccount`. + // already registered while handling any of the pre-cursor events, such as + // `Event.SignInShareableAccount`, `Event.InFlightCopyMigration` + // or `Event.InFlightReuseMigration`. logger.info("Registering device constellation observer") account.deviceConstellation().register(deviceEventsIntegration) diff --git a/mobile/android/android-components/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt b/mobile/android/android-components/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt index e36ed4fb31b4..01ee33cccbe1 100644 --- a/mobile/android/android-components/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt +++ b/mobile/android/android-components/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt @@ -602,6 +602,8 @@ class FxaAccountManagerTest { assertNull(account.persistenceCallback) manager.initAsync().await() + // Make sure a persistence callback was registered while pumping the state machine. + assertNotNull(account.persistenceCallback) // Assert that neither ensureCapabilities nor initialization fired. verify(constellation, never()).ensureCapabilitiesAsync(setOf(DeviceCapability.SEND_TAB)) @@ -638,6 +640,8 @@ class FxaAccountManagerTest { assertNull(account.persistenceCallback) manager.initAsync().await() + // Make sure a persistence callback was registered while pumping the state machine. + assertNotNull(account.persistenceCallback) verify(constellation).ensureCapabilitiesAsync(setOf(DeviceCapability.SEND_TAB)) verify(constellation, never()).initDeviceAsync(any(), any(), any()) -- 2.11.4.GIT