Fix enrollment preview surface leak

Bug: 189057897
Test: adb shell dumpsys SurfaceFlinger | grep -A 10 Offscreen | grep EnrollActivity
Change-Id: I9e11d0f1efc4428f2181370cb8be1b933aa5f548

11 files changed, 206 insertions, 89 deletions

diff --git a/core/java/android/hardware/face/FaceManager.java b/core/java/android/hardware/face/FaceManager.java
index 12557f9b73eb..c2a2c4c0678c 100644
--- a/core/java/android/hardware/face/FaceManager.java
+++ b/core/java/android/hardware/face/FaceManager.java
@@ -74,7 +74,7 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan
 
     private final IFaceService mService;
     private final Context mContext;
-    private IBinder mToken = new Binder();
+    private final IBinder mToken = new Binder();
     @Nullable private AuthenticationCallback mAuthenticationCallback;
     @Nullable private FaceDetectionCallback mFaceDetectionCallback;
     @Nullable private EnrollmentCallback mEnrollmentCallback;
@@ -86,7 +86,7 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan
     private Face mRemovalFace;
     private Handler mHandler;
 
-    private IFaceServiceReceiver mServiceReceiver = new IFaceServiceReceiver.Stub() {
+    private final IFaceServiceReceiver mServiceReceiver = new IFaceServiceReceiver.Stub() {
 
         @Override // binder call
         public void onEnrollResult(Face face, int remaining) {
@@ -293,7 +293,7 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan
 
     /**
      * Defaults to {@link FaceManager#enroll(int, byte[], CancellationSignal, EnrollmentCallback,
-     * int[], Surface)} with {@code surface} set to null.
+     * int[], Surface)} with {@code previewSurface} set to null.
      *
      * @see FaceManager#enroll(int, byte[], CancellationSignal, EnrollmentCallback, int[], Surface)
      * @hide
@@ -301,8 +301,8 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan
     @RequiresPermission(MANAGE_BIOMETRIC)
     public void enroll(int userId, byte[] hardwareAuthToken, CancellationSignal cancel,
             EnrollmentCallback callback, int[] disabledFeatures) {
-        enroll(userId, hardwareAuthToken, cancel, callback, disabledFeatures, null /* surface */,
-                false /* debugConsent */);
+        enroll(userId, hardwareAuthToken, cancel, callback, disabledFeatures,
+                null /* previewSurface */, false /* debugConsent */);
     }
 
     /**
@@ -319,14 +319,14 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan
      * @param cancel            an object that can be used to cancel enrollment
      * @param userId            the user to whom this face will belong to
      * @param callback          an object to receive enrollment events
-     * @param surface           optional camera preview surface for a single-camera device.
+     * @param previewSurface    optional camera preview surface for a single-camera device.
      *                          Must be null if not used.
      * @param debugConsent      a feature flag that the user has consented to debug.
      * @hide
      */
     @RequiresPermission(MANAGE_BIOMETRIC)
     public void enroll(int userId, byte[] hardwareAuthToken, CancellationSignal cancel,
-            EnrollmentCallback callback, int[] disabledFeatures, @Nullable Surface surface,
+            EnrollmentCallback callback, int[] disabledFeatures, @Nullable Surface previewSurface,
             boolean debugConsent) {
         if (callback == null) {
             throw new IllegalArgumentException("Must supply an enrollment callback");
@@ -346,7 +346,8 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan
                 mEnrollmentCallback = callback;
                 Trace.beginSection("FaceManager#enroll");
                 mService.enroll(userId, mToken, hardwareAuthToken, mServiceReceiver,
-                        mContext.getOpPackageName(), disabledFeatures, surface, debugConsent);
+                        mContext.getOpPackageName(), disabledFeatures, previewSurface,
+                        debugConsent);
             } catch (RemoteException e) {
                 Slog.w(TAG, "Remote exception in enroll: ", e);
                 // Though this may not be a hardware issue, it will cause apps to give up or
@@ -853,10 +854,10 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan
      * @hide
      */
     public static class AuthenticationResult {
-        private Face mFace;
-        private CryptoObject mCryptoObject;
-        private int mUserId;
-        private boolean mIsStrongBiometric;
+        private final Face mFace;
+        private final CryptoObject mCryptoObject;
+        private final int mUserId;
+        private final boolean mIsStrongBiometric;
 
         /**
          * Authentication result
@@ -1127,7 +1128,7 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan
     }
 
     private class OnAuthenticationCancelListener implements OnCancelListener {
-        private CryptoObject mCrypto;
+        private final CryptoObject mCrypto;
 
         OnAuthenticationCancelListener(CryptoObject crypto) {
             mCrypto = crypto;
diff --git a/core/java/android/hardware/face/IFaceService.aidl b/core/java/android/hardware/face/IFaceService.aidl
index 270d662a02a0..b9a49c6ced09 100644
--- a/core/java/android/hardware/face/IFaceService.aidl
+++ b/core/java/android/hardware/face/IFaceService.aidl
@@ -75,7 +75,7 @@ interface IFaceService {
 
     // Start face enrollment
     void enroll(int userId, IBinder token, in byte [] hardwareAuthToken, IFaceServiceReceiver receiver,
-            String opPackageName, in int [] disabledFeatures, in Surface surface, boolean debugConsent);
+            String opPackageName, in int [] disabledFeatures, in Surface previewSurface, boolean debugConsent);
 
     // Start remote face enrollment
     void enrollRemotely(int userId, IBinder token, in byte [] hardwareAuthToken, IFaceServiceReceiver receiver,
diff --git a/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java b/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java
index 779558e8893e..219e06358afb 100644
--- a/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java
+++ b/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java
@@ -215,7 +215,7 @@ public class FaceService extends SystemService {
         @Override // Binder call
         public void enroll(int userId, final IBinder token, final byte[] hardwareAuthToken,
                 final IFaceServiceReceiver receiver, final String opPackageName,
-                final int[] disabledFeatures, Surface surface, boolean debugConsent) {
+                final int[] disabledFeatures, Surface previewSurface, boolean debugConsent) {
             Utils.checkPermission(getContext(), MANAGE_BIOMETRIC);
 
             final Pair<Integer, ServiceProvider> provider = getSingleProvider();
@@ -225,8 +225,7 @@ public class FaceService extends SystemService {
             }
 
             provider.second.scheduleEnroll(provider.first, token, hardwareAuthToken, userId,
-                    receiver, opPackageName, disabledFeatures,
-                    convertSurfaceToNativeHandle(surface), debugConsent);
+                    receiver, opPackageName, disabledFeatures, previewSurface, debugConsent);
         }
 
         @Override // Binder call
@@ -703,5 +702,27 @@ public class FaceService extends SystemService {
         publishBinderService(Context.FACE_SERVICE, mServiceWrapper);
     }
 
-    private native NativeHandle convertSurfaceToNativeHandle(Surface surface);
+    /**
+     * Acquires a NativeHandle that can be used to access the provided surface. The returned handle
+     * must be explicitly released with {@link #releaseSurfaceHandle(NativeHandle)} to avoid memory
+     * leaks.
+     *
+     * The caller is responsible for ensuring that the surface is valid while using the handle.
+     * This method provides no lifecycle synchronization between the surface and the handle.
+     *
+     * @param surface a valid Surface.
+     * @return {@link android.os.NativeHandle} a NativeHandle for the provided surface.
+     */
+    public static native NativeHandle acquireSurfaceHandle(@NonNull Surface surface);
+
+    /**
+     * Releases resources associated with a NativeHandle that was acquired with
+     * {@link #acquireSurfaceHandle(Surface)}.
+     *
+     * This method has no affect on the surface for which the handle was acquired. It only frees up
+     * the resources that are associated with the handle.
+     *
+     * @param handle a handle that was obtained from {@link #acquireSurfaceHandle(Surface)}.
+     */
+    public static native void releaseSurfaceHandle(@NonNull NativeHandle handle);
 }
diff --git a/services/core/java/com/android/server/biometrics/sensors/face/ServiceProvider.java b/services/core/java/com/android/server/biometrics/sensors/face/ServiceProvider.java
index 6d6c2e9e975f..1d2ac3b3bfdc 100644
--- a/services/core/java/com/android/server/biometrics/sensors/face/ServiceProvider.java
+++ b/services/core/java/com/android/server/biometrics/sensors/face/ServiceProvider.java
@@ -26,8 +26,8 @@ import android.hardware.face.FaceManager;
 import android.hardware.face.FaceSensorPropertiesInternal;
 import android.hardware.face.IFaceServiceReceiver;
 import android.os.IBinder;
-import android.os.NativeHandle;
 import android.util.proto.ProtoOutputStream;
+import android.view.Surface;
 
 import com.android.server.biometrics.sensors.BaseClientMonitor;
 import com.android.server.biometrics.sensors.ClientMonitorCallbackConverter;
@@ -75,8 +75,8 @@ public interface ServiceProvider {
     int getLockoutModeForUser(int sensorId, int userId);
 
     /**
-     * Requests for the authenticatorId (whose source of truth is in the TEE or equivalent) to
-     * be invalidated. See {@link com.android.server.biometrics.sensors.InvalidationRequesterClient}
+     * Requests for the authenticatorId (whose source of truth is in the TEE or equivalent) to be
+     * invalidated. See {@link com.android.server.biometrics.sensors.InvalidationRequesterClient}
      */
     default void scheduleInvalidateAuthenticatorId(int sensorId, int userId,
             @NonNull IInvalidationCallback callback) {
@@ -96,7 +96,7 @@ public interface ServiceProvider {
 
     void scheduleEnroll(int sensorId, @NonNull IBinder token, @NonNull byte[] hardwareAuthToken,
             int userId, @NonNull IFaceServiceReceiver receiver, @NonNull String opPackageName,
-            @NonNull int[] disabledFeatures, @Nullable NativeHandle surfaceHandle,
+            @NonNull int[] disabledFeatures, @Nullable Surface previewSurface,
             boolean debugConsent);
 
     void cancelEnrollment(int sensorId, @NonNull IBinder token);
diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/BiometricTestSessionImpl.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/BiometricTestSessionImpl.java
index 57c1c74a51a8..66b942b085a4 100644
--- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/BiometricTestSessionImpl.java
+++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/BiometricTestSessionImpl.java
@@ -150,8 +150,8 @@ public class BiometricTestSessionImpl extends ITestSession.Stub {
         Utils.checkPermission(mContext, TEST_BIOMETRIC);
 
         mProvider.scheduleEnroll(mSensorId, new Binder(), new byte[69], userId, mReceiver,
-                mContext.getOpPackageName(), new int[0] /* disabledFeatures */, null /* surface */,
-                false /* debugConsent */);
+                mContext.getOpPackageName(), new int[0] /* disabledFeatures */,
+                null /* previewSurface */, false /* debugConsent */);
     }
 
     @Override
diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceEnrollClient.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceEnrollClient.java
index 0400e2257321..55c987a19e45 100644
--- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceEnrollClient.java
+++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceEnrollClient.java
@@ -26,21 +26,24 @@ import android.hardware.biometrics.face.EnrollmentType;
 import android.hardware.biometrics.face.Feature;
 import android.hardware.biometrics.face.IFace;
 import android.hardware.biometrics.face.ISession;
+import android.hardware.common.NativeHandle;
 import android.hardware.face.Face;
 import android.hardware.face.FaceEnrollFrame;
 import android.hardware.face.FaceManager;
 import android.os.IBinder;
-import android.os.NativeHandle;
 import android.os.RemoteException;
 import android.util.Slog;
+import android.view.Surface;
 
 import com.android.internal.R;
 import com.android.server.biometrics.HardwareAuthTokenUtils;
 import com.android.server.biometrics.Utils;
+import com.android.server.biometrics.sensors.BaseClientMonitor;
 import com.android.server.biometrics.sensors.BiometricNotificationUtils;
 import com.android.server.biometrics.sensors.BiometricUtils;
 import com.android.server.biometrics.sensors.ClientMonitorCallbackConverter;
 import com.android.server.biometrics.sensors.EnrollClient;
+import com.android.server.biometrics.sensors.face.FaceService;
 import com.android.server.biometrics.sensors.face.FaceUtils;
 
 import java.io.IOException;
@@ -57,16 +60,31 @@ public class FaceEnrollClient extends EnrollClient<ISession> {
     @NonNull private final int[] mEnrollIgnoreList;
     @NonNull private final int[] mEnrollIgnoreListVendor;
     @NonNull private final int[] mDisabledFeatures;
+    @Nullable private final Surface mPreviewSurface;
+    @Nullable private android.os.NativeHandle mOsPreviewHandle;
+    @Nullable private NativeHandle mHwPreviewHandle;
     @Nullable private ICancellationSignal mCancellationSignal;
-    @Nullable private android.hardware.common.NativeHandle mPreviewSurface;
     private final int mMaxTemplatesPerUser;
     private final boolean mDebugConsent;
 
+    private final BaseClientMonitor.Callback mPreviewHandleDeleterCallback =
+            new BaseClientMonitor.Callback() {
+                @Override
+                public void onClientStarted(@NonNull BaseClientMonitor clientMonitor) {
+                }
+
+                @Override
+                public void onClientFinished(@NonNull BaseClientMonitor clientMonitor,
+                        boolean success) {
+                    releaseSurfaceHandlesIfNeeded();
+                }
+            };
+
     FaceEnrollClient(@NonNull Context context, @NonNull LazyDaemon<ISession> lazyDaemon,
             @NonNull IBinder token, @NonNull ClientMonitorCallbackConverter listener, int userId,
             @NonNull byte[] hardwareAuthToken, @NonNull String opPackageName,
             @NonNull BiometricUtils<Face> utils, @NonNull int[] disabledFeatures, int timeoutSec,
-            @Nullable NativeHandle previewSurface, int sensorId, int maxTemplatesPerUser,
+            @Nullable Surface previewSurface, int sensorId, int maxTemplatesPerUser,
             boolean debugConsent) {
         super(context, lazyDaemon, token, listener, userId, hardwareAuthToken, opPackageName, utils,
                 timeoutSec, BiometricsProtoEnums.MODALITY_FACE, sensorId,
@@ -78,14 +96,7 @@ public class FaceEnrollClient extends EnrollClient<ISession> {
         mMaxTemplatesPerUser = maxTemplatesPerUser;
         mDebugConsent = debugConsent;
         mDisabledFeatures = disabledFeatures;
-        try {
-            // We must manually close the duplicate handle after it's no longer needed.
-            // The caller is responsible for closing the original handle.
-            mPreviewSurface = AidlNativeHandleUtils.dup(previewSurface);
-        } catch (IOException e) {
-            mPreviewSurface = null;
-            Slog.e(TAG, "Failed to dup previewSurface", e);
-        }
+        mPreviewSurface = previewSurface;
     }
 
     @Override
@@ -98,17 +109,7 @@ public class FaceEnrollClient extends EnrollClient<ISession> {
     @NonNull
     @Override
     protected Callback wrapCallbackForStart(@NonNull Callback callback) {
-        return new CompositeCallback(createALSCallback(), callback);
-    }
-
-    @Override
-    public void destroy() {
-        try {
-            AidlNativeHandleUtils.close(mPreviewSurface);
-        } catch (IOException e) {
-            Slog.e(TAG, "Failed to close mPreviewSurface", e);
-        }
-        super.destroy();
+        return new CompositeCallback(mPreviewHandleDeleterCallback, createALSCallback(), callback);
     }
 
     @Override
@@ -153,22 +154,23 @@ public class FaceEnrollClient extends EnrollClient<ISession> {
 
     @Override
     protected void startHalOperation() {
+        obtainSurfaceHandlesIfNeeded();
         try {
             List<Byte> featureList = new ArrayList<Byte>();
             if (mDebugConsent) {
-                featureList.add(new Byte(Feature.DEBUG));
+                featureList.add(Feature.DEBUG);
             }
 
             boolean shouldAddDiversePoses = true;
-            for (int i = 0; i < mDisabledFeatures.length; i++) {
-                if (AidlConversionUtils.convertFrameworkToAidlFeature(mDisabledFeatures[i])
+            for (int disabledFeature : mDisabledFeatures) {
+                if (AidlConversionUtils.convertFrameworkToAidlFeature(disabledFeature)
                         == Feature.REQUIRE_DIVERSE_POSES) {
                     shouldAddDiversePoses = false;
                 }
             }
 
             if (shouldAddDiversePoses) {
-                featureList.add(new Byte(Feature.REQUIRE_DIVERSE_POSES));
+                featureList.add(Feature.REQUIRE_DIVERSE_POSES);
             }
 
             byte[] features = new byte[featureList.size()];
@@ -178,7 +180,7 @@ public class FaceEnrollClient extends EnrollClient<ISession> {
 
             mCancellationSignal = getFreshDaemon().enroll(
                     HardwareAuthTokenUtils.toHardwareAuthToken(mHardwareAuthToken),
-                    EnrollmentType.DEFAULT, features, mPreviewSurface);
+                    EnrollmentType.DEFAULT, features, mHwPreviewHandle);
         } catch (RemoteException | IllegalArgumentException e) {
             Slog.e(TAG, "Exception when requesting enroll", e);
             onError(BiometricFaceConstants.FACE_ERROR_UNABLE_TO_PROCESS, 0 /* vendorCode */);
@@ -198,4 +200,55 @@ public class FaceEnrollClient extends EnrollClient<ISession> {
             }
         }
     }
+
+    private void obtainSurfaceHandlesIfNeeded() {
+        if (mPreviewSurface != null) {
+            // There is no direct way to convert Surface to android.hardware.common.NativeHandle. We
+            // first convert Surface to android.os.NativeHandle, and then android.os.NativeHandle to
+            // android.hardware.common.NativeHandle, which can be passed to the HAL.
+            // The resources for both handles must be explicitly freed to avoid memory leaks.
+            mOsPreviewHandle = FaceService.acquireSurfaceHandle(mPreviewSurface);
+            try {
+                // We must manually free up the resources for both handles after they are no longer
+                // needed. mHwPreviewHandle must be closed, but mOsPreviewHandle must be released
+                // through FaceService.
+                mHwPreviewHandle = AidlNativeHandleUtils.dup(mOsPreviewHandle);
+                Slog.v(TAG, "Obtained handles for the preview surface.");
+            } catch (IOException e) {
+                mHwPreviewHandle = null;
+                Slog.e(TAG, "Failed to dup mOsPreviewHandle", e);
+            }
+        }
+    }
+
+    private void releaseSurfaceHandlesIfNeeded() {
+        if (mPreviewSurface != null && mHwPreviewHandle == null) {
+            Slog.w(TAG, "mHwPreviewHandle is null even though mPreviewSurface is not null.");
+        }
+        if (mHwPreviewHandle != null) {
+            try {
+                Slog.v(TAG, "Closing mHwPreviewHandle");
+                AidlNativeHandleUtils.close(mHwPreviewHandle);
+            } catch (IOException e) {
+                Slog.e(TAG, "Failed to close mPreviewSurface", e);
+            }
+            mHwPreviewHandle = null;
+        }
+        if (mOsPreviewHandle != null) {
+            Slog.v(TAG, "Releasing mOsPreviewHandle");
+            FaceService.releaseSurfaceHandle(mOsPreviewHandle);
+            mOsPreviewHandle = null;
+        }
+        if (mPreviewSurface != null) {
+            Slog.v(TAG, "Releasing mPreviewSurface");
+            // We need to manually release this surface because it's a copy of the original surface
+            // that was sent to us by an app (e.g. Settings). The app cleans up its own surface (as
+            // part of the SurfaceView lifecycle, for example), but there is no mechanism in place
+            // that will clean up this copy.
+            // If this copy isn't cleaned up, it will eventually be garbage collected. However, this
+            // surface could be holding onto the native buffers that the GC is not aware of,
+            // exhausting the native memory before the GC feels the need to garbage collect.
+            mPreviewSurface.release();
+        }
+    }
 }
diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java
index 36a1292e0e7e..5c24108dcded 100644
--- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java
+++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java
@@ -37,13 +37,13 @@ import android.os.Binder;
 import android.os.Handler;
 import android.os.IBinder;
 import android.os.Looper;
-import android.os.NativeHandle;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.os.UserManager;
 import android.util.Slog;
 import android.util.SparseArray;
 import android.util.proto.ProtoOutputStream;
+import android.view.Surface;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.server.biometrics.Utils;
@@ -327,7 +327,7 @@ public class FaceProvider implements IBinder.DeathRecipient, ServiceProvider {
     public void scheduleEnroll(int sensorId, @NonNull IBinder token,
             @NonNull byte[] hardwareAuthToken, int userId, @NonNull IFaceServiceReceiver receiver,
             @NonNull String opPackageName, @NonNull int[] disabledFeatures,
-            @Nullable NativeHandle previewSurface, boolean debugConsent) {
+            @Nullable Surface previewSurface, boolean debugConsent) {
         mHandler.post(() -> {
             final int maxTemplatesPerUser = mSensors.get(
                     sensorId).getSensorProperties().maxEnrollmentsPerUser;
@@ -400,7 +400,7 @@ public class FaceProvider implements IBinder.DeathRecipient, ServiceProvider {
     @Override
     public void scheduleRemove(int sensorId, @NonNull IBinder token, int faceId, int userId,
             @NonNull IFaceServiceReceiver receiver, @NonNull String opPackageName) {
-        scheduleRemoveSpecifiedIds(sensorId, token, new int[] {faceId}, userId, receiver,
+        scheduleRemoveSpecifiedIds(sensorId, token, new int[]{faceId}, userId, receiver,
                 opPackageName);
     }
 
@@ -561,7 +561,8 @@ public class FaceProvider implements IBinder.DeathRecipient, ServiceProvider {
     }
 
     @Override
-    public void dumpHal(int sensorId, @NonNull FileDescriptor fd, @NonNull String[] args) {}
+    public void dumpHal(int sensorId, @NonNull FileDescriptor fd, @NonNull String[] args) {
+    }
 
     @Override
     public void binderDied() {
diff --git a/services/core/java/com/android/server/biometrics/sensors/face/hidl/BiometricTestSessionImpl.java b/services/core/java/com/android/server/biometrics/sensors/face/hidl/BiometricTestSessionImpl.java
index d0580c712610..b45578b4d447 100644
--- a/services/core/java/com/android/server/biometrics/sensors/face/hidl/BiometricTestSessionImpl.java
+++ b/services/core/java/com/android/server/biometrics/sensors/face/hidl/BiometricTestSessionImpl.java
@@ -139,7 +139,7 @@ public class BiometricTestSessionImpl extends ITestSession.Stub {
 
         mFace10.scheduleEnroll(mSensorId, new Binder(), new byte[69], userId, mReceiver,
                 mContext.getOpPackageName(), new int[0] /* disabledFeatures */,
-                null /* surfaceHandle */, false /* debugConsent */);
+                null /* previewSurface */, false /* debugConsent */);
     }
 
     @Override
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 26c5bca7f726..aa6093cef9b9 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
@@ -47,6 +47,7 @@ import android.os.UserManager;
 import android.provider.Settings;
 import android.util.Slog;
 import android.util.proto.ProtoOutputStream;
+import android.view.Surface;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.FrameworkStatsLog;
@@ -88,8 +89,8 @@ import java.util.List;
 import java.util.Map;
 
 /**
- * Supports a single instance of the {@link android.hardware.biometrics.face.V1_0} or
- * its extended minor versions.
+ * Supports a single instance of the {@link android.hardware.biometrics.face.V1_0} or its extended
+ * minor versions.
  */
 public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider {
 
@@ -325,7 +326,8 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider {
         }
     }
 
-    @VisibleForTesting Face10(@NonNull Context context,
+    @VisibleForTesting
+    Face10(@NonNull Context context,
             @NonNull FaceSensorPropertiesInternal sensorProps,
             @NonNull LockoutResetDispatcher lockoutResetDispatcher,
             @NonNull BiometricScheduler scheduler) {
@@ -570,7 +572,7 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider {
     public void scheduleEnroll(int sensorId, @NonNull IBinder token,
             @NonNull byte[] hardwareAuthToken, int userId, @NonNull IFaceServiceReceiver receiver,
             @NonNull String opPackageName, @NonNull int[] disabledFeatures,
-            @Nullable NativeHandle surfaceHandle, boolean debugConsent) {
+            @Nullable Surface previewSurface, boolean debugConsent) {
         mHandler.post(() -> {
             scheduleUpdateActiveUserWithoutHandler(userId);
 
@@ -579,7 +581,7 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider {
             final FaceEnrollClient client = new FaceEnrollClient(mContext, mLazyDaemon, token,
                     new ClientMonitorCallbackConverter(receiver), userId, hardwareAuthToken,
                     opPackageName, FaceUtils.getLegacyInstance(mSensorId), disabledFeatures,
-                    ENROLL_TIMEOUT_SEC, surfaceHandle, mSensorId);
+                    ENROLL_TIMEOUT_SEC, previewSurface, mSensorId);
 
             mScheduler.scheduleClientMonitor(client, new BaseClientMonitor.Callback() {
                 @Override
@@ -857,8 +859,8 @@ public class Face10 implements IHwBinder.DeathRecipient, ServiceProvider {
     }
 
     /**
-     * Schedules the {@link FaceUpdateActiveUserClient} without posting the work onto the
-     * handler. Many/most APIs are user-specific. However, the HAL requires explicit "setActiveUser"
+     * Schedules the {@link FaceUpdateActiveUserClient} without posting the work onto the handler.
+     * Many/most APIs are user-specific. However, the HAL requires explicit "setActiveUser"
      * invocation prior to authenticate/enroll/etc. Thus, internally we usually want to schedule
      * this operation on the same lambda/runnable as those operations so that the ordering is
      * correct.
diff --git a/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceEnrollClient.java b/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceEnrollClient.java
index d3bd18b6f704..455d6f868e65 100644
--- a/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceEnrollClient.java
+++ b/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceEnrollClient.java
@@ -26,9 +26,9 @@ import android.hardware.biometrics.face.V1_0.Status;
 import android.hardware.face.Face;
 import android.hardware.face.FaceManager;
 import android.os.IBinder;
-import android.os.NativeHandle;
 import android.os.RemoteException;
 import android.util.Slog;
+import android.view.Surface;
 
 import com.android.internal.R;
 import com.android.server.biometrics.Utils;
@@ -40,15 +40,14 @@ import java.util.ArrayList;
 import java.util.Arrays;
 
 /**
- * Face-specific enroll client supporting the {@link android.hardware.biometrics.face.V1_0}
- * HIDL interface.
+ * Face-specific enroll client supporting the {@link android.hardware.biometrics.face.V1_0} HIDL
+ * interface.
  */
 public class FaceEnrollClient extends EnrollClient<IBiometricsFace> {
 
     private static final String TAG = "FaceEnrollClient";
 
     @NonNull private final int[] mDisabledFeatures;
-    @Nullable private final NativeHandle mSurfaceHandle;
     @NonNull private final int[] mEnrollIgnoreList;
     @NonNull private final int[] mEnrollIgnoreListVendor;
 
@@ -56,12 +55,11 @@ public class FaceEnrollClient extends EnrollClient<IBiometricsFace> {
             @NonNull IBinder token, @NonNull ClientMonitorCallbackConverter listener, int userId,
             @NonNull byte[] hardwareAuthToken, @NonNull String owner,
             @NonNull BiometricUtils<Face> utils, @NonNull int[] disabledFeatures, int timeoutSec,
-            @Nullable NativeHandle surfaceHandle, int sensorId) {
+            @Nullable Surface previewSurface, int sensorId) {
         super(context, lazyDaemon, token, listener, userId, hardwareAuthToken, owner, utils,
                 timeoutSec, BiometricsProtoEnums.MODALITY_FACE, sensorId,
                 false /* shouldVibrate */);
         mDisabledFeatures = Arrays.copyOf(disabledFeatures, disabledFeatures.length);
-        mSurfaceHandle = surfaceHandle;
         mEnrollIgnoreList = getContext().getResources()
                 .getIntArray(R.array.config_face_acquire_enroll_ignorelist);
         mEnrollIgnoreListVendor = getContext().getResources()
diff --git a/services/core/jni/com_android_server_biometrics_SurfaceToNativeHandleConverter.cpp b/services/core/jni/com_android_server_biometrics_SurfaceToNativeHandleConverter.cpp
index e994c03a9239..d3d532bb882d 100644
--- a/services/core/jni/com_android_server_biometrics_SurfaceToNativeHandleConverter.cpp
+++ b/services/core/jni/com_android_server_biometrics_SurfaceToNativeHandleConverter.cpp
@@ -16,18 +16,19 @@
 
 #define LOG_TAG "SurfaceToNativeHandleConverter"
 
-#include <nativehelper/JNIHelp.h>
-#include "jni.h"
-
-#include <android/native_window_jni.h>
 #include <android_os_NativeHandle.h>
+#include <android_runtime/AndroidRuntime.h>
+#include <android_runtime/android_view_Surface.h>
 #include <gui/IGraphicBufferProducer.h>
 #include <gui/Surface.h>
 #include <gui/bufferqueue/1.0/WGraphicBufferProducer.h>
+#include <utils/Log.h>
 
-namespace android {
+#include "jni.h"
 
+namespace android {
 namespace {
+
 constexpr int WINDOW_HAL_TOKEN_SIZE_MAX = 256;
 
 native_handle_t* convertHalTokenToNativeHandle(const HalToken& halToken) {
@@ -54,33 +55,73 @@ native_handle_t* convertHalTokenToNativeHandle(const HalToken& halToken) {
     memcpy(&(nh->data[1]), halToken.data(), nhDataByteSize);
     return nh;
 }
-} // namespace
 
-using ::android::sp;
+HalToken convertNativeHandleToHalToken(native_handle_t* handle) {
+    int size = handle->data[0];
+    auto data = reinterpret_cast<uint8_t*>(&handle->data[1]);
+    HalToken halToken;
+    halToken.setToExternal(data, size);
+    return halToken;
+}
+
+jobject acquireSurfaceHandle(JNIEnv* env, jobject /* clazz */, jobject jSurface) {
+    ALOGD("%s", __func__);
+    if (jSurface == nullptr) {
+        ALOGE("%s: jSurface is null", __func__);
+        return nullptr;
+    }
 
-static jobject convertSurfaceToNativeHandle(JNIEnv* env, jobject /* clazz */,
-                                            jobject previewSurface) {
-    if (previewSurface == nullptr) {
+    sp<Surface> surface = android_view_Surface_getSurface(env, jSurface);
+    if (surface == nullptr) {
+        ALOGE("%s: surface is null", __func__);
         return nullptr;
     }
-    ANativeWindow* previewAnw = ANativeWindow_fromSurface(env, previewSurface);
-    sp<Surface> surface = static_cast<Surface*>(previewAnw);
+
     sp<IGraphicBufferProducer> igbp = surface->getIGraphicBufferProducer();
     sp<HGraphicBufferProducer> hgbp = new TWGraphicBufferProducer<HGraphicBufferProducer>(igbp);
+    // The HAL token will be closed in releaseSurfaceHandle.
     HalToken halToken;
     createHalToken(hgbp, &halToken);
+
     native_handle_t* native_handle = convertHalTokenToNativeHandle(halToken);
-    return JNativeHandle::MakeJavaNativeHandleObj(env, native_handle);
+    if (native_handle == nullptr) {
+        ALOGE("%s: native_handle is null", __func__);
+        return nullptr;
+    }
+    jobject jHandle = JNativeHandle::MakeJavaNativeHandleObj(env, native_handle);
+    native_handle_delete(native_handle);
+
+    return jHandle;
 }
 
-static const JNINativeMethod method_table[] = {
-        {"convertSurfaceToNativeHandle", "(Landroid/view/Surface;)Landroid/os/NativeHandle;",
-         reinterpret_cast<void*>(convertSurfaceToNativeHandle)},
+void releaseSurfaceHandle(JNIEnv* env, jobject /* clazz */, jobject jHandle) {
+    ALOGD("%s", __func__);
+    // Creates a native handle from a Java handle. We must call native_handle_delete when we're done
+    // with it because we created it, but we shouldn't call native_handle_close because we don't own
+    // the underlying FDs.
+    native_handle_t* handle =
+            JNativeHandle::MakeCppNativeHandle(env, jHandle, nullptr /* storage */);
+    if (handle == nullptr) {
+        ALOGE("%s: handle is null", __func__);
+        return;
+    }
+
+    HalToken token = convertNativeHandleToHalToken(handle);
+    ALOGD("%s: deleteHalToken, success: %d", __func__, deleteHalToken(token));
+    ALOGD("%s: native_handle_delete, success: %d", __func__, !native_handle_delete(handle));
+}
+
+const JNINativeMethod method_table[] = {
+        {"acquireSurfaceHandle", "(Landroid/view/Surface;)Landroid/os/NativeHandle;",
+         reinterpret_cast<void*>(acquireSurfaceHandle)},
+        {"releaseSurfaceHandle", "(Landroid/os/NativeHandle;)V",
+         reinterpret_cast<void*>(releaseSurfaceHandle)},
 };
+} // namespace
 
 int register_android_server_FaceService(JNIEnv* env) {
-    return jniRegisterNativeMethods(env, "com/android/server/biometrics/sensors/face/FaceService",
-                                    method_table, NELEM(method_table));
+    return AndroidRuntime::
+            registerNativeMethods(env, "com/android/server/biometrics/sensors/face/FaceService",
+                                  method_table, NELEM(method_table));
 }
-
-}; // namespace android
+} // namespace android