From 38a5f2252d8056adfaf75017db28c78641bfe389 Mon Sep 17 00:00:00 2001 From: Josh Tsuji Date: Mon, 4 Apr 2022 16:14:36 +0000 Subject: [PATCH] Revert "Revert "Fix shouldControlUnlockedScreenOff."" This reverts commit d0e5677884690980db257f6ac108044d1b308ae8. Reason for revert: I think that test was already broken Change-Id: Iede5b711aa9dff333148f0a99d82c2ec9b1e10bc --- .../statusbar/phone/DozeParameters.java | 31 ++++++++++++------- .../UnlockedScreenOffAnimationController.kt | 23 ++++++++++++-- .../statusbar/phone/DozeParametersTest.java | 10 +++++- ...nlockedScreenOffAnimationControllerTest.kt | 14 +++++++-- 4 files changed, 61 insertions(+), 17 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/DozeParameters.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/DozeParameters.java index 8b25c2bc20b98..b04f17ad3a16f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/DozeParameters.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/DozeParameters.java @@ -31,6 +31,8 @@ import android.provider.Settings; import android.util.Log; import android.util.MathUtils; +import android.util.Log; + import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; @@ -292,6 +294,11 @@ public class DozeParameters implements } public void updateControlScreenOff() { + Log.i("TEST", "Display needs blanking?" + getDisplayNeedsBlanking()); + Log.i("TEST", "Should control screen off?" + shouldControlUnlockedScreenOff()); + Log.i("TEST", "alwaysOn?" + getAlwaysOn()); + Log.i("TEST", "keyguard showing?" + mKeyguardShowing); + Log.i("TEST", "Flag enabled? " + mFeatureFlags.isEnabled(Flags.LOCKSCREEN_ANIMATIONS)); if (!getDisplayNeedsBlanking()) { final boolean controlScreenOff = getAlwaysOn() && (mKeyguardShowing || shouldControlUnlockedScreenOff()); @@ -299,6 +306,17 @@ public class DozeParameters implements } } + /** + * Whether we're capable of controlling the screen off animation if we want to. This isn't + * possible if AOD isn't even enabled or if the flag is disabled, or if the display needs + * blanking. + */ + public boolean canControlUnlockedScreenOff() { + return getAlwaysOn() + && mFeatureFlags.isEnabled(Flags.LOCKSCREEN_ANIMATIONS) + && !getDisplayNeedsBlanking(); + } + /** * Whether we want to control the screen off animation when the device is unlocked. If we do, * we'll animate in AOD before turning off the screen, rather than simply fading to black and @@ -309,8 +327,7 @@ public class DozeParameters implements * disabled for a11y. */ public boolean shouldControlUnlockedScreenOff() { - return canControlUnlockedScreenOff() - && mUnlockedScreenOffAnimationController.shouldPlayUnlockedScreenOffAnimation(); + return mUnlockedScreenOffAnimationController.shouldPlayUnlockedScreenOffAnimation(); } public boolean shouldDelayKeyguardShow() { @@ -342,16 +359,6 @@ public class DozeParameters implements return getAlwaysOn() && mKeyguardShowing; } - /** - * Whether we're capable of controlling the screen off animation if we want to. This isn't - * possible if AOD isn't even enabled or if the flag is disabled. - */ - public boolean canControlUnlockedScreenOff() { - return getAlwaysOn() - && mFeatureFlags.isEnabled(Flags.LOCKSCREEN_ANIMATIONS) - && !getDisplayNeedsBlanking(); - } - private boolean getBoolean(String propName, int resId) { return SystemProperties.getBoolean(propName, mResources.getBoolean(resId)); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationController.kt index c11d450e47b25..ea935be334c17 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationController.kt @@ -59,8 +59,14 @@ class UnlockedScreenOffAnimationController @Inject constructor( private val powerManager: PowerManager, private val handler: Handler = Handler() ) : WakefulnessLifecycle.Observer, ScreenOffAnimation { - private lateinit var mCentralSurfaces: CentralSurfaces + /** + * Whether or not [initialize] has been called to provide us with the StatusBar, + * NotificationPanelViewController, and LightRevealSrim so that we can run the unlocked screen + * off animation. + */ + private var initialized = false + private lateinit var lightRevealScrim: LightRevealScrim private var animatorDurationScale = 1f @@ -116,6 +122,7 @@ class UnlockedScreenOffAnimationController @Inject constructor( centralSurfaces: CentralSurfaces, lightRevealScrim: LightRevealScrim ) { + this.initialized = true this.lightRevealScrim = lightRevealScrim this.mCentralSurfaces = centralSurfaces @@ -262,6 +269,18 @@ class UnlockedScreenOffAnimationController @Inject constructor( * on the current state of the device. */ fun shouldPlayUnlockedScreenOffAnimation(): Boolean { + // If we haven't been initialized yet, we don't have a StatusBar/LightRevealScrim yet, so we + // can't perform the animation. + if (!initialized) { + return false + } + + // If the device isn't in a state where we can control unlocked screen off (no AOD enabled, + // power save, etc.) then we shouldn't try to do so. + if (!dozeParameters.get().canControlUnlockedScreenOff()) { + return false + } + // If we explicitly already decided not to play the screen off animation, then never change // our mind. if (decidedToAnimateGoingToSleep == false) { @@ -304,7 +323,7 @@ class UnlockedScreenOffAnimationController @Inject constructor( } override fun shouldDelayDisplayDozeTransition(): Boolean = - dozeParameters.get().shouldControlUnlockedScreenOff() + shouldPlayUnlockedScreenOffAnimation() /** * Whether we're doing the light reveal animation or we're done with that and animating in the diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/DozeParametersTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/DozeParametersTest.java index 5f2bbd3419628..077b41a0aa900 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/DozeParametersTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/DozeParametersTest.java @@ -126,6 +126,12 @@ public class DozeParametersTest extends SysuiTestCase { setAodEnabledForTest(true); setShouldControlUnlockedScreenOffForTest(true); setDisplayNeedsBlankingForTest(false); + + // Default to false here (with one test to make sure that when it returns true, we respect + // that). We'll test the specific conditions for this to return true/false in the + // UnlockedScreenOffAnimationController's tests. + when(mUnlockedScreenOffAnimationController.shouldPlayUnlockedScreenOffAnimation()) + .thenReturn(false); } @Test @@ -174,9 +180,12 @@ public class DozeParametersTest extends SysuiTestCase { */ @Test public void testControlUnlockedScreenOffAnimation_dozeAfterScreenOff_false() { + mDozeParameters.mKeyguardVisibilityCallback.onKeyguardVisibilityChanged(true); + // If AOD is disabled, we shouldn't want to control screen off. Also, let's double check // that when that value is updated, we called through to PowerManager. setAodEnabledForTest(false); + assertFalse(mDozeParameters.shouldControlScreenOff()); assertTrue(mPowerManagerDozeAfterScreenOff); @@ -188,7 +197,6 @@ public class DozeParametersTest extends SysuiTestCase { @Test public void testControlUnlockedScreenOffAnimationDisabled_dozeAfterScreenOff() { - setShouldControlUnlockedScreenOffForTest(true); when(mFeatureFlags.isEnabled(Flags.LOCKSCREEN_ANIMATIONS)).thenReturn(false); assertFalse(mDozeParameters.shouldControlUnlockedScreenOff()); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationControllerTest.kt index 050563a5707ce..0936b773d4b3e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationControllerTest.kt @@ -31,6 +31,7 @@ import com.android.systemui.statusbar.LightRevealScrim import com.android.systemui.statusbar.StatusBarStateControllerImpl import com.android.systemui.statusbar.policy.KeyguardStateController import com.android.systemui.util.settings.GlobalSettings +import junit.framework.Assert.assertFalse import org.junit.After import org.junit.Before import org.junit.Test @@ -133,7 +134,7 @@ class UnlockedScreenOffAnimationControllerTest : SysuiTestCase() { */ @Test fun testAodUiShownIfNotInteractive() { - `when`(dozeParameters.shouldControlUnlockedScreenOff()).thenReturn(true) + `when`(dozeParameters.canControlUnlockedScreenOff()).thenReturn(true) `when`(powerManager.isInteractive).thenReturn(false) val callbackCaptor = ArgumentCaptor.forClass(Runnable::class.java) @@ -156,7 +157,7 @@ class UnlockedScreenOffAnimationControllerTest : SysuiTestCase() { */ @Test fun testAodUiNotShownIfInteractive() { - `when`(dozeParameters.shouldControlUnlockedScreenOff()).thenReturn(true) + `when`(dozeParameters.canControlUnlockedScreenOff()).thenReturn(true) `when`(powerManager.isInteractive).thenReturn(true) val callbackCaptor = ArgumentCaptor.forClass(Runnable::class.java) @@ -167,4 +168,13 @@ class UnlockedScreenOffAnimationControllerTest : SysuiTestCase() { verify(notificationPanelViewController, never()).showAodUi() } + + @Test + fun testNoAnimationPlaying_dozeParamsCanNotControlScreenOff() { + `when`(dozeParameters.canControlUnlockedScreenOff()).thenReturn(false) + + assertFalse(controller.shouldPlayUnlockedScreenOffAnimation()) + controller.startAnimation() + assertFalse(controller.isAnimationPlaying()) + } } \ No newline at end of file