From 4cf3ab66c16943110b9e17b6b4ae61d5d3b539d2 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Thu, 7 Oct 2021 18:26:08 -0600 Subject: [PATCH] Request transition for explicit finishing visible activity Set finishing may be a part of cleanup, it is to aggressive for handling transition. Such as a transition is created even if when removing background tasks. It is also unnecessary to request transition when finishing invisible activity. Otherwise the pending transition may affect the next launch animation. Bug: 202378926 Test: atest ActivityRecordTests# \ testFinishActivityIfPossible_nonVisibleNoAppTransition testFinishActivityIfPossible_collectToExistingTransition Test: setprop persist.debug.shell_transit 1; reboot Remove task from recents and launch any app, the transition animation runs normally. Change-Id: Ib1509d9ad6a4e8f733e968f5f3e753dd44ab57f7 --- .../com/android/server/wm/ActivityRecord.java | 16 ++------- .../server/wm/ActivityTaskSupervisor.java | 15 +-------- .../com/android/server/wm/Transition.java | 5 +++ .../server/wm/TransitionController.java | 21 ++++++++++-- .../server/wm/ActivityRecordTests.java | 33 ++++++++++++++++--- .../server/wm/DisplayContentTests.java | 6 +--- .../android/server/wm/WindowTestsBase.java | 10 +++++- 7 files changed, 64 insertions(+), 42 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index 5672ef32c00ae..bc3ea416db1e3 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -3080,9 +3080,6 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A mAtmService.deferWindowLayout(); try { - final Transition newTransition = (!mTransitionController.isCollecting() - && mTransitionController.getTransitionPlayer() != null) - ? mTransitionController.createTransition(TRANSIT_CLOSE) : null; mTaskSupervisor.mNoHistoryActivities.remove(this); makeFinishingLocked(); // Make a local reference to its task since this.task could be set to null once this @@ -3114,10 +3111,7 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A final boolean endTask = task.getTopNonFinishingActivity() == null && !task.isClearingToReuseTask(); - if (newTransition != null) { - mTransitionController.requestStartTransition(newTransition, - endTask ? task : null, null /* remote */); - } + mTransitionController.requestCloseTransitionIfNeeded(endTask ? task : this); if (isState(RESUMED)) { if (endTask) { mAtmService.getTaskChangeNotificationController().notifyTaskRemovalStarted( @@ -3543,13 +3537,6 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A if (stopped) { abortAndClearOptionsAnimation(); } - if (mTransitionController.isCollecting()) { - // We don't want the finishing to change the transition ready state since there will not - // be corresponding setReady for finishing. - mTransitionController.collectExistenceChange(this); - } else { - mTransitionController.requestTransitionIfNeeded(TRANSIT_CLOSE, this); - } } /** @@ -3731,6 +3718,7 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A // to the restarted activity. nowVisible = mVisibleRequested; } + mTransitionController.requestCloseTransitionIfNeeded(this); cleanUp(true /* cleanServices */, true /* setState */); if (remove) { if (mStartingData != null && mVisible && task != null) { diff --git a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java index ba305929d808a..7c5f059fb89a4 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java +++ b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java @@ -44,7 +44,6 @@ import static android.os.PowerManager.PARTIAL_WAKE_LOCK; import static android.os.Process.INVALID_UID; import static android.os.Trace.TRACE_TAG_WINDOW_MANAGER; import static android.view.Display.DEFAULT_DISPLAY; -import static android.view.WindowManager.TRANSIT_CLOSE; import static android.view.WindowManager.TRANSIT_TO_FRONT; import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_STATES; @@ -1562,19 +1561,7 @@ public class ActivityTaskSupervisor implements RecentTasks.Callbacks { // Prevent recursion. return; } - if (task.isVisible()) { - if (task.mTransitionController.isCollecting()) { - // We don't want the finishing to change the transition ready state since there will - // not be corresponding setReady for finishing. - task.mTransitionController.collectExistenceChange(task); - } else { - task.mTransitionController.requestTransitionIfNeeded(TRANSIT_CLOSE, task); - } - } else { - // Removing a non-visible task doesn't require a transition, but if there is one - // collecting, this should be a member just in case. - task.mTransitionController.collect(task); - } + task.mTransitionController.requestCloseTransitionIfNeeded(task); task.mInRemoveTask = true; try { task.performClearTask(reason); diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index 5d82553ad2848..4db8ef49a11ae 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -338,6 +338,11 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe applyReady(); } + @VisibleForTesting + boolean allReady() { + return mReadyTracker.allReady(); + } + /** * Build a transaction that "resets" all the re-parenting and layer changes. This is * intended to be applied at the end of the transition but before the finish callback. This diff --git a/services/core/java/com/android/server/wm/TransitionController.java b/services/core/java/com/android/server/wm/TransitionController.java index 91825ccf98e79..a21e4f2fee0d6 100644 --- a/services/core/java/com/android/server/wm/TransitionController.java +++ b/services/core/java/com/android/server/wm/TransitionController.java @@ -35,7 +35,6 @@ import android.util.ArrayMap; import android.util.Slog; import android.util.proto.ProtoOutputStream; import android.view.WindowManager; -import android.window.IRemoteTransition; import android.window.ITransitionMetricsReporter; import android.window.ITransitionPlayer; import android.window.RemoteTransition; @@ -226,7 +225,7 @@ class TransitionController { } /** - * @see #requestTransitionIfNeeded(int, int, WindowContainer, IRemoteTransition) + * @see #requestTransitionIfNeeded(int, int, WindowContainer, WindowContainer, RemoteTransition) */ @Nullable Transition requestTransitionIfNeeded(@WindowManager.TransitionType int type, @@ -235,7 +234,7 @@ class TransitionController { } /** - * @see #requestTransitionIfNeeded(int, int, WindowContainer, IRemoteTransition) + * @see #requestTransitionIfNeeded(int, int, WindowContainer, WindowContainer, RemoteTransition) */ @Nullable Transition requestTransitionIfNeeded(@WindowManager.TransitionType int type, @@ -306,6 +305,22 @@ class TransitionController { return transition; } + /** Requests transition for a window container which will be removed or invisible. */ + void requestCloseTransitionIfNeeded(@NonNull WindowContainer wc) { + if (mTransitionPlayer == null) return; + if (wc.isVisibleRequested()) { + if (!isCollecting()) { + requestStartTransition(createTransition(TRANSIT_CLOSE, 0 /* flags */), + wc.asTask(), null /* remoteTransition */); + } + collectExistenceChange(wc); + } else { + // Removing a non-visible window doesn't require a transition, but if there is one + // collecting, this should be a member just in case. + collect(wc); + } + } + /** @see Transition#collect */ void collect(@NonNull WindowContainer wc) { if (mCollectingTransition == null) return; diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java index 44cff33d61469..128bfa8a28f3a 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java @@ -166,6 +166,8 @@ public class ActivityRecordTests extends WindowTestsBase { @Before public void setUp() throws Exception { setBooted(mAtm); + // Because the booted state is set, avoid starting real home if there is no task. + doReturn(false).when(mRootWindowContainer).resumeHomeActivity(any(), anyString(), any()); } private TestStartingWindowOrganizer registerTestStartingWindowOrganizer() { @@ -1083,6 +1085,7 @@ public class ActivityRecordTests extends WindowTestsBase { */ @Test public void testFinishActivityIfPossible_nonVisibleNoAppTransition() { + registerTestTransitionPlayer(); final ActivityRecord activity = createActivityWithTask(); // Put an activity on top of test activity to make it invisible and prevent us from // accidentally resuming the topmost one again. @@ -1093,6 +1096,7 @@ public class ActivityRecordTests extends WindowTestsBase { activity.finishIfPossible("test", false /* oomAdj */); verify(activity.mDisplayContent, never()).prepareAppTransition(eq(TRANSIT_CLOSE)); + assertFalse(activity.inTransition()); } /** @@ -1101,11 +1105,7 @@ public class ActivityRecordTests extends WindowTestsBase { */ @Test public void testFinishActivityIfPossible_lastInTaskRequestsTransitionWithTrigger() { - // Set-up mock shell transitions - final TestTransitionPlayer testPlayer = new TestTransitionPlayer( - mAtm.getTransitionController(), mAtm.mWindowOrganizerController); - mAtm.getTransitionController().registerTransitionPlayer(testPlayer); - + final TestTransitionPlayer testPlayer = registerTestTransitionPlayer(); final ActivityRecord activity = createActivityWithTask(); activity.finishing = false; activity.mVisibleRequested = true; @@ -1116,6 +1116,29 @@ public class ActivityRecordTests extends WindowTestsBase { assertEquals(activity.getTask().mTaskId, testPlayer.mLastRequest.getTriggerTask().taskId); } + /** + * Verify that when collecting activity to the existing close transition, it should not affect + * ready state. + */ + @Test + public void testFinishActivityIfPossible_collectToExistingTransition() { + final TestTransitionPlayer testPlayer = registerTestTransitionPlayer(); + final ActivityRecord activity = createActivityWithTask(); + activity.setState(PAUSED, "test"); + activity.finishIfPossible("test", false /* oomAdj */); + final Transition lastTransition = testPlayer.mLastTransit; + assertTrue(lastTransition.allReady()); + assertTrue(activity.inTransition()); + + // Collect another activity to the existing transition without changing ready state. + final ActivityRecord activity2 = createActivityRecord(activity.getTask()); + activity2.setState(PAUSING, "test"); + activity2.finishIfPossible("test", false /* oomAdj */); + assertTrue(activity2.inTransition()); + assertEquals(lastTransition, testPlayer.mLastTransit); + assertTrue(lastTransition.allReady()); + } + /** * Verify that complete finish request for non-finishing activity is invalid. */ diff --git a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java index 24bbf4682157a..f3c1ec5b200e9 100644 --- a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java @@ -1674,11 +1674,7 @@ public class DisplayContentTests extends WindowTestsBase { public void testShellTransitRotation() { DisplayContent dc = createNewDisplay(); - // Set-up mock shell transitions - final TestTransitionPlayer testPlayer = new TestTransitionPlayer( - mAtm.getTransitionController(), mAtm.mWindowOrganizerController); - mAtm.getTransitionController().registerTransitionPlayer(testPlayer); - + final TestTransitionPlayer testPlayer = registerTestTransitionPlayer(); final DisplayRotation dr = dc.getDisplayRotation(); doCallRealMethod().when(dr).updateRotationUnchecked(anyBoolean()); // Rotate 180 degree so the display doesn't have configuration change. This condition is diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java b/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java index ac61bb15ab06c..81b00eae00214 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java @@ -798,6 +798,14 @@ class WindowTestsBase extends SystemServiceTestsBase { }; } + /** Sets up a simple implementation of transition player for shell transitions. */ + TestTransitionPlayer registerTestTransitionPlayer() { + final TestTransitionPlayer testPlayer = new TestTransitionPlayer( + mAtm.getTransitionController(), mAtm.mWindowOrganizerController); + testPlayer.mController.registerTransitionPlayer(testPlayer); + return testPlayer; + } + /** * Avoids rotating screen disturbed by some conditions. It is usually used for the default * display that is not the instance of {@link TestDisplayContent} (it bypasses the conditions). @@ -1606,7 +1614,7 @@ class WindowTestsBase extends SystemServiceTestsBase { } } - class TestTransitionPlayer extends ITransitionPlayer.Stub { + static class TestTransitionPlayer extends ITransitionPlayer.Stub { final TransitionController mController; final WindowOrganizerController mOrganizer; Transition mLastTransit = null;