diff --git a/components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenter.kt b/components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenter.kt index bd6b20e602c..4b474542b7f 100644 --- a/components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenter.kt +++ b/components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenter.kt @@ -81,7 +81,8 @@ internal class DefaultPresenter( } override fun stop() { - // no-op + accountManager.unregisterForSyncEvents(eventObserver) + accountManager.unregister(accountObserver) } companion object { diff --git a/components/feature/syncedtabs/src/test/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenterTest.kt b/components/feature/syncedtabs/src/test/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenterTest.kt index b43a89555f8..49423dde2cf 100644 --- a/components/feature/syncedtabs/src/test/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenterTest.kt +++ b/components/feature/syncedtabs/src/test/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenterTest.kt @@ -236,4 +236,20 @@ class DefaultPresenterTest { verify(view).onError(ErrorType.SYNC_ENGINE_UNAVAILABLE) } + + @Test + fun `GIVEN the presenter is started WHEN it is stopped THEN unregister the account and sync events observers`() { + val presenter = DefaultPresenter( + context, + controller, + accountManager, + view, + lifecycleOwner, + ) + + presenter.stop() + + verify(accountManager).unregisterForSyncEvents(presenter.eventObserver) + verify(accountManager).unregister(presenter.accountObserver) + } } diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt index 538c349ad88..65b45229496 100644 --- a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt @@ -178,7 +178,9 @@ open class FxaAccountManager( @VisibleForTesting val accountEventObserverRegistry = ObserverRegistry() - private val syncStatusObserverRegistry = ObserverRegistry() + + @VisibleForTesting + open val syncStatusObserverRegistry = ObserverRegistry() // We always obtain a "profile" scope, as that's assumed to be needed for any application integration. // We obtain a sync scope only if this was requested by the application via SyncConfig. @@ -487,6 +489,14 @@ open class FxaAccountManager( syncStatusObserverRegistry.register(observer, owner, autoPause) } + /** + * Unregister a [SyncStatusObserver] from being informed about "sync lifecycle" events. + * The method is safe to call even if the provided observer was not registered before. + */ + fun unregisterForSyncEvents(observer: SyncStatusObserver) { + syncStatusObserverRegistry.unregister(observer) + } + override fun close() { GlobalAccountManager.close() coroutineContext.cancel() diff --git a/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt b/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt index 7a8fd1b286e..c8354ccffca 100644 --- a/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt +++ b/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt @@ -5,6 +5,7 @@ package mozilla.components.service.fxa import android.content.Context +import androidx.lifecycle.LifecycleOwner import androidx.test.ext.junit.runners.AndroidJUnit4 import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -99,6 +100,9 @@ internal open class TestableFxaAccountManager( } ) : FxaAccountManager(context, config, DeviceConfig("test", DeviceType.UNKNOWN, capabilities), syncConfig, emptySet(), crashReporter, coroutineContext) { private val testableStorageWrapper = TestableStorageWrapper(this, accountEventObserverRegistry, serverConfig, block) + + override var syncStatusObserverRegistry = ObserverRegistry() + override fun getStorageWrapper(): StorageWrapper { return testableStorageWrapper } @@ -1734,6 +1738,45 @@ class FxaAccountManagerTest { verifyNoMoreInteractions(syncManager) } + @Test + fun `GIVEN a sync observer WHEN registering it THEN add it to the sync observer registry`() { + val fxaManager = TestableFxaAccountManager( + context = testContext, + config = mock(), + storage = mock(), + capabilities = setOf(DeviceCapability.SEND_TAB), + syncConfig = null, + coroutineContext = mock(), + ) + fxaManager.syncStatusObserverRegistry = mock() + val observer: SyncStatusObserver = mock() + val lifecycleOwner: LifecycleOwner = mock() + + fxaManager.registerForSyncEvents(observer, lifecycleOwner, false) + + verify(fxaManager.syncStatusObserverRegistry).register(observer, lifecycleOwner, false) + verifyNoMoreInteractions(fxaManager.syncStatusObserverRegistry) + } + + @Test + fun `GIVEN a sync observer WHEN unregistering it THEN remove it from the sync observer registry`() { + val fxaManager = TestableFxaAccountManager( + context = testContext, + config = mock(), + storage = mock(), + capabilities = setOf(DeviceCapability.SEND_TAB), + syncConfig = null, + coroutineContext = mock(), + ) + fxaManager.syncStatusObserverRegistry = mock() + val observer: SyncStatusObserver = mock() + + fxaManager.unregisterForSyncEvents(observer) + + verify(fxaManager.syncStatusObserverRegistry).unregister(observer) + verifyNoMoreInteractions(fxaManager.syncStatusObserverRegistry) + } + private suspend fun prepareHappyAuthenticationFlow( mockAccount: OAuthAccount, profile: Profile, diff --git a/docs/changelog.md b/docs/changelog.md index 32785959e3b..18f30780925 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -11,6 +11,9 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/main/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/main/.config.yml) +* **feature-syncedtabs** + * 🚒 Bug fixed [issue #12930](https://github.com/mozilla-mobile/android-components/issues/12930) Ensure `DefaultPresenter` will unregister it's `FxaAccountManager` observers when it's `lifecycleOwner` is stopped to prevent memory leaks. + * **feature-app-links** * 🚒 Bug fixed [issue #12804](https://github.com/mozilla-mobile/android-components/issues/12804) Speculative fix for a TransactionTooLargeException or RuntimeException when querying activities.