From 4f5454b4b7a0d51109f3028266630f13d8786980 Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 27 Jun 2024 13:56:55 -0500 Subject: [PATCH] Fix a screen capture bug that clears the setting when the app language changes (#3372) --- .../java/com/x8bit/bitwarden/MainActivity.kt | 22 ++++----- .../java/com/x8bit/bitwarden/MainViewModel.kt | 26 ++++++---- .../com/x8bit/bitwarden/MainViewModelTest.kt | 47 +++++++------------ 3 files changed, 43 insertions(+), 52 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/MainActivity.kt b/app/src/main/java/com/x8bit/bitwarden/MainActivity.kt index 6463ca58d..d696bfce1 100644 --- a/app/src/main/java/com/x8bit/bitwarden/MainActivity.kt +++ b/app/src/main/java/com/x8bit/bitwarden/MainActivity.kt @@ -66,6 +66,7 @@ class MainActivity : AppCompatActivity() { } setContent { val state by mainViewModel.stateFlow.collectAsStateWithLifecycle() + updateScreenCapture(isScreenCaptureAllowed = state.isScreenCaptureAllowed) LocalManagerProvider { BitwardenTheme(theme = state.theme) { RootNavScreen( @@ -97,15 +98,8 @@ class MainActivity : AppCompatActivity() { .eventFlow .onEach { event -> when (event) { - is MainEvent.CompleteAutofill -> { - handleCompleteAutofill(event) - } - + is MainEvent.CompleteAutofill -> handleCompleteAutofill(event) MainEvent.Recreate -> handleRecreate() - - is MainEvent.ScreenCaptureSettingChange -> { - handleScreenCaptureSettingChange(event) - } } } .launchIn(lifecycleScope) @@ -118,15 +112,15 @@ class MainActivity : AppCompatActivity() { ) } - private fun handleScreenCaptureSettingChange(event: MainEvent.ScreenCaptureSettingChange) { - if (event.isAllowed) { + private fun handleRecreate() { + recreate() + } + + private fun updateScreenCapture(isScreenCaptureAllowed: Boolean) { + if (isScreenCaptureAllowed) { window.clearFlags(WindowManager.LayoutParams.FLAG_SECURE) } else { window.addFlags(WindowManager.LayoutParams.FLAG_SECURE) } } - - private fun handleRecreate() { - recreate() - } } diff --git a/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt index bc4c59b56..487bade64 100644 --- a/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt @@ -50,8 +50,9 @@ class MainViewModel @Inject constructor( private val authRepository: AuthRepository, private val savedStateHandle: SavedStateHandle, ) : BaseViewModel( - MainState( + initialState = MainState( theme = settingsRepository.appTheme, + isScreenCaptureAllowed = settingsRepository.isScreenCaptureAllowed, ), ) { private var specialCircumstance: SpecialCircumstance? @@ -81,9 +82,8 @@ class MainViewModel @Inject constructor( settingsRepository .isScreenCaptureAllowedStateFlow - .onEach { isAllowed -> - sendEvent(MainEvent.ScreenCaptureSettingChange(isAllowed)) - } + .map { MainAction.Internal.ScreenCaptureUpdate(it) } + .onEach(::trySendAction) .launchIn(viewModelScope) authRepository @@ -124,6 +124,7 @@ class MainViewModel @Inject constructor( } is MainAction.Internal.CurrentUserStateChange -> handleCurrentUserStateChange() + is MainAction.Internal.ScreenCaptureUpdate -> handleScreenCaptureUpdate(action) is MainAction.Internal.ThemeUpdate -> handleAppThemeUpdated(action) is MainAction.Internal.VaultUnlockStateChange -> handleVaultUnlockStateChange() is MainAction.ReceiveFirstIntent -> handleFirstIntentReceived(action) @@ -141,6 +142,10 @@ class MainViewModel @Inject constructor( recreateUiAndGarbageCollect() } + private fun handleScreenCaptureUpdate(action: MainAction.Internal.ScreenCaptureUpdate) { + mutableStateFlow.update { it.copy(isScreenCaptureAllowed = action.isScreenCaptureEnabled) } + } + private fun handleAppThemeUpdated(action: MainAction.Internal.ThemeUpdate) { mutableStateFlow.update { it.copy(theme = action.theme) } } @@ -249,6 +254,7 @@ class MainViewModel @Inject constructor( @Parcelize data class MainState( val theme: AppTheme, + val isScreenCaptureAllowed: Boolean, ) : Parcelable /** @@ -281,6 +287,13 @@ sealed class MainAction { */ data object CurrentUserStateChange : Internal() + /** + * Indicates that the screen capture state has changed. + */ + data class ScreenCaptureUpdate( + val isScreenCaptureEnabled: Boolean, + ) : Internal() + /** * Indicates that the app theme has changed. */ @@ -305,11 +318,6 @@ sealed class MainEvent { */ data class CompleteAutofill(val cipherView: CipherView) : MainEvent() - /** - * Event indicating a change in the screen capture setting. - */ - data class ScreenCaptureSettingChange(val isAllowed: Boolean) : MainEvent() - /** * Event indicating that the UI should recreate itself. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt index edbf11c08..f4ee9bed6 100644 --- a/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt @@ -59,6 +59,7 @@ class MainViewModelTest : BaseViewModelTest() { private val settingsRepository = mockk { every { appTheme } returns AppTheme.DEFAULT every { appThemeStateFlow } returns mutableAppThemeFlow + every { isScreenCaptureAllowed } returns true every { isScreenCaptureAllowedStateFlow } returns mutableScreenCaptureAllowedFlow } private val authRepository = mockk { @@ -129,9 +130,6 @@ class MainViewModelTest : BaseViewModelTest() { val viewModel = createViewModel() viewModel.eventFlow.test { - // Ignore initial screen capture event - awaitItem() - mutableUserStateFlow.value = UserState( activeUserId = userId1, accounts = listOf( @@ -179,9 +177,6 @@ class MainViewModelTest : BaseViewModelTest() { val viewModel = createViewModel() viewModel.eventFlow.test { - // Ignore initial screen capture event - awaitItem() - mutableVaultStateEventFlow.tryEmit(VaultStateEvent.Unlocked(userId = "userId")) expectNoEvents() @@ -198,9 +193,6 @@ class MainViewModelTest : BaseViewModelTest() { val viewModel = createViewModel() val cipherView = mockk() viewModel.eventFlow.test { - // Ignore initial screen capture event - awaitItem() - autofillSelectionManager.emitAutofillSelection(cipherView = cipherView) assertEquals( MainEvent.CompleteAutofill(cipherView = cipherView), @@ -229,9 +221,7 @@ class MainViewModelTest : BaseViewModelTest() { val viewModel = createViewModel() assertEquals( - MainState( - theme = AppTheme.DEFAULT, - ), + DEFAULT_STATE, viewModel.stateFlow.value, ) viewModel.trySendAction( @@ -240,7 +230,7 @@ class MainViewModelTest : BaseViewModelTest() { ), ) assertEquals( - MainState( + DEFAULT_STATE.copy( theme = AppTheme.DARK, ), viewModel.stateFlow.value, @@ -623,25 +613,19 @@ class MainViewModelTest : BaseViewModelTest() { ) } - @Suppress("MaxLineLength") @Test - fun `changes in the allowed screen capture value should result in emissions of ScreenCaptureSettingChange `() = - runTest { - val viewModel = createViewModel() + fun `changes in the allowed screen capture value should update the state`() { + val viewModel = createViewModel() - viewModel.eventFlow.test { - assertEquals( - MainEvent.ScreenCaptureSettingChange(isAllowed = true), - awaitItem(), - ) + assertEquals(DEFAULT_STATE, viewModel.stateFlow.value) - mutableScreenCaptureAllowedFlow.value = false - assertEquals( - MainEvent.ScreenCaptureSettingChange(isAllowed = false), - awaitItem(), - ) - } - } + mutableScreenCaptureAllowedFlow.value = false + + assertEquals( + DEFAULT_STATE.copy(isScreenCaptureAllowed = false), + viewModel.stateFlow.value, + ) + } private fun createViewModel( initialSpecialCircumstance: SpecialCircumstance? = null, @@ -659,6 +643,11 @@ class MainViewModelTest : BaseViewModelTest() { ) } +private val DEFAULT_STATE: MainState = MainState( + theme = AppTheme.DEFAULT, + isScreenCaptureAllowed = true, +) + private const val SPECIAL_CIRCUMSTANCE_KEY: String = "special-circumstance" private val DEFAULT_ACCOUNT = UserState.Account( userId = "activeUserId",