From 9177c77262fb190659ce0b031d7eb3696196d067 Mon Sep 17 00:00:00 2001 From: chaviw Date: Tue, 24 Mar 2020 11:35:22 -0700 Subject: [PATCH 1/3] Ignore specified animation types when checking isAnimating The check isAnimating is used in a lot of places to determine if the apps can hide, show, removed, etc. However, we don't always want to check if any animation is running, just if a specific animation. This change adds a new method that allows the caller to check if isAnmating but ignore certain animationTypes. This is specifically needed for ScreenRotationAnimation refactor to ensure we can ignore screen rotation animation when checking isAnimating. Otherwise, screen rotation will block hiding closing apps if rotation occurs at the same time. Test: WindowContainerTests Bug: 152333373 Bug: 149490428 Change-Id: I922e3a5698f5db4dcc819a59ebfde2cd0301d7d2 --- .../com/android/server/wm/ActivityRecord.java | 4 +- .../android/server/wm/SurfaceAnimator.java | 21 ++++++--- .../android/server/wm/WindowContainer.java | 43 +++++++++++++++---- .../com/android/server/wm/TaskStackTests.java | 4 +- .../server/wm/WindowContainerTests.java | 19 ++++++++ 5 files changed, 76 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index c1c844078d6fb..8fa6f59331102 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -196,6 +196,7 @@ import static com.android.server.wm.ProtoLogGroup.WM_DEBUG_FOCUS_LIGHT; import static com.android.server.wm.ProtoLogGroup.WM_DEBUG_ORIENTATION; import static com.android.server.wm.ProtoLogGroup.WM_DEBUG_STARTING_WINDOW; import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_APP_TRANSITION; +import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_SCREEN_ROTATION; import static com.android.server.wm.TaskPersister.DEBUG; import static com.android.server.wm.TaskPersister.IMAGE_EXTENSION; import static com.android.server.wm.WindowContainer.AnimationFlags.CHILDREN; @@ -5918,7 +5919,8 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A @Override void prepareSurfaces() { - final boolean show = isVisible() || isAnimating(PARENTS); + final boolean show = isVisible() || isAnimatingExcluding(PARENTS, + ANIMATION_TYPE_SCREEN_ROTATION); if (mSurfaceControl != null) { if (show && !mLastSurfaceShowing) { diff --git a/services/core/java/com/android/server/wm/SurfaceAnimator.java b/services/core/java/com/android/server/wm/SurfaceAnimator.java index 6358e4719fde1..18e32c0683d64 100644 --- a/services/core/java/com/android/server/wm/SurfaceAnimator.java +++ b/services/core/java/com/android/server/wm/SurfaceAnimator.java @@ -202,6 +202,11 @@ class SurfaceAnimator { return mAnimation != null; } + @AnimationType + int getAnimationType() { + return mAnimationType; + } + /** * @return The current animation spec if we are running an animation, or {@code null} otherwise. */ @@ -453,32 +458,38 @@ class SurfaceAnimator { * Animation for screen rotation. * @hide */ - static final int ANIMATION_TYPE_SCREEN_ROTATION = 2; + static final int ANIMATION_TYPE_SCREEN_ROTATION = 1 << 1; /** * Animation for dimming. * @hide */ - static final int ANIMATION_TYPE_DIMMER = 3; + static final int ANIMATION_TYPE_DIMMER = 1 << 2; /** * Animation for recent apps. * @hide */ - static final int ANIMATION_TYPE_RECENTS = 4; + static final int ANIMATION_TYPE_RECENTS = 1 << 3; /** * Animation for a {@link WindowState} without animating the activity. * @hide */ - static final int ANIMATION_TYPE_WINDOW_ANIMATION = 5; + static final int ANIMATION_TYPE_WINDOW_ANIMATION = 1 << 4; /** * Animation to control insets. This is actually not an animation, but is used to give the * client a leash over the system window causing insets. * @hide */ - static final int ANIMATION_TYPE_INSETS_CONTROL = 6; + static final int ANIMATION_TYPE_INSETS_CONTROL = 1 << 5; + + /** + * Bitmask to include all animation types. This is NOT an {@link AnimationType} + * @hide + */ + static final int ANIMATION_TYPE_ALL = -1; /** * The type of the animation. diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java index 58119c2ab3c24..569b8f61c4f45 100644 --- a/services/core/java/com/android/server/wm/WindowContainer.java +++ b/services/core/java/com/android/server/wm/WindowContainer.java @@ -36,6 +36,7 @@ import static com.android.server.wm.IdentifierProto.USER_ID; import static com.android.server.wm.ProtoLogGroup.WM_DEBUG_APP_TRANSITIONS; import static com.android.server.wm.ProtoLogGroup.WM_DEBUG_APP_TRANSITIONS_ANIM; import static com.android.server.wm.ProtoLogGroup.WM_DEBUG_ORIENTATION; +import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_ALL; import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_APP_TRANSITION; import static com.android.server.wm.WindowContainer.AnimationFlags.CHILDREN; import static com.android.server.wm.WindowContainer.AnimationFlags.PARENTS; @@ -790,7 +791,7 @@ class WindowContainer extends ConfigurationContainer< * By default this predicate only checks if this container itself is actually running an * animation, but you can extend the check target over its relatives, or relax the condition * so that this can return {@code true} if an animation starts soon by giving a combination - * of {@link #AnimationFlags}. + * of {@link AnimationFlags}. * * Note that you can give a combination of bitmask flags to specify targets and condition for * checking animating status. @@ -800,12 +801,18 @@ class WindowContainer extends ConfigurationContainer< * * Note that TRANSITION propagates to parents and children as well. * - * {@see AnimationFlags#TRANSITION} - * {@see AnimationFlags#PARENTS} - * {@see AnimationFlags#CHILDREN} + * @param flags The combination of bitmask flags to specify targets and condition for + * checking animating status. + * @param typesToCheck The combination of bitmask {@link AnimationType} to compare when + * determining if animating. + * + * @see AnimationFlags#TRANSITION + * @see AnimationFlags#PARENTS + * @see AnimationFlags#CHILDREN */ - boolean isAnimating(int flags) { - if (mSurfaceAnimator.isAnimating()) { + boolean isAnimating(int flags, int typesToCheck) { + int animationType = mSurfaceAnimator.getAnimationType(); + if (mSurfaceAnimator.isAnimating() && (animationType & typesToCheck) > 0) { return true; } if ((flags & TRANSITION) != 0 && isWaitingForTransitionStart()) { @@ -813,14 +820,14 @@ class WindowContainer extends ConfigurationContainer< } if ((flags & PARENTS) != 0) { final WindowContainer parent = getParent(); - if (parent != null && parent.isAnimating(flags & ~CHILDREN)) { + if (parent != null && parent.isAnimating(flags & ~CHILDREN, typesToCheck)) { return true; } } if ((flags & CHILDREN) != 0) { for (int i = 0; i < mChildren.size(); ++i) { final WindowContainer wc = mChildren.get(i); - if (wc.isAnimating(flags & ~PARENTS)) { + if (wc.isAnimating(flags & ~PARENTS, typesToCheck)) { return true; } } @@ -828,6 +835,26 @@ class WindowContainer extends ConfigurationContainer< return false; } + /** + * Similar to {@link #isAnimating(int, int)} except provide a bitmask of + * {@link AnimationType} to exclude, rather than include + * @param flags The combination of bitmask flags to specify targets and condition for + * checking animating status. + * @param typesToExclude The combination of bitmask {@link AnimationType} to exclude when + * checking if animating. + */ + boolean isAnimatingExcluding(int flags, int typesToExclude) { + return isAnimating(flags, ANIMATION_TYPE_ALL & ~typesToExclude); + } + + /** + * @see #isAnimating(int, int) + * TODO (b/152333373): Migrate calls to use isAnimating with specified animation type + */ + boolean isAnimating(int flags) { + return isAnimating(flags, ANIMATION_TYPE_ALL); + } + /** * @return {@code true} when the container is waiting the app transition start, {@code false} * otherwise. diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskStackTests.java b/services/tests/wmtests/src/com/android/server/wm/TaskStackTests.java index 6387a3b7c4744..35adcfa8b4bbe 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskStackTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskStackTests.java @@ -33,6 +33,8 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.clearInvocations; import android.app.WindowConfiguration; @@ -144,7 +146,7 @@ public class TaskStackTests extends WindowTestsBase { // Stack removal is deferred if one of its child is animating. doReturn(true).when(stack).hasWindowsAlive(); - doReturn(true).when(task).isAnimating(TRANSITION | CHILDREN); + doReturn(true).when(task).isAnimating(eq(TRANSITION | CHILDREN), anyInt()); stack.removeIfPossible(); // For the case of deferred removal the task controller will still be connected to the its diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java index 27ea37dfeb193..bfaf5f2a0678f 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java @@ -35,6 +35,7 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.times; import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify; import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_APP_TRANSITION; +import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_SCREEN_ROTATION; import static com.android.server.wm.WindowContainer.AnimationFlags.CHILDREN; import static com.android.server.wm.WindowContainer.AnimationFlags.PARENTS; import static com.android.server.wm.WindowContainer.AnimationFlags.TRANSITION; @@ -422,6 +423,23 @@ public class WindowContainerTests extends WindowTestsBase { assertFalse(child21.isAnimating(TRANSITION | CHILDREN)); } + @Test + public void testIsAnimating_typesToCheck() { + final TestWindowContainerBuilder builder = new TestWindowContainerBuilder(mWm); + final TestWindowContainer window = builder.setIsAnimating(true).setLayer(0).build(); + + assertTrue(window.isAnimating()); + assertFalse(window.isAnimating(0, ANIMATION_TYPE_SCREEN_ROTATION)); + assertTrue(window.isAnimating(0, ANIMATION_TYPE_APP_TRANSITION)); + assertFalse(window.isAnimatingExcluding(0, ANIMATION_TYPE_APP_TRANSITION)); + + final TestWindowContainer child = window.addChildWindow(); + assertFalse(child.isAnimating()); + assertTrue(child.isAnimating(PARENTS)); + assertTrue(child.isAnimating(PARENTS, ANIMATION_TYPE_APP_TRANSITION)); + assertFalse(child.isAnimating(PARENTS, ANIMATION_TYPE_SCREEN_ROTATION)); + } + @Test public void testIsVisible() { final TestWindowContainerBuilder builder = new TestWindowContainerBuilder(mWm); @@ -895,6 +913,7 @@ public class WindowContainerTests extends WindowTestsBase { mWaitForTransitStart = waitTransitStart; spyOn(mSurfaceAnimator); doReturn(mIsAnimating).when(mSurfaceAnimator).isAnimating(); + doReturn(ANIMATION_TYPE_APP_TRANSITION).when(mSurfaceAnimator).getAnimationType(); } TestWindowContainer getParentWindow() { From 6103c92a56900a8a68ae8b2d3701da4688115254 Mon Sep 17 00:00:00 2001 From: chaviw Date: Mon, 30 Mar 2020 14:56:54 -0700 Subject: [PATCH 2/3] Override base isAnimating in WindowState WindowState overrides the isAnimating call. A new base method was added to handle specific animation types, but the WindowState class was still overriding the old isAnimating call. Therefore, the check for insets provider was not handled. Test: NexusLauncherTests Fixes: 152657145 Change-Id: I32f5469fa992adb712779a6cb961bd3a1b44a227 --- services/core/java/com/android/server/wm/WindowState.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index b87d18143fc7e..b386a7ae0a8ff 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -4981,7 +4981,7 @@ class WindowState extends WindowContainer implements WindowManagerP } @Override - boolean isAnimating(int flags) { + boolean isAnimating(int flags, int typesToCheck) { // If we are an inset provider, all our animations are driven by the inset client, so we // aren't really animating. @@ -4989,7 +4989,7 @@ class WindowState extends WindowContainer implements WindowManagerP if (mControllableInsetProvider != null) { return false; } - return super.isAnimating(flags); + return super.isAnimating(flags, typesToCheck); } void startAnimation(Animation anim) { From 45c25bfc1b5108042f7019c0ef6777906897f8f7 Mon Sep 17 00:00:00 2001 From: chaviw Date: Tue, 31 Mar 2020 15:35:44 -0700 Subject: [PATCH 3/3] Added test for isAnimating bug Fixes: 152657145 Test: WindowContainerTests#testIsAnimating_typesToCheck Change-Id: Id2b99e6d50ae1ee6cc8df870077cb542b2127601 --- .../com/android/server/wm/WindowContainerTests.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java index bfaf5f2a0678f..4c807324364cf 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java @@ -21,6 +21,7 @@ import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE; import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_PORTRAIT; import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_UNSET; import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED; +import static android.view.WindowManager.LayoutParams.TYPE_BASE_APPLICATION; import static android.view.WindowManager.TRANSIT_TASK_OPEN; import static com.android.dx.mockito.inline.extended.ExtendedMockito.any; @@ -438,6 +439,18 @@ public class WindowContainerTests extends WindowTestsBase { assertTrue(child.isAnimating(PARENTS)); assertTrue(child.isAnimating(PARENTS, ANIMATION_TYPE_APP_TRANSITION)); assertFalse(child.isAnimating(PARENTS, ANIMATION_TYPE_SCREEN_ROTATION)); + + final WindowState windowState = createWindow(null /* parent */, TYPE_BASE_APPLICATION, + mDisplayContent, "TestWindowState"); + WindowContainer parent = windowState.getParent(); + spyOn(windowState.mSurfaceAnimator); + doReturn(true).when(windowState.mSurfaceAnimator).isAnimating(); + doReturn(ANIMATION_TYPE_APP_TRANSITION).when( + windowState.mSurfaceAnimator).getAnimationType(); + assertTrue(parent.isAnimating(CHILDREN)); + + windowState.setControllableInsetProvider(mock(InsetsSourceProvider.class)); + assertFalse(parent.isAnimating(CHILDREN)); } @Test