diff --git a/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java b/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java index 4f900208841e3..a0999771a1be8 100644 --- a/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java +++ b/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java @@ -57,30 +57,25 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { @Nullable private StopUserClient mStopUserClient; private class ClientFinishedCallback implements ClientMonitorCallback { - private final BaseClientMonitor mOwner; + @NonNull private final BaseClientMonitor mOwner; - ClientFinishedCallback(BaseClientMonitor owner) { + ClientFinishedCallback(@NonNull BaseClientMonitor owner) { mOwner = owner; } @Override public void onClientFinished(@NonNull BaseClientMonitor clientMonitor, boolean success) { mHandler.post(() -> { - if (mOwner != clientMonitor) { - Slog.e(getTag(), "[Wrong client finished], actual: " - + clientMonitor + ", expected: " + mOwner); - return; - } - Slog.d(getTag(), "[Client finished] " + clientMonitor + ", success: " + success); if (mCurrentOperation != null && mCurrentOperation.isFor(mOwner)) { mCurrentOperation = null; - startNextOperationIfIdle(); } else { - // can usually be ignored (hal died, etc.) - Slog.d(getTag(), "operation is already null or different (reset?): " + // can happen if the hal dies and is usually okay + // do not unset the current operation that may be newer + Slog.w(getTag(), "operation is already null or different (reset?): " + mCurrentOperation); } + startNextOperationIfIdle(); }); } } diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java index 52eee9a55cc70..8391914a0bb60 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java @@ -18,9 +18,8 @@ package com.android.server.biometrics.sensors; import static android.testing.TestableLooper.RunWithLooper; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; +import static com.google.common.truth.Truth.assertThat; + import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -45,11 +44,15 @@ import com.android.server.biometrics.log.BiometricContext; import com.android.server.biometrics.log.BiometricLogger; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.MockitoAnnotations; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import java.util.ArrayList; +import java.util.List; import java.util.function.Supplier; @Presubmit @@ -61,9 +64,12 @@ public class UserAwareBiometricSchedulerTest { private static final String TAG = "UserAwareBiometricSchedulerTest"; private static final int TEST_SENSOR_ID = 0; + @Rule + public final MockitoRule mockito = MockitoJUnit.rule(); + private Handler mHandler; private UserAwareBiometricScheduler mScheduler; - private IBinder mToken = new Binder(); + private final IBinder mToken = new Binder(); @Mock private Context mContext; @@ -74,15 +80,14 @@ public class UserAwareBiometricSchedulerTest { @Mock private BiometricContext mBiometricContext; - private TestUserStartedCallback mUserStartedCallback = new TestUserStartedCallback(); - private TestUserStoppedCallback mUserStoppedCallback = new TestUserStoppedCallback(); + private final TestUserStartedCallback mUserStartedCallback = new TestUserStartedCallback(); + private final TestUserStoppedCallback mUserStoppedCallback = new TestUserStoppedCallback(); private int mCurrentUserId = UserHandle.USER_NULL; private boolean mStartOperationsFinish = true; private int mStartUserClientCount = 0; @Before public void setUp() { - MockitoAnnotations.initMocks(this); mHandler = new Handler(TestableLooper.get(this).getLooper()); mScheduler = new UserAwareBiometricScheduler(TAG, mHandler, @@ -121,8 +126,8 @@ public class UserAwareBiometricSchedulerTest { mScheduler.scheduleClientMonitor(nextClient); waitForIdle(); - assertEquals(0, mUserStoppedCallback.numInvocations); - assertEquals(1, mUserStartedCallback.numInvocations); + assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(0); + assertThat(mUserStartedCallback.mStartedUsers).containsExactly(0); verify(nextClient).start(any()); } @@ -142,9 +147,9 @@ public class UserAwareBiometricSchedulerTest { waitForIdle(); } - assertEquals(0, mUserStoppedCallback.numInvocations); - assertEquals(0, mUserStartedCallback.numInvocations); - assertEquals(1, mStartUserClientCount); + assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(0); + assertThat(mUserStartedCallback.mStartedUsers).isEmpty(); + assertThat(mStartUserClientCount).isEqualTo(1); for (BaseClientMonitor client : nextClients) { verify(client, never()).start(any()); } @@ -163,13 +168,13 @@ public class UserAwareBiometricSchedulerTest { final TestStartUserClient startUserClient = (TestStartUserClient) mScheduler.mCurrentOperation.getClientMonitor(); mScheduler.reset(); - assertNull(mScheduler.mCurrentOperation); + assertThat(mScheduler.mCurrentOperation).isNull(); final BiometricSchedulerOperation fakeOperation = new BiometricSchedulerOperation( mock(BaseClientMonitor.class), new ClientMonitorCallback() {}); mScheduler.mCurrentOperation = fakeOperation; startUserClient.mCallback.onClientFinished(startUserClient, true); - assertSame(fakeOperation, mScheduler.mCurrentOperation); + assertThat(fakeOperation).isSameInstanceAs(mScheduler.mCurrentOperation); } @Test @@ -184,8 +189,8 @@ public class UserAwareBiometricSchedulerTest { waitForIdle(); verify(nextClient).start(any()); - assertEquals(0, mUserStoppedCallback.numInvocations); - assertEquals(0, mUserStartedCallback.numInvocations); + assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(0); + assertThat(mUserStartedCallback.mStartedUsers).isEmpty(); } @Test @@ -199,36 +204,67 @@ public class UserAwareBiometricSchedulerTest { mScheduler.scheduleClientMonitor(nextClient); waitForIdle(); - assertEquals(1, mUserStoppedCallback.numInvocations); + assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(1); waitForIdle(); - assertEquals(1, mUserStartedCallback.numInvocations); + assertThat(mUserStartedCallback.mStartedUsers).containsExactly(nextUserId); waitForIdle(); verify(nextClient).start(any()); } + @Test + public void testStartUser_alwaysStartsNextOperation() { + BaseClientMonitor nextClient = mock(BaseClientMonitor.class); + when(nextClient.getTargetUserId()).thenReturn(10); + + mScheduler.scheduleClientMonitor(nextClient); + + waitForIdle(); + verify(nextClient).start(any()); + + // finish first operation + mScheduler.getInternalCallback().onClientFinished(nextClient, true /* success */); + waitForIdle(); + + // schedule second operation but swap out the current operation + // before it runs so that it's not current when it's completion callback runs + nextClient = mock(BaseClientMonitor.class); + when(nextClient.getTargetUserId()).thenReturn(11); + mUserStartedCallback.mAfterStart = () -> mScheduler.mCurrentOperation = null; + mScheduler.scheduleClientMonitor(nextClient); + + waitForIdle(); + verify(nextClient).start(any()); + assertThat(mUserStartedCallback.mStartedUsers).containsExactly(10, 11).inOrder(); + assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(1); + } + private void waitForIdle() { TestableLooper.get(this).processAllMessages(); } private class TestUserStoppedCallback implements StopUserClient.UserStoppedCallback { - int numInvocations; + int mNumInvocations; @Override public void onUserStopped() { - numInvocations++; + mNumInvocations++; mCurrentUserId = UserHandle.USER_NULL; } } private class TestUserStartedCallback implements StartUserClient.UserStartedCallback { - int numInvocations; + final List mStartedUsers = new ArrayList<>(); + Runnable mAfterStart = null; @Override public void onUserStarted(int newUserId, Object newObject, int halInterfaceVersion) { - numInvocations++; + mStartedUsers.add(newUserId); mCurrentUserId = newUserId; + if (mAfterStart != null) { + mAfterStart.run(); + } } }