Merge "Catch exception for deferred window transaction" into udc-dev

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

@@ -58,6 +58,7 @@ import com.android.internal.protolog.common.ProtoLog;
 import com.android.server.FgThread;
 
 import java.util.ArrayList;
+import java.util.function.Consumer;
 import java.util.function.LongConsumer;
 
 /**
@@ -1314,18 +1315,18 @@ class TransitionController {
         return transit;
     }
 
-    /** Returns {@code true} if it started collecting, {@code false} if it was queued. */
+    /** Starts the sync set if there is no pending or active syncs, otherwise enqueue the sync. */
-    boolean startLegacySyncOrQueue(BLASTSyncEngine.SyncGroup syncGroup, Runnable applySync) {
+    void startLegacySyncOrQueue(BLASTSyncEngine.SyncGroup syncGroup, Consumer<Boolean> applySync) {
         if (!mQueuedTransitions.isEmpty() || mSyncEngine.hasActiveSync()) {
             // Just add to queue since we already have a queue.
-            mQueuedTransitions.add(new QueuedTransition(syncGroup, (d) -> applySync.run()));
+            mQueuedTransitions.add(new QueuedTransition(syncGroup,
+                    (deferred) -> applySync.accept(true /* deferred */)));
             ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS_MIN,
                     "Queueing legacy sync-set: %s", syncGroup.mSyncId);
-            return false;
+            return;
         }
         mSyncEngine.startSyncSet(syncGroup);
-        applySync.run();
+        applySync.accept(false /* deferred */);
-        return true;
     }
 
     interface OnStartCollect {

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

@@ -232,8 +232,8 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                 final BLASTSyncEngine.SyncGroup syncGroup = prepareSyncWithOrganizer(callback);
                 final int syncId = syncGroup.mSyncId;
                 if (mTransitionController.isShellTransitionsEnabled()) {
-                    mTransitionController.startLegacySyncOrQueue(syncGroup, () -> {
+                    mTransitionController.startLegacySyncOrQueue(syncGroup, (deferred) -> {
-                        applyTransaction(t, syncId, null /*transition*/, caller);
+                        applyTransaction(t, syncId, null /* transition */, caller, deferred);
                         setSyncReady(syncId);
                     });
                 } else {
@@ -304,7 +304,8 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                             (deferred) -> {
                                 nextTransition.start();
                                 nextTransition.mLogger.mStartWCT = wct;
-                                applyTransaction(wct, -1 /*syncId*/, nextTransition, caller);
+                                applyTransaction(wct, -1 /* syncId */, nextTransition, caller,
+                                        deferred);
                                 if (needsSetReady) {
                                     nextTransition.setAllReady();
                                 }
@@ -456,7 +457,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                     transition.abort();
                     return;
                 }
-                if (applyTransaction(wct, -1 /* syncId */, transition, caller)
+                if (applyTransaction(wct, -1 /* syncId */, transition, caller, deferred)
                         == TRANSACT_EFFECTS_NONE && transition.mParticipants.isEmpty()) {
                     transition.abort();
                     return;
@@ -476,6 +477,23 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
         return applyTransaction(t, syncId, transition, caller, null /* finishTransition */);
     }
 
+    private int applyTransaction(@NonNull WindowContainerTransaction t, int syncId,
+            @Nullable Transition transition, @NonNull CallerInfo caller, boolean deferred) {
+        if (deferred) {
+            try {
+                return applyTransaction(t, syncId, transition, caller);
+            } catch (RuntimeException e) {
+                // If the transaction is deferred, the caller could be from TransitionController
+                // #tryStartCollectFromQueue that executes on system's worker thread rather than
+                // binder thread. And the operation in the WCT may be outdated that violates the
+                // current state. So catch the exception to avoid crashing the system.
+                Slog.e(TAG, "Failed to execute deferred applyTransaction", e);
+            }
+            return TRANSACT_EFFECTS_NONE;
+        }
+        return applyTransaction(t, syncId, transition, caller);
+    }
+
     /**
      * @param syncId If non-null, this will be a sync-transaction.
      * @param transition A transition to collect changes into.
@@ -838,13 +856,21 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
         switch (type) {
             case HIERARCHY_OP_TYPE_REMOVE_TASK: {
                 final WindowContainer wc = WindowContainer.fromBinder(hop.getContainer());
-                final Task task = wc != null ? wc.asTask() : null;
+                if (wc == null || wc.asTask() == null || !wc.isAttached()) {
+                    Slog.e(TAG, "Attempt to remove invalid task: " + wc);
+                    break;
+                }
+                final Task task = wc.asTask();
                 task.remove(true, "Applying remove task Hierarchy Op");
                 break;
             }
             case HIERARCHY_OP_TYPE_SET_LAUNCH_ROOT: {
                 final WindowContainer wc = WindowContainer.fromBinder(hop.getContainer());
-                final Task task = wc != null ? wc.asTask() : null;
+                if (wc == null || !wc.isAttached()) {
+                    Slog.e(TAG, "Attempt to set launch root to a detached container: " + wc);
+                    break;
+                }
+                final Task task = wc.asTask();
                 if (task == null) {
                     throw new IllegalArgumentException("Cannot set non-task as launch root: " + wc);
                 } else if (task.getTaskDisplayArea() == null) {
@@ -858,7 +884,11 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
             }
             case HIERARCHY_OP_TYPE_SET_LAUNCH_ADJACENT_FLAG_ROOT: {
                 final WindowContainer wc = WindowContainer.fromBinder(hop.getContainer());
-                final Task task = wc != null ? wc.asTask() : null;
+                if (wc == null || !wc.isAttached()) {
+                    Slog.e(TAG, "Attempt to set launch adjacent to a detached container: " + wc);
+                    break;
+                }
+                final Task task = wc.asTask();
                 final boolean clearRoot = hop.getToTop();
                 if (task == null) {
                     throw new IllegalArgumentException("Cannot set non-task as launch root: " + wc);

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

@@ -2191,8 +2191,11 @@ public class TransitionTests extends WindowTestsBase {
 
         BLASTSyncEngine.SyncGroup legacySync = mSyncEngine.prepareSyncSet(
                 mock(BLASTSyncEngine.TransactionReadyListener.class), "test");
-        final boolean[] applyLegacy = new boolean[]{false};
+        final boolean[] applyLegacy = new boolean[2];
-        controller.startLegacySyncOrQueue(legacySync, () -> applyLegacy[0] = true);
+        controller.startLegacySyncOrQueue(legacySync, (deferred) -> {
+            applyLegacy[0] = true;
+            applyLegacy[1] = deferred;
+        });
         assertFalse(applyLegacy[0]);
         waitUntilHandlersIdle();
 
@@ -2208,6 +2211,7 @@ public class TransitionTests extends WindowTestsBase {
         assertTrue(transitA.isPlaying());
         // legacy sync should start now
         assertTrue(applyLegacy[0]);
+        assertTrue(applyLegacy[1]);
         // transitB must wait
         assertTrue(transitB.isPending());