From d9701abf0482d644551657124a315ee1e6638e85 Mon Sep 17 00:00:00 2001 From: Santos Cordon Date: Thu, 1 Aug 2019 18:33:20 +0100 Subject: [PATCH] Acquire display suspend blocker for DozeService If DozeService explicitly requests the display state to be on, then PowerManager needs to acquire the display suspend blocker even while in DOZE mode to prevent the system from constantly trying to suspend. Bug: 138195405 Test: atest PowerManagerServiceTest Change-Id: I05f5b86789ced084d0814480b2fe89d74f96a54e --- .../server/power/AttentionDetector.java | 2 +- .../server/power/PowerManagerService.java | 11 ++- .../server/power/PowerManagerServiceTest.java | 73 ++++++++++++++++++- 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/power/AttentionDetector.java b/services/core/java/com/android/server/power/AttentionDetector.java index 56d839633ab54..4e9b724f528d4 100644 --- a/services/core/java/com/android/server/power/AttentionDetector.java +++ b/services/core/java/com/android/server/power/AttentionDetector.java @@ -161,7 +161,7 @@ public class AttentionDetector { context.getContentResolver().registerContentObserver(Settings.System.getUriFor( Settings.System.ADAPTIVE_SLEEP), - false, new ContentObserver(new Handler()) { + false, new ContentObserver(new Handler(context.getMainLooper())) { @Override public void onChange(boolean selfChange) { updateEnabledFromSettings(context); diff --git a/services/core/java/com/android/server/power/PowerManagerService.java b/services/core/java/com/android/server/power/PowerManagerService.java index cfd3ae6ef5943..aa49ba62f48b1 100644 --- a/services/core/java/com/android/server/power/PowerManagerService.java +++ b/services/core/java/com/android/server/power/PowerManagerService.java @@ -2724,6 +2724,14 @@ public final class PowerManagerService extends SystemService return true; } } + + if (mDisplayPowerRequest.policy == DisplayPowerRequest.POLICY_DOZE + && mDisplayPowerRequest.dozeScreenState == Display.STATE_ON) { + // Although we are in DOZE and would normally allow the device to suspend, + // the doze service has explicitly requested the display to remain in the ON + // state which means we should hold the display suspend blocker. + return true; + } if (mScreenBrightnessBoostInProgress) { return true; } @@ -4858,7 +4866,8 @@ public final class PowerManagerService extends SystemService } } - private final class LocalService extends PowerManagerInternal { + @VisibleForTesting + final class LocalService extends PowerManagerInternal { @Override public void setScreenBrightnessOverrideFromWindowManager(int screenBrightness) { if (screenBrightness < PowerManager.BRIGHTNESS_DEFAULT diff --git a/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java index 1bda412f2f89d..88de250e4b0da 100644 --- a/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java @@ -23,7 +23,10 @@ import static android.os.PowerManagerInternal.WAKEFULNESS_DREAMING; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; @@ -59,6 +62,7 @@ import android.os.SystemClock; import android.os.SystemProperties; import android.os.UserHandle; import android.provider.Settings; +import android.view.Display; import androidx.test.InstrumentationRegistry; @@ -157,6 +161,10 @@ public class PowerManagerServiceTest { mResourcesSpy = spy(mContextSpy.getResources()); when(mContextSpy.getResources()).thenReturn(mResourcesSpy); + when(mDisplayManagerInternalMock.requestPowerState(any(), anyBoolean())).thenReturn(true); + } + + private PowerManagerService createService() { mService = new PowerManagerService(mContextSpy, new Injector() { @Override Notifier createNotifier(Looper looper, Context context, IBatteryStats batteryStats, @@ -166,7 +174,7 @@ public class PowerManagerServiceTest { @Override SuspendBlocker createSuspendBlocker(PowerManagerService service, String name) { - return mock(SuspendBlocker.class); + return super.createSuspendBlocker(service, name); } @Override @@ -191,6 +199,7 @@ public class PowerManagerServiceTest { return mAmbientDisplayConfigurationMock; } }); + return mService; } @After @@ -262,6 +271,7 @@ public class PowerManagerServiceTest { @Test public void testUpdatePowerScreenPolicy_UpdateDisplayPowerRequest() { + createService(); mService.updatePowerRequestFromBatterySaverPolicy(mDisplayPowerRequest); assertThat(mDisplayPowerRequest.lowPowerMode).isEqualTo(BATTERY_SAVER_ENABLED); assertThat(mDisplayPowerRequest.screenLowPowerBrightnessFactor) @@ -270,6 +280,7 @@ public class PowerManagerServiceTest { @Test public void testGetLastShutdownReasonInternal() { + createService(); SystemProperties.set(TEST_LAST_REBOOT_PROPERTY, "shutdown,thermal"); int reason = mService.getLastShutdownReasonInternal(TEST_LAST_REBOOT_PROPERTY); SystemProperties.set(TEST_LAST_REBOOT_PROPERTY, ""); @@ -278,6 +289,7 @@ public class PowerManagerServiceTest { @Test public void testGetDesiredScreenPolicy_WithVR() throws Exception { + createService(); // Brighten up the screen mService.setWakefulnessLocked(WAKEFULNESS_AWAKE, PowerManager.WAKE_REASON_UNKNOWN, 0); assertThat(mService.getDesiredScreenPolicyLocked()).isEqualTo( @@ -307,11 +319,13 @@ public class PowerManagerServiceTest { @Test public void testWakefulnessAwake_InitialValue() throws Exception { + createService(); assertThat(mService.getWakefulness()).isEqualTo(WAKEFULNESS_AWAKE); } @Test public void testWakefulnessSleep_NoDozeSleepFlag() throws Exception { + createService(); // Start with AWAKE state startSystem(); assertThat(mService.getWakefulness()).isEqualTo(WAKEFULNESS_AWAKE); @@ -324,6 +338,7 @@ public class PowerManagerServiceTest { @Test public void testWakefulnessAwake_AcquireCausesWakeup() throws Exception { + createService(); startSystem(); forceSleep(); @@ -355,6 +370,7 @@ public class PowerManagerServiceTest { @Test public void testWakefulnessAwake_IPowerManagerWakeUp() throws Exception { + createService(); startSystem(); forceSleep(); mService.getBinderServiceInstance().wakeUp(SystemClock.uptimeMillis(), @@ -369,6 +385,8 @@ public class PowerManagerServiceTest { @Test public void testWakefulnessAwake_ShouldWakeUpWhenPluggedIn() throws Exception { boolean powerState; + + createService(); startSystem(); forceSleep(); @@ -444,6 +462,7 @@ public class PowerManagerServiceTest { @Test public void testWakefulnessDoze_goToSleep() throws Exception { + createService(); // Start with AWAKE state startSystem(); assertThat(mService.getWakefulness()).isEqualTo(WAKEFULNESS_AWAKE); @@ -457,6 +476,7 @@ public class PowerManagerServiceTest { @Test public void testWasDeviceIdleFor_true() { int interval = 1000; + createService(); mService.onUserActivity(); SystemClock.sleep(interval + 1 /* just a little more */); assertThat(mService.wasDeviceIdleForInternal(interval)).isTrue(); @@ -465,12 +485,14 @@ public class PowerManagerServiceTest { @Test public void testWasDeviceIdleFor_false() { int interval = 1000; + createService(); mService.onUserActivity(); assertThat(mService.wasDeviceIdleForInternal(interval)).isFalse(); } @Test public void testForceSuspend_putsDeviceToSleep() { + createService(); mService.systemReady(null); mService.onBootPhase(SystemService.PHASE_BOOT_COMPLETED); @@ -497,6 +519,8 @@ public class PowerManagerServiceTest { final int flags = PowerManager.PARTIAL_WAKE_LOCK; final String pkg = mContextSpy.getOpPackageName(); + createService(); + // Set up the Notification mock to keep track of the wakelocks that are currently // active or disabled. We'll use this to verify that wakelocks are disabled when // they should be. @@ -541,7 +565,54 @@ public class PowerManagerServiceTest { @Test public void testForceSuspend_forceSuspendFailurePropogated() { + createService(); when(mNativeWrapperMock.nativeForceSuspend()).thenReturn(false); assertThat(mService.getBinderServiceInstance().forceSuspend()).isFalse(); } + + @Test + public void testSetDozeOverrideFromDreamManager_triggersSuspendBlocker() throws Exception { + final String suspendBlockerName = "PowerManagerService.Display"; + final String tag = "acq_causes_wakeup"; + final String packageName = "pkg.name"; + final IBinder token = new Binder(); + + final boolean[] isAcquired = new boolean[1]; + doAnswer(inv -> { + if (suspendBlockerName.equals(inv.getArguments()[0])) { + isAcquired[0] = false; + } + return null; + }).when(mNativeWrapperMock).nativeReleaseSuspendBlocker(any()); + + doAnswer(inv -> { + if (suspendBlockerName.equals(inv.getArguments()[0])) { + isAcquired[0] = true; + } + return null; + }).when(mNativeWrapperMock).nativeAcquireSuspendBlocker(any()); + + // Need to create the service after we stub the mocks for this test because some of the + // mocks are used during the constructor. + createService(); + + // Start with AWAKE state + startSystem(); + assertThat(mService.getWakefulness()).isEqualTo(WAKEFULNESS_AWAKE); + assertTrue(isAcquired[0]); + + // Take a nap and verify we no longer hold the blocker + int flags = PowerManager.DOZE_WAKE_LOCK; + mService.getBinderServiceInstance().acquireWakeLock(token, flags, tag, packageName, + null /* workSource */, null /* historyTag */); + mService.getBinderServiceInstance().goToSleep(SystemClock.uptimeMillis(), + PowerManager.GO_TO_SLEEP_REASON_APPLICATION, 0); + assertThat(mService.getWakefulness()).isEqualTo(WAKEFULNESS_DOZING); + assertFalse(isAcquired[0]); + + // Override the display state by DreamManager and verify is reacquires the blocker. + mService.getLocalServiceInstance() + .setDozeOverrideFromDreamManager(Display.STATE_ON, PowerManager.BRIGHTNESS_DEFAULT); + assertTrue(isAcquired[0]); + } }