Merge "Move the pendingTransaction stuff into BLASTSyncEngine" into tm-dev

data/etc/services.core.protolog.json

@@ -3439,12 +3439,6 @@
       "group": "WM_ERROR",
       "at": "com\/android\/server\/wm\/WindowManagerService.java"
     },
-    "1333520287": {
-      "message": "Creating PendingTransition: %s",
-      "level": "VERBOSE",
-      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
-      "at": "com\/android\/server\/wm\/TransitionController.java"
-    },
     "1335791109": {
       "message": "createSurface %s: mDrawState=DRAW_PENDING",
       "level": "INFO",
@@ -3739,6 +3733,12 @@
       "group": "WM_DEBUG_IME",
       "at": "com\/android\/server\/wm\/InsetsStateController.java"
     },
+    "1667162379": {
+      "message": "Creating Pending Transition: %s",
+      "level": "VERBOSE",
+      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
+      "at": "com\/android\/server\/wm\/WindowOrganizerController.java"
+    },
     "1670933628": {
       "message": " Setting allReady override",
       "level": "VERBOSE",
@@ -3793,6 +3793,12 @@
       "group": "WM_ERROR",
       "at": "com\/android\/server\/wm\/WindowManagerService.java"
     },
+    "1730300180": {
+      "message": "PendingStartTransaction found",
+      "level": "VERBOSE",
+      "group": "WM_DEBUG_SYNC_ENGINE",
+      "at": "com\/android\/server\/wm\/BLASTSyncEngine.java"
+    },
     "1739298851": {
       "message": "removeWindowToken: Attempted to remove token: %s for non-exiting displayId=%d",
       "level": "WARN",
@@ -4105,12 +4111,6 @@
       "group": "WM_DEBUG_BOOT",
       "at": "com\/android\/server\/wm\/WindowManagerService.java"
     },
-    "2034988903": {
-      "message": "PendingStartTransaction found",
-      "level": "VERBOSE",
-      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
-      "at": "com\/android\/server\/wm\/WindowOrganizerController.java"
-    },
     "2039056415": {
       "message": "Found matching affinity candidate!",
       "level": "DEBUG",

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

@@ -30,6 +30,8 @@ import android.view.SurfaceControl;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.protolog.common.ProtoLog;
 
+import java.util.ArrayList;
+
 /**
  * Utility class for collecting WindowContainers that will merge transactions.
  * For example to use to synchronously resize all the children of a window container
@@ -64,9 +66,17 @@ class BLASTSyncEngine {
         void onTransactionReady(int mSyncId, SurfaceControl.Transaction transaction);
     }
 
-    interface SyncEngineListener {
+    /**
-        /** Called when there is no more active sync set. */
+     * Represents the desire to make a {@link BLASTSyncEngine.SyncGroup} while another is active.
-        void onSyncEngineFree();
+     *
+     * @see #queueSyncSet
+     */
+    private static class PendingSyncSet {
+        /** Called immediately when the {@link BLASTSyncEngine} is free. */
+        private Runnable mStartSync;
+
+        /** Posted to the main handler after {@link #mStartSync} is called. */
+        private Runnable mApplySync;
     }
 
     /**
@@ -142,8 +152,21 @@ class BLASTSyncEngine {
             Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER);
             mActiveSyncs.remove(mSyncId);
             mWm.mH.removeCallbacks(mOnTimeout);
-            if (mSyncEngineListener != null && mActiveSyncs.size() == 0) {
+
-                mSyncEngineListener.onSyncEngineFree();
+            // Immediately start the next pending sync-transaction if there is one.
+            if (mActiveSyncs.size() == 0 && !mPendingSyncSets.isEmpty()) {
+                ProtoLog.v(WM_DEBUG_SYNC_ENGINE, "PendingStartTransaction found");
+                final PendingSyncSet pt = mPendingSyncSets.remove(0);
+                pt.mStartSync.run();
+                if (mActiveSyncs.size() == 0) {
+                    throw new IllegalStateException("Pending Sync Set didn't start a sync.");
+                }
+                // Post this so that the now-playing transition setup isn't interrupted.
+                mWm.mH.post(() -> {
+                    synchronized (mWm.mGlobalLock) {
+                        pt.mApplySync.run();
+                    }
+                });
             }
         }
 
@@ -183,17 +206,18 @@ class BLASTSyncEngine {
     private final WindowManagerService mWm;
     private int mNextSyncId = 0;
     private final SparseArray<SyncGroup> mActiveSyncs = new SparseArray<>();
-    private SyncEngineListener mSyncEngineListener;
+
+    /**
+     * A queue of pending sync-sets waiting for their turn to run.
+     *
+     * @see #queueSyncSet
+     */
+    private final ArrayList<PendingSyncSet> mPendingSyncSets = new ArrayList<>();
 
     BLASTSyncEngine(WindowManagerService wms) {
         mWm = wms;
     }
 
-    /** Sets listener listening to whether the sync engine is free. */
-    void setSyncEngineListener(SyncEngineListener listener) {
-        mSyncEngineListener = listener;
-    }
-
     /**
      * Prepares a {@link SyncGroup} that is not active yet. Caller must call {@link #startSyncSet}
      * before calling {@link #addToSyncSet(int, WindowContainer)} on any {@link WindowContainer}.
@@ -275,4 +299,31 @@ class BLASTSyncEngine {
             mActiveSyncs.valueAt(i).onSurfacePlacement();
         }
     }
+
+    /**
+     * Queues a sync operation onto this engine. It will wait until any current/prior sync-sets
+     * have finished to run. This is needed right now because currently {@link BLASTSyncEngine}
+     * only supports 1 sync at a time.
+     *
+     * Code-paths should avoid using this unless absolutely necessary. Usually, we use this for
+     * difficult edge-cases that we hope to clean-up later.
+     *
+     * @param startSync will be called immediately when the {@link BLASTSyncEngine} is free to
+     *                  "reserve" the {@link BLASTSyncEngine} by calling one of the
+     *                  {@link BLASTSyncEngine#startSyncSet} variants.
+     * @param applySync will be posted to the main handler after {@code startSync} has been
+     *                  called. This is posted so that it doesn't interrupt any clean-up for the
+     *                  prior sync-set.
+     */
+    void queueSyncSet(@NonNull Runnable startSync, @NonNull Runnable applySync) {
+        final PendingSyncSet pt = new PendingSyncSet();
+        pt.mStartSync = startSync;
+        pt.mApplySync = applySync;
+        mPendingSyncSets.add(pt);
+    }
+
+    /** @return {@code true} if there are any sync-sets waiting to start. */
+    boolean hasPendingSyncSets() {
+        return !mPendingSyncSets.isEmpty();
+    }
 }

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

@@ -99,9 +99,6 @@ class TransitionController {
     // TODO(b/188595497): remove when not needed.
     final StatusBarManagerInternal mStatusBar;
 
-    /** Pending transitions from Shell that are waiting the SyncEngine to be free. */
-    private final ArrayList<PendingStartTransition> mPendingTransitions = new ArrayList<>();
-
     TransitionController(ActivityTaskManagerService atm,
             TaskSnapshotController taskSnapshotController) {
         mAtm = atm;
@@ -146,7 +143,7 @@ class TransitionController {
     }
 
     /** Starts Collecting */
-    private void moveToCollecting(@NonNull Transition transition) {
+    void moveToCollecting(@NonNull Transition transition) {
         if (mCollectingTransition != null) {
             throw new IllegalStateException("Simultaneous transition collection not supported.");
         }
@@ -160,26 +157,6 @@ class TransitionController {
         dispatchLegacyAppTransitionPending();
     }
 
-    /** Creates a transition representation but doesn't start collecting. */
-    @NonNull
-    PendingStartTransition createPendingTransition(@WindowManager.TransitionType int type) {
-        if (mTransitionPlayer == null) {
-            throw new IllegalStateException("Shell Transitions not enabled");
-        }
-        final PendingStartTransition out = new PendingStartTransition(new Transition(type,
-                0 /* flags */, this, mAtm.mWindowManager.mSyncEngine));
-        mPendingTransitions.add(out);
-        // We want to start collecting immediately when the engine is free, otherwise it may
-        // be busy again.
-        out.setStartSync(() -> {
-            mPendingTransitions.remove(out);
-            moveToCollecting(out.mTransition);
-        });
-        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Creating PendingTransition: %s",
-                out.mTransition);
-        return out;
-    }
-
     void registerTransitionPlayer(@Nullable ITransitionPlayer player,
             @Nullable IApplicationThread appThread) {
         try {
@@ -601,24 +578,16 @@ class TransitionController {
         if (!mPlayingTransitions.isEmpty()) {
             state = LEGACY_STATE_RUNNING;
         } else if ((mCollectingTransition != null && mCollectingTransition.getLegacyIsReady())
-                || !mPendingTransitions.isEmpty()) {
+                || mAtm.mWindowManager.mSyncEngine.hasPendingSyncSets()) {
-            // The transition may not be "ready", but we have transition waiting to start, so it
+            // The transition may not be "ready", but we have a sync-transaction waiting to start.
-            // can't be IDLE for test purpose. Ideally, we should have a STATE_COLLECTING.
+            // Usually the pending transaction is for a transition, so assuming that is the case,
+            // we can't be IDLE for test purposes. Ideally, we should have a STATE_COLLECTING.
             state = LEGACY_STATE_READY;
         }
         proto.write(AppTransitionProto.APP_TRANSITION_STATE, state);
         proto.end(token);
     }
 
-    /** Represents a startTransition call made while there is other active BLAST SyncGroup. */
-    class PendingStartTransition extends WindowOrganizerController.PendingTransaction {
-        final Transition mTransition;
-
-        PendingStartTransition(Transition transition) {
-            mTransition = transition;
-        }
-    }
-
     static class TransitionMetricsReporter extends ITransitionMetricsReporter.Stub {
         private final ArrayMap<IBinder, LongConsumer> mMetricConsumers = new ArrayMap<>();
 

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

@@ -99,7 +99,7 @@ import java.util.function.IntSupplier;
  * @see android.window.WindowOrganizer
  */
 class WindowOrganizerController extends IWindowOrganizerController.Stub
-        implements BLASTSyncEngine.TransactionReadyListener, BLASTSyncEngine.SyncEngineListener {
+        implements BLASTSyncEngine.TransactionReadyListener {
 
     private static final String TAG = "WindowOrganizerController";
 
@@ -122,21 +122,6 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
     private final HashMap<Integer, IWindowContainerTransactionCallback>
             mTransactionCallbacksByPendingSyncId = new HashMap();
 
-    /**
-     * A queue of transaction waiting for their turn to sync. Currently {@link BLASTSyncEngine} only
-     * supports 1 sync at a time, so we have to queue them.
-     *
-     * WMCore has enough information to ensure that it won't end up collecting multiple transitions
-     * in parallel by itself; however, Shell can start transitions/apply sync transaction at
-     * arbitrary times via {@link WindowOrganizerController#startTransition} and
-     * {@link WindowOrganizerController#applySyncTransaction}, so we have to support those coming in
-     * at any time (even while already syncing).
-     *
-     * This is really just a back-up for unrealistic situations (eg. during tests). In practice,
-     * this shouldn't ever happen.
-     */
-    private final ArrayList<PendingTransaction> mPendingTransactions = new ArrayList<>();
-
     final TaskOrganizerController mTaskOrganizerController;
     final DisplayAreaOrganizerController mDisplayAreaOrganizerController;
     final TaskFragmentOrganizerController mTaskFragmentOrganizerController;
@@ -160,7 +145,6 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
     void setWindowManager(WindowManagerService wms) {
         mTransitionController = new TransitionController(mService, wms.mTaskSnapshotController);
         mTransitionController.registerLegacyListener(wms.mActivityManagerAppTransitionNotifier);
-        wms.mSyncEngine.setSyncEngineListener(this);
     }
 
     TransitionController getTransitionController() {
@@ -231,16 +215,12 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                 } else {
                     // Because the BLAST engine only supports one sync at a time, queue the
                     // transaction.
-                    final PendingTransaction pt = new PendingTransaction();
+                    mService.mWindowManager.mSyncEngine.queueSyncSet(
-                    // Start sync group immediately when the SyncEngine is free.
+                            () -> mService.mWindowManager.mSyncEngine.startSyncSet(syncGroup),
-                    pt.setStartSync(() ->
+                            () -> {
-                            mService.mWindowManager.mSyncEngine.startSyncSet(syncGroup));
-                    // Those will be post so that it won't interrupt ongoing transition.
-                    pt.setStartTransaction(() -> {
                                 applyTransaction(t, syncId, null /*transition*/, caller);
                                 setSyncReady(syncId);
                             });
-                    mPendingTransactions.add(pt);
                 }
                 return syncId;
             }
@@ -283,19 +263,24 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                     // transition starts collecting. This should almost never happen except during
                     // tests.
                     if (mService.mWindowManager.mSyncEngine.hasActiveSync()) {
-                        Slog.e(TAG, "startTransition() while one is already collecting.");
+                        Slog.w(TAG, "startTransition() while one is already collecting.");
-                        final TransitionController.PendingStartTransition pt =
+                        final Transition nextTransition = new Transition(type, 0 /* flags */,
-                                mTransitionController.createPendingTransition(type);
+                                mTransitionController, mService.mWindowManager.mSyncEngine);
-                        // Those will be post so that it won't interrupt ongoing transition.
+                        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS,
-                        pt.setStartTransaction(() -> {
+                                "Creating Pending Transition: %s", nextTransition);
-                            pt.mTransition.start();
+                        mService.mWindowManager.mSyncEngine.queueSyncSet(
-                            applyTransaction(wct, -1 /*syncId*/, pt.mTransition, caller);
+                                // Make sure to collect immediately to prevent another transition
+                                // from sneaking in before it. Note: moveToCollecting internally
+                                // calls startSyncSet.
+                                () -> mTransitionController.moveToCollecting(nextTransition),
+                                () -> {
+                                    nextTransition.start();
+                                    applyTransaction(wct, -1 /*syncId*/, nextTransition, caller);
                                     if (needsSetReady) {
-                                pt.mTransition.setAllReady();
+                                        nextTransition.setAllReady();
                                     }
                                 });
-                        mPendingTransactions.add(pt);
+                        return nextTransition;
-                        return pt.mTransition;
                     }
                     transition = mTransitionController.createTransition(type);
                 }
@@ -1197,23 +1182,6 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
         mTransactionCallbacksByPendingSyncId.remove(syncId);
     }
 
-    @Override
-    public void onSyncEngineFree() {
-        if (mPendingTransactions.isEmpty()) {
-            return;
-        }
-
-        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "PendingStartTransaction found");
-        final PendingTransaction pt = mPendingTransactions.remove(0);
-        pt.startSync();
-        // Post this so that the now-playing transition setup isn't interrupted.
-        mService.mH.post(() -> {
-            synchronized (mGlobalLock) {
-                pt.startTransaction();
-            }
-        });
-    }
-
     @Override
     public void registerTransitionPlayer(ITransitionPlayer player) {
         enforceTaskPermission("registerTransitionPlayer()");
@@ -1563,38 +1531,4 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                         + result + " when starting " + intent);
         }
     }
-
-    /**
-     *  Represents a sync {@link WindowContainerTransaction} call made while there is other active
-     *  {@link BLASTSyncEngine.SyncGroup}.
-     */
-    static class PendingTransaction {
-        private Runnable mStartSync;
-        private Runnable mStartTransaction;
-
-        /**
-         * The callback will be called immediately when the {@link BLASTSyncEngine} is free. One
-         * should call {@link BLASTSyncEngine#startSyncSet(BLASTSyncEngine.SyncGroup)} here to
-         * reserve the {@link BLASTSyncEngine}.
-         */
-        void setStartSync(@NonNull Runnable callback) {
-            mStartSync = callback;
-        }
-
-        /**
-         * The callback will be post to the main handler after the {@link BLASTSyncEngine} is free
-         * to apply the pending {@link WindowContainerTransaction}.
-         */
-        void setStartTransaction(@NonNull Runnable callback) {
-            mStartTransaction = callback;
-        }
-
-        private void startSync() {
-            mStartSync.run();
-        }
-
-        private void startTransaction() {
-            mStartTransaction.run();
-        }
-    }
 }