Check current user when the operations runs for fingerprint hidl.
The current and target user are both provided to the switch
client constructor which is brittle unless operations are scheduled and
run synchronously. Commit 9a99503870074d40d794245d9e3ac7a076f5b2f2
changed the handlers and somehow caused a bug where the cached current
user was out of sync. The switch client tries to optimize for this and
can skip switching when that occurs.
This also includes two additonal changes 1) a few updated logs from
the original change and 2) restores each scheduling having its own
handler for clarity.
Bug: 213962104
Bug: 210090299
Test: atest UserAwareBiometricSchedulerTest BiometricSchedulerTest BiometricSchedulerOperationTest SensorTest Face10Test
Test: manual (flash, enroll fingerprint, add work profile account, reboot, verify fingerprint still works)
Change-Id: Ifb73b0145aeb8afb62d1f55d2f881347b0d2ef8a
Merged-In: Ifb73b0145aeb8afb62d1f55d2f881347b0d2ef8a
diff --git a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java
index 1f91c4d..39c5944 100644
--- a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java
+++ b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java
@@ -25,6 +25,7 @@
import android.hardware.fingerprint.FingerprintSensorPropertiesInternal;
import android.os.Handler;
import android.os.IBinder;
+import android.os.Looper;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.util.Slog;
@@ -232,17 +233,14 @@
* Creates a new scheduler.
*
* @param tag for the specific instance of the scheduler. Should be unique.
- * @param handler handler for callbacks (all methods of this class must be called on the
- * thread associated with this handler)
* @param sensorType the sensorType that this scheduler is handling.
* @param gestureAvailabilityDispatcher may be null if the sensor does not support gestures
* (such as fingerprint swipe).
*/
public BiometricScheduler(@NonNull String tag,
- @NonNull Handler handler,
@SensorType int sensorType,
@Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher) {
- this(tag, handler, sensorType, gestureAvailabilityDispatcher,
+ this(tag, new Handler(Looper.getMainLooper()), sensorType, gestureAvailabilityDispatcher,
IBiometricService.Stub.asInterface(
ServiceManager.getService(Context.BIOMETRIC_SERVICE)),
LOG_NUM_RECENT_OPERATIONS, CoexCoordinator.getInstance());
@@ -376,8 +374,9 @@
// send ERROR_CANCELED and skip the operation.
if (clientMonitor.interruptsPrecedingClients()) {
for (BiometricSchedulerOperation operation : mPendingOperations) {
- Slog.d(getTag(), "New client, marking pending op as canceling: " + operation);
- operation.markCanceling();
+ if (operation.markCanceling()) {
+ Slog.d(getTag(), "New client, marking pending op as canceling: " + operation);
+ }
}
}
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 a8cce15..e8b50d9 100644
--- a/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java
+++ b/services/core/java/com/android/server/biometrics/sensors/BiometricSchedulerOperation.java
@@ -225,12 +225,13 @@
Slog.v(TAG, "Aborted: " + this);
}
- /** Flags this operation as canceled, but does not cancel it until started. */
- public void markCanceling() {
+ /** Flags this operation as canceled, if possible, but does not cancel it until started. */
+ public boolean markCanceling() {
if (mState == STATE_WAITING_IN_QUEUE && isInterruptable()) {
mState = STATE_WAITING_IN_QUEUE_CANCELING;
- Slog.v(TAG, "Marked cancelling: " + this);
+ return true;
}
+ return false;
}
/**
@@ -280,6 +281,7 @@
@Override
public void onClientFinished(@NonNull BaseClientMonitor clientMonitor,
boolean success) {
+ Slog.d(TAG, "[Finished / destroy]: " + clientMonitor);
mClientMonitor.destroy();
mState = STATE_FINISHED;
}
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 19eaa17..603cc22 100644
--- a/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java
+++ b/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java
@@ -23,6 +23,7 @@
import android.content.Context;
import android.hardware.biometrics.IBiometricService;
import android.os.Handler;
+import android.os.Looper;
import android.os.ServiceManager;
import android.os.UserHandle;
import android.util.Slog;
@@ -85,7 +86,7 @@
}
@VisibleForTesting
- UserAwareBiometricScheduler(@NonNull String tag,
+ public UserAwareBiometricScheduler(@NonNull String tag,
@NonNull Handler handler,
@SensorType int sensorType,
@Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher,
@@ -101,12 +102,11 @@
}
public UserAwareBiometricScheduler(@NonNull String tag,
- @NonNull Handler handler,
@SensorType int sensorType,
@Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher,
@NonNull CurrentUserRetriever currentUserRetriever,
@NonNull UserSwitchCallback userSwitchCallback) {
- this(tag, handler, sensorType, gestureAvailabilityDispatcher,
+ this(tag, new Handler(Looper.getMainLooper()), sensorType, gestureAvailabilityDispatcher,
IBiometricService.Stub.asInterface(
ServiceManager.getService(Context.BIOMETRIC_SERVICE)),
currentUserRetriever, userSwitchCallback, CoexCoordinator.getInstance());
diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java
index 3927043..206b8f0 100644
--- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java
+++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java
@@ -494,7 +494,7 @@
mToken = new Binder();
mHandler = handler;
mSensorProperties = sensorProperties;
- mScheduler = new UserAwareBiometricScheduler(tag, mHandler,
+ mScheduler = new UserAwareBiometricScheduler(tag,
BiometricScheduler.SENSOR_TYPE_FACE, null /* gestureAvailabilityDispatcher */,
() -> mCurrentSession != null ? mCurrentSession.mUserId : UserHandle.USER_NULL,
new UserAwareBiometricScheduler.UserSwitchCallback() {
diff --git a/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java b/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java
index 493c0a0..e957794 100644
--- a/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java
+++ b/services/core/java/com/android/server/biometrics/sensors/face/hidl/Face10.java
@@ -363,7 +363,7 @@
@NonNull LockoutResetDispatcher lockoutResetDispatcher) {
final Handler handler = new Handler(Looper.getMainLooper());
return new Face10(context, sensorProps, lockoutResetDispatcher, handler,
- new BiometricScheduler(TAG, handler, BiometricScheduler.SENSOR_TYPE_FACE,
+ new BiometricScheduler(TAG, BiometricScheduler.SENSOR_TYPE_FACE,
null /* gestureAvailabilityTracker */));
}
@@ -896,6 +896,8 @@
boolean success) {
if (success) {
mCurrentUserId = targetUserId;
+ } else {
+ Slog.w(TAG, "Failed to change user, still: " + mCurrentUserId);
}
}
});
diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java
index 256761a..59e4b58 100644
--- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java
+++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java
@@ -449,7 +449,7 @@
mHandler = handler;
mSensorProperties = sensorProperties;
mLockoutCache = new LockoutCache();
- mScheduler = new UserAwareBiometricScheduler(tag, handler,
+ mScheduler = new UserAwareBiometricScheduler(tag,
BiometricScheduler.sensorTypeFromFingerprintProperties(mSensorProperties),
gestureAvailabilityDispatcher,
() -> mCurrentSession != null ? mCurrentSession.mUserId : UserHandle.USER_NULL,
diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java
index d352cda..6feb5fa 100644
--- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java
+++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java
@@ -360,7 +360,7 @@
@NonNull LockoutResetDispatcher lockoutResetDispatcher,
@NonNull GestureAvailabilityDispatcher gestureAvailabilityDispatcher) {
final BiometricScheduler scheduler =
- new BiometricScheduler(TAG, handler,
+ new BiometricScheduler(TAG,
BiometricScheduler.sensorTypeFromFingerprintProperties(sensorProps),
gestureAvailabilityDispatcher);
final HalResultController controller = new HalResultController(sensorProps.sensorId,
@@ -490,19 +490,25 @@
!getEnrolledFingerprints(mSensorProperties.sensorId, targetUserId).isEmpty();
final FingerprintUpdateActiveUserClient client =
new FingerprintUpdateActiveUserClient(mContext, mLazyDaemon, targetUserId,
- mContext.getOpPackageName(), mSensorProperties.sensorId, mCurrentUserId,
- hasEnrolled, mAuthenticatorIds, force);
+ mContext.getOpPackageName(), mSensorProperties.sensorId,
+ this::getCurrentUser, hasEnrolled, mAuthenticatorIds, force);
mScheduler.scheduleClientMonitor(client, new BaseClientMonitor.Callback() {
@Override
public void onClientFinished(@NonNull BaseClientMonitor clientMonitor,
boolean success) {
if (success) {
mCurrentUserId = targetUserId;
+ } else {
+ Slog.w(TAG, "Failed to change user, still: " + mCurrentUserId);
}
}
});
}
+ private int getCurrentUser() {
+ return mCurrentUserId;
+ }
+
@Override
public boolean containsSensor(int sensorId) {
return mSensorProperties.sensorId == sensorId;
diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java
index 20dab55..273f8a5 100644
--- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java
+++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java
@@ -138,8 +138,7 @@
TestableBiometricScheduler(@NonNull String tag, @NonNull Handler handler,
@Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher) {
- super(tag, handler, BiometricScheduler.SENSOR_TYPE_FP_OTHER,
- gestureAvailabilityDispatcher);
+ super(tag, BiometricScheduler.SENSOR_TYPE_FP_OTHER, gestureAvailabilityDispatcher);
}
void init(@NonNull Fingerprint21UdfpsMock fingerprint21) {
diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintUpdateActiveUserClient.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintUpdateActiveUserClient.java
index fd38bdd..a2c1892 100644
--- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintUpdateActiveUserClient.java
+++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintUpdateActiveUserClient.java
@@ -31,6 +31,7 @@
import java.io.File;
import java.util.Map;
+import java.util.function.Supplier;
/**
* Sets the HAL's current active user, and updates the framework's authenticatorId cache.
@@ -40,7 +41,7 @@
private static final String TAG = "FingerprintUpdateActiveUserClient";
private static final String FP_DATA_DIR = "fpdata";
- private final int mCurrentUserId;
+ private final Supplier<Integer> mCurrentUserId;
private final boolean mForceUpdateAuthenticatorId;
private final boolean mHasEnrolledBiometrics;
private final Map<Integer, Long> mAuthenticatorIds;
@@ -48,8 +49,9 @@
FingerprintUpdateActiveUserClient(@NonNull Context context,
@NonNull LazyDaemon<IBiometricsFingerprint> lazyDaemon, int userId,
- @NonNull String owner, int sensorId, int currentUserId, boolean hasEnrolledBiometrics,
- @NonNull Map<Integer, Long> authenticatorIds, boolean forceUpdateAuthenticatorId) {
+ @NonNull String owner, int sensorId, Supplier<Integer> currentUserId,
+ boolean hasEnrolledBiometrics, @NonNull Map<Integer, Long> authenticatorIds,
+ boolean forceUpdateAuthenticatorId) {
super(context, lazyDaemon, null /* token */, null /* listener */, userId, owner,
0 /* cookie */, sensorId, BiometricsProtoEnums.MODALITY_UNKNOWN,
BiometricsProtoEnums.ACTION_UNKNOWN, BiometricsProtoEnums.CLIENT_UNKNOWN);
@@ -63,7 +65,7 @@
public void start(@NonNull Callback callback) {
super.start(callback);
- if (mCurrentUserId == getTargetUserId() && !mForceUpdateAuthenticatorId) {
+ if (mCurrentUserId.get() == getTargetUserId() && !mForceUpdateAuthenticatorId) {
Slog.d(TAG, "Already user: " + mCurrentUserId + ", returning");
callback.onClientFinished(this, true /* success */);
return;
@@ -109,8 +111,10 @@
@Override
protected void startHalOperation() {
try {
- getFreshDaemon().setActiveGroup(getTargetUserId(), mDirectory.getAbsolutePath());
- mAuthenticatorIds.put(getTargetUserId(), mHasEnrolledBiometrics
+ final int targetId = getTargetUserId();
+ Slog.d(TAG, "Setting active user: " + targetId);
+ getFreshDaemon().setActiveGroup(targetId, mDirectory.getAbsolutePath());
+ mAuthenticatorIds.put(targetId, mHasEnrolledBiometrics
? getFreshDaemon().getAuthenticatorId() : 0L);
mCallback.onClientFinished(this, true /* success */);
} catch (RemoteException e) {
diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/aidl/SensorTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/aidl/SensorTest.java
index 0891eca..2718bf9 100644
--- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/aidl/SensorTest.java
+++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/aidl/SensorTest.java
@@ -33,6 +33,7 @@
import androidx.test.filters.SmallTest;
import com.android.server.biometrics.sensors.BiometricScheduler;
+import com.android.server.biometrics.sensors.CoexCoordinator;
import com.android.server.biometrics.sensors.LockoutCache;
import com.android.server.biometrics.sensors.LockoutResetDispatcher;
import com.android.server.biometrics.sensors.LockoutTracker;
@@ -82,8 +83,10 @@
new Handler(mLooper.getLooper()),
BiometricScheduler.SENSOR_TYPE_FACE,
null /* gestureAvailabilityDispatcher */,
+ mBiometricService,
() -> USER_ID,
- mUserSwitchCallback);
+ mUserSwitchCallback,
+ CoexCoordinator.getInstance());
mHalCallback = new Sensor.HalSessionCallback(mContext, new Handler(mLooper.getLooper()),
TAG, mScheduler, SENSOR_ID,
USER_ID, mLockoutCache, mLockoutResetDispatcher, mHalSessionCallback);
diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java
index a012b8b..d4609b5 100644
--- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java
+++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/fingerprint/aidl/SensorTest.java
@@ -33,6 +33,7 @@
import androidx.test.filters.SmallTest;
import com.android.server.biometrics.sensors.BiometricScheduler;
+import com.android.server.biometrics.sensors.CoexCoordinator;
import com.android.server.biometrics.sensors.LockoutCache;
import com.android.server.biometrics.sensors.LockoutResetDispatcher;
import com.android.server.biometrics.sensors.LockoutTracker;
@@ -82,8 +83,10 @@
new Handler(mLooper.getLooper()),
BiometricScheduler.SENSOR_TYPE_FP_OTHER,
null /* gestureAvailabilityDispatcher */,
+ mBiometricService,
() -> USER_ID,
- mUserSwitchCallback);
+ mUserSwitchCallback,
+ CoexCoordinator.getInstance());
mHalCallback = new Sensor.HalSessionCallback(mContext, new Handler(mLooper.getLooper()),
TAG, mScheduler, SENSOR_ID,
USER_ID, mLockoutCache, mLockoutResetDispatcher, mHalSessionCallback);
