From 4815db0d69590773197f4264d7bdb7f071c866a2 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Wed, 28 Jul 2021 17:30:10 -0700 Subject: [PATCH] Fix condition where FP would require confirmation Adds IconController#deactivate to allow clean transfer of view ownership. Now we deactivate the face IconController once we know we need to animate to the fingerprint state, and use the fingerprint IconController to display subsequent animations. This also fixes a hidden issue where the face IconController was leaking (pulsing logic keeps running) even after BP is dismissed. Bug: 194351208 Test: manual, for requireConfirmation=true and requireConfirmation=false Test: atest com.android.systemui.biometrics Change-Id: Iefd3df7a8706ce9213109862d509235a74aa8099 --- .../AuthBiometricFaceToFingerprintView.java | 49 +++++++++---------- .../biometrics/AuthBiometricFaceView.java | 38 ++++++++++---- ...uthBiometricFaceToFingerprintViewTest.java | 49 ++++++++++--------- .../biometrics/AuthBiometricFaceViewTest.java | 8 +-- 4 files changed, 83 insertions(+), 61 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintView.java index 205054d68280f..98ad875068192 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintView.java @@ -93,6 +93,8 @@ public class AuthBiometricFaceToFingerprintView extends AuthBiometricFaceView { @Nullable private ModalityListener mModalityListener; @Nullable private FingerprintSensorPropertiesInternal mFingerprintSensorProps; @Nullable private UdfpsDialogMeasureAdapter mUdfpsMeasureAdapter; + @Nullable @VisibleForTesting UdfpsIconController mUdfpsIconController; + public AuthBiometricFaceToFingerprintView(Context context) { super(context); @@ -107,6 +109,12 @@ public class AuthBiometricFaceToFingerprintView extends AuthBiometricFaceView { super(context, attrs, injector); } + @Override + protected void onFinishInflate() { + super.onFinishInflate(); + mUdfpsIconController = new UdfpsIconController(mContext, mIconView, mIndicatorView); + } + @Modality int getActiveSensorType() { return mActiveSensorType; @@ -167,36 +175,27 @@ public class AuthBiometricFaceToFingerprintView extends AuthBiometricFaceView { return super.getStateForAfterError(); } - @Override - @NonNull - protected IconController getIconController() { - if (mActiveSensorType == TYPE_FINGERPRINT) { - if (!(mIconController instanceof UdfpsIconController)) { - mIconController = createUdfpsIconController(); - } - return mIconController; - } - return super.getIconController(); - } - - @NonNull - protected IconController createUdfpsIconController() { - return new UdfpsIconController(getContext(), mIconView, mIndicatorView); - } - @Override public void updateState(@BiometricState int newState) { - if (mState == STATE_HELP || mState == STATE_ERROR) { - @Modality final int currentType = mActiveSensorType; - mActiveSensorType = TYPE_FINGERPRINT; + if (mActiveSensorType == TYPE_FACE) { + if (newState == STATE_HELP || newState == STATE_ERROR) { + mActiveSensorType = TYPE_FINGERPRINT; - setRequireConfirmation(false); - mConfirmButton.setEnabled(false); - mConfirmButton.setVisibility(View.GONE); + setRequireConfirmation(false); + mConfirmButton.setEnabled(false); + mConfirmButton.setVisibility(View.GONE); - if (mModalityListener != null && currentType != mActiveSensorType) { - mModalityListener.onModalitySwitched(currentType, mActiveSensorType); + if (mModalityListener != null) { + mModalityListener.onModalitySwitched(TYPE_FACE, mActiveSensorType); + } + + // Deactivate the face icon controller so it stops drawing to the view + mFaceIconController.deactivate(); + // Then, activate this icon controller. We need to start in the "error" state + mUdfpsIconController.updateState(mState, newState); } + } else { // Fingerprint + mUdfpsIconController.updateState(mState, newState); } super.updateState(newState); diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceView.java index e8da7c51bf0d0..c32c1a2f3e53b 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceView.java @@ -49,6 +49,7 @@ public class AuthBiometricFaceView extends AuthBiometricView { protected Handler mHandler; protected boolean mLastPulseLightToDark; // false = dark to light, true = light to dark protected @BiometricState int mState; + protected boolean mDeactivated; protected IconController(Context context, ImageView iconView, TextView textView) { mContext = context; @@ -67,6 +68,11 @@ public class AuthBiometricFaceView extends AuthBiometricView { } protected void animateIcon(int iconRes, boolean repeat) { + Log.d(TAG, "animateIcon, state: " + mState + ", deactivated: " + mDeactivated); + if (mDeactivated) { + return; + } + final AnimatedVectorDrawable icon = (AnimatedVectorDrawable) mContext.getDrawable(iconRes); mIconView.setImageDrawable(icon); @@ -92,12 +98,26 @@ public class AuthBiometricFaceView extends AuthBiometricView { @Override public void onAnimationEnd(Drawable drawable) { super.onAnimationEnd(drawable); + Log.d(TAG, "onAnimationEnd, mState: " + mState + ", deactivated: " + mDeactivated); + if (mDeactivated) { + return; + } + if (mState == STATE_AUTHENTICATING || mState == STATE_HELP) { pulseInNextDirection(); } } + protected void deactivate() { + mDeactivated = true; + } + protected void updateState(int lastState, int newState) { + if (mDeactivated) { + Log.w(TAG, "Ignoring updateState when deactivated: " + newState); + return; + } + final boolean lastStateIsErrorIcon = lastState == STATE_ERROR || lastState == STATE_HELP; @@ -142,7 +162,7 @@ public class AuthBiometricFaceView extends AuthBiometricView { } } - protected IconController mIconController; + @Nullable @VisibleForTesting IconController mFaceIconController; public AuthBiometricFaceView(Context context) { this(context, null); @@ -157,6 +177,12 @@ public class AuthBiometricFaceView extends AuthBiometricView { super(context, attrs, injector); } + @Override + protected void onFinishInflate() { + super.onFinishInflate(); + mFaceIconController = new IconController(mContext, mIconView, mIndicatorView); + } + @Override protected int getDelayAfterAuthenticatedDurationMs() { return HIDE_DELAY_MS; @@ -187,17 +213,9 @@ public class AuthBiometricFaceView extends AuthBiometricView { return true; } - @NonNull - protected IconController getIconController() { - if (mIconController == null) { - mIconController = new IconController(mContext, mIconView, mIndicatorView); - } - return mIconController; - } - @Override public void updateState(@BiometricState int newState) { - getIconController().updateState(mState, newState); + mFaceIconController.updateState(mState, newState); if (newState == STATE_AUTHENTICATING_ANIMATING_IN || (newState == STATE_AUTHENTICATING && getSize() == AuthDialog.SIZE_MEDIUM)) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintViewTest.java index 5cd781085b155..090cf9f63eaff 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintViewTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintViewTest.java @@ -23,7 +23,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; @@ -50,7 +49,6 @@ import com.android.systemui.SysuiTestCase; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -78,14 +76,16 @@ public class AuthBiometricFaceToFingerprintViewTest extends SysuiTestCase { @Mock private TextView mIndicatorView; @Mock private ImageView mIconView; @Mock private View mIconHolderView; - @Mock private AuthBiometricFaceView.IconController mIconController; + @Mock private AuthBiometricFaceView.IconController mFaceIconController; + @Mock private AuthBiometricFaceToFingerprintView.UdfpsIconController mUdfpsIconController; @Before public void setup() { MockitoAnnotations.initMocks(this); mFaceToFpView = new TestableView(mContext); - mFaceToFpView.mIconController = mIconController; + mFaceToFpView.mFaceIconController = mFaceIconController; + mFaceToFpView.mUdfpsIconController = mUdfpsIconController; mFaceToFpView.setCallback(mCallback); mFaceToFpView.mNegativeButton = mNegativeButton; @@ -99,20 +99,23 @@ public class AuthBiometricFaceToFingerprintViewTest extends SysuiTestCase { @Test public void testStateUpdated_whenDialogAnimatedIn() { mFaceToFpView.onDialogAnimatedIn(); - verify(mFaceToFpView.mIconController) + verify(mFaceToFpView.mFaceIconController) .updateState(anyInt(), eq(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING)); + verify(mFaceToFpView.mUdfpsIconController, never()).updateState(anyInt(), anyInt()); } @Test public void testIconUpdatesState_whenDialogStateUpdated() { mFaceToFpView.onDialogAnimatedIn(); - verify(mFaceToFpView.mIconController) + verify(mFaceToFpView.mFaceIconController) .updateState(anyInt(), eq(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING)); + verify(mFaceToFpView.mUdfpsIconController, never()).updateState(anyInt(), anyInt()); mFaceToFpView.updateState(AuthBiometricFaceView.STATE_AUTHENTICATED); - verify(mFaceToFpView.mIconController).updateState( + verify(mFaceToFpView.mFaceIconController).updateState( eq(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING), eq(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATED)); + verify(mFaceToFpView.mUdfpsIconController, never()).updateState(anyInt(), anyInt()); assertEquals(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATED, mFaceToFpView.mState); } @@ -120,21 +123,22 @@ public class AuthBiometricFaceToFingerprintViewTest extends SysuiTestCase { @Test public void testStateUpdated_whenSwitchToFingerprint() { mFaceToFpView.onDialogAnimatedIn(); - verify(mFaceToFpView.mIconController) + verify(mFaceToFpView.mFaceIconController) .updateState(anyInt(), eq(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING)); mFaceToFpView.updateState(AuthBiometricFaceToFingerprintView.STATE_ERROR); - mFaceToFpView.updateState(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING); - InOrder order = inOrder(mFaceToFpView.mIconController); - order.verify(mFaceToFpView.mIconController).updateState( + verify(mFaceToFpView.mFaceIconController).deactivate(); + verify(mFaceToFpView.mUdfpsIconController).updateState( eq(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING), eq(AuthBiometricFaceToFingerprintView.STATE_ERROR)); - order.verify(mFaceToFpView.mIconController).updateState( + verify(mConfirmButton).setVisibility(eq(View.GONE)); + + mFaceToFpView.updateState(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING); + + verify(mFaceToFpView.mUdfpsIconController).updateState( eq(AuthBiometricFaceToFingerprintView.STATE_ERROR), eq(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING)); - - verify(mConfirmButton).setVisibility(eq(View.GONE)); } @Test @@ -172,7 +176,10 @@ public class AuthBiometricFaceToFingerprintViewTest extends SysuiTestCase { eq(mContext.getString(R.string.fingerprint_dialog_use_fingerprint_instead))); verify(mCallback).onAction( eq(AuthBiometricView.Callback.ACTION_START_DELAYED_FINGERPRINT_SENSOR)); - assertEquals(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING, mFaceToFpView.mState); + + // First we enter the error state, since we need to show the error animation/text. The + // error state is later cleared based on a timer, and we enter STATE_AUTHENTICATING. + assertEquals(AuthBiometricFaceToFingerprintView.STATE_ERROR, mFaceToFpView.mState); } @Test @@ -185,13 +192,16 @@ public class AuthBiometricFaceToFingerprintViewTest extends SysuiTestCase { eq(mContext.getString(R.string.fingerprint_dialog_use_fingerprint_instead))); verify(mCallback).onAction( eq(AuthBiometricView.Callback.ACTION_START_DELAYED_FINGERPRINT_SENSOR)); - assertEquals(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING, mFaceToFpView.mState); + + // First we enter the error state, since we need to show the error animation/text. The + // error state is later cleared based on a timer, and we enter STATE_AUTHENTICATING. + assertEquals(AuthBiometricFaceToFingerprintView.STATE_ERROR, mFaceToFpView.mState); } @Test public void testFingerprintOnlyStartsOnFirstError() { mFaceToFpView.onDialogAnimatedIn(); - verify(mFaceToFpView.mIconController) + verify(mFaceToFpView.mFaceIconController) .updateState(anyInt(), eq(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING)); mFaceToFpView.onDialogAnimatedIn(); @@ -260,11 +270,6 @@ public class AuthBiometricFaceToFingerprintViewTest extends SysuiTestCase { protected int getDelayAfterAuthenticatedDurationMs() { return 0; } - - @Override - protected IconController createUdfpsIconController() { - return AuthBiometricFaceToFingerprintViewTest.this.mIconController; - } } private class MockInjector extends AuthBiometricView.Injector { diff --git a/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceViewTest.java index 043bd5cd6ba5f..b93381d2b5c9b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceViewTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceViewTest.java @@ -62,7 +62,7 @@ public class AuthBiometricFaceViewTest extends SysuiTestCase { public void setup() { MockitoAnnotations.initMocks(this); mFaceView = new TestableFaceView(mContext); - mFaceView.mIconController = mock(TestableFaceView.TestableIconController.class); + mFaceView.mFaceIconController = mock(TestableFaceView.TestableIconController.class); mFaceView.setCallback(mCallback); mFaceView.mNegativeButton = mNegativeButton; @@ -78,18 +78,18 @@ public class AuthBiometricFaceViewTest extends SysuiTestCase { @Test public void testStateUpdated_whenDialogAnimatedIn() { mFaceView.onDialogAnimatedIn(); - verify(mFaceView.mIconController) + verify(mFaceView.mFaceIconController) .updateState(anyInt(), eq(AuthBiometricFaceView.STATE_AUTHENTICATING)); } @Test public void testIconUpdatesState_whenDialogStateUpdated() { mFaceView.updateState(AuthBiometricFaceView.STATE_AUTHENTICATING); - verify(mFaceView.mIconController) + verify(mFaceView.mFaceIconController) .updateState(anyInt(), eq(AuthBiometricFaceView.STATE_AUTHENTICATING)); mFaceView.updateState(AuthBiometricFaceView.STATE_AUTHENTICATED); - verify(mFaceView.mIconController).updateState( + verify(mFaceView.mFaceIconController).updateState( eq(AuthBiometricFaceView.STATE_AUTHENTICATING), eq(AuthBiometricFaceView.STATE_AUTHENTICATED)); }