Fix power manager display wake lock bugs.

Under certain circumstances, the power manager might continue to
hold the display wakelock long after the display had been turned
off due to the mDisplayReady flag having an incorrect value.

1. An inverted conditional caused DisplayPowerState to incorrectly
signal the screen on ready state.

2. The DisplayPowerController failed to clear the block screen on
flag in the case where the screen was turned off before it became
unblocked from turning on.  This could happen when the display was
rapidly turned on-off-on-off.

Bug: 13248135
Change-Id: I8faa3034695c83c8cd35613d81acccf40d22128d

3 files changed, 67 insertions, 55 deletions

diff --git a/services/core/java/com/android/server/power/DisplayPowerController.java b/services/core/java/com/android/server/power/DisplayPowerController.java
index bd9acb9..e83f734 100644
--- a/services/core/java/com/android/server/power/DisplayPowerController.java
+++ b/services/core/java/com/android/server/power/DisplayPowerController.java
@@ -611,7 +611,6 @@ final class DisplayPowerController {
                         && mProximity == PROXIMITY_POSITIVE) {
                     mScreenOffBecauseOfProximity = true;
                     sendOnProximityPositiveWithWakelock();
-                    setScreenOn(false);
                 }
             } else if (mWaitingForNegativeProximity
                     && mScreenOffBecauseOfProximity
@@ -675,59 +674,62 @@ final class DisplayPowerController {
             mUsingScreenAutoBrightness = false;
         }
 
-        // Animate the screen on or off.
-        if (!mScreenOffBecauseOfProximity) {
-            if (mPowerRequest.wantScreenOnAny()) {
-                // Want screen on.
-                // Wait for previous off animation to complete beforehand.
-                // It is relatively short but if we cancel it and switch to the
-                // on animation immediately then the results are pretty ugly.
-                if (!mElectronBeamOffAnimator.isStarted()) {
-                    // Turn the screen on.  The contents of the screen may not yet
-                    // be visible if the electron beam has not been dismissed because
-                    // its last frame of animation is solid black.
-                    setScreenOn(true);
-
-                    if (mPowerRequest.blockScreenOn
-                            && mPowerState.getElectronBeamLevel() == 0.0f) {
-                        blockScreenOn();
-                    } else {
-                        unblockScreenOn();
-                        if (USE_ELECTRON_BEAM_ON_ANIMATION) {
-                            if (!mElectronBeamOnAnimator.isStarted()) {
-                                if (mPowerState.getElectronBeamLevel() == 1.0f) {
-                                    mPowerState.dismissElectronBeam();
-                                } else if (mPowerState.prepareElectronBeam(
-                                        mElectronBeamFadesConfig ?
-                                                ElectronBeam.MODE_FADE :
-                                                        ElectronBeam.MODE_WARM_UP)) {
-                                    mElectronBeamOnAnimator.start();
-                                } else {
-                                    mElectronBeamOnAnimator.end();
-                                }
+        // Animate the screen on or off unless blocked.
+        if (mScreenOffBecauseOfProximity) {
+            // Screen off due to proximity.
+            setScreenOn(false);
+            unblockScreenOn();
+        } else if (mPowerRequest.wantScreenOnAny()) {
+            // Want screen on.
+            // Wait for previous off animation to complete beforehand.
+            // It is relatively short but if we cancel it and switch to the
+            // on animation immediately then the results are pretty ugly.
+            if (!mElectronBeamOffAnimator.isStarted()) {
+                // Turn the screen on.  The contents of the screen may not yet
+                // be visible if the electron beam has not been dismissed because
+                // its last frame of animation is solid black.
+                setScreenOn(true);
+
+                if (mPowerRequest.blockScreenOn
+                        && mPowerState.getElectronBeamLevel() == 0.0f) {
+                    blockScreenOn();
+                } else {
+                    unblockScreenOn();
+                    if (USE_ELECTRON_BEAM_ON_ANIMATION) {
+                        if (!mElectronBeamOnAnimator.isStarted()) {
+                            if (mPowerState.getElectronBeamLevel() == 1.0f) {
+                                mPowerState.dismissElectronBeam();
+                            } else if (mPowerState.prepareElectronBeam(
+                                    mElectronBeamFadesConfig ?
+                                            ElectronBeam.MODE_FADE :
+                                                    ElectronBeam.MODE_WARM_UP)) {
+                                mElectronBeamOnAnimator.start();
+                            } else {
+                                mElectronBeamOnAnimator.end();
                             }
-                        } else {
-                            mPowerState.setElectronBeamLevel(1.0f);
-                            mPowerState.dismissElectronBeam();
                         }
+                    } else {
+                        mPowerState.setElectronBeamLevel(1.0f);
+                        mPowerState.dismissElectronBeam();
                     }
                 }
-            } else {
-                // Want screen off.
-                // Wait for previous on animation to complete beforehand.
-                if (!mElectronBeamOnAnimator.isStarted()) {
-                    if (!mElectronBeamOffAnimator.isStarted()) {
-                        if (mPowerState.getElectronBeamLevel() == 0.0f) {
-                            setScreenOn(false);
-                        } else if (mPowerState.prepareElectronBeam(
-                                mElectronBeamFadesConfig ?
-                                        ElectronBeam.MODE_FADE :
-                                                ElectronBeam.MODE_COOL_DOWN)
-                                && mPowerState.isScreenOn()) {
-                            mElectronBeamOffAnimator.start();
-                        } else {
-                            mElectronBeamOffAnimator.end();
-                        }
+            }
+        } else {
+            // Want screen off.
+            // Wait for previous on animation to complete beforehand.
+            unblockScreenOn();
+            if (!mElectronBeamOnAnimator.isStarted()) {
+                if (!mElectronBeamOffAnimator.isStarted()) {
+                    if (mPowerState.getElectronBeamLevel() == 0.0f) {
+                        setScreenOn(false);
+                    } else if (mPowerState.prepareElectronBeam(
+                            mElectronBeamFadesConfig ?
+                                    ElectronBeam.MODE_FADE :
+                                            ElectronBeam.MODE_COOL_DOWN)
+                            && mPowerState.isScreenOn()) {
+                        mElectronBeamOffAnimator.start();
+                    } else {
+                        mElectronBeamOffAnimator.end();
                     }
                 }
             }
@@ -767,15 +769,15 @@ final class DisplayPowerController {
     private void unblockScreenOn() {
         if (mScreenOnWasBlocked) {
             mScreenOnWasBlocked = false;
-            if (DEBUG) {
-                Slog.d(TAG, "Unblocked screen on after " +
-                        (SystemClock.elapsedRealtime() - mScreenOnBlockStartRealTime) + " ms");
+            long delay = SystemClock.elapsedRealtime() - mScreenOnBlockStartRealTime;
+            if (delay > 1000 || DEBUG) {
+                Slog.d(TAG, "Unblocked screen on after " + delay + " ms");
             }
         }
     }
 
     private void setScreenOn(boolean on) {
-        if (!mPowerState.isScreenOn() == on) {
+        if (mPowerState.isScreenOn() != on) {
             mPowerState.setScreenOn(on);
             if (on) {
                 mNotifier.onScreenOn();
diff --git a/services/core/java/com/android/server/power/DisplayPowerState.java b/services/core/java/com/android/server/power/DisplayPowerState.java
index 42af4b4..8e331ad 100644
--- a/services/core/java/com/android/server/power/DisplayPowerState.java
+++ b/services/core/java/com/android/server/power/DisplayPowerState.java
@@ -304,8 +304,15 @@ final class DisplayPowerState {
 
             int brightness = mScreenOn && mElectronBeamLevel > 0f ? mScreenBrightness : 0;
             if (mPhotonicModulator.setState(mScreenOn, brightness)) {
+                if (DEBUG) {
+                    Slog.d(TAG, "Screen ready");
+                }
                 mScreenReady = true;
                 invokeCleanListenerIfNeeded();
+            } else {
+                if (DEBUG) {
+                    Slog.d(TAG, "Screen not ready");
+                }
             }
         }
     };
@@ -355,7 +362,7 @@ final class DisplayPowerState {
                         AsyncTask.THREAD_POOL_EXECUTOR.execute(mTask);
                     }
                 }
-                return mChangeInProgress;
+                return !mChangeInProgress;
             }
         }
 
diff --git a/services/core/java/com/android/server/power/PowerManagerService.java b/services/core/java/com/android/server/power/PowerManagerService.java
index c31de05..159268c 100644
--- a/services/core/java/com/android/server/power/PowerManagerService.java
+++ b/services/core/java/com/android/server/power/PowerManagerService.java
@@ -1036,6 +1036,9 @@ public final class PowerManagerService extends com.android.server.SystemService
         if (!mSystemReady || mDirty == 0) {
             return;
         }
+        if (!Thread.holdsLock(mLock)) {
+            Slog.wtf(TAG, "Power manager lock was not held when calling updatePowerStateLocked");
+        }
 
         // Phase 0: Basic state updates.
         updateIsPoweredLocked(mDirty);