Merge "Fix crash when continueTransitionReady without collecting transition" into tm-qpr-dev

data/etc/services.core.protolog.json

@@ -2041,12 +2041,6 @@
       "group": "WM_DEBUG_CONFIGURATION",
       "at": "com\/android\/server\/wm\/ActivityRecord.java"
     },
-    "-108248992": {
-      "message": "Defer transition ready for TaskFragmentTransaction=%s",
-      "level": "VERBOSE",
-      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
-      "at": "com\/android\/server\/wm\/TaskFragmentOrganizerController.java"
-    },
     "-106400104": {
       "message": "Preload recents with %s",
       "level": "DEBUG",
@@ -2095,12 +2089,6 @@
       "group": "WM_DEBUG_STATES",
       "at": "com\/android\/server\/wm\/TaskFragment.java"
     },
-    "-79016993": {
-      "message": "Continue transition ready for TaskFragmentTransaction=%s",
-      "level": "VERBOSE",
-      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
-      "at": "com\/android\/server\/wm\/TaskFragmentOrganizerController.java"
-    },
     "-70719599": {
       "message": "Unregister remote animations for organizer=%s uid=%d pid=%d",
       "level": "VERBOSE",
@@ -3085,6 +3073,12 @@
       "group": "WM_DEBUG_REMOTE_ANIMATIONS",
       "at": "com\/android\/server\/wm\/RemoteAnimationController.java"
     },
+    "851368695": {
+      "message": "Deferred transition id=%d has been continued before the TaskFragmentTransaction=%s is finished",
+      "level": "WARN",
+      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
+      "at": "com\/android\/server\/wm\/TaskFragmentOrganizerController.java"
+    },
     "872933199": {
       "message": "Changing focus from %s to %s displayId=%d Callers=%s",
       "level": "DEBUG",
@@ -3277,6 +3271,12 @@
       "group": "WM_DEBUG_CONFIGURATION",
       "at": "com\/android\/server\/wm\/ActivityRecord.java"
     },
+    "1046228706": {
+      "message": "Defer transition id=%d for TaskFragmentTransaction=%s",
+      "level": "VERBOSE",
+      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
+      "at": "com\/android\/server\/wm\/TaskFragmentOrganizerController.java"
+    },
     "1046922686": {
       "message": "requestScrollCapture: caught exception dispatching callback: %s",
       "level": "WARN",
@@ -3319,6 +3319,12 @@
       "group": "WM_DEBUG_REMOTE_ANIMATIONS",
       "at": "com\/android\/server\/wm\/WallpaperAnimationAdapter.java"
     },
+    "1075460705": {
+      "message": "Continue transition id=%d for TaskFragmentTransaction=%s",
+      "level": "VERBOSE",
+      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
+      "at": "com\/android\/server\/wm\/TaskFragmentOrganizerController.java"
+    },
     "1087494661": {
       "message": "Clear window stuck on animatingExit status: %s",
       "level": "WARN",

services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java

@@ -138,12 +138,12 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
                 new SparseArray<>();
 
         /**
-         * List of {@link TaskFragmentTransaction#getTransactionToken()} that have been sent to the
+         * Map from {@link TaskFragmentTransaction#getTransactionToken()} to the
-         * organizer. If the transaction is sent during a transition, the
+         * {@link Transition#getSyncId()} that has been deferred. {@link TransitionController} will
-         * {@link TransitionController} will wait until the transaction is finished.
+         * wait until the organizer finished handling the {@link TaskFragmentTransaction}.
          * @see #onTransactionFinished(IBinder)
          */
-        private final List<IBinder> mRunningTransactions = new ArrayList<>();
+        private final ArrayMap<IBinder, Integer> mDeferredTransitions = new ArrayMap<>();
 
         TaskFragmentOrganizerState(ITaskFragmentOrganizer organizer, int pid, int uid) {
             mOrganizer = organizer;
@@ -190,9 +190,9 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
                 taskFragment.removeImmediately();
                 mOrganizedTaskFragments.remove(taskFragment);
             }
-            for (int i = mRunningTransactions.size() - 1; i >= 0; i--) {
+            for (int i = mDeferredTransitions.size() - 1; i >= 0; i--) {
                 // Cleanup any running transaction to unblock the current transition.
-                onTransactionFinished(mRunningTransactions.get(i));
+                onTransactionFinished(mDeferredTransitions.keyAt(i));
             }
             mOrganizer.asBinder().unlinkToDeath(this, 0 /*flags*/);
         }
@@ -357,19 +357,34 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
             if (!mWindowOrganizerController.getTransitionController().isCollecting()) {
                 return;
             }
+            final int transitionId = mWindowOrganizerController.getTransitionController()
+                    .getCollectingTransitionId();
             ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS,
-                    "Defer transition ready for TaskFragmentTransaction=%s", transactionToken);
+                    "Defer transition id=%d for TaskFragmentTransaction=%s", transitionId,
-            mRunningTransactions.add(transactionToken);
+                    transactionToken);
+            mDeferredTransitions.put(transactionToken, transitionId);
             mWindowOrganizerController.getTransitionController().deferTransitionReady();
         }
 
         /** Called when the transaction is finished. */
         void onTransactionFinished(@NonNull IBinder transactionToken) {
-            if (!mRunningTransactions.remove(transactionToken)) {
+            if (!mDeferredTransitions.containsKey(transactionToken)) {
+                return;
+            }
+            final int transitionId = mDeferredTransitions.remove(transactionToken);
+            if (!mWindowOrganizerController.getTransitionController().isCollecting()
+                    || mWindowOrganizerController.getTransitionController()
+                    .getCollectingTransitionId() != transitionId) {
+                // This can happen when the transition is timeout or abort.
+                ProtoLog.w(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS,
+                        "Deferred transition id=%d has been continued before the"
+                                + " TaskFragmentTransaction=%s is finished",
+                        transitionId, transactionToken);
                 return;
             }
             ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS,
-                    "Continue transition ready for TaskFragmentTransaction=%s", transactionToken);
+                    "Continue transition id=%d for TaskFragmentTransaction=%s", transitionId,
+                    transactionToken);
             mWindowOrganizerController.getTransitionController().continueTransitionReady();
         }
     }

services/core/java/com/android/server/wm/Transition.java

@@ -1894,6 +1894,8 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
      */
     void deferTransitionReady() {
         ++mReadyTracker.mDeferReadyDepth;
+        // Make sure it wait until #continueTransitionReady() is called.
+        mSyncEngine.setReady(mSyncId, false);
     }
 
     /** This undoes one call to {@link #deferTransitionReady}. */

services/core/java/com/android/server/wm/TransitionController.java

@@ -226,6 +226,17 @@ class TransitionController {
         return mCollectingTransition != null;
     }
 
+    /**
+     * @return the collecting transition sync Id. This should only be called when there is a
+     * collecting transition.
+     */
+    int getCollectingTransitionId() {
+        if (mCollectingTransition == null) {
+            throw new IllegalStateException("There is no collecting transition");
+        }
+        return mCollectingTransition.getSyncId();
+    }
+
     /**
      * @return {@code true} if transition is actively collecting changes and `wc` is one of them.
      *                      This is {@code false} once a transition is playing.

services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java

@@ -1119,6 +1119,7 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
         final ArgumentCaptor<WindowContainerTransaction> wctCaptor =
                 ArgumentCaptor.forClass(WindowContainerTransaction.class);
         doReturn(true).when(mTransitionController).isCollecting();
+        doReturn(10).when(mTransitionController).getCollectingTransitionId();
 
         mController.onTaskFragmentAppeared(mTaskFragment.getTaskFragmentOrganizer(), mTaskFragment);
         mController.dispatchPendingEvents();

services/tests/wmtests/src/com/android/server/wm/TransitionTests.java

@@ -95,6 +95,7 @@ import java.util.function.Function;
 @RunWith(WindowTestRunner.class)
 public class TransitionTests extends WindowTestsBase {
     final SurfaceControl.Transaction mMockT = mock(SurfaceControl.Transaction.class);
+    private BLASTSyncEngine mSyncEngine;
 
     private Transition createTestTransition(int transitType) {
         TransitionTracer tracer = mock(TransitionTracer.class);
@@ -102,8 +103,8 @@ public class TransitionTests extends WindowTestsBase {
                 mock(ActivityTaskManagerService.class), mock(TaskSnapshotController.class),
                 mock(TransitionTracer.class));
 
-        final BLASTSyncEngine sync = createTestBLASTSyncEngine();
+        mSyncEngine = createTestBLASTSyncEngine();
-        final Transition t = new Transition(transitType, 0 /* flags */, controller, sync);
+        final Transition t = new Transition(transitType, 0 /* flags */, controller, mSyncEngine);
         t.startCollecting(0 /* timeoutMs */);
         return t;
     }
@@ -1146,6 +1147,26 @@ public class TransitionTests extends WindowTestsBase {
         transition.abort();
     }
 
+    @Test
+    public void testDeferTransitionReady_deferStartedTransition() {
+        final Transition transition = createTestTransition(TRANSIT_OPEN);
+        transition.setAllReady();
+        transition.start();
+
+        assertTrue(mSyncEngine.isReady(transition.getSyncId()));
+
+        transition.deferTransitionReady();
+
+        // Both transition ready tracker and sync engine should be deferred.
+        assertFalse(transition.allReady());
+        assertFalse(mSyncEngine.isReady(transition.getSyncId()));
+
+        transition.continueTransitionReady();
+
+        assertTrue(transition.allReady());
+        assertTrue(mSyncEngine.isReady(transition.getSyncId()));
+    }
+
     private static void makeTaskOrganized(Task... tasks) {
         final ITaskOrganizer organizer = mock(ITaskOrganizer.class);
         for (Task t : tasks) {