For mozilla-mobile#27381 - Unregister FXA observer to prevent memory …

…leaks
MatthewTighe · Oct 12, 2022 · 09b11da · 09b11da
1 parent 8ee0a8a
commit 09b11da
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 8 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt
@@ -67,7 +67,8 @@ class SettingsFragment : PreferenceFragmentCompat() {
     private lateinit var accountUiView: AccountUiView
     private val profilerViewModel: ProfilerViewModel by activityViewModels()
 
-    private val accountObserver = object : AccountObserver {
+    @VisibleForTesting
+    internal val accountObserver = object : AccountObserver {
         private fun updateAccountUi(profile: Profile? = null) {
             val context = context ?: return
             lifecycleScope.launch {
@@ -101,13 +102,6 @@ class SettingsFragment : PreferenceFragmentCompat() {
             updateFxAAllowDomesticChinaServerMenu = ::updateFxAAllowDomesticChinaServerMenu,
         )
 
-        // Observe account changes to keep the UI up-to-date.
-        requireComponents.backgroundServices.accountManager.register(
-            accountObserver,
-            owner = this,
-            autoPause = true,
-        )
-
         // It's important to update the account UI state in onCreate since that ensures we'll never
         // display an incorrect state in the UI. We take care to not also call it as part of onResume
         // if it was just called here (via the 'creatingFragment' flag).
@@ -186,6 +180,23 @@ class SettingsFragment : PreferenceFragmentCompat() {
         creatingFragment = false
     }
 
+    override fun onStart() {
+        super.onStart()
+        // Observe account changes to keep the UI up-to-date.
+        requireComponents.backgroundServices.accountManager.register(
+            accountObserver,
+            owner = this,
+            autoPause = true,
+        )
+    }
+
+    override fun onStop() {
+        super.onStop()
+        // If the screen isn't visible we don't need to show updates.
+        // Also prevent the observer registered to the FXA singleton causing memory leaks.
+        requireComponents.backgroundServices.accountManager.unregister(accountObserver)
+    }
+
     override fun onDestroyView() {
         super.onDestroyView()
         accountUiView.cancel()

diff --git a/app/src/test/java/org/mozilla/fenix/settings/SettingsFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/settings/SettingsFragmentTest.kt
@@ -10,8 +10,10 @@ import io.mockk.every
 import io.mockk.mockk
 import io.mockk.mockkObject
 import io.mockk.unmockkObject
+import io.mockk.verify
 import kotlinx.coroutines.test.advanceUntilIdle
 import mozilla.components.concept.fetch.Client
+import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.support.test.robolectric.testContext
 import mozilla.components.support.test.rule.MainCoroutineRule
 import mozilla.components.support.test.rule.runTestOnMain
@@ -177,6 +179,26 @@ class SettingsFragmentTest {
         assertEquals(summary, httpsOnlyPreference.summary)
     }
 
+    @Test
+    fun `GIVEN an account observer WHEN the fragment is visible THEN register it for updates`() {
+        val accountManager: FxaAccountManager = mockk(relaxed = true)
+        every { testContext.components.backgroundServices.accountManager } returns accountManager
+
+        settingsFragment.onStart()
+
+        verify { accountManager.register(settingsFragment.accountObserver, settingsFragment, true) }
+    }
+
+    @Test
+    fun `GIVEN an account observer WHEN the fragment stops being visible THEN unregister it for updates`() {
+        val accountManager: FxaAccountManager = mockk(relaxed = true)
+        every { testContext.components.backgroundServices.accountManager } returns accountManager
+
+        settingsFragment.onStop()
+
+        verify { accountManager.unregister(settingsFragment.accountObserver) }
+    }
+
     @After
     fun tearDown() {
         unmockkObject(FeatureFlags)