From 27b1385f50cc7fb21eea4e801a6004ce0760e19a Mon Sep 17 00:00:00 2001
From: Evan Rosky <erosky@google.com>
Date: Mon, 10 Apr 2023 17:35:30 -0700
Subject: [PATCH] Don't collect redundant activities in transient launch

Transitions shouldn't collect any changes they don't actually
modify. This isn't a big problem now, however, once we have
simultaneous transitions, this will force transitions to
serialize when they aren't actually interdependent.

This was collecting the activities only for the purposes of
tracking what to commit in finish. So instead, just calculate
the commit targets in finish.

Bug: 264536014
Test: atest TransitionTests#testTransientLaunch
Change-Id: I4eb34adefb35ad001e6ef2734b10e9d5aacac611
---
 .../com/android/server/wm/Transition.java     | 25 +++++++++++++------
 .../android/server/wm/TransitionTests.java    |  6 +++--
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java
index 3a909cedc5abf..ec00d2f107e89 100644
--- a/services/core/java/com/android/server/wm/Transition.java
+++ b/services/core/java/com/android/server/wm/Transition.java
@@ -287,7 +287,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
 
         if (restoreBelow != null) {
             final Task transientRootTask = activity.getRootTask();
-            // Collect all visible activities which can be occluded by the transient activity to
+            // Collect all visible tasks which can be occluded by the transient activity to
             // make sure they are in the participants so their visibilities can be updated when
             // finishing transition.
             ((WindowContainer<?>) restoreBelow.getParent()).forAllTasks(t -> {
@@ -297,11 +297,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
                         mTransientHideTasks.add(t);
                     }
                     if (t.isLeafTask()) {
-                        t.forAllActivities(r -> {
-                            if (r.isVisibleRequested()) {
-                                collect(r);
-                            }
-                        });
+                        collect(t);
                     }
                 }
                 return t == restoreBelow;
@@ -904,6 +900,18 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
         mController.mFinishingTransition = this;
 
         if (mTransientHideTasks != null && !mTransientHideTasks.isEmpty()) {
+            // Record all the now-hiding activities so that they are committed after
+            // recalculating visibilities. We just use mParticipants because we can and it will
+            // ensure proper reporting of `isInFinishTransition`.
+            for (int i = 0; i < mTransientHideTasks.size(); ++i) {
+                mTransientHideTasks.get(i).forAllActivities(r -> {
+                    // Only check leaf-tasks that were collected
+                    if (!mParticipants.contains(r.getTask())) return;
+                    // Only concern ourselves with anything that can become invisible
+                    if (!r.isVisible()) return;
+                    mParticipants.add(r);
+                });
+            }
             // The transient hide tasks could be occluded now, e.g. returning to home. So trigger
             // the update to make the activities in the tasks invisible-requested, then the next
             // step can continue to commit the visibility.
@@ -953,7 +961,10 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
                         enterAutoPip = true;
                     }
                 }
-                if (mChanges.get(ar).mVisible != visibleAtTransitionEnd) {
+                final ChangeInfo changeInfo = mChanges.get(ar);
+                // Due to transient-hide, there may be some activities here which weren't in the
+                // transition.
+                if (changeInfo != null && changeInfo.mVisible != visibleAtTransitionEnd) {
                     // Legacy dispatch relies on this (for now).
                     ar.mEnteringAnimation = visibleAtTransitionEnd;
                 } else if (mTransientLaunches != null && mTransientLaunches.containsKey(ar)
diff --git a/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java b/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java
index 1e2fdec07d00b..43b429c767491 100644
--- a/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java
@@ -1410,9 +1410,9 @@ public class TransitionTests extends WindowTestsBase {
         final Transition.ChangeInfo task1ChangeInfo = closeTransition.mChanges.get(task1);
         assertNotNull(task1ChangeInfo);
         assertTrue(task1ChangeInfo.hasChanged());
+        // Make sure the unrelated activity is NOT collected.
         final Transition.ChangeInfo activity1ChangeInfo = closeTransition.mChanges.get(activity1);
-        assertNotNull(activity1ChangeInfo);
-        assertTrue(activity1ChangeInfo.hasChanged());
+        assertNull(activity1ChangeInfo);
         // No need to wait for the activity in transient hide task.
         assertEquals(WindowContainer.SYNC_STATE_NONE, activity1.mSyncState);
 
@@ -1442,6 +1442,7 @@ public class TransitionTests extends WindowTestsBase {
                 }
             }
         });
+        assertTrue(activity1.isVisible());
         controller.finishTransition(closeTransition);
         assertTrue(wasInFinishingTransition[0]);
         assertNull(controller.mFinishingTransition);
@@ -1450,6 +1451,7 @@ public class TransitionTests extends WindowTestsBase {
         assertEquals(ActivityTaskManagerService.APP_SWITCH_DISALLOW, mAtm.getBalAppSwitchesState());
         // Because task1 is occluded by task2, finishTransition should make activity1 invisible.
         assertFalse(activity1.isVisibleRequested());
+        // Make sure activity1 visibility was committed
         assertFalse(activity1.isVisible());
         assertFalse(activity1.app.hasActivityInVisibleTask());