Merge "Fix races in GHBM surface creation and usage" into sc-dev

packages/SystemUI/src/com/android/systemui/biometrics/UdfpsController.java

@@ -71,6 +71,7 @@ import com.android.systemui.statusbar.LockscreenShadeTransitionController;
 import com.android.systemui.statusbar.phone.StatusBar;
 import com.android.systemui.statusbar.phone.StatusBarKeyguardViewManager;
 import com.android.systemui.util.concurrency.DelayableExecutor;
+import com.android.systemui.util.concurrency.Execution;
 
 import java.util.Optional;
 
@@ -97,6 +98,7 @@ public class UdfpsController implements DozeReceiver {
     private static final long MIN_TOUCH_LOG_INTERVAL = 50;
 
     private final Context mContext;
+    private final Execution mExecution;
     private final FingerprintManager mFingerprintManager;
     @NonNull private final LayoutInflater mInflater;
     private final WindowManager mWindowManager;
@@ -496,6 +498,7 @@ public class UdfpsController implements DozeReceiver {
 
     @Inject
     public UdfpsController(@NonNull Context context,
+            @NonNull Execution execution,
             @NonNull LayoutInflater inflater,
             @Nullable FingerprintManager fingerprintManager,
             @NonNull WindowManager windowManager,
@@ -514,6 +517,7 @@ public class UdfpsController implements DozeReceiver {
             @Nullable Vibrator vibrator,
             @NonNull Optional<UdfpsHbmProvider> hbmProvider) {
         mContext = context;
+        mExecution = execution;
         // TODO (b/185124905): inject main handler and vibrator once done prototyping
         mMainHandler = new Handler(Looper.getMainLooper());
         mVibrator = vibrator;
@@ -820,8 +824,8 @@ public class UdfpsController implements DozeReceiver {
         mIsAodInterruptActive = false;
     }
 
-    // This method can be called from the UI thread.
     private void onFingerDown(int x, int y, float minor, float major) {
+        mExecution.assertIsMainThread();
         if (mView == null) {
             Log.w(TAG, "Null view in onFingerDown");
             return;
@@ -835,8 +839,8 @@ public class UdfpsController implements DozeReceiver {
         });
     }
 
-    // This method can be called from the UI thread.
     private void onFingerUp() {
+        mExecution.assertIsMainThread();
         mActivePointerId = -1;
         mGoodCaptureReceived = false;
         mMainHandler.removeCallbacks(mAcquiredVibration);

packages/SystemUI/src/com/android/systemui/biometrics/UdfpsSurfaceView.java

@@ -23,43 +23,36 @@ import android.graphics.Canvas;
 import android.graphics.Paint;
 import android.graphics.PixelFormat;
 import android.graphics.RectF;
-import android.os.Build;
-import android.os.UserHandle;
-import android.provider.Settings;
 import android.util.AttributeSet;
 import android.util.Log;
 import android.view.Surface;
 import android.view.SurfaceHolder;
 import android.view.SurfaceView;
 
-import com.android.systemui.biometrics.UdfpsHbmTypes.HbmType;
-
 /**
- * Under-display fingerprint sensor Surface View. The surface should be used for HBM-specific things
+ * Surface View for providing the Global High-Brightness Mode (GHBM) illumination for UDFPS.
- * only. All other animations should be done on the other view.
  */
-public class UdfpsSurfaceView extends SurfaceView implements UdfpsIlluminator {
+public class UdfpsSurfaceView extends SurfaceView implements SurfaceHolder.Callback {
     private static final String TAG = "UdfpsSurfaceView";
-    private static final String SETTING_HBM_TYPE =
-            "com.android.systemui.biometrics.UdfpsSurfaceView.hbmType";
-    private static final @HbmType int DEFAULT_HBM_TYPE = UdfpsHbmTypes.GLOBAL_HBM;
 
     /**
-     * This is used instead of {@link android.graphics.drawable.Drawable}, because the latter has
+     * Notifies {@link UdfpsView} when to enable GHBM illumination.
-     * several abstract methods that are not used here but require implementation.
      */
-    private interface SimpleDrawable {
+    interface GhbmIlluminationListener {
-        void draw(Canvas canvas);
+        /**
+         * @param surface the surface for which GHBM should be enabled.
+         * @param onIlluminatedRunnable a runnable that should be run after GHBM is enabled.
+         */
+        void enableGhbm(@NonNull Surface surface, @Nullable Runnable onIlluminatedRunnable);
     }
 
     @NonNull private final SurfaceHolder mHolder;
     @NonNull private final Paint mSensorPaint;
-    @NonNull private final SimpleDrawable mIlluminationDotDrawable;
-    private final int mOnIlluminatedDelayMs;
-    private final @HbmType int mHbmType;
 
-    @NonNull private RectF mSensorRect;
+    @Nullable private GhbmIlluminationListener mGhbmIlluminationListener;
-    @Nullable private UdfpsHbmProvider mHbmProvider;
+    @Nullable private Runnable mOnIlluminatedRunnable;
+    boolean mAwaitingSurfaceToStartIllumination;
+    boolean mHasValidSurface;
 
     public UdfpsSurfaceView(Context context, AttributeSet attrs) {
         super(context, attrs);
@@ -71,82 +64,77 @@ public class UdfpsSurfaceView extends SurfaceView implements UdfpsIlluminator {
         setZOrderOnTop(true);
 
         mHolder = getHolder();
+        mHolder.addCallback(this);
         mHolder.setFormat(PixelFormat.RGBA_8888);
 
-        mSensorRect = new RectF();
         mSensorPaint = new Paint(0 /* flags */);
         mSensorPaint.setAntiAlias(true);
         mSensorPaint.setARGB(255, 255, 255, 255);
         mSensorPaint.setStyle(Paint.Style.FILL);
+    }
 
-        mIlluminationDotDrawable = canvas -> {
+    @Override public void surfaceCreated(SurfaceHolder holder) {
-            canvas.drawOval(mSensorRect, mSensorPaint);
+        mHasValidSurface = true;
-        };
+        if (mAwaitingSurfaceToStartIllumination) {
-
+            doIlluminate(mOnIlluminatedRunnable);
-        mOnIlluminatedDelayMs = mContext.getResources().getInteger(
+            mOnIlluminatedRunnable = null;
-                com.android.internal.R.integer.config_udfps_illumination_transition_ms);
+            mAwaitingSurfaceToStartIllumination = false;
-
-        if (Build.IS_ENG || Build.IS_USERDEBUG) {
-            mHbmType = Settings.Secure.getIntForUser(mContext.getContentResolver(),
-                    SETTING_HBM_TYPE, DEFAULT_HBM_TYPE, UserHandle.USER_CURRENT);
-        } else {
-            mHbmType = DEFAULT_HBM_TYPE;
         }
     }
 
     @Override
-    public void setHbmProvider(@Nullable UdfpsHbmProvider hbmProvider) {
+    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
-        mHbmProvider = hbmProvider;
+        // Unused.
     }
 
-    @Override
+    @Override public void surfaceDestroyed(SurfaceHolder holder) {
-    public void startIllumination(@Nullable Runnable onIlluminatedRunnable) {
+        mHasValidSurface = false;
-        if (mHbmProvider != null) {
-            final Surface surface =
-                    (mHbmType == UdfpsHbmTypes.GLOBAL_HBM) ? mHolder.getSurface() : null;
-
-            final Runnable onHbmEnabled = () -> {
-                if (mHbmType == UdfpsHbmTypes.GLOBAL_HBM) {
-                    drawImmediately(mIlluminationDotDrawable);
-                }
-                if (onIlluminatedRunnable != null) {
-                    // No framework API can reliably tell when a frame reaches the panel. A timeout
-                    // is the safest solution.
-                    postDelayed(onIlluminatedRunnable, mOnIlluminatedDelayMs);
-                } else {
-                    Log.w(TAG, "startIllumination | onIlluminatedRunnable is null");
-                }
-            };
-
-            mHbmProvider.enableHbm(mHbmType, surface, onHbmEnabled);
-        } else {
-            Log.e(TAG, "startIllumination | mHbmProvider is null");
-        }
     }
 
-    @Override
+    void setGhbmIlluminationListener(@Nullable GhbmIlluminationListener listener) {
-    public void stopIllumination() {
+        mGhbmIlluminationListener = listener;
-        if (mHbmProvider != null) {
-            final Runnable onHbmDisabled =
-                    (mHbmType == UdfpsHbmTypes.GLOBAL_HBM) ? this::invalidate : null;
-            mHbmProvider.disableHbm(onHbmDisabled);
-        } else {
-            Log.e(TAG, "stopIllumination | mHbmProvider is null");
-        }
-    }
-
-    void onSensorRectUpdated(@NonNull RectF sensorRect) {
-        mSensorRect = sensorRect;
     }
 
     /**
-     * Immediately draws the provided drawable on this SurfaceView's surface.
+     * Note: there is no corresponding method to stop GHBM illumination. It is expected that
+     * {@link UdfpsView} will hide this view, which would destroy the surface and remove the
+     * illumination dot.
      */
-    private void drawImmediately(@NonNull SimpleDrawable drawable) {
+    void startGhbmIllumination(@Nullable Runnable onIlluminatedRunnable) {
+        if (mGhbmIlluminationListener == null) {
+            Log.e(TAG, "startIllumination | mGhbmIlluminationListener is null");
+            return;
+        }
+
+        if (mHasValidSurface) {
+            doIlluminate(onIlluminatedRunnable);
+        } else {
+            mAwaitingSurfaceToStartIllumination = true;
+            mOnIlluminatedRunnable = onIlluminatedRunnable;
+        }
+    }
+
+    private void doIlluminate(@Nullable Runnable onIlluminatedRunnable) {
+        if (mGhbmIlluminationListener == null) {
+            Log.e(TAG, "doIlluminate | mGhbmIlluminationListener is null");
+            return;
+        }
+
+        mGhbmIlluminationListener.enableGhbm(mHolder.getSurface(), onIlluminatedRunnable);
+    }
+
+    /**
+     * Immediately draws the illumination dot on this SurfaceView's surface.
+     */
+    void drawIlluminationDot(@NonNull RectF sensorRect) {
+        if (!mHasValidSurface) {
+            Log.e(TAG, "drawIlluminationDot | the surface is destroyed or was never created.");
+            return;
+        }
         Canvas canvas = null;
         try {
             canvas = mHolder.lockCanvas();
-            drawable.draw(canvas);
+            canvas.drawOval(sensorRect, mSensorPaint);
         } finally {
             // Make sure the surface is never left in a bad state.
             if (canvas != null) {

packages/SystemUI/src/com/android/systemui/biometrics/UdfpsView.java

@@ -26,14 +26,19 @@ import android.graphics.Paint;
 import android.graphics.PointF;
 import android.graphics.RectF;
 import android.hardware.fingerprint.FingerprintSensorPropertiesInternal;
+import android.os.Build;
+import android.os.UserHandle;
+import android.provider.Settings;
 import android.text.TextUtils;
 import android.util.AttributeSet;
 import android.util.Log;
 import android.view.MotionEvent;
+import android.view.Surface;
 import android.view.View;
 import android.widget.FrameLayout;
 
 import com.android.systemui.R;
+import com.android.systemui.biometrics.UdfpsHbmTypes.HbmType;
 import com.android.systemui.doze.DozeReceiver;
 
 /**
@@ -43,18 +48,25 @@ import com.android.systemui.doze.DozeReceiver;
 public class UdfpsView extends FrameLayout implements DozeReceiver, UdfpsIlluminator {
     private static final String TAG = "UdfpsView";
 
+    private static final String SETTING_HBM_TYPE =
+            "com.android.systemui.biometrics.UdfpsSurfaceView.hbmType";
+    private static final @HbmType int DEFAULT_HBM_TYPE = UdfpsHbmTypes.LOCAL_HBM;
+
     private static final int DEBUG_TEXT_SIZE_PX = 32;
 
     @NonNull private final RectF mSensorRect;
     @NonNull private final Paint mDebugTextPaint;
+    private final float mSensorTouchAreaCoefficient;
+    private final int mOnIlluminatedDelayMs;
+    private final @HbmType int mHbmType;
 
-    @NonNull private UdfpsSurfaceView mHbmSurfaceView;
+    // Only used for UdfpsHbmTypes.GLOBAL_HBM.
+    @Nullable private UdfpsSurfaceView mGhbmView;
+    // Can be different for enrollment, BiometricPrompt, Keyguard, etc.
     @Nullable private UdfpsAnimationViewController mAnimationViewController;
-
     // Used to obtain the sensor location.
     @NonNull private FingerprintSensorPropertiesInternal mSensorProps;
-
+    @Nullable private UdfpsHbmProvider mHbmProvider;
-    private final float mSensorTouchAreaCoefficient;
     @Nullable private String mDebugMessage;
     private boolean mIlluminationRequested;
 
@@ -81,7 +93,15 @@ public class UdfpsView extends FrameLayout implements DozeReceiver, UdfpsIllumin
         mDebugTextPaint.setColor(Color.BLUE);
         mDebugTextPaint.setTextSize(DEBUG_TEXT_SIZE_PX);
 
-        mIlluminationRequested = false;
+        mOnIlluminatedDelayMs = mContext.getResources().getInteger(
+                com.android.internal.R.integer.config_udfps_illumination_transition_ms);
+
+        if (Build.IS_ENG || Build.IS_USERDEBUG) {
+            mHbmType = Settings.Secure.getIntForUser(mContext.getContentResolver(),
+                    SETTING_HBM_TYPE, DEFAULT_HBM_TYPE, UserHandle.USER_CURRENT);
+        } else {
+            mHbmType = DEFAULT_HBM_TYPE;
+        }
     }
 
     // Don't propagate any touch events to the child views.
@@ -93,7 +113,9 @@ public class UdfpsView extends FrameLayout implements DozeReceiver, UdfpsIllumin
 
     @Override
     protected void onFinishInflate() {
-        mHbmSurfaceView = findViewById(R.id.hbm_view);
+        if (mHbmType == UdfpsHbmTypes.GLOBAL_HBM) {
+            mGhbmView = findViewById(R.id.hbm_view);
+        }
     }
 
     void setSensorProperties(@NonNull FingerprintSensorPropertiesInternal properties) {
@@ -102,7 +124,7 @@ public class UdfpsView extends FrameLayout implements DozeReceiver, UdfpsIllumin
 
     @Override
     public void setHbmProvider(@Nullable UdfpsHbmProvider hbmProvider) {
-        mHbmSurfaceView.setHbmProvider(hbmProvider);
+        mHbmProvider = hbmProvider;
     }
 
     @Override
@@ -125,7 +147,6 @@ public class UdfpsView extends FrameLayout implements DozeReceiver, UdfpsIllumin
                 2 * mSensorProps.sensorRadius + paddingX,
                 2 * mSensorProps.sensorRadius + paddingY);
 
-        mHbmSurfaceView.onSensorRectUpdated(new RectF(mSensorRect));
         if (mAnimationViewController != null) {
             mAnimationViewController.onSensorRectUpdated(new RectF(mSensorRect));
         }
@@ -204,8 +225,32 @@ public class UdfpsView extends FrameLayout implements DozeReceiver, UdfpsIllumin
         if (mAnimationViewController != null) {
             mAnimationViewController.onIlluminationStarting();
         }
-        mHbmSurfaceView.setVisibility(View.VISIBLE);
+
-        mHbmSurfaceView.startIllumination(onIlluminatedRunnable);
+        if (mGhbmView != null) {
+            mGhbmView.setGhbmIlluminationListener(this::doIlluminate);
+            mGhbmView.setVisibility(View.VISIBLE);
+            mGhbmView.startGhbmIllumination(onIlluminatedRunnable);
+        } else {
+            doIlluminate(null /* surface */, onIlluminatedRunnable);
+        }
+    }
+
+    private void doIlluminate(@Nullable Surface surface, @Nullable Runnable onIlluminatedRunnable) {
+        if (mGhbmView != null && surface == null) {
+            Log.e(TAG, "doIlluminate | surface must be non-null for GHBM");
+        }
+        mHbmProvider.enableHbm(mHbmType, surface, () -> {
+            if (mGhbmView != null) {
+                mGhbmView.drawIlluminationDot(mSensorRect);
+            }
+            if (onIlluminatedRunnable != null) {
+                // No framework API can reliably tell when a frame reaches the panel. A timeout
+                // is the safest solution.
+                postDelayed(onIlluminatedRunnable, mOnIlluminatedDelayMs);
+            } else {
+                Log.w(TAG, "doIlluminate | onIlluminatedRunnable is null");
+            }
+        });
     }
 
     @Override
@@ -214,7 +259,10 @@ public class UdfpsView extends FrameLayout implements DozeReceiver, UdfpsIllumin
         if (mAnimationViewController != null) {
             mAnimationViewController.onIlluminationStopped();
         }
-        mHbmSurfaceView.setVisibility(View.INVISIBLE);
+        if (mGhbmView != null) {
-        mHbmSurfaceView.stopIllumination();
+            mGhbmView.setGhbmIlluminationListener(null);
+            mGhbmView.setVisibility(View.INVISIBLE);
+        }
+        mHbmProvider.disableHbm(null /* onHbmDisabled */);
     }
 }

packages/SystemUI/tests/src/com/android/systemui/biometrics/UdfpsControllerTest.java

@@ -56,6 +56,8 @@ import com.android.systemui.plugins.statusbar.StatusBarStateController;
 import com.android.systemui.statusbar.LockscreenShadeTransitionController;
 import com.android.systemui.statusbar.phone.StatusBar;
 import com.android.systemui.statusbar.phone.StatusBarKeyguardViewManager;
+import com.android.systemui.util.concurrency.Execution;
+import com.android.systemui.util.concurrency.FakeExecution;
 import com.android.systemui.util.concurrency.FakeExecutor;
 import com.android.systemui.util.time.FakeSystemClock;
 
@@ -75,7 +77,7 @@ import java.util.Optional;
 
 @SmallTest
 @RunWith(AndroidTestingRunner.class)
-@RunWithLooper
+@RunWithLooper(setAsMainLooper = true)
 public class UdfpsControllerTest extends SysuiTestCase {
 
     // Use this for inputs going into SystemUI. Use UdfpsController.mUdfpsSensorId for things
@@ -89,6 +91,7 @@ public class UdfpsControllerTest extends SysuiTestCase {
     private UdfpsController mUdfpsController;
 
     // Dependencies
+    private Execution mExecution;
     @Mock
     private LayoutInflater mLayoutInflater;
     @Mock
@@ -145,6 +148,8 @@ public class UdfpsControllerTest extends SysuiTestCase {
     @Before
     public void setUp() {
         setUpResources();
+        mExecution = new FakeExecution();
+
         when(mLayoutInflater.inflate(R.layout.udfps_view, null, false)).thenReturn(mUdfpsView);
         final List<FingerprintSensorPropertiesInternal> props = new ArrayList<>();
 
@@ -166,6 +171,7 @@ public class UdfpsControllerTest extends SysuiTestCase {
         mFgExecutor = new FakeExecutor(new FakeSystemClock());
         mUdfpsController = new UdfpsController(
                 mContext,
+                mExecution,
                 mLayoutInflater,
                 mFingerprintManager,
                 mWindowManager,