From 382622fb5bf6e900eac99e1a79e745bd26973b2b Mon Sep 17 00:00:00 2001 From: Jordan Demeulenaere Date: Wed, 3 Aug 2022 14:16:57 +0200 Subject: [PATCH] Do not animate dialogs into activites when locked (1/n) This CL ensures that we don't try to animate a dialog into an Activity if the device is currently locked. Test: atest DialogLaunchAnimatorTest Bug: 240572073 Change-Id: I5adf650cf9f42781803dce1bebe14b2a78f43fff --- .../animation/DialogLaunchAnimator.kt | 34 +++++++++++--- .../CentralSurfacesDependenciesModule.java | 29 +++++++++++- .../phone/StatusBarKeyguardViewManager.java | 3 +- .../animation/ActivityLaunchAnimatorTest.kt | 46 ++++++++++++------- .../animation/DialogLaunchAnimatorTest.kt | 30 ++++++++---- .../android/systemui/animation/TestValues.kt | 23 ---------- .../com/android/systemui/SysuiTestCase.java | 4 ++ .../animation/FakeDialogLaunchAnimator.kt | 46 +++++++++++++++++++ .../systemui/animation/FakeLaunchAnimator.kt | 44 ++++++++++++++++++ 9 files changed, 202 insertions(+), 57 deletions(-) delete mode 100644 packages/SystemUI/tests/src/com/android/systemui/animation/TestValues.kt create mode 100644 packages/SystemUI/tests/utils/src/com/android/systemui/animation/FakeDialogLaunchAnimator.kt create mode 100644 packages/SystemUI/tests/utils/src/com/android/systemui/animation/FakeLaunchAnimator.kt diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/DialogLaunchAnimator.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/DialogLaunchAnimator.kt index 2f36ab9aa93dc..8f9ced6956ca7 100644 --- a/packages/SystemUI/animation/src/com/android/systemui/animation/DialogLaunchAnimator.kt +++ b/packages/SystemUI/animation/src/com/android/systemui/animation/DialogLaunchAnimator.kt @@ -23,7 +23,6 @@ import android.app.Dialog import android.graphics.Color import android.graphics.Rect import android.os.Looper -import android.service.dreams.IDreamManager import android.util.Log import android.util.MathUtils import android.view.GhostView @@ -54,7 +53,7 @@ private const val TAG = "DialogLaunchAnimator" class DialogLaunchAnimator @JvmOverloads constructor( - private val dreamManager: IDreamManager, + private val callback: Callback, private val interactionJankMonitor: InteractionJankMonitor, private val launchAnimator: LaunchAnimator = LaunchAnimator(TIMINGS, INTERPOLATORS), private val isForTesting: Boolean = false @@ -126,7 +125,7 @@ constructor( val animatedDialog = AnimatedDialog( launchAnimator, - dreamManager, + callback, interactionJankMonitor, animateFrom, onDialogDismissed = { openedDialogs.remove(it) }, @@ -194,8 +193,12 @@ constructor( val dialog = animatedDialog.dialog - // Don't animate if the dialog is not showing. - if (!dialog.isShowing) { + // Don't animate if the dialog is not showing or if we are locked and going to show the + // bouncer. + if ( + !dialog.isShowing || + (!callback.isUnlocked() && !callback.isShowingAlternateAuthOnUnlock()) + ) { return null } @@ -285,6 +288,23 @@ constructor( ?.let { it.touchSurface = it.prepareForStackDismiss() } dialog.dismiss() } + + interface Callback { + /** Whether the device is currently in dreaming (screensaver) mode. */ + fun isDreaming(): Boolean + + /** + * Whether the device is currently unlocked, i.e. if it is *not* on the keyguard or if the + * keyguard can be dismissed. + */ + fun isUnlocked(): Boolean + + /** + * Whether we are going to show alternate authentication (like UDFPS) instead of the + * traditional bouncer when unlocking the device. + */ + fun isShowingAlternateAuthOnUnlock(): Boolean + } } /** @@ -296,7 +316,7 @@ data class DialogCuj(@CujType val cujType: Int, val tag: String? = null) private class AnimatedDialog( private val launchAnimator: LaunchAnimator, - private val dreamManager: IDreamManager, + private val callback: DialogLaunchAnimator.Callback, private val interactionJankMonitor: InteractionJankMonitor, /** The view that triggered the dialog after being tapped. */ @@ -850,7 +870,7 @@ private class AnimatedDialog( // If we are dreaming, the dialog was probably closed because of that so we don't animate // into the touchSurface. - if (dreamManager.isDreaming) { + if (callback.isDreaming()) { return false } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java index 48e34501ef593..0951e821cdc2e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java @@ -19,7 +19,9 @@ package com.android.systemui.statusbar.dagger; import android.app.IActivityManager; import android.content.Context; import android.os.Handler; +import android.os.RemoteException; import android.service.dreams.IDreamManager; +import android.util.Log; import com.android.internal.jank.InteractionJankMonitor; import com.android.internal.statusbar.IStatusBarService; @@ -60,10 +62,12 @@ import com.android.systemui.statusbar.phone.ManagedProfileControllerImpl; import com.android.systemui.statusbar.phone.StatusBarIconController; import com.android.systemui.statusbar.phone.StatusBarIconControllerImpl; import com.android.systemui.statusbar.phone.StatusBarIconList; +import com.android.systemui.statusbar.phone.StatusBarKeyguardViewManager; import com.android.systemui.statusbar.phone.StatusBarRemoteInputCallback; import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallController; import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallFlags; import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallLogger; +import com.android.systemui.statusbar.policy.KeyguardStateController; import com.android.systemui.statusbar.policy.RemoteInputUriController; import com.android.systemui.statusbar.window.StatusBarWindowController; import com.android.systemui.tracing.ProtoTracer; @@ -274,7 +278,30 @@ public interface CentralSurfacesDependenciesModule { @Provides @SysUISingleton static DialogLaunchAnimator provideDialogLaunchAnimator(IDreamManager dreamManager, + KeyguardStateController keyguardStateController, + Lazy statusBarKeyguardViewManager, InteractionJankMonitor interactionJankMonitor) { - return new DialogLaunchAnimator(dreamManager, interactionJankMonitor); + DialogLaunchAnimator.Callback callback = new DialogLaunchAnimator.Callback() { + @Override + public boolean isDreaming() { + try { + return dreamManager.isDreaming(); + } catch (RemoteException e) { + Log.e("DialogLaunchAnimator.Callback", "dreamManager.isDreaming failed", e); + return false; + } + } + + @Override + public boolean isUnlocked() { + return keyguardStateController.isUnlocked(); + } + + @Override + public boolean isShowingAlternateAuthOnUnlock() { + return statusBarKeyguardViewManager.get().shouldShowAltAuth(); + } + }; + return new DialogLaunchAnimator(callback, interactionJankMonitor); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java index 3880ee36cd1f9..d5c6f89bfb6b1 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java @@ -471,7 +471,8 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb showBouncer(scrimmed); } - private boolean shouldShowAltAuth() { + /** Whether we should show the alternate authentication instead of the traditional bouncer. */ + public boolean shouldShowAltAuth() { return mAlternateAuthInterceptor != null && mKeyguardUpdateManager.isUnlockingWithBiometricAllowed(true); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/animation/ActivityLaunchAnimatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/animation/ActivityLaunchAnimatorTest.kt index c48cbb19b40a5..0f112415df0d4 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/animation/ActivityLaunchAnimatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/animation/ActivityLaunchAnimatorTest.kt @@ -26,6 +26,7 @@ import junit.framework.Assert.assertNotNull import junit.framework.Assert.assertNull import junit.framework.Assert.assertTrue import junit.framework.AssertionFailedError +import kotlin.concurrent.thread import org.junit.After import org.junit.Before import org.junit.Rule @@ -34,19 +35,18 @@ import org.junit.runner.RunWith import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.Mock -import org.mockito.Mockito.`when` import org.mockito.Mockito.never import org.mockito.Mockito.verify +import org.mockito.Mockito.`when` import org.mockito.Spy import org.mockito.junit.MockitoJUnit -import kotlin.concurrent.thread @SmallTest @RunWith(AndroidTestingRunner::class) @RunWithLooper class ActivityLaunchAnimatorTest : SysuiTestCase() { private val launchContainer = LinearLayout(mContext) - private val testLaunchAnimator = LaunchAnimator(TEST_TIMINGS, TEST_INTERPOLATORS) + private val testLaunchAnimator = fakeLaunchAnimator() @Mock lateinit var callback: ActivityLaunchAnimator.Callback @Mock lateinit var listener: ActivityLaunchAnimator.Listener @Spy private val controller = TestLaunchAnimatorController(launchContainer) @@ -77,12 +77,13 @@ class ActivityLaunchAnimatorTest : SysuiTestCase() { // We start in a new thread so that we can ensure that the callbacks are called in the main // thread. thread { - animator.startIntentWithAnimation( + animator.startIntentWithAnimation( controller = controller, animate = animate, intentStarter = intentStarter - ) - }.join() + ) + } + .join() } @Test @@ -197,14 +198,25 @@ class ActivityLaunchAnimatorTest : SysuiTestCase() { val bounds = Rect(10 /* left */, 20 /* top */, 30 /* right */, 40 /* bottom */) val taskInfo = ActivityManager.RunningTaskInfo() taskInfo.topActivity = ComponentName("com.android.systemui", "FakeActivity") - taskInfo.topActivityInfo = ActivityInfo().apply { - applicationInfo = ApplicationInfo() - } + taskInfo.topActivityInfo = ActivityInfo().apply { applicationInfo = ApplicationInfo() } return RemoteAnimationTarget( - 0, RemoteAnimationTarget.MODE_OPENING, SurfaceControl(), false, Rect(), Rect(), 0, - Point(), Rect(), bounds, WindowConfiguration(), false, SurfaceControl(), Rect(), - taskInfo, false + 0, + RemoteAnimationTarget.MODE_OPENING, + SurfaceControl(), + false, + Rect(), + Rect(), + 0, + Point(), + Rect(), + bounds, + WindowConfiguration(), + false, + SurfaceControl(), + Rect(), + taskInfo, + false ) } } @@ -213,17 +225,17 @@ class ActivityLaunchAnimatorTest : SysuiTestCase() { * A simple implementation of [ActivityLaunchAnimator.Controller] which throws if it is called * outside of the main thread. */ -private class TestLaunchAnimatorController( - override var launchContainer: ViewGroup -) : ActivityLaunchAnimator.Controller { - override fun createAnimatorState() = LaunchAnimator.State( +private class TestLaunchAnimatorController(override var launchContainer: ViewGroup) : + ActivityLaunchAnimator.Controller { + override fun createAnimatorState() = + LaunchAnimator.State( top = 100, bottom = 200, left = 300, right = 400, topCornerRadius = 10f, bottomCornerRadius = 20f - ) + ) private fun assertOnMainThread() { if (Looper.myLooper() != Looper.getMainLooper()) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/animation/DialogLaunchAnimatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/animation/DialogLaunchAnimatorTest.kt index 4218e0904c431..7c1e384f8c305 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/animation/DialogLaunchAnimatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/animation/DialogLaunchAnimatorTest.kt @@ -5,7 +5,6 @@ import android.content.Context import android.graphics.Color import android.graphics.drawable.ColorDrawable import android.os.Bundle -import android.service.dreams.IDreamManager import android.testing.AndroidTestingRunner import android.testing.TestableLooper import android.testing.ViewUtils @@ -38,19 +37,16 @@ import org.mockito.junit.MockitoJUnit @RunWith(AndroidTestingRunner::class) @TestableLooper.RunWithLooper class DialogLaunchAnimatorTest : SysuiTestCase() { - private val launchAnimator = LaunchAnimator(TEST_TIMINGS, TEST_INTERPOLATORS) private lateinit var dialogLaunchAnimator: DialogLaunchAnimator private val attachedViews = mutableSetOf() - @Mock lateinit var dreamManager: IDreamManager @Mock lateinit var interactionJankMonitor: InteractionJankMonitor @get:Rule val rule = MockitoJUnit.rule() @Before fun setUp() { - dialogLaunchAnimator = DialogLaunchAnimator( - dreamManager, interactionJankMonitor, launchAnimator, isForTesting = true - ) + dialogLaunchAnimator = + fakeDialogLaunchAnimator(interactionJankMonitor = interactionJankMonitor) } @After @@ -152,6 +148,22 @@ class DialogLaunchAnimatorTest : SysuiTestCase() { assertNull(dialogLaunchAnimator.createActivityLaunchController(dialog.contentView)) } + @Test + fun testActivityLaunchWhenLockedWithoutAlternateAuth() { + val dialogLaunchAnimator = + fakeDialogLaunchAnimator(isUnlocked = false, isShowingAlternateAuthOnUnlock = false) + val dialog = createAndShowDialog(dialogLaunchAnimator) + assertNull(dialogLaunchAnimator.createActivityLaunchController(dialog.contentView)) + } + + @Test + fun testActivityLaunchWhenLockedWithAlternateAuth() { + val dialogLaunchAnimator = + fakeDialogLaunchAnimator(isUnlocked = false, isShowingAlternateAuthOnUnlock = true) + val dialog = createAndShowDialog(dialogLaunchAnimator) + assertNotNull(dialogLaunchAnimator.createActivityLaunchController(dialog.contentView)) + } + @Test fun testDialogAnimationIsChangedByAnimator() { // Important: the power menu animation relies on this behavior to know when to animate (see @@ -193,11 +205,13 @@ class DialogLaunchAnimatorTest : SysuiTestCase() { verify(interactionJankMonitor).end(InteractionJankMonitor.CUJ_USER_DIALOG_OPEN) } - private fun createAndShowDialog(): TestDialog { + private fun createAndShowDialog( + animator: DialogLaunchAnimator = dialogLaunchAnimator, + ): TestDialog { val touchSurface = createTouchSurface() return runOnMainThreadAndWaitForIdleSync { val dialog = TestDialog(context) - dialogLaunchAnimator.showFromView(dialog, touchSurface) + animator.showFromView(dialog, touchSurface) dialog } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/animation/TestValues.kt b/packages/SystemUI/tests/src/com/android/systemui/animation/TestValues.kt deleted file mode 100644 index dadf94e2a9dda..0000000000000 --- a/packages/SystemUI/tests/src/com/android/systemui/animation/TestValues.kt +++ /dev/null @@ -1,23 +0,0 @@ -package com.android.systemui.animation - -/** - * A [LaunchAnimator.Timings] to be used in tests. - * - * Note that all timings except the total duration are non-zero to avoid divide-by-zero exceptions - * when computing the progress of a sub-animation (the contents fade in/out). - */ -val TEST_TIMINGS = LaunchAnimator.Timings( - totalDuration = 0L, - contentBeforeFadeOutDelay = 1L, - contentBeforeFadeOutDuration = 1L, - contentAfterFadeInDelay = 1L, - contentAfterFadeInDuration = 1L -) - -/** A [LaunchAnimator.Interpolators] to be used in tests. */ -val TEST_INTERPOLATORS = LaunchAnimator.Interpolators( - positionInterpolator = Interpolators.STANDARD, - positionXInterpolator = Interpolators.STANDARD, - contentBeforeFadeOutInterpolator = Interpolators.STANDARD, - contentAfterFadeInInterpolator = Interpolators.STANDARD -) \ No newline at end of file diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/SysuiTestCase.java b/packages/SystemUI/tests/utils/src/com/android/systemui/SysuiTestCase.java index c52ea60f0bfc6..c83189dbc6166 100644 --- a/packages/SystemUI/tests/utils/src/com/android/systemui/SysuiTestCase.java +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/SysuiTestCase.java @@ -15,6 +15,8 @@ */ package com.android.systemui; +import static com.android.systemui.animation.FakeDialogLaunchAnimatorKt.fakeDialogLaunchAnimator; + import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -34,6 +36,7 @@ import androidx.test.uiautomator.UiDevice; import com.android.keyguard.KeyguardUpdateMonitor; import com.android.settingslib.bluetooth.LocalBluetoothManager; +import com.android.systemui.animation.DialogLaunchAnimator; import com.android.systemui.broadcast.BroadcastDispatcher; import com.android.systemui.broadcast.FakeBroadcastDispatcher; import com.android.systemui.broadcast.logging.BroadcastDispatcherLogger; @@ -119,6 +122,7 @@ public abstract class SysuiTestCase { // is missing (constructing the actual one would throw). // TODO(b/219008720): Remove this. mDependency.injectMockDependency(SystemUIDialogManager.class); + mDependency.injectTestDependency(DialogLaunchAnimator.class, fakeDialogLaunchAnimator()); } @After diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/animation/FakeDialogLaunchAnimator.kt b/packages/SystemUI/tests/utils/src/com/android/systemui/animation/FakeDialogLaunchAnimator.kt new file mode 100644 index 0000000000000..990db77463f61 --- /dev/null +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/animation/FakeDialogLaunchAnimator.kt @@ -0,0 +1,46 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the + * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package com.android.systemui.animation + +import com.android.internal.jank.InteractionJankMonitor +import org.mockito.Mockito.mock + +/** A [DialogLaunchAnimator] to be used in tests. */ +@JvmOverloads +fun fakeDialogLaunchAnimator( + isUnlocked: Boolean = true, + isShowingAlternateAuthOnUnlock: Boolean = false, + interactionJankMonitor: InteractionJankMonitor = mock(InteractionJankMonitor::class.java), +): DialogLaunchAnimator { + return DialogLaunchAnimator( + FakeCallback( + isUnlocked = isUnlocked, + isShowingAlternateAuthOnUnlock = isShowingAlternateAuthOnUnlock, + ), + interactionJankMonitor, + fakeLaunchAnimator(), + isForTesting = true, + ) +} + +private class FakeCallback( + private val isDreaming: Boolean = false, + private val isUnlocked: Boolean = true, + private val isShowingAlternateAuthOnUnlock: Boolean = false, +) : DialogLaunchAnimator.Callback { + override fun isDreaming(): Boolean = isDreaming + override fun isUnlocked(): Boolean = isUnlocked + override fun isShowingAlternateAuthOnUnlock() = isShowingAlternateAuthOnUnlock +} diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/animation/FakeLaunchAnimator.kt b/packages/SystemUI/tests/utils/src/com/android/systemui/animation/FakeLaunchAnimator.kt new file mode 100644 index 0000000000000..5b431e72e2ac5 --- /dev/null +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/animation/FakeLaunchAnimator.kt @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the + * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package com.android.systemui.animation + +/** A [LaunchAnimator] to be used in tests. */ +fun fakeLaunchAnimator(): LaunchAnimator { + return LaunchAnimator(TEST_TIMINGS, TEST_INTERPOLATORS) +} + +/** + * A [LaunchAnimator.Timings] to be used in tests. + * + * Note that all timings except the total duration are non-zero to avoid divide-by-zero exceptions + * when computing the progress of a sub-animation (the contents fade in/out). + */ +private val TEST_TIMINGS = + LaunchAnimator.Timings( + totalDuration = 0L, + contentBeforeFadeOutDelay = 1L, + contentBeforeFadeOutDuration = 1L, + contentAfterFadeInDelay = 1L, + contentAfterFadeInDuration = 1L + ) + +/** A [LaunchAnimator.Interpolators] to be used in tests. */ +private val TEST_INTERPOLATORS = + LaunchAnimator.Interpolators( + positionInterpolator = Interpolators.STANDARD, + positionXInterpolator = Interpolators.STANDARD, + contentBeforeFadeOutInterpolator = Interpolators.STANDARD, + contentAfterFadeInInterpolator = Interpolators.STANDARD + )