From 0207f039d25eece5f04512bfc1198ef72e5c2e4f Mon Sep 17 00:00:00 2001
From: Chris Li <lihongyu@google.com>
Date: Thu, 27 Jan 2022 11:52:51 +0800
Subject: [DO NOT MERGE] Fix regression for enter PIP when onUserLeaveHint

This was changed in ag/15886711, but it shouldn't be needed anymore as
for case we need to animate the close transition, isVisible() will stay
true until the app transition is finished. Otherwise we shouldn't need
to keep the surface as visible even if it is in the closing app list.

Without this fix, we may accidentally show the surface that has been
commited to be invisible, such as enter pip onUserLeaveHint.

Fix: 216145863
Test: manually verify with Google Meet.
Test: atest WmTests:ActivityRecordTests
Change-Id: Ib28651f5c4dc728d19f968adc56015836e307ef8
Merged-In: Ib28651f5c4dc728d19f968adc56015836e307ef8
---
 .../java/com/android/server/wm/ActivityRecord.java |  2 +-
 .../com/android/server/wm/ActivityRecordTests.java | 42 ++++++++++++++++++----
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java
index ffafb8a7d2ce..618e421038c7 100644
--- a/services/core/java/com/android/server/wm/ActivityRecord.java
+++ b/services/core/java/com/android/server/wm/ActivityRecord.java
@@ -6913,7 +6913,7 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
 
     @Override
     void prepareSurfaces() {
-        final boolean show = isVisible() || isAnimating(TRANSITION | PARENTS,
+        final boolean show = isVisible() || isAnimating(PARENTS,
                 ANIMATION_TYPE_APP_TRANSITION | ANIMATION_TYPE_RECENTS);
 
         if (mSurfaceControl != null) {
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 dd4cc5022cd2..b770b3e3a55b 100644
--- a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java
@@ -80,7 +80,6 @@ import static com.android.server.wm.ActivityRecord.State.RESUMED;
 import static com.android.server.wm.ActivityRecord.State.STARTED;
 import static com.android.server.wm.ActivityRecord.State.STOPPED;
 import static com.android.server.wm.ActivityRecord.State.STOPPING;
-import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_APP_TRANSITION;
 import static com.android.server.wm.TaskFragment.TASK_FRAGMENT_VISIBILITY_INVISIBLE;
 import static com.android.server.wm.TaskFragment.TASK_FRAGMENT_VISIBILITY_VISIBLE;
 import static com.android.server.wm.TaskFragment.TASK_FRAGMENT_VISIBILITY_VISIBLE_BEHIND_TRANSLUCENT;
@@ -3124,7 +3123,7 @@ public class ActivityRecordTests extends WindowTestsBase {
     }
 
     @Test
-    public void testInClosingAnimation_doNotHideSurface() {
+    public void testInClosingAnimation_visibilityNotCommitted_doNotHideSurface() {
         final WindowState app = createWindow(null, TYPE_APPLICATION, "app");
         makeWindowVisibleAndDrawn(app);
 
@@ -3133,16 +3132,45 @@ public class ActivityRecordTests extends WindowTestsBase {
         mDisplayContent.mClosingApps.add(app.mActivityRecord);
         mDisplayContent.prepareAppTransition(TRANSIT_CLOSE);
 
-        // Update visibility and call to remove window
-        app.mActivityRecord.commitVisibility(false, false);
+        // Remove window during transition, so it is requested to hide, but won't be committed until
+        // the transition is finished.
+        app.mActivityRecord.onRemovedFromDisplay();
+
+        assertTrue(mDisplayContent.mClosingApps.contains(app.mActivityRecord));
+        assertFalse(app.mActivityRecord.isVisibleRequested());
+        assertTrue(app.mActivityRecord.isVisible());
+        assertTrue(app.mActivityRecord.isSurfaceShowing());
+
+        // Start transition.
         app.mActivityRecord.prepareSurfaces();
 
         // Because the app is waiting for transition, it should not hide the surface.
         assertTrue(app.mActivityRecord.isSurfaceShowing());
+    }
+
+    @Test
+    public void testInClosingAnimation_visibilityCommitted_hideSurface() {
+        final WindowState app = createWindow(null, TYPE_APPLICATION, "app");
+        makeWindowVisibleAndDrawn(app);
+
+        // Put the activity in close transition.
+        mDisplayContent.mOpeningApps.clear();
+        mDisplayContent.mClosingApps.add(app.mActivityRecord);
+        mDisplayContent.prepareAppTransition(TRANSIT_CLOSE);
+
+        // Commit visibility before start transition.
+        app.mActivityRecord.commitVisibility(false, false);
+
+        assertFalse(app.mActivityRecord.isVisibleRequested());
+        assertFalse(app.mActivityRecord.isVisible());
+        assertTrue(app.mActivityRecord.isSurfaceShowing());
+
+        // Start transition.
+        app.mActivityRecord.prepareSurfaces();
 
-        // Ensure onAnimationFinished will callback when the closing animation is finished.
-        verify(app.mActivityRecord).onAnimationFinished(eq(ANIMATION_TYPE_APP_TRANSITION),
-                eq(null));
+        // Because the app visibility has been committed before the transition start, it should hide
+        // the surface.
+        assertFalse(app.mActivityRecord.isSurfaceShowing());
     }
 
     private void assertHasStartingWindow(ActivityRecord atoken) {
-- 
cgit v1.2.3-59-g8ed1b