Defer committing visible until transition ready

So some logic depend on ActivityRecord#isVisible() won't be called
earlier before transition ready that may delay the animation.
Such as updateFocusedWindow in activityPaused and
updateSystemBarAttributes when starting window performs relayout.

Skip the assertion pipAppWindowAlwaysVisible for "swipe up and
fade in pip" because either "always visible" or "visible->invisible
->visible" is fine for internal state. The visual appearance is
still asserted by layer visibility.

Bug: 260059642
Test: TransitionTests#testIntermediateVisibility
Test: OpenAppMicrobenchmark with shell transition.

Change-Id: I4584880dd91f9b406bc5241a67eebae179a7fe40

libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/pip/EnterPipOnUserLeaveHintTest.kt

@@ -68,6 +68,18 @@ open class EnterPipOnUserLeaveHintTest(flicker: FlickerTest) : EnterPipTest(flic
             transitions { tapl.goHome() }
         }
 
+    @Presubmit
+    @Test
+    override fun pipAppWindowAlwaysVisible() {
+        // In gestural nav the pip will first move behind home and then above home. The visual
+        // appearance visible->invisible->visible is asserted by pipAppLayerAlwaysVisible().
+        // But the internal states of activity don't need to follow that, such as a temporary
+        // visibility state can be changed quickly outside a transaction so the test doesn't
+        // detect that. Hence, skip the case to avoid restricting the internal implementation.
+        Assume.assumeFalse(flicker.scenario.isGesturalNavigation)
+        super.pipAppWindowAlwaysVisible()
+    }
+
     @Presubmit
     @Test
     override fun pipAppLayerAlwaysVisible() {

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

@@ -5257,9 +5257,9 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
             transferStartingWindowFromHiddenAboveTokenIfNeeded();
         }
 
-        // If in a transition, defer commits for activities that are going invisible
-        if (!visible && inTransition()) {
-            if (mTransitionController.inPlayingTransition(this)
+        // Defer committing visibility until transition starts.
+        if (inTransition()) {
+            if (!visible && mTransitionController.inPlayingTransition(this)
                     && mTransitionController.isCollecting(this)) {
                 mTransitionChangeFlags |= FLAG_IS_OCCLUDED;
             }
@@ -5509,6 +5509,17 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
         }
     }
 
+    /** Updates draw state and shows drawn windows. */
+    void commitFinishDrawing(SurfaceControl.Transaction t) {
+        boolean committed = false;
+        for (int i = mChildren.size() - 1; i >= 0; i--) {
+            committed |= mChildren.get(i).commitFinishDrawing(t);
+        }
+        if (committed) {
+            requestUpdateWallpaperIfNeeded();
+        }
+    }
+
     /**
      * Check if visibility of this {@link ActivityRecord} should be updated as part of an app
      * transition.

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

@@ -845,7 +845,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
             final ActivityRecord ar = mParticipants.valueAt(i).asActivityRecord();
             if (ar == null || !ar.isVisible() || ar.getParent() == null) continue;
             if (inputSinkTransaction == null) {
-                inputSinkTransaction = new SurfaceControl.Transaction();
+                inputSinkTransaction = ar.mWmService.mTransactionFactory.get();
             }
             ar.mActivityRecordInputSink.applyChangesToSurfaceIfChanged(inputSinkTransaction);
         }
@@ -961,6 +961,12 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
         // time being, we don't have full cross-display transitions so it isn't a problem.
         final DisplayContent dc = mTargetDisplays.get(0);
 
+        // Commit the visibility of visible activities before calculateTransitionInfo(), so the
+        // TaskInfo can be visible. Also it needs to be done before moveToPlaying(), otherwise
+        // ActivityRecord#canShowWindows() may reject to show its window. The visibility also
+        // needs to be updated for STATE_ABORT.
+        commitVisibleActivities(transaction);
+
         if (mState == STATE_ABORT) {
             mController.abort(this);
             dc.getPendingTransaction().merge(transaction);
@@ -1132,6 +1138,19 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
         }
     }
 
+    /** The transition is ready to play. Make the start transaction show the surfaces. */
+    private void commitVisibleActivities(SurfaceControl.Transaction transaction) {
+        for (int i = mParticipants.size() - 1; i >= 0; --i) {
+            final ActivityRecord ar = mParticipants.valueAt(i).asActivityRecord();
+            if (ar == null || !ar.isVisibleRequested()) {
+                continue;
+            }
+            ar.commitVisibility(true /* visible */, false /* performLayout */,
+                    true /* fromTransition */);
+            ar.commitFinishDrawing(transaction);
+        }
+    }
+
     /** @see RecentsAnimationController#attachNavigationBarToApp */
     private void handleLegacyRecentsStartBehavior(DisplayContent dc, TransitionInfo info) {
         if ((mFlags & TRANSIT_FLAG_IS_RECENTS) == 0) {

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

@@ -4616,6 +4616,19 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
         }
     }
 
+    /** Makes the surface of drawn window (COMMIT_DRAW_PENDING) to be visible. */
+    boolean commitFinishDrawing(SurfaceControl.Transaction t) {
+        boolean committed = mWinAnimator.commitFinishDrawingLocked();
+        if (committed) {
+            // Ensure that the visibility of buffer layer is set.
+            mWinAnimator.prepareSurfaceLocked(t);
+        }
+        for (int i = mChildren.size() - 1; i >= 0; i--) {
+            committed |= mChildren.get(i).commitFinishDrawing(t);
+        }
+        return committed;
+    }
+
     // This must be called while inside a transaction.
     boolean performShowLocked() {
         if (!showToCurrentUser()) {

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

@@ -564,20 +564,23 @@ public class TransitionTests extends WindowTestsBase {
         final WindowProcessController delegateProc = mSystemServicesTestRule.addProcess(
                 "pkg.delegate", "proc.delegate", 6000 /* pid */, 6000 /* uid */);
         doReturn(mock(IBinder.class)).when(delegateProc.getThread()).asBinder();
-        final ActivityRecord app = new ActivityBuilder(mAtm).setCreateTask(true).build();
+        final ActivityRecord app = new ActivityBuilder(mAtm).setCreateTask(true)
+                .setVisible(false).build();
+        app.setVisibleRequested(true);
         final TransitionController controller = app.mTransitionController;
         final Transition transition = controller.createTransition(TRANSIT_OPEN);
         final RemoteTransition remoteTransition = new RemoteTransition(
                 mock(IRemoteTransition.class));
         remoteTransition.setAppThread(delegateProc.getThread());
-        transition.collectExistenceChange(app.getTask());
-        controller.requestStartTransition(transition, app.getTask(), remoteTransition,
+        transition.collect(app);
+        controller.requestStartTransition(transition, null /* startTask */, remoteTransition,
                 null /* displayChange */);
         testPlayer.startTransition();
         testPlayer.onTransactionReady(app.getSyncTransaction());
         assertTrue(playerProc.isRunningRemoteTransition());
         assertTrue(delegateProc.isRunningRemoteTransition());
         assertTrue(controller.mRemotePlayer.reportRunning(delegateProc.getThread()));
+        assertTrue(app.isVisible());
 
         testPlayer.finish();
         assertFalse(playerProc.isRunningRemoteTransition());
@@ -1114,6 +1117,14 @@ public class TransitionTests extends WindowTestsBase {
 
         assertFalse(activity1.isVisible());
         assertTrue(activity2.isVisible());
+
+        // The abort should still commit visible-requested to visible.
+        final Transition abortTransition = controller.createTransition(TRANSIT_OPEN);
+        abortTransition.collect(activity1);
+        activity1.setVisibleRequested(true);
+        activity1.setVisible(false);
+        abortTransition.abort();
+        assertTrue(activity1.isVisible());
     }
 
     @Test