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

Commit f4740576 authored by Evan Rosky's avatar Evan Rosky
Browse files

Fix task-organizer + shell transit interractions during tests 

During tests, task-organizers sometimes get registered/unregistered
overtop the shell task-organizer. This can race with transition
animations (most common during teardown where the instrumentation
isn't waiting for anything and doesn't know what the test was
doing).

To remedy this, add a waiting mechanism to transition controller so
that these operations (like register/unregister) won't run during
transition animations. This is designed to have basically 0 impact
on normal usage.

Also, because chanhing organizers goes through a "migrateSurface"
flow, it can also mix-up unfavorably with transitions (esp. in cases
where Shell uses transitions to handle visibility). For this one
case, we manually "show" task surfaces after they've been migrated
even for organized tasks. This simulates the desired effect that
register/unregister shouldn't alter the system state.

Bug: 183993924
Test: atest CrossAppDragAndDropTests
Change-Id: I6a8f80fe92442e1ac12be8e5ff4322b58567157a
parent 6173c266

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

+31 −6
Original line number Diff line number Diff line
@@ -388,6 +388,15 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub { 
                                mOrganizer.mTaskOrganizer, t);

                    }

                }

                if (mService.getTransitionController().isShellTransitionsEnabled()) {

                    // dispose is only called outside of transitions (eg during unregister). Since

                    // we "migrate" surfaces when replacing organizers, visibility gets delegated

                    // to transitions; however, since there is no transition at this point, we have

                    // to manually show the surface here.

                    if (t.mTaskOrganizer != null && t.getSurfaceControl() != null) {

                        t.getSyncTransaction().show(t.getSurfaceControl());

                    }

                }

            }



            // Remove organizer state after removing tasks so we get a chance to send
@@ -480,7 +489,8 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub { 
        final int uid = Binder.getCallingUid();

        final long origId = Binder.clearCallingIdentity();

        try {

            synchronized (mGlobalLock) {

            final ArrayList<TaskAppearedInfo> taskInfos = new ArrayList<>();

            final Runnable withGlobalLock = () -> {

                ProtoLog.v(WM_DEBUG_WINDOW_ORGANIZER, "Register task organizer=%s uid=%d",

                        organizer.asBinder(), uid);

                if (!mTaskOrganizerStates.containsKey(organizer.asBinder())) {
@@ -489,10 +499,10 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub { 
                            new TaskOrganizerState(organizer, uid));

                }



                final ArrayList<TaskAppearedInfo> taskInfos = new ArrayList<>();

                final TaskOrganizerState state = mTaskOrganizerStates.get(organizer.asBinder());

                mService.mRootWindowContainer.forAllTasks((task) -> {

                    if (ArrayUtils.contains(UNSUPPORTED_WINDOWING_MODES, task.getWindowingMode())) {

                    if (ArrayUtils.contains(UNSUPPORTED_WINDOWING_MODES,

                            task.getWindowingMode())) {

                        return;

                    }
@@ -502,11 +512,19 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub { 
                    if (returnTask) {

                        SurfaceControl outSurfaceControl = state.addTaskWithoutCallback(task,

                                "TaskOrganizerController.registerTaskOrganizer");

                        taskInfos.add(new TaskAppearedInfo(task.getTaskInfo(), outSurfaceControl));

                        taskInfos.add(

                                new TaskAppearedInfo(task.getTaskInfo(), outSurfaceControl));

                    }

                });

                return new ParceledListSlice<>(taskInfos);

            };

            if (mService.getTransitionController().isShellTransitionsEnabled()) {

                mService.getTransitionController().mRunningLock.runWhenIdle(1000, withGlobalLock);

            } else {

                synchronized (mGlobalLock) {

                    withGlobalLock.run();

                }

            }

            return new ParceledListSlice<>(taskInfos);

        } finally {

            Binder.restoreCallingIdentity(origId);

        }
@@ -518,7 +536,7 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub { 
        final int uid = Binder.getCallingUid();

        final long origId = Binder.clearCallingIdentity();

        try {

            synchronized (mGlobalLock) {

            final Runnable withGlobalLock = () -> {

                final TaskOrganizerState state = mTaskOrganizerStates.get(organizer.asBinder());

                if (state == null) {

                    return;
@@ -527,6 +545,13 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub { 
                        organizer.asBinder(), uid);

                state.unlinkDeath();

                state.dispose();

            };

            if (mService.getTransitionController().isShellTransitionsEnabled()) {

                mService.getTransitionController().mRunningLock.runWhenIdle(1000, withGlobalLock);

            } else {

                synchronized (mGlobalLock) {

                    withGlobalLock.run();

                }

            }

        } finally {

            Binder.restoreCallingIdentity(origId);

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

+43 −0
Original line number Diff line number Diff line
@@ -63,6 +63,8 @@ class TransitionController { 
     */

    private final ArrayList<Transition> mPlayingTransitions = new ArrayList<>();



    final Lock mRunningLock = new Lock();



    private final IBinder.DeathRecipient mTransitionPlayerDeath = () -> {

        // clean-up/finish any playing transitions.

        for (int i = 0; i < mPlayingTransitions.size(); ++i) {
@@ -70,6 +72,7 @@ class TransitionController { 
        }

        mPlayingTransitions.clear();

        mTransitionPlayer = null;

        mRunningLock.doNotifyLocked();

    };



    /** The transition currently being constructed (collecting participants). */
@@ -285,6 +288,7 @@ class TransitionController { 
        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Finish Transition: %s", record);

        mPlayingTransitions.remove(record);

        record.finishTransition();

        mRunningLock.doNotifyLocked();

    }



    void moveToPlaying(Transition transition) {
@@ -357,4 +361,43 @@ class TransitionController { 
                    false /* keyguardGoingAway */);

        }

    }



    class Lock {

        private int mTransitionWaiters = 0;

        void runWhenIdle(long timeout, Runnable r) {

            synchronized (mAtm.mGlobalLock) {

                if (!inTransition()) {

                    r.run();

                    return;

                }

                mTransitionWaiters += 1;

            }

            final long startTime = SystemClock.uptimeMillis();

            final long endTime = startTime + timeout;

            while (true) {

                synchronized (mAtm.mGlobalLock) {

                    if (!inTransition() || SystemClock.uptimeMillis() > endTime) {

                        mTransitionWaiters -= 1;

                        r.run();

                        return;

                    }

                }

                synchronized (this) {

                    try {

                        this.wait(timeout);

                    } catch (InterruptedException e) {

                        return;

                    }

                }

            }

        }



        void doNotifyLocked() {

            synchronized (this) {

                if (mTransitionWaiters > 0) {

                    this.notifyAll();

                }

            }

        }

    }

}