From 1aef022bf6927c73fe828e477d781976843ae82c Mon Sep 17 00:00:00 2001 From: Helen Qin Date: Tue, 6 Dec 2022 02:29:49 +0000 Subject: [PATCH] Fix race condition between provider ui launching and credman ui update. Hide CredMan UI has to happen before provider UI is launched. This fix ensures this order. Bug: 261229077 Test: local deployment Change-Id: I7e7ef275f119386e833b6762801e3ee9eed6a423 --- .../CredentialSelectorActivity.kt | 2 ++ .../createflow/CreateCredentialComponents.kt | 20 +++++++--------- .../createflow/CreateCredentialViewModel.kt | 23 +++++++++++-------- .../getflow/GetCredentialComponents.kt | 9 ++++---- .../getflow/GetCredentialViewModel.kt | 21 +++++++++++------ 5 files changed, 42 insertions(+), 33 deletions(-) diff --git a/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorActivity.kt b/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorActivity.kt index d324f875c85a5..6a4c599f682ba 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorActivity.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorActivity.kt @@ -69,6 +69,7 @@ class CredentialSelectorActivity : ComponentActivity() { ) providerActivityResult.value?.let { viewModel.onProviderActivityResult(it) + providerActivityResult.value = null } CreateCredentialScreen(viewModel = viewModel, providerActivityLauncher = launcher) } @@ -80,6 +81,7 @@ class CredentialSelectorActivity : ComponentActivity() { ) providerActivityResult.value?.let { viewModel.onProviderActivityResult(it) + providerActivityResult.value = null } GetCredentialScreen(viewModel = viewModel, providerActivityLauncher = launcher) } diff --git a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialComponents.kt b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialComponents.kt index 5efedd1a88634..57e20beb62639 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialComponents.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialComponents.kt @@ -60,12 +60,6 @@ fun CreateCredentialScreen( viewModel: CreateCredentialViewModel, providerActivityLauncher: ManagedActivityResultLauncher ) { - val selectEntryCallback: (EntryInfo) -> Unit = { - viewModel.onEntrySelected(it, providerActivityLauncher) - } - val confirmEntryCallback: () -> Unit = { - viewModel.onConfirmEntrySelected(providerActivityLauncher) - } val state = rememberModalBottomSheetState( initialValue = ModalBottomSheetValue.Expanded, skipHalfExpanded = true @@ -89,7 +83,7 @@ fun CreateCredentialScreen( onOptionSelected = viewModel::onEntrySelectedFromFirstUseScreen, onDisabledPasswordManagerSelected = viewModel::onDisabledPasswordManagerSelected, - onRemoteEntrySelected = selectEntryCallback, + onRemoteEntrySelected = viewModel::onEntrySelected, ) CreateScreenState.CREATION_OPTION_SELECTION -> CreationSelectionCard( requestDisplayInfo = uiState.requestDisplayInfo, @@ -97,8 +91,8 @@ fun CreateCredentialScreen( providerInfo = uiState.activeEntry?.activeProvider!!, createOptionInfo = uiState.activeEntry.activeEntryInfo as CreateOptionInfo, showActiveEntryOnly = uiState.showActiveEntryOnly, - onOptionSelected = selectEntryCallback, - onConfirm = confirmEntryCallback, + onOptionSelected = viewModel::onEntrySelected, + onConfirm = viewModel::onConfirmEntrySelected, onCancel = viewModel::onCancel, onMoreOptionsSelected = viewModel::onMoreOptionsSelected, ) @@ -110,7 +104,7 @@ fun CreateCredentialScreen( onOptionSelected = viewModel::onEntrySelectedFromMoreOptionScreen, onDisabledPasswordManagerSelected = viewModel::onDisabledPasswordManagerSelected, - onRemoteEntrySelected = selectEntryCallback, + onRemoteEntrySelected = viewModel::onEntrySelected, ) CreateScreenState.MORE_OPTIONS_ROW_INTRO -> MoreOptionsRowIntroCard( providerInfo = uiState.activeEntry?.activeProvider!!, @@ -119,11 +113,13 @@ fun CreateCredentialScreen( CreateScreenState.EXTERNAL_ONLY_SELECTION -> ExternalOnlySelectionCard( requestDisplayInfo = uiState.requestDisplayInfo, activeRemoteEntry = uiState.activeEntry?.activeEntryInfo!!, - onOptionSelected = selectEntryCallback, - onConfirm = confirmEntryCallback, + onOptionSelected = viewModel::onEntrySelected, + onConfirm = viewModel::onConfirmEntrySelected, onCancel = viewModel::onCancel, ) } + } else if (uiState.hidden && uiState.selectedEntry != null) { + viewModel.launchProviderUi(providerActivityLauncher) } }, scrimColor = MaterialTheme.colorScheme.scrim.copy(alpha = 0.8f), diff --git a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt index f81f08babdac0..6f749988308ac 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt @@ -130,10 +130,7 @@ class CreateCredentialViewModel( // TODO: implement the if choose as default or not logic later } - fun onEntrySelected( - selectedEntry: EntryInfo, - launcher: ManagedActivityResultLauncher - ) { + fun onEntrySelected(selectedEntry: EntryInfo) { val providerId = selectedEntry.providerId val entryKey = selectedEntry.entryKey val entrySubkey = selectedEntry.entrySubkey @@ -145,9 +142,6 @@ class CreateCredentialViewModel( selectedEntry = selectedEntry, hidden = true, ) - val intentSenderRequest = IntentSenderRequest.Builder(selectedEntry.pendingIntent) - .setFillInIntent(selectedEntry.fillInIntent).build() - launcher.launch(intentSenderRequest) } else { CredentialManagerRepo.getInstance().onOptionSelected( providerId, @@ -160,12 +154,23 @@ class CreateCredentialViewModel( } } - fun onConfirmEntrySelected( + fun launchProviderUi( launcher: ManagedActivityResultLauncher ) { + val entry = uiState.selectedEntry + if (entry != null && entry.pendingIntent != null) { + val intentSenderRequest = IntentSenderRequest.Builder(entry.pendingIntent) + .setFillInIntent(entry.fillInIntent).build() + launcher.launch(intentSenderRequest) + } else { + Log.w("Account Selector", "No provider UI to launch") + } + } + + fun onConfirmEntrySelected() { val selectedEntry = uiState.activeEntry?.activeEntryInfo if (selectedEntry != null) { - onEntrySelected(selectedEntry, launcher) + onEntrySelected(selectedEntry) } else { Log.w("Account Selector", "Illegal state: confirm is pressed but activeEntry isn't set.") diff --git a/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialComponents.kt b/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialComponents.kt index 7f65865b3b5e9..21342a15aca58 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialComponents.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialComponents.kt @@ -71,9 +71,6 @@ fun GetCredentialScreen( viewModel: GetCredentialViewModel, providerActivityLauncher: ManagedActivityResultLauncher ) { - val entrySelectionCallback: (EntryInfo) -> Unit = { - viewModel.onEntrySelected(it, providerActivityLauncher) - } val state = rememberModalBottomSheetState( initialValue = ModalBottomSheetValue.Expanded, skipHalfExpanded = true @@ -89,17 +86,19 @@ fun GetCredentialScreen( GetScreenState.PRIMARY_SELECTION -> PrimarySelectionCard( requestDisplayInfo = uiState.requestDisplayInfo, providerDisplayInfo = uiState.providerDisplayInfo, - onEntrySelected = entrySelectionCallback, + onEntrySelected = viewModel::onEntrySelected, onCancel = viewModel::onCancel, onMoreOptionSelected = viewModel::onMoreOptionSelected, ) GetScreenState.ALL_SIGN_IN_OPTIONS -> AllSignInOptionCard( providerInfoList = uiState.providerInfoList, providerDisplayInfo = uiState.providerDisplayInfo, - onEntrySelected = entrySelectionCallback, + onEntrySelected = viewModel::onEntrySelected, onBackButtonClicked = viewModel::onBackToPrimarySelectionScreen, ) } + } else if (uiState.hidden && uiState.selectedEntry != null) { + viewModel.launchProviderUi(providerActivityLauncher) } }, scrimColor = MaterialTheme.colorScheme.scrim.copy(alpha = 0.8f), diff --git a/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialViewModel.kt b/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialViewModel.kt index c64dae3d5a132..33e7021033893 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialViewModel.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialViewModel.kt @@ -58,10 +58,7 @@ class GetCredentialViewModel( return dialogResult } - fun onEntrySelected( - entry: EntryInfo, - launcher: ManagedActivityResultLauncher - ) { + fun onEntrySelected(entry: EntryInfo) { Log.d("Account Selector", "credential selected:" + " {provider=${entry.providerId}, key=${entry.entryKey}, subkey=${entry.entrySubkey}}") if (entry.pendingIntent != null) { @@ -69,9 +66,6 @@ class GetCredentialViewModel( selectedEntry = entry, hidden = true, ) - val intentSenderRequest = IntentSenderRequest.Builder(entry.pendingIntent) - .setFillInIntent(entry.fillInIntent).build() - launcher.launch(intentSenderRequest) } else { CredentialManagerRepo.getInstance().onOptionSelected( entry.providerId, entry.entryKey, entry.entrySubkey, @@ -80,6 +74,19 @@ class GetCredentialViewModel( } } + fun launchProviderUi( + launcher: ManagedActivityResultLauncher + ) { + val entry = uiState.selectedEntry + if (entry != null && entry.pendingIntent != null) { + val intentSenderRequest = IntentSenderRequest.Builder(entry.pendingIntent) + .setFillInIntent(entry.fillInIntent).build() + launcher.launch(intentSenderRequest) + } else { + Log.w("Account Selector", "No provider UI to launch") + } + } + fun onProviderActivityResult(providerActivityResult: ProviderActivityResult) { val entry = uiState.selectedEntry val resultCode = providerActivityResult.resultCode