Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 84683363 authored by Fiona Campbell's avatar Fiona Campbell
Browse files

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
Merged-In: Ia0688642189b050353b45dd31431d5f157b428b8
parent 267f49b1

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

+69 −20
Original line number Diff line number Diff line
@@ -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();
@@ -3331,6 +3364,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;

            }

        }

    }
@@ -3398,21 +3444,18 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 


    @Override

    public void setDisplayWhiteBalanceLoggingEnabled(boolean enabled) {

        if (mDisplayWhiteBalanceController != null) {

            mDisplayWhiteBalanceController.setLoggingEnabled(enabled);

            mDisplayWhiteBalanceSettings.setLoggingEnabled(enabled);

        }

        Message msg = mHandler.obtainMessage();

        msg.what = MSG_SET_DWBC_LOGGING_ENABLED;

        msg.arg1 = enabled ? 1 : 0;

        msg.sendToTarget();

    }



    @Override

    public void setAmbientColorTemperatureOverride(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!

            sendUpdatePowerState();

        }

        Message msg = mHandler.obtainMessage();

        msg.what = MSG_SET_DWBC_COLOR_OVERRIDE;

        msg.arg1 = Float.floatToIntBits(cct);

        msg.sendToTarget();

    }



    @VisibleForTesting
@@ -3543,6 +3586,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

+68 −20
Original line number Diff line number Diff line
@@ -142,6 +142,9 @@ 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;
@@ -368,6 +371,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
@@ -572,9 +576,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) {
@@ -850,10 +854,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();

            }
@@ -1164,9 +1164,10 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal 
                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() {
@@ -1221,8 +1222,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");
@@ -1726,6 +1732,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();
@@ -2755,6 +2787,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;

            }

        }

    }
@@ -2805,21 +2850,18 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal 


    @Override

    public void setDisplayWhiteBalanceLoggingEnabled(boolean enabled) {

        if (mDisplayWhiteBalanceController != null) {

            mDisplayWhiteBalanceController.setLoggingEnabled(enabled);

            mDisplayWhiteBalanceSettings.setLoggingEnabled(enabled);

        }

        Message msg = mHandler.obtainMessage();

        msg.what = MSG_SET_DWBC_LOGGING_ENABLED;

        msg.arg1 = enabled ? 1 : 0;

        msg.sendToTarget();

    }



    @Override

    public void setAmbientColorTemperatureOverride(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!

            sendUpdatePowerState();

        }

        Message msg = mHandler.obtainMessage();

        msg.what = MSG_SET_DWBC_COLOR_OVERRIDE;

        msg.arg1 = Float.floatToIntBits(cct);

        msg.sendToTarget();

    }



    /** Functional interface for providing time. */
@@ -2946,6 +2988,12 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal 
        boolean isColorFadeEnabled() {

            return !ActivityManager.isLowRamDeviceStatic();

        }



        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

+4 −1
Original line number Diff line number Diff line
@@ -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

+20 −1
Original line number Diff line number Diff line
@@ -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;
@@ -1089,6 +1090,18 @@ public final class DisplayPowerController2Test { 
                .getThermalBrightnessThrottlingDataMapByThrottlingId();

    }



    @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}.

     */
@@ -1378,5 +1391,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

+20 −1
Original line number Diff line number Diff line
@@ -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;
@@ -1092,6 +1093,18 @@ public final class DisplayPowerControllerTest { 
                .getThermalBrightnessThrottlingDataMapByThrottlingId();

    }



    @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}.

     */
@@ -1351,5 +1364,11 @@ public final class DisplayPowerControllerTest { 
                Context context) {

            return mHighBrightnessModeController;

        }



        @Override

        DisplayWhiteBalanceController getDisplayWhiteBalanceController(Handler handler,

                SensorManager sensorManager, Resources resources) {

            return mDisplayWhiteBalanceControllerMock;

        }

    }

}