From 14cdb17059c98a1687036d31de3fbe72ebeb1e63 Mon Sep 17 00:00:00 2001
From: Fiona Campbell <flc@google.com>
Date: Fri, 14 Oct 2022 17:18:03 +0000
Subject: [PATCH] Ensure BrightnessTracker is only "start()"ed once

Here, we use mStarted to ensure that we dont reregister any broadcast
recievers. This was happening because initialize() would be called for
each DPC (of which there is one per display). This would cause start()
and stop() each to be called multiple times - causing a crash. There is
only one BrightnessTracker per device, however it needs to be
initialized within DPC because we need an initial brightness value,
which the first-initialised DPC provides us with.

Test: atest BrightnessTrackerTest
Bug: 253267570
Change-Id: I8f2aa74f5938cb370e97bf55dd97823be2a32627
---
 .../server/display/BrightnessTracker.java     | 10 ++++++++
 .../server/display/BrightnessTrackerTest.java | 24 +++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/services/core/java/com/android/server/display/BrightnessTracker.java b/services/core/java/com/android/server/display/BrightnessTracker.java
index 6de08aed96873..1686cb2b246d9 100644
--- a/services/core/java/com/android/server/display/BrightnessTracker.java
+++ b/services/core/java/com/android/server/display/BrightnessTracker.java
@@ -220,6 +220,11 @@ public class BrightnessTracker {
     }
 
     private void backgroundStart(float initialBrightness) {
+        synchronized (mDataCollectionLock) {
+            if (mStarted) {
+                return;
+            }
+        }
         if (DEBUG) {
             Slog.d(TAG, "Background start");
         }
@@ -250,6 +255,11 @@ public class BrightnessTracker {
 
     /** Stop listening for events */
     void stop() {
+        synchronized (mDataCollectionLock) {
+            if (!mStarted) {
+                return;
+            }
+        }
         if (DEBUG) {
             Slog.d(TAG, "Stop");
         }
diff --git a/services/tests/servicestests/src/com/android/server/display/BrightnessTrackerTest.java b/services/tests/servicestests/src/com/android/server/display/BrightnessTrackerTest.java
index 0a5df410bcdb3..c2e8417f2ff0e 100644
--- a/services/tests/servicestests/src/com/android/server/display/BrightnessTrackerTest.java
+++ b/services/tests/servicestests/src/com/android/server/display/BrightnessTrackerTest.java
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -885,6 +886,29 @@ public class BrightnessTrackerTest {
         assertNull(mInjector.mLightSensor);
     }
 
+    @Test
+    public void testOnlyOneReceiverRegistered() {
+        assertNull(mInjector.mLightSensor);
+        assertNull(mInjector.mSensorListener);
+        startTracker(mTracker, 0.3f, false);
+
+        assertNotNull(mInjector.mLightSensor);
+        assertNotNull(mInjector.mSensorListener);
+        Sensor registeredLightSensor = mInjector.mLightSensor;
+        SensorEventListener registeredSensorListener = mInjector.mSensorListener;
+
+        mTracker.start(0.3f);
+        assertSame(registeredLightSensor, mInjector.mLightSensor);
+        assertSame(registeredSensorListener, mInjector.mSensorListener);
+
+        mTracker.stop();
+        assertNull(mInjector.mLightSensor);
+        assertNull(mInjector.mSensorListener);
+
+        // mInjector asserts that we aren't removing a null receiver
+        mTracker.stop();
+    }
+
     private InputStream getInputStream(String data) {
         return new ByteArrayInputStream(data.getBytes(StandardCharsets.UTF_8));
     }