From 6ef340b2ef6d89e5e9302884445fe0df1b30e467 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Wed, 7 Apr 2021 17:25:56 -0700 Subject: [PATCH] Split error and cancellation into separate interfaces Not all clients need to be cancelled in order for it to receive onError. For example, IFace#resetLockout. Bug: 184774513 Test: atest CtsBiometricsTestCases Test: atest com.android.server.biometrics Change-Id: I4db4747fe03725976c6c8cfa5248420f82287140 --- .../biometrics/sensors/AcquisitionClient.java | 3 +- .../sensors/BiometricScheduler.java | 6 ++-- .../biometrics/sensors/ErrorConsumer.java | 30 +++++++++++++++++++ .../biometrics/sensors/Interruptable.java | 10 +------ .../biometrics/sensors/face/aidl/Sensor.java | 11 +++---- .../biometrics/sensors/face/hidl/Face10.java | 13 ++++---- .../sensors/fingerprint/aidl/Sensor.java | 13 ++++---- .../fingerprint/hidl/Fingerprint21.java | 13 ++++---- 8 files changed, 63 insertions(+), 36 deletions(-) create mode 100644 services/core/java/com/android/server/biometrics/sensors/ErrorConsumer.java diff --git a/services/core/java/com/android/server/biometrics/sensors/AcquisitionClient.java b/services/core/java/com/android/server/biometrics/sensors/AcquisitionClient.java index 93fea90cd89ae..f7443740943a3 100644 --- a/services/core/java/com/android/server/biometrics/sensors/AcquisitionClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/AcquisitionClient.java @@ -32,7 +32,8 @@ import android.util.Slog; * Abstract {@link HalClientMonitor} subclass that operations eligible/interested in acquisition * messages should extend. */ -public abstract class AcquisitionClient extends HalClientMonitor implements Interruptable { +public abstract class AcquisitionClient extends HalClientMonitor implements Interruptable, + ErrorConsumer { private static final String TAG = "Biometrics/AcquisitionClient"; 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 85d849f6ff9a9..cc27127ac58f4 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java +++ b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java @@ -422,9 +422,9 @@ public class BiometricScheduler { + mCurrentOperation); // This should trigger the internal onClientFinished callback, which clears the // operation and starts the next one. - final Interruptable interruptable = - (Interruptable) mCurrentOperation.mClientMonitor; - interruptable.onError(BiometricConstants.BIOMETRIC_ERROR_CANCELED, + final ErrorConsumer errorConsumer = + (ErrorConsumer) mCurrentOperation.mClientMonitor; + errorConsumer.onError(BiometricConstants.BIOMETRIC_ERROR_CANCELED, 0 /* vendorCode */); return; } else { diff --git a/services/core/java/com/android/server/biometrics/sensors/ErrorConsumer.java b/services/core/java/com/android/server/biometrics/sensors/ErrorConsumer.java new file mode 100644 index 0000000000000..9fe8bdcf992df --- /dev/null +++ b/services/core/java/com/android/server/biometrics/sensors/ErrorConsumer.java @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.biometrics.sensors; + +/** + * Interface that {@link BaseClientMonitor} subclasses eligible/interested in error callbacks should + * implement. + */ +public interface ErrorConsumer { + /** + * Notifies the client of errors from the HAL. + * @param errorCode defined by the HIDL interface + * @param vendorCode defined by the vendor + */ + void onError(int errorCode, int vendorCode); +} diff --git a/services/core/java/com/android/server/biometrics/sensors/Interruptable.java b/services/core/java/com/android/server/biometrics/sensors/Interruptable.java index 70d9b26f4af1e..fab98b6581a3e 100644 --- a/services/core/java/com/android/server/biometrics/sensors/Interruptable.java +++ b/services/core/java/com/android/server/biometrics/sensors/Interruptable.java @@ -19,8 +19,7 @@ package com.android.server.biometrics.sensors; import android.annotation.NonNull; /** - * Interface that {@link BaseClientMonitor} subclasses eligible/interested in error callbacks should - * implement. + * Interface that {@link BaseClientMonitor} subclasses eligible for cancellation should implement. */ public interface Interruptable { /** @@ -28,13 +27,6 @@ public interface Interruptable { */ void cancel(); - /** - * Notifies the client of errors from the HAL. - * @param errorCode defined by the HIDL interface - * @param vendorCode defined by the vendor - */ - void onError(int errorCode, int vendorCode); - /** * Notifies the client that it needs to finish before * {@link BaseClientMonitor#start(BaseClientMonitor.Callback)} was invoked. This usually happens 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 a533c8f7ecc65..d56fd1221aa9e 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 @@ -52,6 +52,7 @@ import com.android.server.biometrics.sensors.AuthenticationConsumer; import com.android.server.biometrics.sensors.BaseClientMonitor; import com.android.server.biometrics.sensors.BiometricScheduler; import com.android.server.biometrics.sensors.EnumerateConsumer; +import com.android.server.biometrics.sensors.ErrorConsumer; import com.android.server.biometrics.sensors.HalClientMonitor; import com.android.server.biometrics.sensors.Interruptable; import com.android.server.biometrics.sensors.LockoutCache; @@ -215,14 +216,14 @@ public class Sensor { + ", client: " + Utils.getClientName(client) + ", error: " + error + ", vendorCode: " + vendorCode); - if (!(client instanceof Interruptable)) { + if (!(client instanceof ErrorConsumer)) { Slog.e(mTag, "onError for non-error consumer: " + Utils.getClientName(client)); return; } - final Interruptable interruptable = (Interruptable) client; - interruptable.onError(error, vendorCode); + final ErrorConsumer errorConsumer = (ErrorConsumer) client; + errorConsumer.onError(error, vendorCode); if (error == Error.HW_UNAVAILABLE) { mCallback.onHardwareUnavailable(); @@ -581,8 +582,8 @@ public class Sensor { final BaseClientMonitor client = mScheduler.getCurrentClient(); if (client instanceof Interruptable) { Slog.e(mTag, "Sending ERROR_HW_UNAVAILABLE for client: " + client); - final Interruptable interruptable = (Interruptable) client; - interruptable.onError(FaceManager.FACE_ERROR_HW_UNAVAILABLE, + final ErrorConsumer errorConsumer = (ErrorConsumer) client; + errorConsumer.onError(FaceManager.FACE_ERROR_HW_UNAVAILABLE, 0 /* vendorCode */); FrameworkStatsLog.write(FrameworkStatsLog.BIOMETRIC_SYSTEM_HEALTH_ISSUE_DETECTED, 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 6bcc9e6f85d47..2cb2939cde1d3 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 @@ -64,6 +64,7 @@ import com.android.server.biometrics.sensors.BaseClientMonitor; import com.android.server.biometrics.sensors.BiometricScheduler; import com.android.server.biometrics.sensors.ClientMonitorCallbackConverter; import com.android.server.biometrics.sensors.EnumerateConsumer; +import com.android.server.biometrics.sensors.ErrorConsumer; import com.android.server.biometrics.sensors.HalClientMonitor; import com.android.server.biometrics.sensors.Interruptable; import com.android.server.biometrics.sensors.LockoutResetDispatcher; @@ -227,14 +228,14 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider { + ", client: " + (client != null ? client.getOwnerString() : null) + ", error: " + error + ", vendorCode: " + vendorCode); - if (!(client instanceof Interruptable)) { + if (!(client instanceof ErrorConsumer)) { Slog.e(TAG, "onError for non-error consumer: " + Utils.getClientName( client)); return; } - final Interruptable interruptable = (Interruptable) client; - interruptable.onError(error, vendorCode); + final ErrorConsumer errorConsumer = (ErrorConsumer) client; + errorConsumer.onError(error, vendorCode); if (error == BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE) { Slog.e(TAG, "Got ERROR_HW_UNAVAILABLE"); @@ -379,10 +380,10 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider { mCurrentUserId = UserHandle.USER_NULL; final BaseClientMonitor client = mScheduler.getCurrentClient(); - if (client instanceof Interruptable) { + if (client instanceof ErrorConsumer) { Slog.e(TAG, "Sending ERROR_HW_UNAVAILABLE for client: " + client); - final Interruptable interruptable = (Interruptable) client; - interruptable.onError(BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE, + final ErrorConsumer errorConsumer = (ErrorConsumer) client; + errorConsumer.onError(BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE, 0 /* vendorCode */); FrameworkStatsLog.write(FrameworkStatsLog.BIOMETRIC_SYSTEM_HEALTH_ISSUE_DETECTED, 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 4862d849613d9..6a9b125454084 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 @@ -50,6 +50,7 @@ import com.android.server.biometrics.sensors.AuthenticationConsumer; import com.android.server.biometrics.sensors.BaseClientMonitor; import com.android.server.biometrics.sensors.BiometricScheduler; import com.android.server.biometrics.sensors.EnumerateConsumer; +import com.android.server.biometrics.sensors.ErrorConsumer; import com.android.server.biometrics.sensors.HalClientMonitor; import com.android.server.biometrics.sensors.Interruptable; import com.android.server.biometrics.sensors.LockoutCache; @@ -190,14 +191,14 @@ class Sensor { + ", client: " + Utils.getClientName(client) + ", error: " + error + ", vendorCode: " + vendorCode); - if (!(client instanceof Interruptable)) { + if (!(client instanceof ErrorConsumer)) { Slog.e(mTag, "onError for non-error consumer: " + Utils.getClientName(client)); return; } - final Interruptable interruptable = (Interruptable) client; - interruptable.onError(error, vendorCode); + final ErrorConsumer errorConsumer = (ErrorConsumer) client; + errorConsumer.onError(error, vendorCode); if (error == Error.HW_UNAVAILABLE) { mCallback.onHardwareUnavailable(); @@ -549,10 +550,10 @@ class Sensor { public void onBinderDied() { final BaseClientMonitor client = mScheduler.getCurrentClient(); - if (client instanceof Interruptable) { + if (client instanceof ErrorConsumer) { Slog.e(mTag, "Sending ERROR_HW_UNAVAILABLE for client: " + client); - final Interruptable interruptable = (Interruptable) client; - interruptable.onError(FingerprintManager.FINGERPRINT_ERROR_HW_UNAVAILABLE, + final ErrorConsumer errorConsumer = (ErrorConsumer) client; + errorConsumer.onError(FingerprintManager.FINGERPRINT_ERROR_HW_UNAVAILABLE, 0 /* vendorCode */); FrameworkStatsLog.write(FrameworkStatsLog.BIOMETRIC_SYSTEM_HEALTH_ISSUE_DETECTED, 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 07affb18270eb..eb78245b0df56 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 @@ -68,6 +68,7 @@ import com.android.server.biometrics.sensors.BaseClientMonitor; import com.android.server.biometrics.sensors.BiometricScheduler; import com.android.server.biometrics.sensors.ClientMonitorCallbackConverter; import com.android.server.biometrics.sensors.EnumerateConsumer; +import com.android.server.biometrics.sensors.ErrorConsumer; import com.android.server.biometrics.sensors.HalClientMonitor; import com.android.server.biometrics.sensors.Interruptable; import com.android.server.biometrics.sensors.LockoutResetDispatcher; @@ -262,13 +263,13 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider + ", client: " + Utils.getClientName(client) + ", error: " + error + ", vendorCode: " + vendorCode); - if (!(client instanceof Interruptable)) { + if (!(client instanceof ErrorConsumer)) { Slog.e(TAG, "onError for non-error consumer: " + Utils.getClientName(client)); return; } - final Interruptable interruptable = (Interruptable) client; - interruptable.onError(error, vendorCode); + final ErrorConsumer errorConsumer = (ErrorConsumer) client; + errorConsumer.onError(error, vendorCode); if (error == BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE) { Slog.e(TAG, "Got ERROR_HW_UNAVAILABLE"); @@ -392,10 +393,10 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider mCurrentUserId = UserHandle.USER_NULL; final BaseClientMonitor client = mScheduler.getCurrentClient(); - if (client instanceof Interruptable) { + if (client instanceof ErrorConsumer) { Slog.e(TAG, "Sending ERROR_HW_UNAVAILABLE for client: " + client); - final Interruptable interruptable = (Interruptable) client; - interruptable.onError(BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE, + final ErrorConsumer errorConsumer = (ErrorConsumer) client; + errorConsumer.onError(BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE, 0 /* vendorCode */); FrameworkStatsLog.write(FrameworkStatsLog.BIOMETRIC_SYSTEM_HEALTH_ISSUE_DETECTED,