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

Commit 4febc8a8 authored by Evan Rosky's avatar Evan Rosky
Browse files

Make sure PiP transition triggered by WM is setReady 

This was kinda working by luck previously. However, WM was
making a PiP transition in a way that it would never be ready
without other systems "accidentally" making it ready. This
results in time-outs. While this didn't pose any immediate
issues, its a potential source of some of the CTS timeout bugs.

Resolving this required adding a "push/pop NotReady" counter
to Transition so that we can have "code regions" which make
WM changes interleaved with performSurfacePlacement in one
transition in situations where we might need to nest into
an already-collecting transition.

Bug: 223321458
Test: atest RootWindowContainerTests TransitionTests
Change-Id: I5bc9c63b7f67b050e7164a6581b91eb2c0b00704
parent 7b8af988

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

+6 −0
Original line number Diff line number Diff line
@@ -193,12 +193,18 @@ class BLASTSyncEngine { 


        private void onTimeout() {

            if (!mActiveSyncs.contains(mSyncId)) return;

            boolean allFinished = true;

            for (int i = mRootMembers.size() - 1; i >= 0; --i) {

                final WindowContainer<?> wc = mRootMembers.valueAt(i);

                if (!wc.isSyncFinished()) {

                    allFinished = false;

                    Slog.i(TAG, "Unfinished container: " + wc);

                }

            }

            if (allFinished && !mReady) {

                Slog.w(TAG, "Sync group " + mSyncId + " timed-out because not ready. If you see "

                        + "this, please file a bug.");

            }

            finishNow();

        }

    }

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

+31 −22
Original line number Diff line number Diff line
@@ -1976,23 +1976,23 @@ class RootWindowContainer extends WindowContainer<DisplayContent> 


    void moveActivityToPinnedRootTask(@NonNull ActivityRecord r,

            @Nullable ActivityRecord launchIntoPipHostActivity, String reason) {

        mService.deferWindowLayout();



        final TaskDisplayArea taskDisplayArea = r.getDisplayArea();



        try {

        final Task task = r.getTask();

        final Task rootTask;



        Transition newTransition = null;

        // Create a transition now to collect the current pinned Task dismiss. Only do the

        // create here as the Task (trigger) to enter PIP is not ready yet.

        final TransitionController transitionController = task.mTransitionController;

            Transition newTransition = null;

            if (transitionController.isCollecting()) {

                transitionController.setReady(task, false /* ready */);

            } else if (transitionController.getTransitionPlayer() != null) {

        if (!transitionController.isCollecting()

                && transitionController.getTransitionPlayer() != null) {

            newTransition = transitionController.createTransition(TRANSIT_PIP);

        }



        transitionController.deferTransitionReady();

        mService.deferWindowLayout();

        try {

            // This will change the root pinned task's windowing mode to its original mode, ensuring

            // we only have one root task that is in pinned mode.

            final Task rootPinnedTask = taskDisplayArea.getRootPinnedTask();
@@ -2006,7 +2006,6 @@ class RootWindowContainer extends WindowContainer<DisplayContent> 
            r.getDisplayContent().prepareAppTransition(TRANSIT_NONE);



            final boolean singleActivity = task.getChildCount() == 1;

            final Task rootTask;

            if (singleActivity) {

                rootTask = task;
@@ -2064,6 +2063,9 @@ class RootWindowContainer extends WindowContainer<DisplayContent> 
                    oldTopActivity.mRequestForceTransition = true;

                }

            }



            transitionController.collect(rootTask);



            // The intermediate windowing mode to be set on the ActivityRecord later.

            // This needs to happen before the re-parenting, otherwise we will always set the

            // ActivityRecord to be fullscreen.
@@ -2074,13 +2076,6 @@ class RootWindowContainer extends WindowContainer<DisplayContent> 
                rootTask.reparent(taskDisplayArea, true /* onTop */);

            }



            // The new PIP Task is ready, start the transition before updating the windowing mode.

            if (newTransition != null) {

                transitionController.requestStartTransition(newTransition, rootTask,

                        null /* remoteTransition */, null /* displayChange */);

            }

            transitionController.collect(rootTask);



            // Defer the windowing mode change until after the transition to prevent the activity

            // from doing work and changing the activity visuals while animating

            // TODO(task-org): Figure-out more structured way to do this long term.
@@ -2098,9 +2093,23 @@ class RootWindowContainer extends WindowContainer<DisplayContent> 
            r.supportsEnterPipOnTaskSwitch = false;

        } finally {

            mService.continueWindowLayout();

            try {

                ensureActivitiesVisible(null, 0, false /* preserveWindows */);

            } finally {

                transitionController.continueTransitionReady();

            }

        }



        if (newTransition != null) {

            // Request at end since we want task-organizer events from ensureActivitiesVisible

            // to be recognized.

            transitionController.requestStartTransition(newTransition, rootTask,

                    null /* remoteTransition */, null /* displayChange */);

            // A new transition was created just for this operations. Since the operation is

            // complete, mark it as ready.

            newTransition.setReady(rootTask, true /* ready */);

        }



        ensureActivitiesVisible(null, 0, false /* preserveWindows */);

        resumeFocusedTasksTopActivities();



        notifyActivityPipModeChanged(r.getTask(), r);

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

+27 −0
Original line number Diff line number Diff line
@@ -1594,6 +1594,19 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe 
        }

    }



    /**

     * This transition will be considered not-ready until a corresponding call to

     * {@link #continueTransitionReady}

     */

    void deferTransitionReady() {

        ++mReadyTracker.mDeferReadyDepth;

    }



    /** This undoes one call to {@link #deferTransitionReady}. */

    void continueTransitionReady() {

        --mReadyTracker.mDeferReadyDepth;

    }



    /**

     * The transition sync mechanism has 2 parts:

     *   1. Whether all WM operations for a particular transition are "ready" (eg. did the app
@@ -1623,6 +1636,14 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe 
         */

        private boolean mReadyOverride = false;



        /**

         * When non-zero, this transition is forced not-ready (even over setAllReady()). Use this

         * (via deferTransitionReady/continueTransitionReady) for situations where we want to do

         * bulk operations which could trigger surface-placement but the existing ready-state

         * isn't known.

         */

        private int mDeferReadyDepth = 0;



        /**

         * Adds a ready-group. Any setReady calls in this subtree will be tracked together. For

         * now these are only DisplayContents.
@@ -1664,7 +1685,13 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe 
        boolean allReady() {

            ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, " allReady query: used=%b "

                    + "override=%b states=[%s]", mUsed, mReadyOverride, groupsToString());

            // If the readiness has never been touched, mUsed will be false. We never want to

            // consider a transition ready if nothing has been reported on it.

            if (!mUsed) return false;

            // If we are deferring readiness, we never report ready. This is usually temporary.

            if (mDeferReadyDepth > 0) return false;

            // Next check all the ready groups to see if they are ready. We can short-cut this if

            // ready-override is set (which is treated as "everything is marked ready").

            if (mReadyOverride) return true;

            for (int i = mReadyGroups.size() - 1; i >= 0; --i) {

                final WindowContainer wc = mReadyGroups.keyAt(i);

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

+18 −0
Original line number Diff line number Diff line
@@ -455,6 +455,24 @@ class TransitionController { 
        setReady(wc, true);

    }



    /** @see Transition#deferTransitionReady */

    void deferTransitionReady() {

        if (!isShellTransitionsEnabled()) return;

        if (mCollectingTransition == null) {

            throw new IllegalStateException("No collecting transition to defer readiness for.");

        }

        mCollectingTransition.deferTransitionReady();

    }



    /** @see Transition#continueTransitionReady */

    void continueTransitionReady() {

        if (!isShellTransitionsEnabled()) return;

        if (mCollectingTransition == null) {

            throw new IllegalStateException("No collecting transition to defer readiness for.");

        }

        mCollectingTransition.continueTransitionReady();

    }



    /** @see Transition#finishTransition */

    void finishTransition(@NonNull IBinder token) {

        // It is usually a no-op but make sure that the metric consumer is removed.

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

+23 −0
Original line number Diff line number Diff line
@@ -955,6 +955,29 @@ public class TransitionTests extends WindowTestsBase { 
        verify(snapshotController, times(1)).recordTaskSnapshot(eq(task1), eq(false));

    }



    @Test

    public void testNotReadyPushPop() {

        final TaskSnapshotController snapshotController = mock(TaskSnapshotController.class);

        final TransitionController controller = new TransitionController(mAtm, snapshotController);

        final ITransitionPlayer player = new ITransitionPlayer.Default();

        controller.registerTransitionPlayer(player, null /* appThread */);

        final Transition openTransition = controller.createTransition(TRANSIT_OPEN);



        // Start out with task2 visible and set up a transition that closes task2 and opens task1

        final Task task1 = createTask(mDisplayContent);

        openTransition.collectExistenceChange(task1);



        assertFalse(openTransition.allReady());



        openTransition.setAllReady();



        openTransition.deferTransitionReady();

        assertFalse(openTransition.allReady());



        openTransition.continueTransitionReady();

        assertTrue(openTransition.allReady());

    }



    private static void makeTaskOrganized(Task... tasks) {

        final ITaskOrganizer organizer = mock(ITaskOrganizer.class);

        for (Task t : tasks) {