From d9e9bac7931275db029a23e61d2e6c3bdf5d4876 Mon Sep 17 00:00:00 2001 From: Beverly Date: Wed, 16 Sep 2020 15:38:33 -0400 Subject: Don't trust DozeState onScreenState changes In DozeScreenBrightness, we should only rely on the passed displayScreenState because our DozeState (updated by transitionTo) may not be updated yet. Therefore, only register the brightness sensor when the display state is Display.STATE_DOZE/DOZE_SUSPENDED. Test: atest DozeScreenBrightnessTest Bug: 168689990 Change-Id: Ie32f91c5a46bd987649a8a17e6543071847ad97c Merged-In: Ie32f91c5a46bd987649a8a17e6543071847ad97c (cherry picked from commit a3c5a6a5975367e5d13509ef579b940b9ee21553) --- .../src/com/android/systemui/doze/DozeMachine.java | 8 ++++++-- .../systemui/doze/DozeScreenBrightness.java | 7 +------ .../systemui/doze/DozeScreenBrightnessTest.java | 22 ++++++++++++++++------ 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/doze/DozeMachine.java b/packages/SystemUI/src/com/android/systemui/doze/DozeMachine.java index ae7d82ac4a5e..253a35c55698 100644 --- a/packages/SystemUI/src/com/android/systemui/doze/DozeMachine.java +++ b/packages/SystemUI/src/com/android/systemui/doze/DozeMachine.java @@ -430,8 +430,12 @@ public class DozeMachine { /** Give the Part a chance to clean itself up. */ default void destroy() {} - /** Alerts that the screenstate is being changed. */ - default void onScreenState(int state) {} + /** + * Alerts that the screenstate is being changed. + * Note: This may be called from within a call to transitionTo, so local DozeState may not + * be accurate nor match with the new displayState. + */ + default void onScreenState(int displayState) {} } /** A wrapper interface for {@link android.service.dreams.DreamService} */ diff --git a/packages/SystemUI/src/com/android/systemui/doze/DozeScreenBrightness.java b/packages/SystemUI/src/com/android/systemui/doze/DozeScreenBrightness.java index 64cfb4bcd058..a11997b6b845 100644 --- a/packages/SystemUI/src/com/android/systemui/doze/DozeScreenBrightness.java +++ b/packages/SystemUI/src/com/android/systemui/doze/DozeScreenBrightness.java @@ -69,7 +69,6 @@ public class DozeScreenBrightness extends BroadcastReceiver implements DozeMachi * --ei brightness_bucket 1} */ private int mDebugBrightnessBucket = -1; - private DozeMachine.State mState; @VisibleForTesting public DozeScreenBrightness(Context context, DozeMachine.Service service, @@ -109,7 +108,6 @@ public class DozeScreenBrightness extends BroadcastReceiver implements DozeMachi @Override public void transitionTo(DozeMachine.State oldState, DozeMachine.State newState) { - mState = newState; switch (newState) { case INITIALIZED: case DOZE: @@ -127,10 +125,7 @@ public class DozeScreenBrightness extends BroadcastReceiver implements DozeMachi @Override public void onScreenState(int state) { - if (!mScreenOff - && (mState == DozeMachine.State.DOZE_AOD - || mState == DozeMachine.State.DOZE_AOD_DOCKED) - && (state == Display.STATE_DOZE || state == Display.STATE_DOZE_SUSPEND)) { + if (state == Display.STATE_DOZE || state == Display.STATE_DOZE_SUSPEND) { setLightSensorEnabled(true); } else { setLightSensorEnabled(false); diff --git a/packages/SystemUI/tests/src/com/android/systemui/doze/DozeScreenBrightnessTest.java b/packages/SystemUI/tests/src/com/android/systemui/doze/DozeScreenBrightnessTest.java index 3ef60274cd76..c82bb9bbd37c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/doze/DozeScreenBrightnessTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/doze/DozeScreenBrightnessTest.java @@ -103,8 +103,6 @@ public class DozeScreenBrightnessTest extends SysuiTestCase { @Test public void testAod_usesLightSensor() { - mScreen.transitionTo(UNINITIALIZED, INITIALIZED); - mScreen.transitionTo(INITIALIZED, DOZE_AOD); mScreen.onScreenState(Display.STATE_DOZE); mSensor.sendSensorEvent(3); @@ -114,8 +112,7 @@ public class DozeScreenBrightnessTest extends SysuiTestCase { @Test public void testAod_usesDebugValue() throws Exception { - mScreen.transitionTo(UNINITIALIZED, INITIALIZED); - mScreen.transitionTo(INITIALIZED, DOZE_AOD); + mScreen.onScreenState(Display.STATE_DOZE); Intent intent = new Intent(DozeScreenBrightness.ACTION_AOD_BRIGHTNESS); intent.putExtra(DozeScreenBrightness.BRIGHTNESS_BUCKET, 1); @@ -166,20 +163,30 @@ public class DozeScreenBrightnessTest extends SysuiTestCase { } @Test - public void testDozingAfterPulsing_pausesLightSensor() throws Exception { + public void testScreenOffAfterPulsing_pausesLightSensor() throws Exception { mScreen.transitionTo(UNINITIALIZED, INITIALIZED); mScreen.transitionTo(INITIALIZED, DOZE); mScreen.transitionTo(DOZE, DOZE_REQUEST_PULSE); mScreen.transitionTo(DOZE_REQUEST_PULSE, DOZE_PULSING); mScreen.transitionTo(DOZE_PULSING, DOZE_PULSE_DONE); mScreen.transitionTo(DOZE_PULSE_DONE, DOZE); - mScreen.onScreenState(Display.STATE_DOZE); mSensor.sendSensorEvent(1); assertEquals(DEFAULT_BRIGHTNESS, mServiceFake.screenBrightness); } + @Test + public void testOnScreenStateSetBeforeTransition_stillRegistersSensor() { + mScreen.transitionTo(UNINITIALIZED, INITIALIZED); + mScreen.onScreenState(Display.STATE_DOZE); + mScreen.transitionTo(INITIALIZED, DOZE_AOD); + + mSensor.sendSensorEvent(1); + + assertEquals(1, mServiceFake.screenBrightness); + } + @Test public void testNullSensor() throws Exception { mScreen = new DozeScreenBrightness(mContext, mServiceFake, mSensorManager, @@ -191,12 +198,15 @@ public class DozeScreenBrightnessTest extends SysuiTestCase { mScreen.transitionTo(INITIALIZED, DOZE_AOD); mScreen.transitionTo(DOZE_AOD, DOZE_AOD_PAUSING); mScreen.transitionTo(DOZE_AOD_PAUSING, DOZE_AOD_PAUSED); + mScreen.onScreenState(Display.STATE_DOZE); + mScreen.onScreenState(Display.STATE_OFF); } @Test public void testNoBrightnessDeliveredAfterFinish() throws Exception { mScreen.transitionTo(UNINITIALIZED, INITIALIZED); mScreen.transitionTo(INITIALIZED, DOZE_AOD); + mScreen.onScreenState(Display.STATE_DOZE); mScreen.transitionTo(DOZE_AOD, FINISH); mSensor.sendSensorEvent(1); -- cgit v1.2.3-59-g8ed1b