From 0f533e914b40b2d8e5a956fe9622230c5ef8702c Mon Sep 17 00:00:00 2001
From: Robin Lee <rgl@google.com>
Date: Mon, 12 Jun 2023 20:06:48 +0200
Subject: [PATCH] Fix re-entrant issue in Keyguard / Mixed handler

We need to:

- Increment mInFlightSubAnimations optimistically before startAnimation
  because it might immediately call back before even returning true,
  which is allowed. If it doesn't do this and returns false instead, no
  problem; we'll just decrement it again.

- Post finish callback from Keyguard's "Remote" Transitions (which are
  just remote for API compatibility and execute synchronously in the
  same process) so that housekeeping from startAnimation can be
  guaranteed to finish before the onFinish callback comes back around

Bug: 286242775
Bug: 286507622 # to be confirmed
Test: atest WMShellUnitTests:ShellTransitionTests
Test: atest KeyguardTests
Change-Id: Ida3035f1a99992a1309033ffdd7498298e44e81f
---
 .../wm/shell/keyguard/KeyguardTransitionHandler.java     | 9 +++++----
 .../android/wm/shell/transition/DefaultMixedHandler.java | 3 ++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/keyguard/KeyguardTransitionHandler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/keyguard/KeyguardTransitionHandler.java
index cef7e16663312..56bd188a0d7d2 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/keyguard/KeyguardTransitionHandler.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/keyguard/KeyguardTransitionHandler.java
@@ -156,6 +156,8 @@ public class KeyguardTransitionHandler implements Transitions.TransitionHandler
                 "start keyguard %s transition, info = %s", description, info);
 
         try {
+            mStartedTransitions.put(transition,
+                    new StartedTransition(info, finishTransaction, remoteHandler));
             remoteHandler.startAnimation(transition, info, startTransaction,
                     new IRemoteTransitionFinishedCallback.Stub() {
                         @Override
@@ -164,14 +166,13 @@ public class KeyguardTransitionHandler implements Transitions.TransitionHandler
                             if (sct != null) {
                                 finishTransaction.merge(sct);
                             }
-                            mMainExecutor.execute(() -> {
+                            // Post our finish callback to let startAnimation finish first.
+                            mMainExecutor.executeDelayed(() -> {
                                 mStartedTransitions.remove(transition);
                                 finishCallback.onTransitionFinished(wct, null);
-                            });
+                            }, 0);
                         }
                     });
-            mStartedTransitions.put(transition,
-                    new StartedTransition(info, finishTransaction, remoteHandler));
         } catch (RemoteException e) {
             Log.wtf(TAG, "RemoteException thrown from local IRemoteTransition", e);
             return false;
diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultMixedHandler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultMixedHandler.java
index 3ec433e55da96..3e8a4e0ae514b 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultMixedHandler.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultMixedHandler.java
@@ -574,11 +574,12 @@ public class DefaultMixedHandler implements Transitions.TransitionHandler,
                 finishCallback.onTransitionFinished(wct, wctCB);
             }
         };
+        mixed.mInFlightSubAnimations++;
         if (!mKeyguardHandler.startAnimation(
                 mixed.mTransition, info, startTransaction, finishTransaction, finishCB)) {
+            mixed.mInFlightSubAnimations--;
             return false;
         }
-        mixed.mInFlightSubAnimations++;
         // Sync pip state.
         if (mPipHandler != null) {
             // We don't know when to apply `startTransaction` so use a separate transaction here.