From f9b29b44a0ef497fbe954b11d2869f132ab9a321 Mon Sep 17 00:00:00 2001 From: eddielan Date: Tue, 6 Dec 2022 20:43:42 +0800 Subject: [PATCH] Increase power-press timeout Preivously, Keyguard would try to authenticate after a cancellation too quickly so the HAL would drop the attempt. Repro steps: 1. User press power key 2. Keyguard cancel 1st authentication and send cancellation reqeust to HAL 3. HAL is dealing with cancelation request 4. Keyguard runs 2nd authentication. 5. HAL report cancellation to keyguard and cancel 2nd authentication. 6. Fingerprint does not respond to user's touch. Bug: 261565425 Test: atest KeyguardUpdateMonitorTest Test: verified locally on device - attempt side FPS after pressing the power button (on or off) Change-Id: If84b8765622e47872b6492eaf09c0e46aeabdf27 --- .../keyguard/KeyguardUpdateMonitor.java | 18 +++++++++++++----- .../logging/KeyguardUpdateMonitorLogger.kt | 2 +- .../keyguard/KeyguardUpdateMonitorTest.java | 16 +++++++++++++--- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java index d1a59cc334ab9..71d5bf57baf60 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java @@ -397,6 +397,9 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab private static final int HAL_ERROR_RETRY_TIMEOUT = 500; // ms private static final int HAL_ERROR_RETRY_MAX = 20; + @VisibleForTesting + protected static final int HAL_POWER_PRESS_TIMEOUT = 50; // ms + @VisibleForTesting protected final Runnable mFpCancelNotReceived = this::onFingerprintCancelNotReceived; @@ -918,7 +921,7 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab } } - private final Runnable mRetryFingerprintAuthentication = new Runnable() { + private final Runnable mRetryFingerprintAuthenticationAfterHwUnavailable = new Runnable() { @SuppressLint("MissingPermission") @Override public void run() { @@ -927,7 +930,8 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab updateFingerprintListeningState(BIOMETRIC_ACTION_UPDATE); } else if (mHardwareFingerprintUnavailableRetryCount < HAL_ERROR_RETRY_MAX) { mHardwareFingerprintUnavailableRetryCount++; - mHandler.postDelayed(mRetryFingerprintAuthentication, HAL_ERROR_RETRY_TIMEOUT); + mHandler.postDelayed(mRetryFingerprintAuthenticationAfterHwUnavailable, + HAL_ERROR_RETRY_TIMEOUT); } } }; @@ -957,12 +961,16 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab if (msgId == FingerprintManager.FINGERPRINT_ERROR_HW_UNAVAILABLE) { mLogger.logRetryAfterFpErrorWithDelay(msgId, errString, HAL_ERROR_RETRY_TIMEOUT); - mHandler.postDelayed(mRetryFingerprintAuthentication, HAL_ERROR_RETRY_TIMEOUT); + mHandler.postDelayed(mRetryFingerprintAuthenticationAfterHwUnavailable, + HAL_ERROR_RETRY_TIMEOUT); } if (msgId == FingerprintManager.BIOMETRIC_ERROR_POWER_PRESSED) { - mLogger.logRetryAfterFpErrorWithDelay(msgId, errString, 0); - updateFingerprintListeningState(BIOMETRIC_ACTION_START); + mLogger.logRetryAfterFpErrorWithDelay(msgId, errString, HAL_POWER_PRESS_TIMEOUT); + mHandler.postDelayed(() -> { + mLogger.d("Retrying fingerprint listening after power pressed error."); + updateFingerprintListeningState(BIOMETRIC_ACTION_START); + }, HAL_POWER_PRESS_TIMEOUT); } boolean lockedOutStateChanged = false; diff --git a/packages/SystemUI/src/com/android/keyguard/logging/KeyguardUpdateMonitorLogger.kt b/packages/SystemUI/src/com/android/keyguard/logging/KeyguardUpdateMonitorLogger.kt index b66ae286171ae..ceebe4c690912 100644 --- a/packages/SystemUI/src/com/android/keyguard/logging/KeyguardUpdateMonitorLogger.kt +++ b/packages/SystemUI/src/com/android/keyguard/logging/KeyguardUpdateMonitorLogger.kt @@ -231,7 +231,7 @@ class KeyguardUpdateMonitorLogger @Inject constructor( int2 = delay str1 = "$errString" }, { - "Fingerprint retrying auth after $int2 ms due to($int1) -> $str1" + "Fingerprint scheduling retry auth after $int2 ms due to($int1) -> $str1" }) } diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java index 0e92a29044368..40542d25689d7 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java @@ -30,6 +30,7 @@ import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STR import static com.android.keyguard.FaceAuthApiRequestReason.NOTIFICATION_PANEL_CLICKED; import static com.android.keyguard.KeyguardUpdateMonitor.BIOMETRIC_STATE_CANCELLING_RESTARTING; import static com.android.keyguard.KeyguardUpdateMonitor.DEFAULT_CANCEL_SIGNAL_TIMEOUT; +import static com.android.keyguard.KeyguardUpdateMonitor.HAL_POWER_PRESS_TIMEOUT; import static com.android.keyguard.KeyguardUpdateMonitor.getCurrentUser; import static com.google.common.truth.Truth.assertThat; @@ -855,12 +856,21 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { } @Test - public void testFingerprintPowerPressed_restartsFingerprintListeningStateImmediately() { + public void testFingerprintPowerPressed_restartsFingerprintListeningStateWithDelay() { mKeyguardUpdateMonitor.mFingerprintAuthenticationCallback .onAuthenticationError(FingerprintManager.BIOMETRIC_ERROR_POWER_PRESSED, ""); - verify(mFingerprintManager).authenticate(any(), any(), any(), any(), anyInt(), anyInt(), - anyInt()); + // THEN doesn't authenticate immediately + verify(mFingerprintManager, never()).authenticate(any(), + any(), any(), any(), anyInt(), anyInt(), anyInt()); + + // WHEN all messages (with delays) are processed + mTestableLooper.moveTimeForward(HAL_POWER_PRESS_TIMEOUT); + mTestableLooper.processAllMessages(); + + // THEN fingerprint manager attempts to authenticate again + verify(mFingerprintManager).authenticate(any(), + any(), any(), any(), anyInt(), anyInt(), anyInt()); } @Test