From 708287bc782f108ab8433057860b996c1c0832f5 Mon Sep 17 00:00:00 2001
From: Arthur Hung <arthurhung@google.com>
Date: Thu, 9 Mar 2023 14:11:16 +0000
Subject: [PATCH] Fix back animation doesn't work when triggered a second time

Check the participated containers to see if any container in the back
target when handling the defered animation, and ensure the targets
which are promoted from the participated containers could be visible.

This also added the timeout mechanism when back gesture released, that
could prevent it stuck for waiting the `startAnimation` called.

Bug: 271599041
Test: atest BackAnimationControllerTest
Change-Id: I473d218f61646932b9e2421096191c215d9a9705
---
 .../shell/back/BackAnimationController.java   |  8 ++--
 .../back/BackAnimationControllerTest.java     | 16 ++++++++
 .../server/wm/BackNavigationController.java   | 40 +++++++++++--------
 3 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java
index 210c9aab14d66..04f8477eb749a 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java
@@ -561,6 +561,9 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
         }
         if (runner.isWaitingAnimation()) {
             ProtoLog.w(WM_SHELL_BACK_PREVIEW, "Gesture released, but animation didn't ready.");
+            // Supposed it is in post commit animation state, and start the timeout to watch
+            // if the animation is ready.
+            mShellExecutor.executeDelayed(mAnimationTimeoutRunnable, MAX_ANIMATION_DURATION);
             return;
         } else if (runner.isAnimationCancelled()) {
             invokeOrCancelBack();
@@ -577,6 +580,8 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
         if (mPostCommitAnimationInProgress) {
             return;
         }
+
+        mShellExecutor.removeCallbacks(mAnimationTimeoutRunnable);
         ProtoLog.d(WM_SHELL_BACK_PREVIEW, "BackAnimationController: startPostCommitAnimation()");
         mPostCommitAnimationInProgress = true;
         mShellExecutor.executeDelayed(mAnimationTimeoutRunnable, MAX_ANIMATION_DURATION);
@@ -595,9 +600,6 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
      */
     @VisibleForTesting
     void onBackAnimationFinished() {
-        if (!mPostCommitAnimationInProgress) {
-            return;
-        }
         // Stop timeout runner.
         mShellExecutor.removeCallbacks(mAnimationTimeoutRunnable);
         mPostCommitAnimationInProgress = false;
diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java
index 806bffebd4cb9..0673484f6336b 100644
--- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java
+++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java
@@ -166,6 +166,9 @@ public class BackAnimationControllerTest extends ShellTestCase {
         doMotionEvent(MotionEvent.ACTION_DOWN, 0);
         doMotionEvent(MotionEvent.ACTION_MOVE, 0);
         mController.setTriggerBack(true);
+    }
+
+    private void releaseBackGesture() {
         doMotionEvent(MotionEvent.ACTION_UP, 0);
     }
 
@@ -201,6 +204,8 @@ public class BackAnimationControllerTest extends ShellTestCase {
                     .setOnBackNavigationDone(new RemoteCallback(result)));
             triggerBackGesture();
             simulateRemoteAnimationStart(type);
+            mShellExecutor.flushAll();
+            releaseBackGesture();
             simulateRemoteAnimationFinished();
             mShellExecutor.flushAll();
 
@@ -252,6 +257,7 @@ public class BackAnimationControllerTest extends ShellTestCase {
         createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, false);
 
         triggerBackGesture();
+        releaseBackGesture();
 
         verify(mAppCallback, times(1)).onBackInvoked();
 
@@ -269,6 +275,8 @@ public class BackAnimationControllerTest extends ShellTestCase {
 
         triggerBackGesture();
         simulateRemoteAnimationStart(BackNavigationInfo.TYPE_RETURN_TO_HOME);
+        releaseBackGesture();
+
         // Check that back invocation is dispatched.
         verify(mAnimatorCallback).onBackInvoked();
         verify(mBackAnimationRunner).onAnimationStart(anyInt(), any(), any(), any(), any());
@@ -308,6 +316,9 @@ public class BackAnimationControllerTest extends ShellTestCase {
 
         triggerBackGesture();
         simulateRemoteAnimationStart(BackNavigationInfo.TYPE_RETURN_TO_HOME);
+        mShellExecutor.flushAll();
+
+        releaseBackGesture();
 
         // Simulate transition timeout.
         mShellExecutor.flushAll();
@@ -369,6 +380,9 @@ public class BackAnimationControllerTest extends ShellTestCase {
             simulateRemoteAnimationStart(type);
             mShellExecutor.flushAll();
 
+            releaseBackGesture();
+            mShellExecutor.flushAll();
+
             assertTrue("Navigation Done callback not called for "
                     + BackNavigationInfo.typeToString(type), result.mBackNavigationDone);
             assertTrue("TriggerBack should have been true", result.mTriggerBack);
@@ -395,6 +409,8 @@ public class BackAnimationControllerTest extends ShellTestCase {
                 .setOnBackNavigationDone(new RemoteCallback(result)));
         triggerBackGesture();
         mShellExecutor.flushAll();
+        releaseBackGesture();
+        mShellExecutor.flushAll();
 
         assertTrue("Navigation Done callback not called for "
                 + BackNavigationInfo.typeToString(type), result.mBackNavigationDone);
diff --git a/services/core/java/com/android/server/wm/BackNavigationController.java b/services/core/java/com/android/server/wm/BackNavigationController.java
index 7b562b0bc964c..745374301263c 100644
--- a/services/core/java/com/android/server/wm/BackNavigationController.java
+++ b/services/core/java/com/android/server/wm/BackNavigationController.java
@@ -648,31 +648,28 @@ class BackNavigationController {
         if (finishedTransition == mWaitTransitionFinish) {
             clearBackAnimations();
         }
+
         if (!mBackAnimationInProgress || mPendingAnimationBuilder == null) {
             return false;
         }
-
         ProtoLog.d(WM_DEBUG_BACK_PREVIEW,
                 "Handling the deferred animation after transition finished");
 
-        // Show the target surface and its parents to prevent it or its parents hidden when
-        // the transition finished.
-        // The target could be affected by transition when :
-        // Open transition -> the open target in back navigation
-        // Close transition -> the close target in back navigation.
+        // Find the participated container collected by transition when :
+        // Open transition -> the open target in back navigation, the close target in transition.
+        // Close transition -> the close target in back navigation, the open target in transition.
         boolean hasTarget = false;
-        final SurfaceControl.Transaction t =
-                mPendingAnimationBuilder.mCloseTarget.getPendingTransaction();
-        for (int i = 0; i < targets.size(); i++) {
-            final WindowContainer wc = targets.get(i).mContainer;
-            if (wc.asActivityRecord() == null && wc.asTask() == null) {
-                continue;
-            } else if (!mPendingAnimationBuilder.containTarget(wc)) {
+        for (int i = 0; i < finishedTransition.mParticipants.size(); i++) {
+            final WindowContainer wc = finishedTransition.mParticipants.valueAt(i);
+            if (wc.asActivityRecord() == null && wc.asTask() == null
+                    && wc.asTaskFragment() == null) {
                 continue;
             }
 
-            hasTarget = true;
-            t.show(wc.getSurfaceControl());
+            if (mPendingAnimationBuilder.containTarget(wc)) {
+                hasTarget = true;
+                break;
+            }
         }
 
         if (!hasTarget) {
@@ -689,6 +686,12 @@ class BackNavigationController {
             return false;
         }
 
+        // Ensure the final animation targets which hidden by transition could be visible.
+        for (int i = 0; i < targets.size(); i++) {
+            final WindowContainer wc = targets.get(i).mContainer;
+            wc.prepareSurfaces();
+        }
+
         scheduleAnimation(mPendingAnimationBuilder);
         mPendingAnimationBuilder = null;
         return true;
@@ -1076,7 +1079,7 @@ class BackNavigationController {
 
             boolean containTarget(@NonNull WindowContainer wc) {
                 return wc == mOpenTarget || wc == mCloseTarget
-                        || wc.hasChild(mOpenTarget) || wc.hasChild(mCloseTarget);
+                        || mOpenTarget.hasChild(wc) || mCloseTarget.hasChild(wc);
             }
 
             /**
@@ -1151,6 +1154,11 @@ class BackNavigationController {
     private static void setLaunchBehind(@NonNull ActivityRecord activity) {
         if (!activity.isVisibleRequested()) {
             activity.setVisibility(true);
+            // The transition could commit the visibility and in the finishing state, that could
+            // skip commitVisibility call in setVisibility cause the activity won't visible here.
+            // Call it again to make sure the activity could be visible while handling the pending
+            // animation.
+            activity.commitVisibility(true, true);
         }
         activity.mLaunchTaskBehind = true;