Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1b97f826 authored by Chris Li's avatar Chris Li
Browse files

Queue applySyncTransaction calls when there is active BLAST sync

We currently queue applySyncTransaction in WM Shell, but it doesn't sync
with Shell transition collection. However, when both happen, it may
cause issue because the BLASTSyncEngine currently only support 1 sync at
a time.

Move the Shell transition queue to WindowOrganizerController as we only
queue organizer sync. Unfortunately, unlike organizer sync that is in
one WCT, we can't prevent any core-initiated transition.

We don't expect it to happen, but if it does, we need to handle them
case by case.

Fix: 213866869
Test: atest WMShellFlickerTests:ExpandBubbleScreen
Change-Id: Ida331585b018e848e2f801da55cb6a4b999950f4
parent 72a8241f
Loading
Loading
Loading
Loading
+12 −12
Original line number Diff line number Diff line
@@ -13,12 +13,6 @@
      "group": "WM_DEBUG_STARTING_WINDOW",
      "at": "com\/android\/server\/wm\/ActivityRecord.java"
    },
    "-2125618712": {
      "message": "PendingStartTransition found",
      "level": "VERBOSE",
      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
      "at": "com\/android\/server\/wm\/TransitionController.java"
    },
    "-2121056984": {
      "message": "%s",
      "level": "WARN",
@@ -2785,6 +2779,12 @@
      "group": "WM_DEBUG_APP_TRANSITIONS",
      "at": "com\/android\/server\/wm\/AppTransitionController.java"
    },
    "800698875": {
      "message": "SyncGroup %d: Started when there is other active SyncGroup",
      "level": "WARN",
      "group": "WM_DEBUG_SYNC_ENGINE",
      "at": "com\/android\/server\/wm\/BLASTSyncEngine.java"
    },
    "806891543": {
      "message": "Setting mOrientationChangeComplete=true because wtoken %s numInteresting=%d numDrawn=%d",
      "level": "INFO",
@@ -3685,12 +3685,6 @@
      "group": "WM_DEBUG_WINDOW_ORGANIZER",
      "at": "com\/android\/server\/wm\/DisplayAreaPolicyBuilder.java"
    },
    "1884961873": {
      "message": "Sleep still need to stop %d activities",
      "level": "VERBOSE",
      "group": "WM_DEBUG_STATES",
      "at": "com\/android\/server\/wm\/Task.java"
    },
    "1891501279": {
      "message": "cancelAnimation(): reason=%s",
      "level": "DEBUG",
@@ -3823,6 +3817,12 @@
      "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",
+57 −9
Original line number Diff line number Diff line
@@ -64,6 +64,11 @@ class BLASTSyncEngine {
        void onTransactionReady(int mSyncId, SurfaceControl.Transaction transaction);
    }

    interface SyncEngineListener {
        /** Called when there is no more active sync set. */
        void onSyncEngineFree();
    }

    /**
     * Holds state associated with a single synchronous set of operations.
     */
@@ -137,6 +142,9 @@ class BLASTSyncEngine {
            Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER);
            mActiveSyncs.remove(mSyncId);
            mWm.mH.removeCallbacks(mOnTimeout);
            if (mSyncEngineListener != null && mActiveSyncs.size() == 0) {
                mSyncEngineListener.onSyncEngineFree();
            }
        }

        private void setReady(boolean ready) {
@@ -175,22 +183,54 @@ class BLASTSyncEngine {
    private final WindowManagerService mWm;
    private int mNextSyncId = 0;
    private final SparseArray<SyncGroup> mActiveSyncs = new SparseArray<>();
    private SyncEngineListener mSyncEngineListener;

    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}.
     */
    SyncGroup prepareSyncSet(TransactionReadyListener listener, String name) {
        return new SyncGroup(listener, mNextSyncId++, name);
    }

    int startSyncSet(TransactionReadyListener listener) {
        return startSyncSet(listener, WindowState.BLAST_TIMEOUT_DURATION, "");
    }

    int startSyncSet(TransactionReadyListener listener, long timeoutMs, String name) {
        final int id = mNextSyncId++;
        final SyncGroup s = new SyncGroup(listener, id, name);
        mActiveSyncs.put(id, s);
        ProtoLog.v(WM_DEBUG_SYNC_ENGINE, "SyncGroup %d: Started for listener: %s", id, listener);
        final SyncGroup s = prepareSyncSet(listener, name);
        startSyncSet(s, timeoutMs);
        return s.mSyncId;
    }

    void startSyncSet(SyncGroup s) {
        startSyncSet(s, WindowState.BLAST_TIMEOUT_DURATION);
    }

    void startSyncSet(SyncGroup s, long timeoutMs) {
        if (mActiveSyncs.size() != 0) {
            // We currently only support one sync at a time, so start a new SyncGroup when there is
            // another may cause issue.
            ProtoLog.w(WM_DEBUG_SYNC_ENGINE,
                    "SyncGroup %d: Started when there is other active SyncGroup", s.mSyncId);
        }
        mActiveSyncs.put(s.mSyncId, s);
        ProtoLog.v(WM_DEBUG_SYNC_ENGINE, "SyncGroup %d: Started for listener: %s",
                s.mSyncId, s.mListener);
        scheduleTimeout(s, timeoutMs);
        return id;
    }

    boolean hasActiveSync() {
        return mActiveSyncs.size() != 0;
    }

    @VisibleForTesting
@@ -199,11 +239,11 @@ class BLASTSyncEngine {
    }

    void addToSyncSet(int id, WindowContainer wc) {
        mActiveSyncs.get(id).addToSync(wc);
        getSyncGroup(id).addToSync(wc);
    }

    void setReady(int id, boolean ready) {
        mActiveSyncs.get(id).setReady(ready);
        getSyncGroup(id).setReady(ready);
    }

    void setReady(int id) {
@@ -211,14 +251,22 @@ class BLASTSyncEngine {
    }

    boolean isReady(int id) {
        return mActiveSyncs.get(id).mReady;
        return getSyncGroup(id).mReady;
    }

    /**
     * Aborts the sync (ie. it doesn't wait for ready or anything to finish)
     */
    void abort(int id) {
        mActiveSyncs.get(id).finishNow();
        getSyncGroup(id).finishNow();
    }

    private SyncGroup getSyncGroup(int id) {
        final SyncGroup syncGroup = mActiveSyncs.get(id);
        if (syncGroup == null) {
            throw new IllegalStateException("SyncGroup is not started yet id=" + id);
        }
        return syncGroup;
    }

    void onSurfacePlacement() {
+7 −38
Original line number Diff line number Diff line
@@ -30,7 +30,6 @@ import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.ActivityManager;
import android.app.IApplicationThread;
import android.os.Handler;
import android.os.IBinder;
import android.os.IRemoteCallback;
import android.os.RemoteException;
@@ -92,20 +91,6 @@ class TransitionController {
    /** The transition currently being constructed (collecting participants). */
    private Transition mCollectingTransition = null;

    /**
     * A queue of transitions waiting for their turn to collect. We can only collect 1 transition
     * at a time because 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 at arbitrary times via
     * {@link WindowOrganizerController#startTransition}, so we have to support those coming in
     * at any time (even while already collecting).
     *
     * This is really just a back-up for unrealistic situations (eg. during tests). In practice,
     * this shouldn't ever happen.
     */
    private final ArrayList<PendingStartTransition> mPendingStartTransitions = new ArrayList<>();

    // TODO(b/188595497): remove when not needed.
    final StatusBarManagerInternal mStatusBar;

@@ -175,9 +160,13 @@ class TransitionController {
        }
        final PendingStartTransition out = new PendingStartTransition(new Transition(type,
                0 /* flags */, this, mAtm.mWindowManager.mSyncEngine));
        // We want to start collecting immediately when the engine is free, otherwise it may
        // be busy again.
        out.setStartSync(() -> {
            moveToCollecting(out.mTransition);
        });
        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Creating PendingTransition: %s",
                out.mTransition);
        mPendingStartTransitions.add(out);
        return out;
    }

@@ -451,15 +440,6 @@ class TransitionController {
            throw new IllegalStateException("Trying to move non-collecting transition to playing");
        }
        mCollectingTransition = null;
        if (!mPendingStartTransitions.isEmpty()) {
            ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "PendingStartTransition found");
            final PendingStartTransition p = mPendingStartTransitions.remove(0);
            moveToCollecting(p.mTransition);
            if (p.mOnStartCollecting != null) {
                // Post this so that the now-playing transition setup isn't interrupted.
                p.mHandler.post(p.mOnStartCollecting);
            }
        }
        if (mPlayingTransitions.isEmpty()) {
            setAnimationRunning(true /* running */);
        }
@@ -557,24 +537,13 @@ class TransitionController {
        proto.end(token);
    }

    /** Represents a startTransition call made while a transition was already collecting. */
    static class PendingStartTransition {
    /** Represents a startTransition call made while there is other active BLAST SyncGroup. */
    class PendingStartTransition extends WindowOrganizerController.PendingTransaction {
        final Transition mTransition;

        /** Handler to post the collecting callback onto. */
        private Handler mHandler;

        /** Runnable to post to `mHandler` once this pending transition starts collecting. */
        private Runnable mOnStartCollecting;

        PendingStartTransition(Transition transition) {
            mTransition = transition;
        }

        void setOnStartCollecting(@NonNull Handler handler, @NonNull Runnable callback) {
            mHandler = handler;
            mOnStartCollecting = callback;
        }
    }

    static class TransitionMetricsReporter extends ITransitionMetricsReporter.Stub {
+5 −2
Original line number Diff line number Diff line
@@ -85,8 +85,8 @@ import android.view.MagnificationSpec;
import android.view.RemoteAnimationDefinition;
import android.view.RemoteAnimationTarget;
import android.view.SurfaceControl;
import android.view.SurfaceControlViewHost;
import android.view.SurfaceControl.Builder;
import android.view.SurfaceControlViewHost;
import android.view.SurfaceSession;
import android.view.TaskTransitionSpec;
import android.view.WindowManager;
@@ -3331,7 +3331,10 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
        ProtoLog.v(WM_DEBUG_SYNC_ENGINE, "setSyncGroup #%d on %s", group.mSyncId, this);
        if (group != null) {
            if (mSyncGroup != null && mSyncGroup != group) {
                throw new IllegalStateException("Can't sync on 2 engines simultaneously");
                // This can still happen if WMCore starts a new transition when there is ongoing
                // sync transaction from Shell. Please file a bug if it happens.
                throw new IllegalStateException("Can't sync on 2 engines simultaneously"
                        + " currentSyncId=" + mSyncGroup.mSyncId + " newSyncId=" + group.mSyncId);
            }
        }
        mSyncGroup = group;
+108 −12
Original line number Diff line number Diff line
@@ -76,6 +76,7 @@ import android.window.TaskFragmentCreationParams;
import android.window.WindowContainerTransaction;

import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.protolog.ProtoLogGroup;
import com.android.internal.protolog.common.ProtoLog;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.function.pooled.PooledConsumer;
@@ -94,7 +95,7 @@ import java.util.Map;
 * @see android.window.WindowOrganizer
 */
class WindowOrganizerController extends IWindowOrganizerController.Stub
    implements BLASTSyncEngine.TransactionReadyListener {
        implements BLASTSyncEngine.TransactionReadyListener, BLASTSyncEngine.SyncEngineListener {

    private static final String TAG = "WindowOrganizerController";

@@ -117,6 +118,21 @@ 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;
@@ -140,6 +156,7 @@ 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() {
@@ -184,6 +201,11 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
        final long ident = Binder.clearCallingIdentity();
        try {
            synchronized (mGlobalLock) {
                if (callback == null) {
                    applyTransaction(t, -1 /* syncId*/, null /*transition*/, caller);
                    return -1;
                }

                /**
                 * If callback is non-null we are looking to synchronize this transaction by
                 * collecting all the results in to a SurfaceFlinger transaction and then delivering
@@ -196,13 +218,25 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                 * all the WindowContainers will eventually finish applying their changes and notify
                 * the BLASTSyncEngine which will deliver the Transaction to the callback.
                 */
                int syncId = -1;
                if (callback != null) {
                    syncId = startSyncWithOrganizer(callback);
                }
                final BLASTSyncEngine.SyncGroup syncGroup = prepareSyncWithOrganizer(callback);
                final int syncId = syncGroup.mSyncId;
                if (!mService.mWindowManager.mSyncEngine.hasActiveSync()) {
                    mService.mWindowManager.mSyncEngine.startSyncSet(syncGroup);
                    applyTransaction(t, syncId, null /*transition*/, caller);
                    setSyncReady(syncId);
                } 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);
                if (syncId >= 0) {
                        setSyncReady(syncId);
                    });
                    mPendingTransactions.add(pt);
                }
                return syncId;
            }
@@ -244,17 +278,19 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                    // return that. The actual start and apply will then be deferred until that
                    // transition starts collecting. This should almost never happen except during
                    // tests.
                    if (mTransitionController.isCollecting()) {
                    if (mService.mWindowManager.mSyncEngine.hasActiveSync()) {
                        Slog.e(TAG, "startTransition() while one is already collecting.");
                        final TransitionController.PendingStartTransition pt =
                                mTransitionController.createPendingTransition(type);
                        pt.setOnStartCollecting(mService.mH, () -> {
                        // 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;
                    }
                    transition = mTransitionController.createTransition(type);
@@ -1052,11 +1088,24 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
        return mTaskFragmentOrganizerController;
    }

    /**
     * This will prepare a {@link BLASTSyncEngine.SyncGroup} for the organizer to track, but the
     * {@link BLASTSyncEngine.SyncGroup} may not be active until the {@link BLASTSyncEngine} is
     * free.
     */
    private BLASTSyncEngine.SyncGroup prepareSyncWithOrganizer(
            IWindowContainerTransactionCallback callback) {
        final BLASTSyncEngine.SyncGroup s = mService.mWindowManager.mSyncEngine
                .prepareSyncSet(this, "");
        mTransactionCallbacksByPendingSyncId.put(s.mSyncId, callback);
        return s;
    }

    @VisibleForTesting
    int startSyncWithOrganizer(IWindowContainerTransactionCallback callback) {
        int id = mService.mWindowManager.mSyncEngine.startSyncSet(this);
        mTransactionCallbacksByPendingSyncId.put(id, callback);
        return id;
        final BLASTSyncEngine.SyncGroup s = prepareSyncWithOrganizer(callback);
        mService.mWindowManager.mSyncEngine.startSyncSet(s);
        return s.mSyncId;
    }

    @VisibleForTesting
@@ -1087,6 +1136,19 @@ 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(pt::startTransaction);
    }

    @Override
    public void registerTransitionPlayer(ITransitionPlayer player) {
        enforceTaskPermission("registerTransitionPlayer()");
@@ -1345,4 +1407,38 @@ 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();
        }
    }
}