From ee8b9563a3f31597c34cd9a5d67a1ee96ed9417f Mon Sep 17 00:00:00 2001
From: Lucas Kivi <125697099+lucas-livefront@users.noreply.github.com>
Date: Wed, 24 Jan 2024 14:03:26 -0600
Subject: [PATCH] Lazily determine autofill setting (#753)

---
 .../platform/repository/SettingsRepository.kt |  3 ++
 .../repository/SettingsRepositoryImpl.kt      | 13 ++++++--
 .../repository/SettingsRepositoryTest.kt      | 31 +++++++++++++------
 3 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt
index e2e53c91e9..9b622c9cd1 100644
--- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt
@@ -87,6 +87,9 @@ interface SettingsRepository {
 
     /**
      * Emits updates whenever there is a change in the app's status for supporting autofill.
+     *
+     * Note that the correct value is only populated upon subscription so calling [StateFlow.value]
+     * may result in an out-of-date value.
      */
     val isAutofillEnabledStateFlow: StateFlow<Boolean>
 
diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt
index bab19a0bf0..f2346aeb2d 100644
--- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt
@@ -12,10 +12,13 @@ import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource
 import com.x8bit.bitwarden.ui.platform.feature.settings.appearance.model.AppLanguage
 import com.x8bit.bitwarden.ui.platform.feature.settings.appearance.model.AppTheme
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
+import kotlinx.coroutines.flow.filter
+import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
@@ -326,9 +329,15 @@ class SettingsRepositoryImpl(
         }
     }
 
+    @OptIn(ExperimentalCoroutinesApi::class)
     private fun observeAutofillEnabledChanges() {
-        appForegroundManager
-            .appForegroundStateFlow
+        mutableIsAutofillEnabledStateFlow
+            // Only observe when subscribed to.
+            .subscriptionCount.map { it > 0 }
+            .filter { hasSubscribers -> hasSubscribers }
+            .flatMapLatest {
+                appForegroundManager.appForegroundStateFlow
+            }
             .onEach {
                 mutableIsAutofillEnabledStateFlow.value = isAutofillEnabledAndSupported
             }
diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt
index eec29a2602..6e9e33c922 100644
--- a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt
@@ -480,6 +480,15 @@ class SettingsRepositoryTest {
     @Test
     fun `isAutofillEnabledStateFlow should emit updates if necessary when the app foreground state changes and disableAutofill is called`() =
         runTest {
+            // Updates are not received without an isAutofillEnabledStateFlow subscriber.
+            isAutofillEnabledAndSupported = true
+            mutableAppForegroundStateFlow.value = AppForegroundState.FOREGROUNDED
+            assertFalse(settingsRepository.isAutofillEnabledStateFlow.value)
+
+            // Revert back to initial state.
+            isAutofillEnabledAndSupported = false
+            mutableAppForegroundStateFlow.value = AppForegroundState.BACKGROUNDED
+
             settingsRepository.isAutofillEnabledStateFlow.test {
                 assertFalse(awaitItem())
 
@@ -511,17 +520,21 @@ class SettingsRepositoryTest {
 
     @Suppress("MaxLineLength")
     @Test
-    fun `disableAutofill should trigger an emission of false from isAutofillEnabledStateFlow and disable autofill with the OS`() {
-        // Start in a state where autofill is enabled
-        isAutofillEnabledAndSupported = true
-        mutableAppForegroundStateFlow.value = AppForegroundState.FOREGROUNDED
-        assertTrue(settingsRepository.isAutofillEnabledStateFlow.value)
+    fun `disableAutofill should trigger an emission of false from isAutofillEnabledStateFlow and disable autofill with the OS`() =
+        runTest {
+            // Start in a state where autofill is enabled
+            isAutofillEnabledAndSupported = true
+            mutableAppForegroundStateFlow.value = AppForegroundState.FOREGROUNDED
+            settingsRepository.isAutofillEnabledStateFlow.test {
+                assertTrue(awaitItem())
+                expectNoEvents()
+            }
 
-        settingsRepository.disableAutofill()
+            settingsRepository.disableAutofill()
 
-        assertFalse(settingsRepository.isAutofillEnabledStateFlow.value)
-        verify { autofillManager.disableAutofillServices() }
-    }
+            assertFalse(settingsRepository.isAutofillEnabledStateFlow.value)
+            verify { autofillManager.disableAutofillServices() }
+        }
 
     @Test
     fun `getPullToRefreshEnabledFlow should react to changes in SettingsDiskSource`() = runTest {