From 4daff35b77bc96016bd52eb8cee881d5f5e80fd0 Mon Sep 17 00:00:00 2001
From: Joe Bolinger <jbolinger@google.com>
Date: Fri, 11 Nov 2022 13:31:58 -0800
Subject: [PATCH] Allow awaitLux to fetch a value after destroy.

The logs are showing -1 lux values at a higher rate than expected (~10%). This may be because the lux probe is being destroyed before a value is ever reqeusted since the scheduler and auth callbacks are confined to the same thread but use different handlers. This allows destroy to be postponed until at least one value has been recorded.

Bug: 253318983
Test: atest ALSProbeTest
Change-Id: Id304bf502bcea9dc313f21253e117adb96324e10
---
 .../server/biometrics/log/ALSProbe.java       | 13 +++++-----
 .../server/biometrics/log/ALSProbeTest.java   | 25 +++++++++++++------
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/services/core/java/com/android/server/biometrics/log/ALSProbe.java b/services/core/java/com/android/server/biometrics/log/ALSProbe.java
index da43618436817..d584c99cea722 100644
--- a/services/core/java/com/android/server/biometrics/log/ALSProbe.java
+++ b/services/core/java/com/android/server/biometrics/log/ALSProbe.java
@@ -120,7 +120,7 @@ final class ALSProbe implements Probe {
 
         // if a final consumer is set it will call destroy/disable on the next value if requested
         if (!mDestroyed && mNextConsumer == null) {
-            disableLightSensorLoggingLocked();
+            disableLightSensorLoggingLocked(false /* destroying */);
         }
     }
 
@@ -130,7 +130,7 @@ final class ALSProbe implements Probe {
 
         // if a final consumer is set it will call destroy/disable on the next value if requested
         if (!mDestroyed && mNextConsumer == null) {
-            disableLightSensorLoggingLocked();
+            disableLightSensorLoggingLocked(true /* destroying */);
             mDestroyed = true;
         }
     }
@@ -177,11 +177,10 @@ final class ALSProbe implements Probe {
         final float current = mLastAmbientLux;
         if (current > -1f) {
             nextConsumer.consume(current);
-        } else if (mDestroyed) {
-            nextConsumer.consume(-1f);
         } else if (mNextConsumer != null) {
             mNextConsumer.add(nextConsumer);
         } else {
+            mDestroyed = false;
             mNextConsumer = nextConsumer;
             enableLightSensorLoggingLocked();
         }
@@ -199,12 +198,14 @@ final class ALSProbe implements Probe {
         resetTimerLocked(true /* start */);
     }
 
-    private void disableLightSensorLoggingLocked() {
+    private void disableLightSensorLoggingLocked(boolean destroying) {
         resetTimerLocked(false /* start */);
 
         if (mEnabled) {
             mEnabled = false;
-            mLastAmbientLux = -1;
+            if (!destroying) {
+                mLastAmbientLux = -1;
+            }
             mSensorManager.unregisterListener(mLightSensorListener);
             Slog.v(TAG, "Disable ALS: " + mLightSensorListener.hashCode());
         }
diff --git a/services/tests/servicestests/src/com/android/server/biometrics/log/ALSProbeTest.java b/services/tests/servicestests/src/com/android/server/biometrics/log/ALSProbeTest.java
index 0cff4f14bf23f..bb00634273396 100644
--- a/services/tests/servicestests/src/com/android/server/biometrics/log/ALSProbeTest.java
+++ b/services/tests/servicestests/src/com/android/server/biometrics/log/ALSProbeTest.java
@@ -125,12 +125,9 @@ public class ALSProbeTest {
         mProbe.destroy();
         mProbe.enable();
 
-        AtomicInteger lux = new AtomicInteger(10);
-        mProbe.awaitNextLux((v) -> lux.set(Math.round(v)), null /* handler */);
-
         verify(mSensorManager, never()).registerListener(any(), any(), anyInt());
         verifyNoMoreInteractions(mSensorManager);
-        assertThat(lux.get()).isLessThan(0);
+        assertThat(mProbe.getMostRecentLux()).isLessThan(0);
     }
 
     @Test
@@ -323,15 +320,27 @@ public class ALSProbeTest {
     }
 
     @Test
-    public void testNoNextLuxWhenDestroyed() {
+    public void testDestroyAllowsAwaitLuxExactlyOnce() {
+        final float lastValue = 5.5f;
         mProbe.destroy();
 
-        AtomicInteger lux = new AtomicInteger(-20);
+        AtomicInteger lux = new AtomicInteger(10);
         mProbe.awaitNextLux((v) -> lux.set(Math.round(v)), null /* handler */);
 
-        assertThat(lux.get()).isEqualTo(-1);
-        verify(mSensorManager, never()).registerListener(
+        verify(mSensorManager).registerListener(
                 mSensorEventListenerCaptor.capture(), any(), anyInt());
+        mSensorEventListenerCaptor.getValue().onSensorChanged(
+                new SensorEvent(mLightSensor, 1, 1, new float[]{lastValue}));
+
+        assertThat(lux.get()).isEqualTo(Math.round(lastValue));
+        verify(mSensorManager).unregisterListener(eq(mSensorEventListenerCaptor.getValue()));
+
+        lux.set(22);
+        mProbe.enable();
+        mProbe.awaitNextLux((v) -> lux.set(Math.round(v)), null /* handler */);
+        mProbe.enable();
+
+        assertThat(lux.get()).isEqualTo(Math.round(lastValue));
         verifyNoMoreInteractions(mSensorManager);
     }