From 4ab85f3d6fced3b32d0a330e2fef7d2f96d37d79 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Fri, 12 May 2023 11:54:05 -0700 Subject: [PATCH] Fix misplaced surface from fold -> unfold while bubbles are expanded This also fixes the surface being misplaced when the theme changes. When setBounds is called in TaskViewTransitions we were skipping creating a new transition whenever there was anything pending. Instead, we should only skip if the pending in flight is for that task view AND an opening type transit, otherwise the bounds never get updated until the next open / if a bounds change is sent without any other pending. Test: manual - have an expanded bubble on folded device, unfold => ensure the expanded view is fully visible - have an expanded bubble & change the theme via QS => ensure the expanded view is properly placed - test various bubble animations: expand, collapse, switch, new bubble, dismissing while expanded, rotation and ensure they all look fine. Test: atest TaskViewTransitionTest Bug: 281855397 Bug: 280440749 Change-Id: I43cbb90bdfbe90bf068c5de21d060193aa38c96f --- .../shell/taskview/TaskViewTransitions.java | 35 +++++- .../taskview/TaskViewTransitionsTest.java | 101 +++++++++++++++++- 2 files changed, 131 insertions(+), 5 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/taskview/TaskViewTransitions.java b/libs/WindowManager/Shell/src/com/android/wm/shell/taskview/TaskViewTransitions.java index fe2faaf79a1a2..2e7fca3f2b469 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/taskview/TaskViewTransitions.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/taskview/TaskViewTransitions.java @@ -134,6 +134,21 @@ public class TaskViewTransitions implements Transitions.TransitionHandler { return null; } + /** + * Looks through the pending transitions for a opening transaction that matches the provided + * `taskView`. + * @param taskView the pending transition should be for this. + */ + private PendingTransition findPendingOpeningTransition(TaskViewTaskController taskView) { + for (int i = mPending.size() - 1; i >= 0; --i) { + if (mPending.get(i).mTaskView != taskView) continue; + if (TransitionUtil.isOpeningType(mPending.get(i).mType)) { + return mPending.get(i); + } + } + return null; + } + /** * Looks through the pending transitions for one matching `taskView`. * @param taskView the pending transition should be for this. @@ -149,6 +164,19 @@ public class TaskViewTransitions implements Transitions.TransitionHandler { return null; } + /** + * Returns all the pending transitions for a given `taskView`. + * @param taskView the pending transition should be for this. + */ + ArrayList findAllPending(TaskViewTaskController taskView) { + ArrayList list = new ArrayList<>(); + for (int i = mPending.size() - 1; i >= 0; --i) { + if (mPending.get(i).mTaskView != taskView) continue; + list.add(mPending.get(i)); + } + return list; + } + private PendingTransition findPending(IBinder claimed) { for (int i = 0; i < mPending.size(); ++i) { if (mPending.get(i).mClaimed != claimed) continue; @@ -249,9 +277,10 @@ public class TaskViewTransitions implements Transitions.TransitionHandler { // Task view isn't visible, the bounds will next visibility update. return; } - if (hasPending()) { - // There is already a transition in-flight, the window bounds will be set in - // prepareOpenAnimation. + PendingTransition pendingOpen = findPendingOpeningTransition(taskView); + if (pendingOpen != null) { + // There is already an opening transition in-flight, the window bounds will be + // set in prepareOpenAnimation (via the window crop) if needed. return; } WindowContainerTransaction wct = new WindowContainerTransaction(); diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/taskview/TaskViewTransitionsTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/taskview/TaskViewTransitionsTest.java index 9d566860c1cdc..71ad0d79eaca3 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/taskview/TaskViewTransitionsTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/taskview/TaskViewTransitionsTest.java @@ -45,6 +45,8 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.List; + @SmallTest @RunWith(AndroidTestingRunner.class) @TestableLooper.RunWithLooper(setAsMainLooper = true) @@ -58,6 +60,12 @@ public class TaskViewTransitionsTest extends ShellTestCase { ActivityManager.RunningTaskInfo mTaskInfo; @Mock WindowContainerToken mToken; + @Mock + TaskViewTaskController mTaskViewTaskController2; + @Mock + ActivityManager.RunningTaskInfo mTaskInfo2; + @Mock + WindowContainerToken mToken2; TaskViewTransitions mTaskViewTransitions; @@ -73,10 +81,16 @@ public class TaskViewTransitionsTest extends ShellTestCase { mTaskInfo.token = mToken; mTaskInfo.taskId = 314; mTaskInfo.taskDescription = mock(ActivityManager.TaskDescription.class); + when(mTaskViewTaskController.getTaskInfo()).thenReturn(mTaskInfo); + + mTaskInfo2 = new ActivityManager.RunningTaskInfo(); + mTaskInfo2.token = mToken2; + mTaskInfo2.taskId = 315; + mTaskInfo2.taskDescription = mock(ActivityManager.TaskDescription.class); + when(mTaskViewTaskController2.getTaskInfo()).thenReturn(mTaskInfo2); mTaskViewTransitions = spy(new TaskViewTransitions(mTransitions)); mTaskViewTransitions.addTaskView(mTaskViewTaskController); - when(mTaskViewTaskController.getTaskInfo()).thenReturn(mTaskInfo); } @Test @@ -119,7 +133,7 @@ public class TaskViewTransitionsTest extends ShellTestCase { } @Test - public void testSetTaskBounds_taskVisibleWithPending_noTransaction() { + public void testSetTaskBounds_taskVisibleWithPendingOpen_noTransaction() { assumeTrue(Transitions.ENABLE_SHELL_TRANSITIONS); mTaskViewTransitions.setTaskViewVisible(mTaskViewTaskController, true); @@ -134,6 +148,43 @@ public class TaskViewTransitionsTest extends ShellTestCase { .isNull(); } + @Test + public void testSetTaskBounds_taskVisibleWithPendingChange_transition() { + assumeTrue(Transitions.ENABLE_SHELL_TRANSITIONS); + + mTaskViewTransitions.setTaskViewVisible(mTaskViewTaskController, true); + + // Consume the pending transition from visibility change + TaskViewTransitions.PendingTransition pending = + mTaskViewTransitions.findPending(mTaskViewTaskController, TRANSIT_TO_FRONT); + assertThat(pending).isNotNull(); + mTaskViewTransitions.startAnimation(pending.mClaimed, + mock(TransitionInfo.class), + new SurfaceControl.Transaction(), + new SurfaceControl.Transaction(), + mock(Transitions.TransitionFinishCallback.class)); + // Verify it was consumed + TaskViewTransitions.PendingTransition checkPending = + mTaskViewTransitions.findPending(mTaskViewTaskController, TRANSIT_TO_FRONT); + assertThat(checkPending).isNull(); + + // Test that set bounds creates a new transition + mTaskViewTransitions.setTaskBounds(mTaskViewTaskController, + new Rect(0, 0, 100, 100)); + assertThat(mTaskViewTransitions.findPending(mTaskViewTaskController, TRANSIT_CHANGE)) + .isNotNull(); + + // Test that set bounds again (with different bounds) creates another transition + mTaskViewTransitions.setTaskBounds(mTaskViewTaskController, + new Rect(0, 0, 300, 200)); + List pendingList = + mTaskViewTransitions.findAllPending(mTaskViewTaskController) + .stream() + .filter(pendingTransition -> pendingTransition.mType == TRANSIT_CHANGE) + .toList(); + assertThat(pendingList.size()).isEqualTo(2); + } + @Test public void testSetTaskBounds_sameBounds_noTransaction() { assumeTrue(Transitions.ENABLE_SHELL_TRANSITIONS); @@ -161,6 +212,16 @@ public class TaskViewTransitionsTest extends ShellTestCase { mTaskViewTransitions.findPending(mTaskViewTaskController, TRANSIT_CHANGE); assertThat(pendingBounds).isNotNull(); + // Test that setting same bounds with in-flight transition doesn't cause another one + mTaskViewTransitions.setTaskBounds(mTaskViewTaskController, + new Rect(0, 0, 100, 100)); + List pendingList = + mTaskViewTransitions.findAllPending(mTaskViewTaskController) + .stream() + .filter(pendingTransition -> pendingTransition.mType == TRANSIT_CHANGE) + .toList(); + assertThat(pendingList.size()).isEqualTo(1); + // Consume the pending bounds transaction mTaskViewTransitions.startAnimation(pendingBounds.mClaimed, mock(TransitionInfo.class), @@ -180,6 +241,42 @@ public class TaskViewTransitionsTest extends ShellTestCase { assertThat(pendingBounds2).isNull(); } + + @Test + public void testSetTaskBounds_taskVisibleWithDifferentTaskViewPendingChange_transition() { + assumeTrue(Transitions.ENABLE_SHELL_TRANSITIONS); + + mTaskViewTransitions.addTaskView(mTaskViewTaskController2); + + mTaskViewTransitions.setTaskViewVisible(mTaskViewTaskController, true); + + // Consume the pending transition from visibility change + TaskViewTransitions.PendingTransition pending = + mTaskViewTransitions.findPending(mTaskViewTaskController, TRANSIT_TO_FRONT); + assertThat(pending).isNotNull(); + mTaskViewTransitions.startAnimation(pending.mClaimed, + mock(TransitionInfo.class), + new SurfaceControl.Transaction(), + new SurfaceControl.Transaction(), + mock(Transitions.TransitionFinishCallback.class)); + // Verify it was consumed + TaskViewTransitions.PendingTransition checkPending = + mTaskViewTransitions.findPending(mTaskViewTaskController, TRANSIT_TO_FRONT); + assertThat(checkPending).isNull(); + + // Set the second taskview as visible & check that it has a pending transition + mTaskViewTransitions.setTaskViewVisible(mTaskViewTaskController2, true); + TaskViewTransitions.PendingTransition pending2 = + mTaskViewTransitions.findPending(mTaskViewTaskController2, TRANSIT_TO_FRONT); + assertThat(pending2).isNotNull(); + + // Test that set bounds on the first taskview will create a new transition + mTaskViewTransitions.setTaskBounds(mTaskViewTaskController, + new Rect(0, 0, 100, 100)); + assertThat(mTaskViewTransitions.findPending(mTaskViewTaskController, TRANSIT_CHANGE)) + .isNotNull(); + } + @Test public void testSetTaskVisibility_taskRemoved_noNPE() { mTaskViewTransitions.removeTaskView(mTaskViewTaskController);