Merge "Don't prematurely commit visibility in consecutive shell transitions" into sc-v2-dev

services/core/java/com/android/server/wm/Transition.java

@@ -139,6 +139,12 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
     /** The final animation targets derived from participants after promotion. */
     private ArraySet<WindowContainer> mTargets = null;
 
+    /**
+     * Set of participating windowtokens (activity/wallpaper) which are visible at the end of
+     * the transition animation.
+     */
+    private final ArraySet<WindowToken> mVisibleAtTransitionEndTokens = new ArraySet<>();
+
     /** Custom activity-level animation options and callbacks. */
     private TransitionInfo.AnimationOptions mOverrideOptions;
     private IRemoteCallback mClientAnimationStartCallback = null;
@@ -345,7 +351,14 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
         for (int i = 0; i < mParticipants.size(); ++i) {
             final ActivityRecord ar = mParticipants.valueAt(i).asActivityRecord();
             if (ar != null) {
-                if (!ar.isVisibleRequested()) {
+                boolean visibleAtTransitionEnd = mVisibleAtTransitionEndTokens.contains(ar);
+                // We need both the expected visibility AND current requested-visibility to be
+                // false. If it is expected-visible but not currently visible, it means that
+                // another animation is queued-up to animate this to invisibility, so we can't
+                // remove the surfaces yet. If it is currently visible, but not expected-visible,
+                // then doing commitVisibility here would actually be out-of-order and leave the
+                // activity in a bad state.
+                if (!visibleAtTransitionEnd && !ar.isVisibleRequested()) {
                     boolean commitVisibility = true;
                     if (ar.getDeferHidingClient() && ar.getTask() != null) {
                         if (ar.pictureInPictureArgs != null
@@ -368,19 +381,22 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
                         activitiesWentInvisible = true;
                     }
                 }
-                if (mChanges.get(ar).mVisible != ar.isVisibleRequested()) {
+                if (mChanges.get(ar).mVisible != visibleAtTransitionEnd) {
                     // Legacy dispatch relies on this (for now).
-                    ar.mEnteringAnimation = ar.isVisibleRequested();
+                    ar.mEnteringAnimation = visibleAtTransitionEnd;
                 }
                 mController.dispatchLegacyAppTransitionFinished(ar);
             }
             final WallpaperWindowToken wt = mParticipants.valueAt(i).asWallpaperToken();
-            if (wt != null && !wt.isVisibleRequested()) {
+            if (wt != null) {
+                final boolean visibleAtTransitionEnd = mVisibleAtTransitionEndTokens.contains(wt);
+                if (!visibleAtTransitionEnd && !wt.isVisibleRequested()) {
                     ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS,
                             "  Commit wallpaper becoming invisible: %s", wt);
                     wt.commitVisibility(false /* visible */);
                 }
             }
+        }
         if (activitiesWentInvisible) {
             // Always schedule stop processing when transition finishes because activities don't
             // stop while they are in a transition thus their stop could still be pending.
@@ -488,6 +504,15 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
             }
         }
 
+        // Record windowtokens (activity/wallpaper) that are expected to be visible after the
+        // transition animation. This will be used in finishTransition to prevent prematurely
+        // committing visibility.
+        for (int i = mParticipants.size() - 1; i >= 0; --i) {
+            final WindowContainer wc = mParticipants.valueAt(i);
+            if (wc.asWindowToken() == null || !wc.isVisibleRequested()) continue;
+            mVisibleAtTransitionEndTokens.add(wc.asWindowToken());
+        }
+
         mStartTransaction = transaction;
         mFinishTransaction = mController.mAtm.mWindowManager.mTransactionFactory.get();
         buildFinishTransaction(mFinishTransaction, info.getRootLeash());

services/core/java/com/android/server/wm/TransitionController.java

@@ -117,11 +117,16 @@ class TransitionController {
 
     void registerTransitionPlayer(@Nullable ITransitionPlayer player) {
         try {
+            // Note: asBinder() can be null if player is same process (likely in a test).
             if (mTransitionPlayer != null) {
+                if (mTransitionPlayer.asBinder() != null) {
                     mTransitionPlayer.asBinder().unlinkToDeath(mTransitionPlayerDeath, 0);
+                }
                 mTransitionPlayer = null;
             }
+            if (player.asBinder() != null) {
                 player.asBinder().linkToDeath(mTransitionPlayerDeath, 0);
+            }
             mTransitionPlayer = player;
         } catch (RemoteException e) {
             throw new RuntimeException("Unable to set transition player");

services/tests/wmtests/src/com/android/server/wm/TransitionTests.java

@@ -20,6 +20,7 @@ import static android.app.WindowConfiguration.ACTIVITY_TYPE_STANDARD;
 import static android.app.WindowConfiguration.WINDOWING_MODE_FREEFORM;
 import static android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN;
 import static android.view.WindowManager.LayoutParams.TYPE_WALLPAPER;
+import static android.view.WindowManager.TRANSIT_CLOSE;
 import static android.view.WindowManager.TRANSIT_OLD_TASK_OPEN;
 import static android.view.WindowManager.TRANSIT_OPEN;
 import static android.view.WindowManager.TRANSIT_TO_BACK;
@@ -41,6 +42,7 @@ import android.platform.test.annotations.Presubmit;
 import android.util.ArrayMap;
 import android.util.ArraySet;
 import android.window.ITaskOrganizer;
+import android.window.ITransitionPlayer;
 import android.window.TransitionInfo;
 
 import androidx.test.filters.SmallTest;
@@ -444,6 +446,71 @@ public class TransitionTests extends WindowTestsBase {
                 info.getChange(openInChangeTask.mRemoteToken.toWindowContainerToken()), info));
     }
 
+    @Test
+    public void testIntermediateVisibility() {
+        final TransitionController controller = new TransitionController(mAtm);
+        final ITransitionPlayer player = new ITransitionPlayer.Default();
+        controller.registerTransitionPlayer(player);
+        ITaskOrganizer mockOrg = mock(ITaskOrganizer.class);
+        final Transition openTransition = controller.createTransition(TRANSIT_OPEN);
+
+        // Start out with task2 visible and set up a transition that closes task2 and opens task1
+        final Task task1 = createTask(mDisplayContent);
+        task1.mTaskOrganizer = mockOrg;
+        final ActivityRecord activity1 = createActivityRecord(task1);
+        activity1.mVisibleRequested = false;
+        activity1.setVisible(false);
+        final Task task2 = createTask(mDisplayContent);
+        task2.mTaskOrganizer = mockOrg;
+        final ActivityRecord activity2 = createActivityRecord(task1);
+        activity2.mVisibleRequested = true;
+        activity2.setVisible(true);
+
+        openTransition.collectExistenceChange(task1);
+        openTransition.collectExistenceChange(activity1);
+        openTransition.collectExistenceChange(task2);
+        openTransition.collectExistenceChange(activity2);
+
+        activity1.mVisibleRequested = true;
+        activity1.setVisible(true);
+        activity2.mVisibleRequested = false;
+
+        // Using abort to force-finish the sync (since we can't wait for drawing in unit test).
+        // We didn't call abort on the transition itself, so it will still run onTransactionReady
+        // normally.
+        mWm.mSyncEngine.abort(openTransition.getSyncId());
+
+        // Before finishing openTransition, we are now going to simulate closing task1 to return
+        // back to (open) task2.
+        final Transition closeTransition = controller.createTransition(TRANSIT_CLOSE);
+
+        closeTransition.collectExistenceChange(task1);
+        closeTransition.collectExistenceChange(activity1);
+        closeTransition.collectExistenceChange(task2);
+        closeTransition.collectExistenceChange(activity2);
+
+        activity1.mVisibleRequested = false;
+        activity2.mVisibleRequested = true;
+
+        openTransition.finishTransition();
+
+        // We finished the openTransition. Even though activity1 is visibleRequested=false, since
+        // the closeTransition animation hasn't played yet, make sure that we didn't commit
+        // visible=false on activity1 since it needs to remain visible for the animation.
+        assertTrue(activity1.isVisible());
+        assertTrue(activity2.isVisible());
+
+        // Using abort to force-finish the sync (since we obviously can't wait for drawing).
+        // We didn't call abort on the actual transition, so it will still run onTransactionReady
+        // normally.
+        mWm.mSyncEngine.abort(closeTransition.getSyncId());
+
+        closeTransition.finishTransition();
+
+        assertFalse(activity1.isVisible());
+        assertTrue(activity2.isVisible());
+    }
+
     /** Fill the change map with all the parents of top. Change maps are usually fully populated */
     private static void fillChangeMap(ArrayMap<WindowContainer, Transition.ChangeInfo> changes,
             WindowContainer top) {