diff --git a/data/etc/services.core.protolog.json b/data/etc/services.core.protolog.json index bde18f36684c..5bbea7769f92 100644 --- a/data/etc/services.core.protolog.json +++ b/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", diff --git a/services/core/java/com/android/server/wm/BLASTSyncEngine.java b/services/core/java/com/android/server/wm/BLASTSyncEngine.java index 5c1ddd964325..6e205be5b574 100644 --- a/services/core/java/com/android/server/wm/BLASTSyncEngine.java +++ b/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. */ - void onSyncEngineFree(); + /** + * Represents the desire to make a {@link BLASTSyncEngine.SyncGroup} while another is active. + * + * @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 mActiveSyncs = new SparseArray<>(); - private SyncEngineListener mSyncEngineListener; + + /** + * A queue of pending sync-sets waiting for their turn to run. + * + * @see #queueSyncSet + */ + private final ArrayList 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(); + } } diff --git a/services/core/java/com/android/server/wm/TransitionController.java b/services/core/java/com/android/server/wm/TransitionController.java index 18851b34ec04..23479a269de7 100644 --- a/services/core/java/com/android/server/wm/TransitionController.java +++ b/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 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()) { - // The transition may not be "ready", but we have transition waiting to start, so it - // can't be IDLE for test purpose. Ideally, we should have a STATE_COLLECTING. + || mAtm.mWindowManager.mSyncEngine.hasPendingSyncSets()) { + // The transition may not be "ready", but we have a sync-transaction waiting to start. + // 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 mMetricConsumers = new ArrayMap<>(); diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java index 3b1d2db0d1c7..b5cf708eb46d 100644 --- a/services/core/java/com/android/server/wm/WindowOrganizerController.java +++ b/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 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 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(); - // Start sync group immediately when the SyncEngine is free. - 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); + mService.mWindowManager.mSyncEngine.queueSyncSet( + () -> mService.mWindowManager.mSyncEngine.startSyncSet(syncGroup), + () -> { + applyTransaction(t, syncId, null /*transition*/, caller); + setSyncReady(syncId); + }); } 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."); - final TransitionController.PendingStartTransition pt = - mTransitionController.createPendingTransition(type); - // Those will be post so that it won't interrupt ongoing transition. - pt.setStartTransaction(() -> { - pt.mTransition.start(); - applyTransaction(wct, -1 /*syncId*/, pt.mTransition, caller); - if (needsSetReady) { - pt.mTransition.setAllReady(); - } - }); - mPendingTransactions.add(pt); - return pt.mTransition; + Slog.w(TAG, "startTransition() while one is already collecting."); + final Transition nextTransition = new Transition(type, 0 /* flags */, + mTransitionController, mService.mWindowManager.mSyncEngine); + ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, + "Creating Pending Transition: %s", nextTransition); + mService.mWindowManager.mSyncEngine.queueSyncSet( + // 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) { + nextTransition.setAllReady(); + } + }); + return nextTransition; } 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(); - } - } }