Merge "Handle some Recents finish corner cases" into udc-dev

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

@@ -5296,6 +5296,10 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
             if (isCollecting) {
                 mTransitionController.collect(this);
             } else {
+                // Failsafe to make sure that we show any activities that were incorrectly hidden
+                // during a transition. If this vis-change is a result of finishing, ignore it.
+                // Finish should only ever commit visibility=false, so we can check full containment
+                // rather than just direct membership.
                 inFinishingTransition = mTransitionController.inFinishingTransition(this);
                 if (!inFinishingTransition && !mDisplayContent.isSleeping()) {
                     Slog.e(TAG, "setVisibility=" + visible

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

@@ -244,6 +244,16 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
     private IContainerFreezer mContainerFreezer = null;
     private final SurfaceControl.Transaction mTmpTransaction = new SurfaceControl.Transaction();
 
+    /**
+     * {@code true} if some other operation may have caused the originally-recorded state (in
+     * mChanges) to be dirty. This is usually due to finishTransition being called mid-collect;
+     * and, the reason that finish can alter the "start" state of other transitions is because
+     * setVisible(false) is deferred until then.
+     * Instead of adding this conditional, we could re-check always; but, this situation isn't
+     * common so it'd be wasted work.
+     */
+    boolean mPriorVisibilityMightBeDirty = false;
+
     final TransitionController.Logger mLogger = new TransitionController.Logger();
 
     /** Whether this transition was forced to play early (eg for a SLEEP signal). */
@@ -966,28 +976,30 @@ 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.
             mController.mAtm.mRootWindowContainer.ensureActivitiesVisible(null /* starting */,
                     0 /* configChanges */, true /* preserveWindows */);
+            // Record all the now-hiding activities so that they are committed. Just use
+            // mParticipants because we can avoid a new list this way.
+            for (int i = 0; i < mTransientHideTasks.size(); ++i) {
+                // Only worry about tasks that were actually hidden. Otherwise, we could end-up
+                // committing visibility for activity-level changes that aren't part of this
+                // transition.
+                if (mTransientHideTasks.get(i).isVisibleRequested()) continue;
+                mTransientHideTasks.get(i).forAllActivities(r -> {
+                    // Only check leaf-tasks that were collected
+                    if (!mParticipants.contains(r.getTask())) return;
+                    mParticipants.add(r);
+                });
+            }
         }
 
         boolean hasParticipatedDisplay = false;
         boolean hasVisibleTransientLaunch = false;
         boolean enterAutoPip = false;
+        boolean committedSomeInvisible = false;
         // Commit all going-invisible containers
         for (int i = 0; i < mParticipants.size(); ++i) {
             final WindowContainer<?> participant = mParticipants.valueAt(i);
@@ -1023,6 +1035,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
                         }
                         ar.commitVisibility(false /* visible */, false /* performLayout */,
                                 true /* fromTransition */);
+                        committedSomeInvisible = true;
                     } else {
                         enterAutoPip = true;
                     }
@@ -1081,6 +1094,9 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
                 }
             }
         }
+        if (committedSomeInvisible) {
+            mController.onCommittedInvisibles();
+        }
 
         if (hasVisibleTransientLaunch) {
             // Notify the change about the transient-below task if entering auto-pip.
@@ -1293,6 +1309,9 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
         // leftover order changes.
         collectOrderChanges(mController.mWaitingTransitions.isEmpty());
 
+        if (mPriorVisibilityMightBeDirty) {
+            updatePriorVisibility();
+        }
         // Resolve the animating targets from the participants.
         mTargets = calculateTargets(mParticipants, mChanges);
         // Check whether the participants were animated from back navigation.
@@ -1806,6 +1825,20 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
         }
     }
 
+    private void updatePriorVisibility() {
+        for (int i = 0; i < mChanges.size(); ++i) {
+            final ChangeInfo chg = mChanges.valueAt(i);
+            // For task/activity, recalculate the current "real" visibility.
+            if (chg.mContainer.asActivityRecord() == null && chg.mContainer.asTask() == null) {
+                continue;
+            }
+            // This ONLY works in the visible -> invisible case (and is only needed for this case)
+            // because commitVisible(false) is deferred until finish.
+            if (!chg.mVisible) continue;
+            chg.mVisible = chg.mContainer.isVisible();
+        }
+    }
+
     /**
      * Under some conditions (eg. all visible targets within a parent container are transitioning
      * the same way) the transition can be "promoted" to the parent container. This means an

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

@@ -408,9 +408,9 @@ class TransitionController {
         return false;
     }
 
-    /** Returns {@code true} if the `wc` is a participant of the finishing transition. */
+    /** Returns {@code true} if the finishing transition contains `wc`. */
     boolean inFinishingTransition(WindowContainer<?> wc) {
-        return mFinishingTransition != null && mFinishingTransition.mParticipants.contains(wc);
+        return mFinishingTransition != null && mFinishingTransition.isInTransition(wc);
     }
 
     /** @return {@code true} if a transition is running */
@@ -827,6 +827,16 @@ class TransitionController {
         }
     }
 
+    /** Called by {@link Transition#finishTransition} if it committed invisible to any activities */
+    void onCommittedInvisibles() {
+        if (mCollectingTransition != null) {
+            mCollectingTransition.mPriorVisibilityMightBeDirty = true;
+        }
+        for (int i = mWaitingTransitions.size() - 1; i >= 0; --i) {
+            mWaitingTransitions.get(i).mPriorVisibilityMightBeDirty = true;
+        }
+    }
+
     private void validateStates() {
         for (int i = 0; i < mStateValidators.size(); ++i) {
             mStateValidators.get(i).run();