From b84dc1866a7f00c5813990e5fd5837c1ba6f26fc Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Fri, 2 Dec 2016 09:01:09 -0800 Subject: [PATCH] DozeMachine: Improve resilience against out-of-order pulse requests Fixes a class of bugs that arise from requesting to pulse at inopportune times. Also improves logging such that we know what state transition failed to validate. Test: runtest -x frameworks/base/packages/SystemUI/tests/src/com/android/systemui/doze/DozeMachineTest.java Change-Id: If0bbe003c4805fd180d013dadbc28addc5bb0dd4 Fixes: 32869355 --- .../android/systemui/doze/DozeMachine.java | 120 ++++++++++-------- .../systemui/doze/DozeMachineTest.java | 12 ++ 2 files changed, 81 insertions(+), 51 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/doze/DozeMachine.java b/packages/SystemUI/src/com/android/systemui/doze/DozeMachine.java index 951b27f75aeb6..13e047cceb30d 100644 --- a/packages/SystemUI/src/com/android/systemui/doze/DozeMachine.java +++ b/packages/SystemUI/src/com/android/systemui/doze/DozeMachine.java @@ -56,7 +56,41 @@ public class DozeMachine { /** Pulse is done showing. Followed by transition to DOZE or DOZE_AOD. */ DOZE_PULSE_DONE, /** Doze is done. DozeService is finished. */ - FINISH, + FINISH; + + boolean canPulse() { + switch (this) { + case DOZE: + case DOZE_AOD: + return true; + default: + return false; + } + } + + boolean staysAwake() { + switch (this) { + case DOZE_REQUEST_PULSE: + case DOZE_PULSING: + return true; + default: + return false; + } + } + + int screenState() { + switch (this) { + case UNINITIALIZED: + case INITIALIZED: + case DOZE: + return Display.STATE_OFF; + case DOZE_PULSING: + case DOZE_AOD: + return Display.STATE_DOZE; // TODO: use STATE_ON if appropriate. + default: + return Display.STATE_UNKNOWN; + } + } } private final Service mDozeService; @@ -165,52 +199,32 @@ public class DozeMachine { } private void validateTransition(State newState) { - switch (mState) { - case FINISH: - Preconditions.checkState(newState == State.FINISH); - break; - case UNINITIALIZED: - Preconditions.checkState(newState == State.INITIALIZED); - break; - } - switch (newState) { - case UNINITIALIZED: - throw new IllegalArgumentException("can't go to UNINITIALIZED"); - case INITIALIZED: - Preconditions.checkState(mState == State.UNINITIALIZED); - break; - case DOZE_PULSING: - Preconditions.checkState(mState == State.DOZE_REQUEST_PULSE); - break; - case DOZE_PULSE_DONE: - Preconditions.checkState(mState == State.DOZE_PULSING); - break; - default: - break; - } - } - - private int screenPolicy(State newState) { - switch (newState) { - case UNINITIALIZED: - case INITIALIZED: - case DOZE: - return Display.STATE_OFF; - case DOZE_PULSING: - case DOZE_AOD: - return Display.STATE_DOZE; // TODO: use STATE_ON if appropriate. - default: - return Display.STATE_UNKNOWN; - } - } - - private boolean wakeLockPolicy(State newState) { - switch (newState) { - case DOZE_REQUEST_PULSE: - case DOZE_PULSING: - return true; - default: - return false; + try { + switch (mState) { + case FINISH: + Preconditions.checkState(newState == State.FINISH); + break; + case UNINITIALIZED: + Preconditions.checkState(newState == State.INITIALIZED); + break; + } + switch (newState) { + case UNINITIALIZED: + throw new IllegalArgumentException("can't transition to UNINITIALIZED"); + case INITIALIZED: + Preconditions.checkState(mState == State.UNINITIALIZED); + break; + case DOZE_PULSING: + Preconditions.checkState(mState == State.DOZE_REQUEST_PULSE); + break; + case DOZE_PULSE_DONE: + Preconditions.checkState(mState == State.DOZE_PULSING); + break; + default: + break; + } + } catch (RuntimeException e) { + throw new IllegalStateException("Illegal Transition: " + mState + " -> " + newState, e); } } @@ -218,22 +232,26 @@ public class DozeMachine { if (mState == State.FINISH) { return State.FINISH; } + if (requestedState == State.DOZE_REQUEST_PULSE && !mState.canPulse()) { + Log.i(TAG, "Dropping pulse request because current state can't pulse: " + mState); + return mState; + } return requestedState; } private void updateWakeLockState(State newState) { - boolean newPolicy = wakeLockPolicy(newState); - if (mWakeLockHeldForCurrentState && !newPolicy) { + boolean staysAwake = newState.staysAwake(); + if (mWakeLockHeldForCurrentState && !staysAwake) { mWakeLock.release(); mWakeLockHeldForCurrentState = false; - } else if (!mWakeLockHeldForCurrentState && newPolicy) { + } else if (!mWakeLockHeldForCurrentState && staysAwake) { mWakeLock.acquire(); mWakeLockHeldForCurrentState = true; } } private void updateScreenState(State newState) { - int state = screenPolicy(newState); + int state = newState.screenState(); if (state != Display.STATE_UNKNOWN) { mDozeService.setDozeScreenState(state); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/doze/DozeMachineTest.java b/packages/SystemUI/tests/src/com/android/systemui/doze/DozeMachineTest.java index 863f0e5a9897a..8b99d725fd734 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/doze/DozeMachineTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/doze/DozeMachineTest.java @@ -200,6 +200,18 @@ public class DozeMachineTest { assertFalse(mWakeLockFake.isHeld()); } + @Test + @UiThreadTest + public void testPulseDuringPulse_doesntCrash() { + mMachine.requestState(INITIALIZED); + + mMachine.requestState(DOZE); + mMachine.requestState(DOZE_REQUEST_PULSE); + mMachine.requestState(DOZE_PULSING); + mMachine.requestState(DOZE_REQUEST_PULSE); + mMachine.requestState(DOZE_PULSE_DONE); + } + @Test @UiThreadTest public void testScreen_offInDoze() {