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

Commit 72a8241f authored by Evan Rosky's avatar Evan Rosky Committed by Chris Li
Browse files

Queue startTransition calls made while already collecting

Shell Transitions only supports 1 transition collecting at a time.
This limitation comes from BLASTSyncEngine. However, because Shell
can call startTransition at any time (and doesn't have a way to
know if WM is already "collecting" for a transition), we can't
just crash in this case. We also can't just use the existing
transition because it has a different "client" expecting results.

So, the best we can do is queue them up. Fortunately, the
startTransition API Shell uses is designed such that all the
relevant operations are in one WCT, so we can maintain server-side
correctness.

This CL adds a queueing mechanism for these situations. In practice,
this should actually never be needed; however, it is *technically*
possible and automated tests are more likely to hit it.

Bug: 183993924
Test: pass existing
Change-Id: I255fb336cb30c7d1e1b82188a31f9c693b9e138c
parent bce8d161
Loading
Loading
Loading
Loading
+18 −30
Original line number Diff line number Diff line
@@ -13,6 +13,12 @@
      "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",
@@ -103,18 +109,6 @@
      "group": "WM_DEBUG_STATES",
      "at": "com\/android\/server\/wm\/TaskFragment.java"
    },
    "-2002500255": {
      "message": "Defer removing snapshot surface in %dms",
      "level": "VERBOSE",
      "group": "WM_DEBUG_STARTING_WINDOW",
      "at": "com\/android\/server\/wm\/TaskSnapshotSurface.java"
    },
    "-1991255017": {
      "message": "Drawing snapshot surface sizeMismatch=%b",
      "level": "VERBOSE",
      "group": "WM_DEBUG_STARTING_WINDOW",
      "at": "com\/android\/server\/wm\/TaskSnapshotSurface.java"
    },
    "-1980468143": {
      "message": "DisplayArea appeared name=%s",
      "level": "VERBOSE",
@@ -331,6 +325,12 @@
      "group": "WM_DEBUG_RECENTS_ANIMATIONS",
      "at": "com\/android\/server\/wm\/RecentsAnimationController.java"
    },
    "-1764792832": {
      "message": "Start collecting in Transition: %s",
      "level": "VERBOSE",
      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
      "at": "com\/android\/server\/wm\/TransitionController.java"
    },
    "-1750384749": {
      "message": "Launch on display check: allow launch on public display",
      "level": "DEBUG",
@@ -1597,12 +1597,6 @@
      "group": "WM_DEBUG_FOCUS_LIGHT",
      "at": "com\/android\/server\/wm\/DisplayContent.java"
    },
    "-405536909": {
      "message": "Removing snapshot surface",
      "level": "VERBOSE",
      "group": "WM_DEBUG_STARTING_WINDOW",
      "at": "com\/android\/server\/wm\/TaskSnapshotSurface.java"
    },
    "-401282500": {
      "message": "destroyIfPossible: r=%s destroy returned removed=%s",
      "level": "DEBUG",
@@ -2005,12 +1999,6 @@
      "group": "WM_ERROR",
      "at": "com\/android\/server\/wm\/WindowManagerService.java"
    },
    "44438983": {
      "message": "performLayout: Activity exiting now removed %s",
      "level": "VERBOSE",
      "group": "WM_DEBUG_ADD_REMOVE",
      "at": "com\/android\/server\/wm\/DisplayContent.java"
    },
    "45285419": {
      "message": "startingWindow was set but startingSurface==null, couldn't remove",
      "level": "VERBOSE",
@@ -3211,6 +3199,12 @@
      "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"
    },
    "1337596507": {
      "message": "Sending to proc %s new compat %s",
      "level": "VERBOSE",
@@ -3271,12 +3265,6 @@
      "group": "WM_DEBUG_LAYER_MIRRORING",
      "at": "com\/android\/server\/wm\/DisplayContent.java"
    },
    "1417601133": {
      "message": "Enqueueing ADD_STARTING",
      "level": "VERBOSE",
      "group": "WM_DEBUG_STARTING_WINDOW",
      "at": "com\/android\/server\/wm\/ActivityRecord.java"
    },
    "1422781269": {
      "message": "Resuming rotation after re-position",
      "level": "DEBUG",
+22 −5
Original line number Diff line number Diff line
@@ -101,6 +101,9 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
    /** The default package for resources */
    private static final String DEFAULT_PACKAGE = "android";

    /** The transition has been created but isn't collecting yet. */
    private static final int STATE_PENDING = -1;

    /** The transition has been created and is collecting, but hasn't formally started. */
    private static final int STATE_COLLECTING = 0;

@@ -122,6 +125,7 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
    private static final int STATE_ABORT = 3;

    @IntDef(prefix = { "STATE_" }, value = {
            STATE_PENDING,
            STATE_COLLECTING,
            STATE_STARTED,
            STATE_PLAYING,
@@ -131,7 +135,7 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
    @interface TransitionState {}

    final @TransitionType int mType;
    private int mSyncId;
    private int mSyncId = -1;
    private @TransitionFlags int mFlags;
    private final TransitionController mController;
    private final BLASTSyncEngine mSyncEngine;
@@ -171,7 +175,7 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
    private IRemoteCallback mClientAnimationStartCallback = null;
    private IRemoteCallback mClientAnimationFinishCallback = null;

    private @TransitionState int mState = STATE_COLLECTING;
    private @TransitionState int mState = STATE_PENDING;
    private final ReadyTracker mReadyTracker = new ReadyTracker();

    // TODO(b/188595497): remove when not needed.
@@ -179,13 +183,12 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
    private boolean mNavBarAttachedToApp = false;
    private int mRecentsDisplayId = INVALID_DISPLAY;

    Transition(@TransitionType int type, @TransitionFlags int flags, long timeoutMs,
    Transition(@TransitionType int type, @TransitionFlags int flags,
            TransitionController controller, BLASTSyncEngine syncEngine) {
        mType = type;
        mFlags = flags;
        mController = controller;
        mSyncEngine = syncEngine;
        mSyncId = mSyncEngine.startSyncSet(this, timeoutMs, TAG);
    }

    void addFlag(int flag) {
@@ -216,13 +219,24 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
        return mFlags;
    }

    /** Starts collecting phase. Once this starts, all relevant surface operations are sync. */
    void startCollecting(long timeoutMs) {
        if (mState != STATE_PENDING) {
            throw new IllegalStateException("Attempting to re-use a transition");
        }
        mState = STATE_COLLECTING;
        mSyncId = mSyncEngine.startSyncSet(this, timeoutMs, TAG);
    }

    /**
     * Formally starts the transition. Participants can be collected before this is started,
     * but this won't consider itself ready until started -- even if all the participants have
     * drawn.
     */
    void start() {
        if (mState >= STATE_STARTED) {
        if (mState < STATE_COLLECTING) {
            throw new IllegalStateException("Can't start Transition which isn't collecting.");
        } else if (mState >= STATE_STARTED) {
            Slog.w(TAG, "Transition already started: " + mSyncId);
        }
        mState = STATE_STARTED;
@@ -235,6 +249,9 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
     * Adds wc to set of WindowContainers participating in this transition.
     */
    void collect(@NonNull WindowContainer wc) {
        if (mState < STATE_COLLECTING) {
            throw new IllegalStateException("Transition hasn't started collecting.");
        }
        if (mSyncId < 0) return;
        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Collecting in transition %d: %s",
                mSyncId, wc);
+76 −6
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ 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;
@@ -91,6 +92,20 @@ 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;

@@ -128,16 +143,42 @@ class TransitionController {
            throw new IllegalStateException("Shell Transitions not enabled");
        }
        if (mCollectingTransition != null) {
            throw new IllegalStateException("Simultaneous transitions not supported yet.");
            throw new IllegalStateException("Simultaneous transition collection not supported"
                    + " yet. Use {@link #createPendingTransition} for explicit queueing.");
        }
        Transition transit = new Transition(type, flags, this, mAtm.mWindowManager.mSyncEngine);
        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Creating Transition: %s", transit);
        moveToCollecting(transit);
        return transit;
    }

    /** Starts Collecting */
    private void moveToCollecting(@NonNull Transition transition) {
        if (mCollectingTransition != null) {
            throw new IllegalStateException("Simultaneous transition collection not supported.");
        }
        mCollectingTransition = transition;
        // Distinguish change type because the response time is usually expected to be not too long.
        final long timeoutMs = type == TRANSIT_CHANGE ? CHANGE_TIMEOUT_MS : DEFAULT_TIMEOUT_MS;
        mCollectingTransition = new Transition(type, flags, timeoutMs, this,
                mAtm.mWindowManager.mSyncEngine);
        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Creating Transition: %s",
        final long timeoutMs =
                transition.mType == TRANSIT_CHANGE ? CHANGE_TIMEOUT_MS : DEFAULT_TIMEOUT_MS;
        mCollectingTransition.startCollecting(timeoutMs);
        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Start collecting in Transition: %s",
                mCollectingTransition);
        dispatchLegacyAppTransitionPending();
        return mCollectingTransition;
    }

    /** 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));
        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Creating PendingTransition: %s",
                out.mTransition);
        mPendingStartTransitions.add(out);
        return out;
    }

    void registerTransitionPlayer(@Nullable ITransitionPlayer player,
@@ -410,6 +451,15 @@ 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 */);
        }
@@ -507,6 +557,26 @@ class TransitionController {
        proto.end(token);
    }

    /** Represents a startTransition call made while a transition was already collecting. */
    static class PendingStartTransition {
        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 {
        private final ArrayMap<IBinder, LongConsumer> mMetricConsumers = new ArrayMap<>();

+29 −13
Original line number Diff line number Diff line
@@ -220,31 +220,47 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
        try {
            synchronized (mGlobalLock) {
                Transition transition = Transition.fromBinder(transitionToken);
                if (mTransitionController.getTransitionPlayer() == null && transition == null) {
                    Slog.w(TAG, "Using shell transitions API for legacy transitions.");
                    if (t == null) {
                        throw new IllegalArgumentException("Can't use legacy transitions in"
                                + " compatibility mode with no WCT.");
                    }
                    applyTransaction(t, -1 /* syncId */, null, caller);
                    return null;
                }
                // In cases where transition is already provided, the "readiness lifecycle" of the
                // transition is determined outside of this transaction. However, if this is a
                // direct call from shell, the entire transition lifecycle is contained in the
                // provided transaction and thus we can setReady immediately after apply.
                boolean needsSetReady = transition == null && t != null;
                final boolean needsSetReady = transition == null && t != null;
                final WindowContainerTransaction wct =
                        t != null ? t : new WindowContainerTransaction();
                if (transition == null) {
                    if (type < 0) {
                        throw new IllegalArgumentException("Can't create transition with no type");
                    }
                    if (mTransitionController.getTransitionPlayer() == null) {
                        Slog.w(TAG, "Using shell transitions API for legacy transitions.");
                        if (t == null) {
                            throw new IllegalArgumentException("Can't use legacy transitions in"
                                    + " compatibility mode with no WCT.");
                    // If there is already a collecting transition, queue up a new transition and
                    // 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()) {
                        Slog.e(TAG, "startTransition() while one is already collecting.");
                        final TransitionController.PendingStartTransition pt =
                                mTransitionController.createPendingTransition(type);
                        pt.setOnStartCollecting(mService.mH, () -> {
                            pt.mTransition.start();
                            applyTransaction(wct, -1 /*syncId*/, pt.mTransition, caller);
                            if (needsSetReady) {
                                pt.mTransition.setAllReady();
                            }
                        applyTransaction(t, -1 /* syncId */, null, caller);
                        return null;
                        });
                        return pt.mTransition;
                    }
                    transition = mTransitionController.createTransition(type);
                }
                transition.start();
                if (t == null) {
                    t = new WindowContainerTransaction();
                }
                applyTransaction(t, -1 /*syncId*/, transition, caller);
                applyTransaction(wct, -1 /*syncId*/, transition, caller);
                if (needsSetReady) {
                    transition.setAllReady();
                }
+5 −2
Original line number Diff line number Diff line
@@ -75,7 +75,9 @@ public class TransitionTests extends WindowTestsBase {
    private Transition createTestTransition(int transitType) {
        TransitionController controller = mock(TransitionController.class);
        final BLASTSyncEngine sync = createTestBLASTSyncEngine();
        return new Transition(transitType, 0 /* flags */, 0 /* timeoutMs */, controller, sync);
        final Transition t = new Transition(transitType, 0 /* flags */, controller, sync);
        t.startCollecting(0 /* timeoutMs */);
        return t;
    }

    @Test
@@ -466,12 +468,13 @@ public class TransitionTests extends WindowTestsBase {
        final BLASTSyncEngine sync = new BLASTSyncEngine(mWm);
        final CountDownLatch latch = new CountDownLatch(1);
        // When the timeout is reached, it will finish the sync-group and notify transaction ready.
        new Transition(TRANSIT_OPEN, 0 /* flags */, 10 /* timeoutMs */, controller, sync) {
        final Transition t = new Transition(TRANSIT_OPEN, 0 /* flags */, controller, sync) {
            @Override
            public void onTransactionReady(int syncId, SurfaceControl.Transaction transaction) {
                latch.countDown();
            }
        };
        t.startCollecting(10 /* timeoutMs */);
        assertTrue(awaitInWmLock(() -> latch.await(3, TimeUnit.SECONDS)));
    }