From 04bcd35776c51fd2b3ab19685a795083cc4dad50 Mon Sep 17 00:00:00 2001 From: David Perez Date: Wed, 11 Mar 2026 16:26:58 -0500 Subject: [PATCH] PM-33411: bug: Defer early navigation until lifecycle is resumed (#6638) --- .../vaultunlock/VaultUnlockViewModel.kt | 4 +- .../feature/search/SearchViewModel.kt | 2 +- .../feature/settings/SettingsViewModel.kt | 4 +- .../VaultUnlockedNavBarViewModel.kt | 8 +- .../ui/vault/feature/vault/VaultViewModel.kt | 2 +- .../ui/auth/unlock/UnlockViewModel.kt | 4 +- .../ui/platform/base/BackgroundEvent.kt | 12 +- .../ui/platform/base/util/EventsEffect.kt | 33 +++- .../ui/platform/base/util/EventsEffectTest.kt | 174 ++++++++++++++++++ 9 files changed, 221 insertions(+), 22 deletions(-) create mode 100644 ui/src/test/kotlin/com/bitwarden/ui/platform/base/util/EventsEffectTest.kt diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt index cc92fc7ef4..1bd1dc9b54 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt @@ -4,8 +4,8 @@ import android.os.Parcelable import androidx.compose.ui.graphics.Color import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope -import com.bitwarden.ui.platform.base.BackgroundEvent import com.bitwarden.ui.platform.base.BaseViewModel +import com.bitwarden.ui.platform.base.DeferredBackgroundEvent import com.bitwarden.ui.platform.base.util.hexToColor import com.bitwarden.ui.platform.components.account.model.AccountSummary import com.bitwarden.ui.platform.components.account.util.initials @@ -541,7 +541,7 @@ sealed class VaultUnlockEvent { /** * Prompts the user for biometrics unlock. */ - data class PromptForBiometrics(val cipher: Cipher) : BackgroundEvent, VaultUnlockEvent() + data class PromptForBiometrics(val cipher: Cipher) : VaultUnlockEvent(), DeferredBackgroundEvent /** * Completes the get credentials request with an error response. diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt index 5025033c7e..89d0a7ebc8 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt @@ -1503,7 +1503,7 @@ sealed class SearchAction { */ data class SnackbarDataReceived( val data: BitwardenSnackbarData, - ) : Internal(), BackgroundEvent + ) : Internal() /** * Indicates a result for updating a cipher during the autofill-and-save process. diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/SettingsViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/SettingsViewModel.kt index 9204f519a1..d57984a730 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/SettingsViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/SettingsViewModel.kt @@ -4,8 +4,8 @@ import androidx.annotation.DrawableRes import androidx.compose.material3.Text import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope -import com.bitwarden.ui.platform.base.BackgroundEvent import com.bitwarden.ui.platform.base.BaseViewModel +import com.bitwarden.ui.platform.base.DeferredBackgroundEvent import com.bitwarden.ui.platform.resource.BitwardenDrawable import com.bitwarden.ui.platform.resource.BitwardenString import com.bitwarden.ui.util.Text @@ -170,7 +170,7 @@ sealed class SettingsEvent { /** * Navigate to the account security screen. */ - data object NavigateAccountSecurityShortcut : SettingsEvent(), BackgroundEvent + data object NavigateAccountSecurityShortcut : SettingsEvent(), DeferredBackgroundEvent /** * Navigate to the appearance screen. diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModel.kt index 640011b4d2..a370b909aa 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModel.kt @@ -2,8 +2,8 @@ package com.x8bit.bitwarden.ui.platform.feature.vaultunlockednavbar import androidx.annotation.StringRes import androidx.lifecycle.viewModelScope -import com.bitwarden.ui.platform.base.BackgroundEvent import com.bitwarden.ui.platform.base.BaseViewModel +import com.bitwarden.ui.platform.base.DeferredBackgroundEvent import com.bitwarden.ui.platform.resource.BitwardenString import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.auth.repository.model.UserState @@ -276,10 +276,10 @@ sealed class VaultUnlockedNavBarEvent { } /** - * Shortcut events should to be considered [BackgroundEvent] as they are fired - * outside of normal lifecycle aware events and should not be ignored by filter. + * Shortcut events should to be considered [DeferredBackgroundEvent] as they are fired + * outside normal lifecycle aware events and should not be ignored by filter. */ - sealed class Shortcut : VaultUnlockedNavBarEvent(), BackgroundEvent { + sealed class Shortcut : VaultUnlockedNavBarEvent(), DeferredBackgroundEvent { /** * Navigate to the Generator screen via a shortcut. */ diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt index c55a0b4c51..d8816c947b 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt @@ -2289,7 +2289,7 @@ sealed class VaultAction { */ data class SnackbarDataReceive( val data: BitwardenSnackbarData, - ) : Internal(), BackgroundEvent + ) : Internal() /** * Indicates that the flight recorder data was received. diff --git a/authenticator/src/main/kotlin/com/bitwarden/authenticator/ui/auth/unlock/UnlockViewModel.kt b/authenticator/src/main/kotlin/com/bitwarden/authenticator/ui/auth/unlock/UnlockViewModel.kt index 4918fa4540..cc38be379b 100644 --- a/authenticator/src/main/kotlin/com/bitwarden/authenticator/ui/auth/unlock/UnlockViewModel.kt +++ b/authenticator/src/main/kotlin/com/bitwarden/authenticator/ui/auth/unlock/UnlockViewModel.kt @@ -5,8 +5,8 @@ import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope import com.bitwarden.authenticator.data.auth.repository.AuthRepository import com.bitwarden.authenticator.data.platform.repository.model.BiometricsUnlockResult -import com.bitwarden.ui.platform.base.BackgroundEvent import com.bitwarden.ui.platform.base.BaseViewModel +import com.bitwarden.ui.platform.base.DeferredBackgroundEvent import com.bitwarden.ui.platform.resource.BitwardenString import com.bitwarden.ui.util.Text import com.bitwarden.ui.util.asText @@ -181,7 +181,7 @@ sealed class UnlockEvent { /** * Prompts the user for biometrics unlock. */ - data class PromptForBiometrics(val cipher: Cipher) : UnlockEvent(), BackgroundEvent + data class PromptForBiometrics(val cipher: Cipher) : UnlockEvent(), DeferredBackgroundEvent } /** diff --git a/ui/src/main/kotlin/com/bitwarden/ui/platform/base/BackgroundEvent.kt b/ui/src/main/kotlin/com/bitwarden/ui/platform/base/BackgroundEvent.kt index f90ebcf713..2021c2942f 100644 --- a/ui/src/main/kotlin/com/bitwarden/ui/platform/base/BackgroundEvent.kt +++ b/ui/src/main/kotlin/com/bitwarden/ui/platform/base/BackgroundEvent.kt @@ -1,8 +1,14 @@ package com.bitwarden.ui.platform.base /** - * Almost all the events in the app involve navigation or toasts. To prevent accidentally - * navigating to the same view twice, by default, events are ignored if the view is not currently - * resumed. To avoid that restriction, specific events can implement [BackgroundEvent]. + * Almost all the events in the app involve navigation. To prevent accidentally navigating to the + * same view twice, by default, events are ignored if the view is not currently resumed. + * To avoid that restriction, specific events can implement [BackgroundEvent]. */ interface BackgroundEvent + +/** + * This implementation of [BackgroundEvent] allows the event to not be filtered but will force it + * to wait until the view is resumed before consuming the event. + */ +interface DeferredBackgroundEvent : BackgroundEvent diff --git a/ui/src/main/kotlin/com/bitwarden/ui/platform/base/util/EventsEffect.kt b/ui/src/main/kotlin/com/bitwarden/ui/platform/base/util/EventsEffect.kt index 9244250672..af1b3ebe6f 100644 --- a/ui/src/main/kotlin/com/bitwarden/ui/platform/base/util/EventsEffect.kt +++ b/ui/src/main/kotlin/com/bitwarden/ui/platform/base/util/EventsEffect.kt @@ -6,16 +6,21 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.compose.LocalLifecycleOwner import com.bitwarden.ui.platform.base.BackgroundEvent import com.bitwarden.ui.platform.base.BaseViewModel +import com.bitwarden.ui.platform.base.DeferredBackgroundEvent import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch /** * Convenience method for observing event flow from [BaseViewModel]. * - * By default, events will only be consumed when the associated screen is - * resumed, to avoid bugs like duplicate navigation calls. To override - * this behavior, a given event type can implement [BackgroundEvent]. + * By default, events will only be consumed when the associated screen is resumed, to avoid bugs + * like duplicate navigation calls. To override this behavior, a given event type can implement + * [BackgroundEvent] which will allow the event to pass through regardless of the lifecycle state. + * Additionally, an event can implement [DeferredBackgroundEvent] which will not be filtered out + * based on the lifecycle but will be processed when the screen resumes. */ @Composable fun EventsEffect( @@ -24,12 +29,26 @@ fun EventsEffect( handler: (E) -> Unit, ) { LaunchedEffect(key1 = Unit) { - viewModel.eventFlow - .filter { - it is BackgroundEvent || + viewModel + .eventFlow + .filter { event -> + event is BackgroundEvent || lifecycleOwner.currentState.isAtLeast(Lifecycle.State.RESUMED) } - .onEach { handler.invoke(it) } + .onEach { event -> + if (event is DeferredBackgroundEvent) { + // Defer processing this event until the screen is resumed. This is launched + // in its own coroutine scope to allow other events to flow unimpeded. + launch { + lifecycleOwner + .currentStateFlow + .first { it.isAtLeast(Lifecycle.State.RESUMED) } + handler(event) + } + } else { + handler(event) + } + } .launchIn(this) } } diff --git a/ui/src/test/kotlin/com/bitwarden/ui/platform/base/util/EventsEffectTest.kt b/ui/src/test/kotlin/com/bitwarden/ui/platform/base/util/EventsEffectTest.kt new file mode 100644 index 0000000000..b654d34077 --- /dev/null +++ b/ui/src/test/kotlin/com/bitwarden/ui/platform/base/util/EventsEffectTest.kt @@ -0,0 +1,174 @@ +package com.bitwarden.ui.platform.base.util + +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleRegistry +import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow +import com.bitwarden.ui.platform.base.BackgroundEvent +import com.bitwarden.ui.platform.base.BaseComposeTest +import com.bitwarden.ui.platform.base.BaseViewModel +import com.bitwarden.ui.platform.base.DeferredBackgroundEvent +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.flow.MutableSharedFlow +import org.junit.Assert.assertEquals +import org.junit.Test + +class EventsEffectTest : BaseComposeTest() { + + private val mutableEventFlow: MutableSharedFlow = bufferedMutableSharedFlow() + private val viewModel: BaseViewModel = mockk { + every { eventFlow } returns mutableEventFlow + } + + @Test + fun `events are dispatched to handler when lifecycle is RESUMED`() { + val handledEvents = mutableListOf() + val lifecycle = createLifecycle(Lifecycle.State.RESUMED) + + setTestContent { + EventsEffect( + viewModel = viewModel, + lifecycleOwner = lifecycle, + handler = { handledEvents.add(it) }, + ) + } + + composeTestRule.runOnIdle { + mutableEventFlow.tryEmit(TestEvent.Regular) + } + composeTestRule.waitForIdle() + + assertEquals(listOf(TestEvent.Regular), handledEvents) + } + + @Test + fun `events are not dispatched to handler when lifecycle is not RESUMED`() { + val handledEvents = mutableListOf() + val lifecycle = createLifecycle(Lifecycle.State.STARTED) + + setTestContent { + EventsEffect( + viewModel = viewModel, + lifecycleOwner = lifecycle, + handler = { handledEvents.add(it) }, + ) + } + + composeTestRule.runOnIdle { + mutableEventFlow.tryEmit(TestEvent.Regular) + } + composeTestRule.waitForIdle() + + assertEquals(emptyList(), handledEvents) + } + + @Test + fun `BackgroundEvent is dispatched to handler even when lifecycle is not RESUMED`() { + val handledEvents = mutableListOf() + val lifecycle = createLifecycle(Lifecycle.State.STARTED) + + setTestContent { + EventsEffect( + viewModel = viewModel, + lifecycleOwner = lifecycle, + handler = { handledEvents.add(it) }, + ) + } + + composeTestRule.runOnIdle { + mutableEventFlow.tryEmit(TestEvent.Background) + } + composeTestRule.waitForIdle() + + assertEquals(listOf(TestEvent.Background), handledEvents) + } + + @Test + fun `DeferredBackgroundEvent is not dispatched until lifecycle transitions to RESUMED`() { + val handledEvents = mutableListOf() + val lifecycle = createLifecycle(Lifecycle.State.STARTED) + + setTestContent { + EventsEffect( + viewModel = viewModel, + lifecycleOwner = lifecycle, + handler = { handledEvents.add(it) }, + ) + } + + composeTestRule.runOnIdle { + mutableEventFlow.tryEmit(TestEvent.DeferredBackground) + } + composeTestRule.waitForIdle() + + assertEquals(emptyList(), handledEvents) + + composeTestRule.runOnIdle { + lifecycle.currentState = Lifecycle.State.RESUMED + } + composeTestRule.waitForIdle() + + assertEquals(listOf(TestEvent.DeferredBackground), handledEvents) + } + + @Test + fun `DeferredBackgroundEvent is dispatched immediately when lifecycle is already RESUMED`() { + val handledEvents = mutableListOf() + val lifecycle = createLifecycle(Lifecycle.State.RESUMED) + + setTestContent { + EventsEffect( + viewModel = viewModel, + lifecycleOwner = lifecycle, + handler = { handledEvents.add(it) }, + ) + } + + composeTestRule.runOnIdle { + mutableEventFlow.tryEmit(TestEvent.DeferredBackground) + } + composeTestRule.waitForIdle() + + assertEquals(listOf(TestEvent.DeferredBackground), handledEvents) + } + + @Test + fun `multiple events are dispatched in order when lifecycle is RESUMED`() { + val handledEvents = mutableListOf() + val lifecycle = createLifecycle(Lifecycle.State.RESUMED) + + setTestContent { + EventsEffect( + viewModel = viewModel, + lifecycleOwner = lifecycle, + handler = { handledEvents.add(it) }, + ) + } + + composeTestRule.runOnIdle { + mutableEventFlow.tryEmit(TestEvent.Regular) + mutableEventFlow.tryEmit(TestEvent.Background) + mutableEventFlow.tryEmit(TestEvent.DeferredBackground) + } + composeTestRule.waitForIdle() + + assertEquals( + listOf( + TestEvent.Regular, + TestEvent.Background, + TestEvent.DeferredBackground, + ), + handledEvents, + ) + } + + private fun createLifecycle( + state: Lifecycle.State, + ): LifecycleRegistry = LifecycleRegistry(mockk()).apply { currentState = state } +} + +private sealed class TestEvent { + data object Regular : TestEvent() + data object Background : TestEvent(), BackgroundEvent + data object DeferredBackground : TestEvent(), DeferredBackgroundEvent +}