From 5924f8373d0082a62d84c927678926e29e83b3e2 Mon Sep 17 00:00:00 2001
From: Riddle Hsu <riddlehsu@google.com>
Date: Sat, 4 Mar 2023 01:58:36 +0800
Subject: [PATCH] Skip merging non-ready transition

Otherwise TransitionHandler#mergeAnimation will receive
null transition info and null transaction. And if it is
passed to remote, the remote may meet NPE on binder thread
and never report transition finish. Which will lead to
a permanent animating state and affect activity life cycle,
e.g. unable to be stopped.

Bug: 269607576
Test: ShellTransitionTests#testTransitionMergingOnFinish
Change-Id: I391ce78c6dcb7ed7bfef41d33d1e633dba3d680a
---
 .../wm/shell/transition/Transitions.java      |  6 +++
 .../transition/ShellTransitionTests.java      | 42 +++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java
index 0a67477212e9d..0a17fa4c4096e 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java
@@ -795,6 +795,12 @@ public class Transitions implements RemoteCallable<Transitions> {
                 ++mergeIdx;
                 continue;
             }
+            if (mergeCandidate.mInfo == null) {
+                ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, "Transition merge candidate"
+                        + " %s is not ready yet", mergeCandidate.mToken);
+                // The later transition should not be merged if the prior one is not ready.
+                return;
+            }
             if (mergeCandidate.mMerged) {
                 throw new IllegalStateException("Can't merge a transition after not-merging"
                         + " a preceding one.");
diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/transition/ShellTransitionTests.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/transition/ShellTransitionTests.java
index 6c9b186b7ede5..e63bbeb055758 100644
--- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/transition/ShellTransitionTests.java
+++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/transition/ShellTransitionTests.java
@@ -563,6 +563,33 @@ public class ShellTransitionTests extends ShellTestCase {
         assertEquals(0, mDefaultHandler.activeCount());
     }
 
+
+    @Test
+    public void testTransitionMergingOnFinish() {
+        final Transitions transitions = createTestTransitions();
+        transitions.replaceDefaultHandlerForTest(mDefaultHandler);
+
+        // The current transition.
+        final IBinder transitToken1 = new Binder();
+        requestStartTransition(transitions, transitToken1);
+        onTransitionReady(transitions, transitToken1);
+
+        // The next ready transition.
+        final IBinder transitToken2 = new Binder();
+        requestStartTransition(transitions, transitToken2);
+        onTransitionReady(transitions, transitToken2);
+
+        // The non-ready merge candidate.
+        final IBinder transitTokenNotReady = new Binder();
+        requestStartTransition(transitions, transitTokenNotReady);
+
+        mDefaultHandler.setSimulateMerge(true);
+        mDefaultHandler.mFinishes.get(0).onTransitionFinished(null /* wct */, null /* wctCB */);
+
+        // Make sure that the non-ready transition is not merged.
+        assertEquals(0, mDefaultHandler.mergeCount());
+    }
+
     @Test
     public void testTransitionOrderMatchesCore() {
         Transitions transitions = createTestTransitions();
@@ -1036,6 +1063,21 @@ public class ShellTransitionTests extends ShellTestCase {
         }
     }
 
+    private static void requestStartTransition(Transitions transitions, IBinder token) {
+        transitions.requestStartTransition(token,
+                new TransitionRequestInfo(TRANSIT_OPEN, null /* trigger */, null /* remote */));
+    }
+
+    private static void onTransitionReady(Transitions transitions, IBinder token) {
+        transitions.onTransitionReady(token, createTransitionInfo(),
+                mock(SurfaceControl.Transaction.class), mock(SurfaceControl.Transaction.class));
+    }
+
+    private static TransitionInfo createTransitionInfo() {
+        return new TransitionInfoBuilder(TRANSIT_OPEN)
+                .addChange(TRANSIT_OPEN).addChange(TRANSIT_CLOSE).build();
+    }
+
     private static SurfaceControl createMockSurface(boolean valid) {
         SurfaceControl sc = mock(SurfaceControl.class);
         doReturn(valid).when(sc).isValid();