From b829ba97bc35209677aeb4dee265cced5fc54845 Mon Sep 17 00:00:00 2001 From: Joe Bolinger Date: Thu, 6 Jul 2023 00:30:10 +0000 Subject: Prioritize fingerprint errors for coex. Bug: 287011195 Test: atest PromptViewModelTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f9a99625e4ff4b657b72604265a9f7c85e4100c1) Merged-In: Ie148ef7fb74ba4546f2c871592a3ef4c2bb8dae6 Change-Id: Ie148ef7fb74ba4546f2c871592a3ef4c2bb8dae6 --- .../biometrics/domain/model/BiometricModality.kt | 35 --------- .../biometrics/shared/model/BiometricModality.kt | 35 +++++++++ .../biometrics/ui/binder/BiometricViewBinder.kt | 25 +++--- .../biometrics/ui/viewmodel/PromptAuthState.kt | 2 +- .../biometrics/ui/viewmodel/PromptMessage.kt | 6 +- .../biometrics/ui/viewmodel/PromptViewModel.kt | 26 +++---- .../biometrics/ui/viewmodel/PromptAuthStateTest.kt | 2 +- .../biometrics/ui/viewmodel/PromptViewModelTest.kt | 90 +++++++++++++++++++--- 8 files changed, 142 insertions(+), 79 deletions(-) delete mode 100644 packages/SystemUI/src/com/android/systemui/biometrics/domain/model/BiometricModality.kt create mode 100644 packages/SystemUI/src/com/android/systemui/biometrics/shared/model/BiometricModality.kt diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/domain/model/BiometricModality.kt b/packages/SystemUI/src/com/android/systemui/biometrics/domain/model/BiometricModality.kt deleted file mode 100644 index 3197c0935d0b..000000000000 --- a/packages/SystemUI/src/com/android/systemui/biometrics/domain/model/BiometricModality.kt +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.systemui.biometrics.domain.model - -import android.hardware.biometrics.BiometricAuthenticator - -/** Shadows [BiometricAuthenticator.Modality] for Kotlin use within SysUI. */ -enum class BiometricModality { - None, - Fingerprint, - Face, -} - -/** Convert a framework [BiometricAuthenticator.Modality] to a SysUI [BiometricModality]. */ -@BiometricAuthenticator.Modality -fun Int.asBiometricModality(): BiometricModality = - when (this) { - BiometricAuthenticator.TYPE_FINGERPRINT -> BiometricModality.Fingerprint - BiometricAuthenticator.TYPE_FACE -> BiometricModality.Face - else -> BiometricModality.None - } diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/shared/model/BiometricModality.kt b/packages/SystemUI/src/com/android/systemui/biometrics/shared/model/BiometricModality.kt new file mode 100644 index 000000000000..fb580ca54aff --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/biometrics/shared/model/BiometricModality.kt @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.biometrics.shared.model + +import android.hardware.biometrics.BiometricAuthenticator + +/** Shadows [BiometricAuthenticator.Modality] for Kotlin use within SysUI. */ +enum class BiometricModality { + None, + Fingerprint, + Face, +} + +/** Convert a framework [BiometricAuthenticator.Modality] to a SysUI [BiometricModality]. */ +@BiometricAuthenticator.Modality +fun Int.asBiometricModality(): BiometricModality = + when (this) { + BiometricAuthenticator.TYPE_FINGERPRINT -> BiometricModality.Fingerprint + BiometricAuthenticator.TYPE_FACE -> BiometricModality.Face + else -> BiometricModality.None + } diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/ui/binder/BiometricViewBinder.kt b/packages/SystemUI/src/com/android/systemui/biometrics/ui/binder/BiometricViewBinder.kt index 64df6a03001d..e5a4d1a644f1 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/ui/binder/BiometricViewBinder.kt +++ b/packages/SystemUI/src/com/android/systemui/biometrics/ui/binder/BiometricViewBinder.kt @@ -46,9 +46,9 @@ import com.android.systemui.biometrics.AuthIconController import com.android.systemui.biometrics.AuthPanelController import com.android.systemui.biometrics.Utils import com.android.systemui.biometrics.domain.model.BiometricModalities -import com.android.systemui.biometrics.domain.model.BiometricModality -import com.android.systemui.biometrics.domain.model.asBiometricModality +import com.android.systemui.biometrics.shared.model.BiometricModality import com.android.systemui.biometrics.shared.model.PromptKind +import com.android.systemui.biometrics.shared.model.asBiometricModality import com.android.systemui.biometrics.ui.BiometricPromptLayout import com.android.systemui.biometrics.ui.viewmodel.FingerprintStartMode import com.android.systemui.biometrics.ui.viewmodel.PromptMessage @@ -396,7 +396,6 @@ private class Spaghetti( private var lifecycleScope: CoroutineScope? = null private var modalities: BiometricModalities = BiometricModalities() - private var faceFailedAtLeastOnce = false private var legacyCallback: Callback? = null override var legacyIconController: AuthIconController? = null @@ -476,19 +475,15 @@ private class Spaghetti( viewModel.ensureFingerprintHasStarted(isDelayed = true) applicationScope.launch { - val suppress = - modalities.hasFaceAndFingerprint && - (failedModality == BiometricModality.Face) && - faceFailedAtLeastOnce - if (failedModality == BiometricModality.Face) { - faceFailedAtLeastOnce = true - } - viewModel.showTemporaryError( failureReason, messageAfterError = modalities.asDefaultHelpMessage(applicationContext), authenticateAfterError = modalities.hasFingerprint, - suppressIfErrorShowing = suppress, + suppressIf = { currentMessage -> + modalities.hasFaceAndFingerprint && + failedModality == BiometricModality.Face && + currentMessage.isError + }, failedModality = failedModality, ) } @@ -501,11 +496,10 @@ private class Spaghetti( } applicationScope.launch { - val suppress = - modalities.hasFaceAndFingerprint && (errorModality == BiometricModality.Face) viewModel.showTemporaryError( error, - suppressIfErrorShowing = suppress, + messageAfterError = modalities.asDefaultHelpMessage(applicationContext), + authenticateAfterError = modalities.hasFingerprint, ) delay(BiometricPrompt.HIDE_DIALOG_DELAY.toLong()) legacyCallback?.onAction(Callback.ACTION_ERROR) @@ -522,6 +516,7 @@ private class Spaghetti( viewModel.showTemporaryError( help, messageAfterError = modalities.asDefaultHelpMessage(applicationContext), + authenticateAfterError = modalities.hasFingerprint, hapticFeedback = false, ) } diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptAuthState.kt b/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptAuthState.kt index 444082ca2742..2f9557f70a32 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptAuthState.kt +++ b/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptAuthState.kt @@ -16,7 +16,7 @@ package com.android.systemui.biometrics.ui.viewmodel -import com.android.systemui.biometrics.domain.model.BiometricModality +import com.android.systemui.biometrics.shared.model.BiometricModality /** * The authenticated state with the [authenticatedModality] (when [isAuthenticated]) with an diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptMessage.kt b/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptMessage.kt index 219da716f7d9..50f491142949 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptMessage.kt +++ b/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptMessage.kt @@ -33,9 +33,9 @@ sealed interface PromptMessage { else -> "" } - /** If this is an [Error] or [Help] message. */ - val isErrorOrHelp: Boolean - get() = this is Error || this is Help + /** If this is an [Error]. */ + val isError: Boolean + get() = this is Error /** An error message. */ data class Error(val errorMessage: String) : PromptMessage diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModel.kt b/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModel.kt index d63bf57013e5..34099a031ee6 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModel.kt @@ -20,7 +20,7 @@ import android.util.Log import com.android.systemui.biometrics.AuthBiometricView import com.android.systemui.biometrics.domain.interactor.PromptSelectorInteractor import com.android.systemui.biometrics.domain.model.BiometricModalities -import com.android.systemui.biometrics.domain.model.BiometricModality +import com.android.systemui.biometrics.shared.model.BiometricModality import com.android.systemui.biometrics.shared.model.PromptKind import com.android.systemui.statusbar.VibratorHelper import javax.inject.Inject @@ -210,35 +210,33 @@ constructor( * Show a temporary error [message] associated with an optional [failedModality] and play * [hapticFeedback]. * - * An optional [messageAfterError] will be shown via [showAuthenticating] when - * [authenticateAfterError] is set (or via [showHelp] when not set) after the error is - * dismissed. + * The [messageAfterError] will be shown via [showAuthenticating] when [authenticateAfterError] + * is set (or via [showHelp] when not set) after the error is dismissed. * - * The error is ignored if the user has already authenticated or if [suppressIfErrorShowing] is - * set and an error message is already showing. + * The error is ignored if the user has already authenticated or if [suppressIf] is true given + * the currently showing [PromptMessage]. */ suspend fun showTemporaryError( message: String, + messageAfterError: String, + authenticateAfterError: Boolean, + suppressIf: (PromptMessage) -> Boolean = { false }, hapticFeedback: Boolean = true, - messageAfterError: String = "", - authenticateAfterError: Boolean = false, - suppressIfErrorShowing: Boolean = false, failedModality: BiometricModality = BiometricModality.None, ) = coroutineScope { if (_isAuthenticated.value.isAuthenticated) { return@coroutineScope } - if (_message.value.isErrorOrHelp && suppressIfErrorShowing) { - if (_isAuthenticated.value.isNotAuthenticated) { - _canTryAgainNow.value = supportsRetry(failedModality) - } + + _canTryAgainNow.value = supportsRetry(failedModality) + + if (suppressIf(_message.value)) { return@coroutineScope } _isAuthenticating.value = false _isAuthenticated.value = PromptAuthState(false) _forceMediumSize.value = true - _canTryAgainNow.value = supportsRetry(failedModality) _message.value = PromptMessage.Error(message) _legacyState.value = AuthBiometricView.STATE_ERROR diff --git a/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptAuthStateTest.kt b/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptAuthStateTest.kt index fff1b81db628..278a43ea1bf1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptAuthStateTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptAuthStateTest.kt @@ -18,7 +18,7 @@ package com.android.systemui.biometrics.ui.viewmodel import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase -import com.android.systemui.biometrics.domain.model.BiometricModality +import com.android.systemui.biometrics.shared.model.BiometricModality import com.google.common.truth.Truth.assertThat import org.junit.Test import org.junit.runner.RunWith diff --git a/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModelTest.kt index 87c9e583af4d..27774ca51592 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModelTest.kt @@ -27,11 +27,12 @@ import com.android.systemui.biometrics.data.repository.FakePromptRepository import com.android.systemui.biometrics.domain.interactor.PromptSelectorInteractor import com.android.systemui.biometrics.domain.interactor.PromptSelectorInteractorImpl import com.android.systemui.biometrics.domain.model.BiometricModalities -import com.android.systemui.biometrics.domain.model.BiometricModality import com.android.systemui.biometrics.extractAuthenticatorTypes import com.android.systemui.biometrics.faceSensorPropertiesInternal import com.android.systemui.biometrics.fingerprintSensorPropertiesInternal +import com.android.systemui.biometrics.shared.model.BiometricModality import com.android.systemui.coroutines.collectLastValue +import com.android.systemui.coroutines.collectValues import com.android.systemui.statusbar.VibratorHelper import com.android.systemui.util.mockito.any import com.google.common.truth.Truth.assertThat @@ -48,7 +49,6 @@ import org.junit.runner.RunWith import org.junit.runners.Parameterized import org.mockito.Mock import org.mockito.Mockito.never -import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.junit.MockitoJUnit @@ -204,7 +204,12 @@ internal class PromptViewModelTest(private val testCase: TestCase) : SysuiTestCa @Test fun plays_haptic_on_errors() = runGenericTest { - viewModel.showTemporaryError("so sad", hapticFeedback = true) + viewModel.showTemporaryError( + "so sad", + messageAfterError = "", + authenticateAfterError = false, + hapticFeedback = true, + ) verify(vibrator).vibrateAuthError(any()) verify(vibrator, never()).vibrateAuthSuccess(any()) @@ -212,7 +217,12 @@ internal class PromptViewModelTest(private val testCase: TestCase) : SysuiTestCa @Test fun plays_haptic_on_errors_unless_skipped() = runGenericTest { - viewModel.showTemporaryError("still sad", hapticFeedback = false) + viewModel.showTemporaryError( + "still sad", + messageAfterError = "", + authenticateAfterError = false, + hapticFeedback = false, + ) verify(vibrator, never()).vibrateAuthError(any()) verify(vibrator, never()).vibrateAuthSuccess(any()) @@ -287,7 +297,13 @@ internal class PromptViewModelTest(private val testCase: TestCase) : SysuiTestCa assertThat(canTryAgain).isFalse() } - val errorJob = launch { viewModel.showTemporaryError("error") } + val errorJob = launch { + viewModel.showTemporaryError( + "error", + messageAfterError = "", + authenticateAfterError = false, + ) + } verifyNoError() errorJob.join() verifyNoError() @@ -306,12 +322,66 @@ internal class PromptViewModelTest(private val testCase: TestCase) : SysuiTestCa assertThat(messageIsShowing).isTrue() } - // @Test - fun `suppress errors`() = runGenericTest { - val errorMessage = "woot" - val message by collectLastValue(viewModel.message) + @Test + fun suppress_temporary_error() = runGenericTest { + val messages by collectValues(viewModel.message) + + for (error in listOf("never", "see", "me")) { + launch { + viewModel.showTemporaryError( + error, + messageAfterError = "or me", + authenticateAfterError = false, + suppressIf = { _ -> true }, + ) + } + } - val errorJob = launch { viewModel.showTemporaryError(errorMessage) } + testScheduler.advanceUntilIdle() + assertThat(messages).containsExactly(PromptMessage.Empty) + } + + @Test + fun suppress_temporary_error_when_already_showing_when_requested() = + suppress_temporary_error_when_already_showing(suppress = true) + + @Test + fun do_not_suppress_temporary_error_when_already_showing_when_not_requested() = + suppress_temporary_error_when_already_showing(suppress = false) + + private fun suppress_temporary_error_when_already_showing(suppress: Boolean) = runGenericTest { + val errors = listOf("woot", "oh yeah", "nope") + val afterSuffix = "(after)" + val expectedErrorMessage = if (suppress) errors.first() else errors.last() + val messages by collectValues(viewModel.message) + + for (error in errors) { + launch { + viewModel.showTemporaryError( + error, + messageAfterError = "$error $afterSuffix", + authenticateAfterError = false, + suppressIf = { currentMessage -> suppress && currentMessage.isError }, + ) + } + } + + testScheduler.runCurrent() + assertThat(messages) + .containsExactly( + PromptMessage.Empty, + PromptMessage.Error(expectedErrorMessage), + ) + .inOrder() + + testScheduler.advanceUntilIdle() + assertThat(messages) + .containsExactly( + PromptMessage.Empty, + PromptMessage.Error(expectedErrorMessage), + PromptMessage.Help("$expectedErrorMessage $afterSuffix"), + ) + .inOrder() } @Test -- cgit v1.2.3-59-g8ed1b From ee846efe13f21c5b9c360684173dee46ee3dead0 Mon Sep 17 00:00:00 2001 From: Joe Bolinger Date: Sat, 8 Jul 2023 00:04:30 +0000 Subject: Move haptic from authenticated to confirmed (when required). Fix: 272832355 Test: atest PromptViewModelTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:e072fe34e4dec7b5c5fbaed14fbac7936d45ee56) Merged-In: I8d9324384680367af7991eab9b5ce42bf378e8bc Change-Id: I8d9324384680367af7991eab9b5ce42bf378e8bc --- .../biometrics/ui/viewmodel/PromptViewModel.kt | 6 +++++- .../biometrics/ui/viewmodel/PromptViewModelTest.kt | 25 ++++++++++++---------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModel.kt b/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModel.kt index 34099a031ee6..8a2e4059ee73 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModel.kt @@ -372,7 +372,9 @@ constructor( AuthBiometricView.STATE_AUTHENTICATED } - vibrator.success(modality) + if (!needsUserConfirmation) { + vibrator.success(modality) + } messageJob?.cancel() messageJob = null @@ -418,6 +420,8 @@ constructor( _message.value = PromptMessage.Empty _legacyState.value = AuthBiometricView.STATE_AUTHENTICATED + vibrator.success(authState.authenticatedModality) + messageJob?.cancel() messageJob = null } diff --git a/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModelTest.kt index 27774ca51592..91140a9b0fc4 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/biometrics/ui/viewmodel/PromptViewModelTest.kt @@ -49,6 +49,7 @@ import org.junit.runner.RunWith import org.junit.runners.Parameterized import org.mockito.Mock import org.mockito.Mockito.never +import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.junit.MockitoJUnit @@ -131,20 +132,22 @@ internal class PromptViewModelTest(private val testCase: TestCase) : SysuiTestCa } @Test - fun plays_haptic_on_authenticated() = runGenericTest { - viewModel.showAuthenticated(testCase.authenticatedModality, 1000L) + fun play_haptic_on_confirm_when_confirmation_required_otherwise_on_authenticated() = + runGenericTest { + val expectConfirmation = testCase.expectConfirmation(atLeastOneFailure = false) - verify(vibrator).vibrateAuthSuccess(any()) - verify(vibrator, never()).vibrateAuthError(any()) - } + viewModel.showAuthenticated(testCase.authenticatedModality, 1_000L) - @Test - fun plays_no_haptic_on_confirm() = runGenericTest { - viewModel.confirmAuthenticated() + verify(vibrator, if (expectConfirmation) never() else times(1)) + .vibrateAuthSuccess(any()) - verify(vibrator, never()).vibrateAuthSuccess(any()) - verify(vibrator, never()).vibrateAuthError(any()) - } + if (expectConfirmation) { + viewModel.confirmAuthenticated() + } + + verify(vibrator).vibrateAuthSuccess(any()) + verify(vibrator, never()).vibrateAuthError(any()) + } private suspend fun TestScope.showAuthenticated( authenticatedModality: BiometricModality, -- cgit v1.2.3-59-g8ed1b