diff options
2 files changed, 29 insertions, 129 deletions
diff --git a/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java b/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java index eb78fe620c0b..a1184156f86e 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java +++ b/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java @@ -20,7 +20,6 @@ import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.hardware.biometrics.BiometricConstants; -import android.os.Build; import android.os.Handler; import android.os.IBinder; import android.os.RemoteException; @@ -28,11 +27,12 @@ import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.ArrayUtils; +import com.android.modules.expresslog.Counter; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.Arrays; -import java.util.function.BooleanSupplier; + /** * Contains all the necessary information for a HAL operation. @@ -89,8 +89,6 @@ public class BiometricSchedulerOperation { private final BaseClientMonitor mClientMonitor; @Nullable private final ClientMonitorCallback mClientCallback; - @NonNull - private final BooleanSupplier mIsDebuggable; @Nullable private ClientMonitorCallback mOnStartCallback; @OperationState @@ -99,6 +97,7 @@ public class BiometricSchedulerOperation { @NonNull final Runnable mCancelWatchdog; + @VisibleForTesting BiometricSchedulerOperation( @NonNull BaseClientMonitor clientMonitor, @Nullable ClientMonitorCallback callback @@ -106,33 +105,14 @@ public class BiometricSchedulerOperation { this(clientMonitor, callback, STATE_WAITING_IN_QUEUE); } - @VisibleForTesting - BiometricSchedulerOperation( - @NonNull BaseClientMonitor clientMonitor, - @Nullable ClientMonitorCallback callback, - @NonNull BooleanSupplier isDebuggable - ) { - this(clientMonitor, callback, STATE_WAITING_IN_QUEUE, isDebuggable); - } - protected BiometricSchedulerOperation( @NonNull BaseClientMonitor clientMonitor, @Nullable ClientMonitorCallback callback, @OperationState int state ) { - this(clientMonitor, callback, state, Build::isDebuggable); - } - - private BiometricSchedulerOperation( - @NonNull BaseClientMonitor clientMonitor, - @Nullable ClientMonitorCallback callback, - @OperationState int state, - @NonNull BooleanSupplier isDebuggable - ) { mClientMonitor = clientMonitor; mClientCallback = callback; mState = state; - mIsDebuggable = isDebuggable; mCancelWatchdog = () -> { if (!isFinished()) { Slog.e(TAG, "[Watchdog Triggered]: " + this); @@ -187,9 +167,7 @@ public class BiometricSchedulerOperation { if (mClientMonitor.getCookie() != 0) { String err = "operation requires cookie"; - if (mIsDebuggable.getAsBoolean()) { - throw new IllegalStateException(err); - } + Counter.logIncrement("biometric.value_biometric_scheduler_operation_state_error_count"); Slog.e(TAG, err); } @@ -456,10 +434,9 @@ public class BiometricSchedulerOperation { private boolean errorWhenOneOf(String op, @OperationState int... states) { final boolean isError = ArrayUtils.contains(states, mState); if (isError) { - String err = op + ": mState must not be " + mState; - if (mIsDebuggable.getAsBoolean()) { - throw new IllegalStateException(err); - } + Counter.logIncrement( + "biometric.value_biometric_scheduler_operation_state_error_count"); + final String err = op + ": mState must not be " + mState; Slog.e(TAG, err); } return isError; @@ -468,10 +445,10 @@ public class BiometricSchedulerOperation { private boolean errorWhenNoneOf(String op, @OperationState int... states) { final boolean isError = !ArrayUtils.contains(states, mState); if (isError) { - String err = op + ": mState=" + mState + " must be one of " + Arrays.toString(states); - if (mIsDebuggable.getAsBoolean()) { - throw new IllegalStateException(err); - } + Counter.logIncrement( + "biometric.value_biometric_scheduler_operation_state_error_count"); + final String err = op + ": mState=" + mState + " must be one of " + + Arrays.toString(states); Slog.e(TAG, err); } return isError; diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerOperationTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerOperationTest.java index f6da41166403..ffc78110d496 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerOperationTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerOperationTest.java @@ -28,7 +28,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.testng.Assert.assertThrows; import android.hardware.biometrics.BiometricConstants; import android.os.Handler; @@ -88,16 +87,14 @@ public class BiometricSchedulerOperationTest { private Handler mHandler; private BiometricSchedulerOperation mInterruptableOperation; private BiometricSchedulerOperation mNonInterruptableOperation; - private boolean mIsDebuggable; @Before public void setUp() { mHandler = new Handler(TestableLooper.get(this).getLooper()); - mIsDebuggable = false; mInterruptableOperation = new BiometricSchedulerOperation(mInterruptableClientMonitor, - mClientCallback, () -> mIsDebuggable); + mClientCallback); mNonInterruptableOperation = new BiometricSchedulerOperation(mNonInterruptableClientMonitor, - mClientCallback, () -> mIsDebuggable); + mClientCallback); when(mInterruptableClientMonitor.isInterruptable()).thenReturn(true); when(mNonInterruptableClientMonitor.isInterruptable()).thenReturn(false); @@ -143,32 +140,13 @@ public class BiometricSchedulerOperationTest { } @Test - public void testSecondStartWithCookieCrashesWhenDebuggable() { + public void testSecondStartWithCookieFails() { final int cookie = 5; - mIsDebuggable = true; when(mInterruptableClientMonitor.getCookie()).thenReturn(cookie); when(mInterruptableClientMonitor.getFreshDaemon()).thenReturn(mHal); - final boolean started = mInterruptableOperation.startWithCookie(mOnStartCallback, cookie); - assertThat(started).isTrue(); - - assertThrows(IllegalStateException.class, - () -> mInterruptableOperation.startWithCookie(mOnStartCallback, cookie)); - } - - @Test - public void testSecondStartWithCookieFailsNicelyWhenNotDebuggable() { - final int cookie = 5; - mIsDebuggable = false; - when(mInterruptableClientMonitor.getCookie()).thenReturn(cookie); - when(mInterruptableClientMonitor.getFreshDaemon()).thenReturn(mHal); - - final boolean started = mInterruptableOperation.startWithCookie(mOnStartCallback, cookie); - assertThat(started).isTrue(); - - final boolean startedAgain = mInterruptableOperation.startWithCookie(mOnStartCallback, - cookie); - assertThat(startedAgain).isFalse(); + assertThat(mInterruptableOperation.startWithCookie(mOnStartCallback, cookie)).isTrue(); + assertThat(mInterruptableOperation.startWithCookie(mOnStartCallback, cookie)).isFalse(); } @Test @@ -217,56 +195,23 @@ public class BiometricSchedulerOperationTest { } @Test - public void secondStartCrashesWhenDebuggable() { - mIsDebuggable = true; + public void secondStartFails() { when(mInterruptableClientMonitor.getCookie()).thenReturn(0); when(mInterruptableClientMonitor.getFreshDaemon()).thenReturn(mHal); - final boolean started = mInterruptableOperation.start(mOnStartCallback); - assertThat(started).isTrue(); - - assertThrows(IllegalStateException.class, () -> mInterruptableOperation.start( - mOnStartCallback)); - } - - @Test - public void secondStartFailsNicelyWhenNotDebuggable() { - mIsDebuggable = false; - when(mInterruptableClientMonitor.getCookie()).thenReturn(0); - when(mInterruptableClientMonitor.getFreshDaemon()).thenReturn(mHal); - - final boolean started = mInterruptableOperation.start(mOnStartCallback); - assertThat(started).isTrue(); - - final boolean startedAgain = mInterruptableOperation.start(mOnStartCallback); - assertThat(startedAgain).isFalse(); + assertThat(mInterruptableOperation.start(mOnStartCallback)).isTrue(); + assertThat(mInterruptableOperation.start(mOnStartCallback)).isFalse(); } @Test public void doesNotStartWithCookie() { - // This class only throws exceptions when debuggable. - mIsDebuggable = true; when(mInterruptableClientMonitor.getCookie()).thenReturn(9); - assertThrows(IllegalStateException.class, - () -> mInterruptableOperation.start(mock(ClientMonitorCallback.class))); - } - @Test - public void cannotRestart() { - // This class only throws exceptions when debuggable. - mIsDebuggable = true; - when(mInterruptableClientMonitor.getFreshDaemon()).thenReturn(mHal); - - mInterruptableOperation.start(mOnStartCallback); - - assertThrows(IllegalStateException.class, - () -> mInterruptableOperation.start(mock(ClientMonitorCallback.class))); + assertThat(mInterruptableOperation.start(mock(ClientMonitorCallback.class))).isFalse(); } @Test - public void abortsNotRunning() { - // This class only throws exceptions when debuggable. - mIsDebuggable = true; + public void abortSuccessfulIfOperationNotRunning() { when(mInterruptableClientMonitor.getFreshDaemon()).thenReturn(mHal); mInterruptableOperation.abort(); @@ -274,28 +219,17 @@ public class BiometricSchedulerOperationTest { assertThat(mInterruptableOperation.isFinished()).isTrue(); verify(mInterruptableClientMonitor).unableToStart(); verify(mInterruptableClientMonitor).destroy(); - assertThrows(IllegalStateException.class, - () -> mInterruptableOperation.start(mock(ClientMonitorCallback.class))); + assertThat(mInterruptableOperation.start(mock(ClientMonitorCallback.class))).isFalse(); } @Test - public void abortCrashesWhenDebuggableIfOperationIsRunning() { - mIsDebuggable = true; + public void abortFailsIfOperationIsRunning() { when(mInterruptableClientMonitor.getFreshDaemon()).thenReturn(mHal); mInterruptableOperation.start(mOnStartCallback); - - assertThrows(IllegalStateException.class, () -> mInterruptableOperation.abort()); - } - - @Test - public void abortFailsNicelyWhenNotDebuggableIfOperationIsRunning() { - mIsDebuggable = false; - when(mInterruptableClientMonitor.getFreshDaemon()).thenReturn(mHal); - - mInterruptableOperation.start(mOnStartCallback); - mInterruptableOperation.abort(); + + assertThat(mInterruptableOperation.isFinished()).isFalse(); } @Test @@ -344,21 +278,7 @@ public class BiometricSchedulerOperationTest { } @Test - public void cancelCrashesWhenDebuggableIfOperationIsFinished() { - mIsDebuggable = true; - when(mInterruptableClientMonitor.getFreshDaemon()).thenReturn(mHal); - - mInterruptableOperation.abort(); - assertThat(mInterruptableOperation.isFinished()).isTrue(); - - final ClientMonitorCallback cancelCb = mock(ClientMonitorCallback.class); - assertThrows(IllegalStateException.class, () -> mInterruptableOperation.cancel(mHandler, - cancelCb)); - } - - @Test - public void cancelFailsNicelyWhenNotDebuggableIfOperationIsFinished() { - mIsDebuggable = false; + public void cancelFailsIfOperationIsFinished() { when(mInterruptableClientMonitor.getFreshDaemon()).thenReturn(mHal); mInterruptableOperation.abort(); @@ -366,6 +286,9 @@ public class BiometricSchedulerOperationTest { final ClientMonitorCallback cancelCb = mock(ClientMonitorCallback.class); mInterruptableOperation.cancel(mHandler, cancelCb); + + verify(mInterruptableClientMonitor, never()).cancel(); + verify(mInterruptableClientMonitor, never()).cancelWithoutStarting(any()); } @Test |