Put DWBC calls on handler

Without DWBC calls on a handler, it was possible for DPC.stop() and
therefore dwbc.disable() to be called whilst querying other dwbc data.

This change puts the critical methods on the dpc/dpc2 handler, so that
this issue does not occur.

Bug: 283886342
Test: atest com.android.server.display
Change-Id: Ia0688642189b050353b45dd31431d5f157b428b8

services/core/java/com/android/server/display/DisplayPowerController.java

@@ -141,6 +141,9 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
     private static final int MSG_STATSD_HBM_BRIGHTNESS = 13;
     private static final int MSG_SWITCH_USER = 14;
     private static final int MSG_BOOT_COMPLETED = 15;
+    private static final int MSG_SET_DWBC_STRONG_MODE = 16;
+    private static final int MSG_SET_DWBC_COLOR_OVERRIDE = 17;
+    private static final int MSG_SET_DWBC_LOGGING_ENABLED = 18;
 
     private static final int PROXIMITY_UNKNOWN = -1;
     private static final int PROXIMITY_NEGATIVE = 0;
@@ -436,6 +439,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
     private final boolean mSkipScreenOnBrightnessRamp;
 
     // Display white balance components.
+    // Critical methods must be called on DPC handler thread.
     @Nullable
     private final DisplayWhiteBalanceSettings mDisplayWhiteBalanceSettings;
     @Nullable
@@ -680,9 +684,9 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
         DisplayWhiteBalanceController displayWhiteBalanceController = null;
         if (mDisplayId == Display.DEFAULT_DISPLAY) {
             try {
+                displayWhiteBalanceController = injector.getDisplayWhiteBalanceController(
+                        mHandler, mSensorManager, resources);
                 displayWhiteBalanceSettings = new DisplayWhiteBalanceSettings(mContext, mHandler);
-                displayWhiteBalanceController = DisplayWhiteBalanceFactory.create(mHandler,
-                        mSensorManager, resources);
                 displayWhiteBalanceSettings.setCallbacks(this);
                 displayWhiteBalanceController.setCallbacks(this);
             } catch (Exception e) {
@@ -1025,10 +1029,6 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
             Message msg = mHandler.obtainMessage(MSG_STOP);
             mHandler.sendMessageAtTime(msg, mClock.uptimeMillis());
 
-            if (mDisplayWhiteBalanceController != null) {
-                mDisplayWhiteBalanceController.setEnabled(false);
-            }
-
             if (mAutomaticBrightnessController != null) {
                 mAutomaticBrightnessController.stop();
             }
@@ -1334,9 +1334,11 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
                 mAutomaticBrightnessController.switchToInteractiveScreenBrightnessMode();
             }
         }
-        if (mDisplayWhiteBalanceController != null) {
+
-            mDisplayWhiteBalanceController.setStrongModeEnabled(isIdle);
+        Message msg = mHandler.obtainMessage();
-        }
+        msg.what = MSG_SET_DWBC_STRONG_MODE;
+        msg.arg1 = isIdle ? 1 : 0;
+        mHandler.sendMessageAtTime(msg, mClock.uptimeMillis());
     }
 
     private final Animator.AnimatorListener mAnimatorListener = new Animator.AnimatorListener() {
@@ -1405,8 +1407,13 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
         if (mScreenOffBrightnessSensorController != null) {
             mScreenOffBrightnessSensorController.stop();
         }
+        if (mDisplayWhiteBalanceController != null) {
+            mDisplayWhiteBalanceController.setEnabled(false);
         }
 
+    }
+
+    // Call from handler thread
     private void updatePowerState() {
         Trace.traceBegin(Trace.TRACE_TAG_POWER,
                 "DisplayPowerController#updatePowerState");
@@ -2058,6 +2065,32 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
         }
     }
 
+    private void setDwbcOverride(float cct) {
+        if (mDisplayWhiteBalanceController != null) {
+            mDisplayWhiteBalanceController.setAmbientColorTemperatureOverride(cct);
+            // The ambient color temperature override is only applied when the ambient color
+            // temperature changes or is updated, so it doesn't necessarily change the screen color
+            // temperature immediately. So, let's make it!
+            // We can call this directly, since we're already on the handler thread.
+            updatePowerState();
+        }
+    }
+
+    private void setDwbcStrongMode(int arg) {
+        if (mDisplayWhiteBalanceController != null) {
+            final boolean isIdle = (arg == 1);
+            mDisplayWhiteBalanceController.setStrongModeEnabled(isIdle);
+        }
+    }
+
+    private void setDwbcLoggingEnabled(int arg) {
+        if (mDisplayWhiteBalanceController != null) {
+            final boolean shouldEnable = (arg == 1);
+            mDisplayWhiteBalanceController.setLoggingEnabled(shouldEnable);
+            mDisplayWhiteBalanceSettings.setLoggingEnabled(shouldEnable);
+        }
+    }
+
     @Override
     public void updateBrightness() {
         sendUpdatePowerState();
@@ -3332,6 +3365,19 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
                     mBootCompleted = true;
                     updatePowerState();
                     break;
+
+                case MSG_SET_DWBC_STRONG_MODE:
+                    setDwbcStrongMode(msg.arg1);
+                    break;
+
+                case MSG_SET_DWBC_COLOR_OVERRIDE:
+                    final float cct = Float.intBitsToFloat(msg.arg1);
+                    setDwbcOverride(cct);
+                    break;
+
+                case MSG_SET_DWBC_LOGGING_ENABLED:
+                    setDwbcLoggingEnabled(msg.arg1);
+                    break;
             }
         }
     }
@@ -3399,21 +3445,18 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
 
     @Override
     public void setDisplayWhiteBalanceLoggingEnabled(boolean enabled) {
-        if (mDisplayWhiteBalanceController != null) {
+        Message msg = mHandler.obtainMessage();
-            mDisplayWhiteBalanceController.setLoggingEnabled(enabled);
+        msg.what = MSG_SET_DWBC_LOGGING_ENABLED;
-            mDisplayWhiteBalanceSettings.setLoggingEnabled(enabled);
+        msg.arg1 = enabled ? 1 : 0;
-        }
+        msg.sendToTarget();
     }
 
     @Override
     public void setAmbientColorTemperatureOverride(float cct) {
-        if (mDisplayWhiteBalanceController != null) {
+        Message msg = mHandler.obtainMessage();
-            mDisplayWhiteBalanceController.setAmbientColorTemperatureOverride(cct);
+        msg.what = MSG_SET_DWBC_COLOR_OVERRIDE;
-            // The ambient color temperature override is only applied when the ambient color
+        msg.arg1 = Float.floatToIntBits(cct);
-            // temperature changes or is updated, so it doesn't necessarily change the screen color
+        msg.sendToTarget();
-            // temperature immediately. So, let's make it!
-            sendUpdatePowerState();
-        }
     }
 
     @VisibleForTesting
@@ -3544,6 +3587,12 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
                     displayUniqueId, brightnessMin, brightnessMax, hbmData, hdrBrightnessCfg,
                     hbmChangeCallback, hbmMetadata, context);
         }
+
+        DisplayWhiteBalanceController getDisplayWhiteBalanceController(Handler handler,
+                SensorManager sensorManager, Resources resources) {
+            return DisplayWhiteBalanceFactory.create(handler,
+                    sensorManager, resources);
+        }
     }
 
     static class CachedBrightnessInfo {

services/core/java/com/android/server/display/DisplayPowerController2.java

@@ -141,6 +141,11 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal
     private static final int MSG_STATSD_HBM_BRIGHTNESS = 11;
     private static final int MSG_SWITCH_USER = 12;
     private static final int MSG_BOOT_COMPLETED = 13;
+    private static final int MSG_SET_DWBC_STRONG_MODE = 14;
+    private static final int MSG_SET_DWBC_COLOR_OVERRIDE = 15;
+    private static final int MSG_SET_DWBC_LOGGING_ENABLED = 16;
+
+
 
     private static final int BRIGHTNESS_CHANGE_STATSD_REPORT_INTERVAL_MS = 500;
 
@@ -367,6 +372,7 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal
     private final boolean mSkipScreenOnBrightnessRamp;
 
     // Display white balance components.
+    // Critical methods must be called on DPC2 handler thread.
     @Nullable
     private final DisplayWhiteBalanceSettings mDisplayWhiteBalanceSettings;
     @Nullable
@@ -561,9 +567,9 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal
         DisplayWhiteBalanceController displayWhiteBalanceController = null;
         if (mDisplayId == Display.DEFAULT_DISPLAY) {
             try {
+                displayWhiteBalanceController = mInjector.getDisplayWhiteBalanceController(
+                        mHandler, mSensorManager, resources);
                 displayWhiteBalanceSettings = new DisplayWhiteBalanceSettings(mContext, mHandler);
-                displayWhiteBalanceController = DisplayWhiteBalanceFactory.create(mHandler,
-                        mSensorManager, resources);
                 displayWhiteBalanceSettings.setCallbacks(this);
                 displayWhiteBalanceController.setCallbacks(this);
             } catch (Exception e) {
@@ -835,10 +841,6 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal
             Message msg = mHandler.obtainMessage(MSG_STOP);
             mHandler.sendMessageAtTime(msg, mClock.uptimeMillis());
 
-            if (mDisplayWhiteBalanceController != null) {
-                mDisplayWhiteBalanceController.setEnabled(false);
-            }
-
             if (mAutomaticBrightnessController != null) {
                 mAutomaticBrightnessController.stop();
             }
@@ -1149,9 +1151,10 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal
                 mAutomaticBrightnessController.switchToInteractiveScreenBrightnessMode();
             }
         }
-        if (mDisplayWhiteBalanceController != null) {
+        Message msg = mHandler.obtainMessage();
-            mDisplayWhiteBalanceController.setStrongModeEnabled(isIdle);
+        msg.what = MSG_SET_DWBC_STRONG_MODE;
-        }
+        msg.arg1 = isIdle ? 1 : 0;
+        mHandler.sendMessageAtTime(msg, mClock.uptimeMillis());
     }
 
     private final Animator.AnimatorListener mAnimatorListener = new Animator.AnimatorListener() {
@@ -1205,8 +1208,13 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal
         if (mScreenOffBrightnessSensorController != null) {
             mScreenOffBrightnessSensorController.stop();
         }
+
+        if (mDisplayWhiteBalanceController != null) {
+            mDisplayWhiteBalanceController.setEnabled(false);
+        }
     }
 
+    // Call from handler thread
     private void updatePowerState() {
         Trace.traceBegin(Trace.TRACE_TAG_POWER,
                 "DisplayPowerController#updatePowerState");
@@ -1705,6 +1713,32 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal
         }
     }
 
+    private void setDwbcOverride(float cct) {
+        if (mDisplayWhiteBalanceController != null) {
+            mDisplayWhiteBalanceController.setAmbientColorTemperatureOverride(cct);
+            // The ambient color temperature override is only applied when the ambient color
+            // temperature changes or is updated, so it doesn't necessarily change the screen color
+            // temperature immediately. So, let's make it!
+            // We can call this directly, since we're already on the handler thread.
+            updatePowerState();
+        }
+    }
+
+    private void setDwbcStrongMode(int arg) {
+        if (mDisplayWhiteBalanceController != null) {
+            final boolean isIdle = (arg == 1);
+            mDisplayWhiteBalanceController.setStrongModeEnabled(isIdle);
+        }
+    }
+
+    private void setDwbcLoggingEnabled(int arg) {
+        if (mDisplayWhiteBalanceController != null) {
+            final boolean enabled = (arg == 1);
+            mDisplayWhiteBalanceController.setLoggingEnabled(enabled);
+            mDisplayWhiteBalanceSettings.setLoggingEnabled(enabled);
+        }
+    }
+
     @Override
     public void updateBrightness() {
         sendUpdatePowerState();
@@ -2730,6 +2764,19 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal
                     mBootCompleted = true;
                     updatePowerState();
                     break;
+
+                case MSG_SET_DWBC_STRONG_MODE:
+                    setDwbcStrongMode(msg.arg1);
+                    break;
+
+                case MSG_SET_DWBC_COLOR_OVERRIDE:
+                    final float cct = Float.intBitsToFloat(msg.arg1);
+                    setDwbcOverride(cct);
+                    break;
+
+                case MSG_SET_DWBC_LOGGING_ENABLED:
+                    setDwbcLoggingEnabled(msg.arg1);
+                    break;
             }
         }
     }
@@ -2780,21 +2827,18 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal
 
     @Override
     public void setDisplayWhiteBalanceLoggingEnabled(boolean enabled) {
-        if (mDisplayWhiteBalanceController != null) {
+        Message msg = mHandler.obtainMessage();
-            mDisplayWhiteBalanceController.setLoggingEnabled(enabled);
+        msg.what = MSG_SET_DWBC_LOGGING_ENABLED;
-            mDisplayWhiteBalanceSettings.setLoggingEnabled(enabled);
+        msg.arg1 = enabled ? 1 : 0;
-        }
+        msg.sendToTarget();
     }
 
     @Override
     public void setAmbientColorTemperatureOverride(float cct) {
-        if (mDisplayWhiteBalanceController != null) {
+        Message msg = mHandler.obtainMessage();
-            mDisplayWhiteBalanceController.setAmbientColorTemperatureOverride(cct);
+        msg.what = MSG_SET_DWBC_COLOR_OVERRIDE;
-            // The ambient color temperature override is only applied when the ambient color
+        msg.arg1 = Float.floatToIntBits(cct);
-            // temperature changes or is updated, so it doesn't necessarily change the screen color
+        msg.sendToTarget();
-            // temperature immediately. So, let's make it!
-            sendUpdatePowerState();
-        }
     }
 
     /** Functional interface for providing time. */
@@ -2917,6 +2961,12 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal
                     displayUniqueId, brightnessMin, brightnessMax, hbmData, hdrBrightnessCfg,
                     hbmChangeCallback, hbmMetadata, context);
         }
+
+        DisplayWhiteBalanceController getDisplayWhiteBalanceController(Handler handler,
+                SensorManager sensorManager, Resources resources) {
+            return DisplayWhiteBalanceFactory.create(handler,
+                    sensorManager, resources);
+        }
     }
 
     static class CachedBrightnessInfo {

services/core/java/com/android/server/display/whitebalance/DisplayWhiteBalanceController.java

@@ -37,8 +37,11 @@ import java.util.Objects;
  * - Uses the AmbientColorTemperatureSensor to detect changes in the ambient color temperature;
  * - Uses the AmbientColorTemperatureFilter to average these changes over time, filter out the
  *   noise, and arrive at an estimate of the actual ambient color temperature;
- * - Uses the DisplayWhiteBalanceThrottler to decide whether the display color tempearture should
+ * - Uses the DisplayWhiteBalanceThrottler to decide whether the display color temperature should
  *   be updated, suppressing changes that are too frequent or too minor.
+ *
+ *   Calls to this class must happen on the DisplayPowerController(2) handler, to ensure
+ *   values do not get out of sync.
  */
 public class DisplayWhiteBalanceController implements
         AmbientSensor.AmbientBrightnessSensor.Callbacks,

services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerController2Test.java

@@ -121,7 +121,8 @@ public final class DisplayPowerController2Test {
     private PowerManager mPowerManagerMock;
     @Mock
     private ColorDisplayService.ColorDisplayServiceInternal mCdsiMock;
-
+    @Mock
+    private DisplayWhiteBalanceController mDisplayWhiteBalanceControllerMock;
     @Captor
     private ArgumentCaptor<SensorEventListener> mSensorEventListenerCaptor;
 
@@ -1096,6 +1097,18 @@ public final class DisplayPowerController2Test {
         verify(mHolder.brightnessSetting).setBrightness(clampedBrightness);
     }
 
+    @Test
+    public void testDwbcCallsHappenOnHandler() {
+        mHolder = createDisplayPowerController(DISPLAY_ID, UNIQUE_ID);
+
+        mHolder.dpc.setAutomaticScreenBrightnessMode(true);
+        verify(mDisplayWhiteBalanceControllerMock, never()).setStrongModeEnabled(true);
+
+        // dispatch handler looper
+        advanceTime(1);
+        verify(mDisplayWhiteBalanceControllerMock, times(1)).setStrongModeEnabled(true);
+    }
+
     /**
      * Creates a mock and registers it to {@link LocalServices}.
      */
@@ -1385,5 +1398,11 @@ public final class DisplayPowerController2Test {
                 Context context) {
             return mHighBrightnessModeController;
         }
+
+        @Override
+        DisplayWhiteBalanceController getDisplayWhiteBalanceController(Handler handler,
+                SensorManager sensorManager, Resources resources) {
+            return mDisplayWhiteBalanceControllerMock;
+        }
     }
 }

services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerControllerTest.java

@@ -121,7 +121,8 @@ public final class DisplayPowerControllerTest {
     private PowerManager mPowerManagerMock;
     @Mock
     private ColorDisplayService.ColorDisplayServiceInternal mCdsiMock;
-
+    @Mock
+    private DisplayWhiteBalanceController mDisplayWhiteBalanceControllerMock;
     @Captor
     private ArgumentCaptor<SensorEventListener> mSensorEventListenerCaptor;
 
@@ -1102,6 +1103,18 @@ public final class DisplayPowerControllerTest {
         verify(mHolder.brightnessSetting).setBrightness(clampedBrightness);
     }
 
+    @Test
+    public void testDwbcCallsHappenOnHandler() {
+        mHolder = createDisplayPowerController(DISPLAY_ID, UNIQUE_ID);
+
+        mHolder.dpc.setAutomaticScreenBrightnessMode(true);
+        verify(mDisplayWhiteBalanceControllerMock, never()).setStrongModeEnabled(true);
+
+        // dispatch handler looper
+        advanceTime(1);
+        verify(mDisplayWhiteBalanceControllerMock, times(1)).setStrongModeEnabled(true);
+    }
+
     /**
      * Creates a mock and registers it to {@link LocalServices}.
      */
@@ -1361,5 +1374,11 @@ public final class DisplayPowerControllerTest {
                 Context context) {
             return mHighBrightnessModeController;
         }
+
+        @Override
+        DisplayWhiteBalanceController getDisplayWhiteBalanceController(Handler handler,
+                SensorManager sensorManager, Resources resources) {
+            return mDisplayWhiteBalanceControllerMock;
+        }
     }
 }