From 8ffedf11e398f9a6aa8b30cff5f06b102fd54536 Mon Sep 17 00:00:00 2001
From: Robin Lee <rgl@google.com>
Date: Fri, 31 Mar 2023 00:09:56 +0000
Subject: [PATCH] Cancel current animation instead of candidate

Otherwise everything along the way confidently ignores this
and Keyguard doesn't even see the timeout and force-abort
coming when it hits. This leaves an animation timer running
for 950ms which sets occlude status on finish callback and
causes unpleasant race conditions.

Change-Id: I7b3dcd06e7483fde745a1d56dfee7c4efc262ed7
Test: atest android.server.wm.KeyguardTests
Test: specific case to be added in b/274003805
Bug: 275650364
---
 .../systemui/keyguard/KeyguardService.java     | 18 ++++++++----------
 .../keyguard/KeyguardViewMediator.java         | 13 +++++++++----
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java
index 4d40db0a0cfd4..f1a8c95469f64 100644
--- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java
+++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java
@@ -260,22 +260,20 @@ public class KeyguardService extends Service {
                 );
             }
 
-            public void mergeAnimation(IBinder transition, TransitionInfo info,
-                    SurfaceControl.Transaction t, IBinder mergeTarget,
-                    IRemoteTransitionFinishedCallback finishCallback) {
+            public void mergeAnimation(IBinder candidateTransition, TransitionInfo candidateInfo,
+                    SurfaceControl.Transaction candidateT, IBinder currentTransition,
+                    IRemoteTransitionFinishedCallback candidateFinishCallback) {
                 try {
-                    final IRemoteTransitionFinishedCallback origFinishCB;
+                    final IRemoteTransitionFinishedCallback currentFinishCB;
                     synchronized (mFinishCallbacks) {
-                        origFinishCB = mFinishCallbacks.remove(transition);
+                        currentFinishCB = mFinishCallbacks.remove(currentTransition);
                     }
-                    info.releaseAllSurfaces();
-                    t.close();
-                    if (origFinishCB == null) {
-                        // already finished (or not started yet), so do nothing.
+                    if (currentFinishCB == null) {
+                        Slog.e(TAG, "Called mergeAnimation, but finish callback is missing");
                         return;
                     }
                     runner.onAnimationCancelled(false /* isKeyguardOccluded */);
-                    origFinishCB.onTransitionFinished(null /* wct */, null /* t */);
+                    currentFinishCB.onTransitionFinished(null /* wct */, null /* t */);
                 } catch (RemoteException e) {
                     // nothing, we'll just let it finish on its own I guess.
                 }
diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java
index 8bd9673d4e871..147961f70bffa 100644
--- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java
+++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java
@@ -951,10 +951,15 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable,
 
                 @Override
                 public void onAnimationCancelled(boolean isKeyguardOccluded) {
-                    if (mOccludeByDreamAnimator != null) {
-                        mOccludeByDreamAnimator.cancel();
-                    }
-                    setOccluded(isKeyguardOccluded /* isOccluded */, false /* animate */);
+                    mContext.getMainExecutor().execute(() -> {
+                        if (mOccludeByDreamAnimator != null) {
+                            mOccludeByDreamAnimator.cancel();
+                        }
+                    });
+                    // The value of isKeyguardOccluded here may come from mergeAnimation, which
+                    // isn't reliable. In all cases, after running or cancelling this animation,
+                    // keyguard should be occluded.
+                    setOccluded(true /* isOccluded */, false /* animate */);
                     if (DEBUG) {
                         Log.d(TAG, "Occlude by Dream animation cancelled. Occluded state is now: "
                                 + mOccluded);