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

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

Fix race between startNew and Core-request transition 

If WMCore requests a transition while a startNew from Shell
is "in flight", Shell could be expecting startNew to be
processed first while WM is actually processing its own
requested transition first. This can lead to incorrect
behavior or an NPE.

To resolve this, make sure the WM request transition is
the first non-started transition in Shell's records (basically
syncronizing Shell to Core's transition order).

Bug: 265861362
Test: atest ShellTransitionTests#testTransitionOrderMatchesCore
Change-Id: I62a25335f270a021a06dd4309e692e8812d43332
parent a19b3110

libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java

+9 −1
Original line number Diff line number Diff line
@@ -829,7 +829,15 @@ public class Transitions implements RemoteCallable<Transitions> { 
        }

        mOrganizer.startTransition(transitionToken, wct != null && wct.isEmpty() ? null : wct);

        active.mToken = transitionToken;

        mActiveTransitions.add(active);

        int insertIdx = 0;

        for (; insertIdx < mActiveTransitions.size(); ++insertIdx) {

            if (mActiveTransitions.get(insertIdx).mInfo == null) {

                // A `startNewTransition` was sent to WMCore, but wasn't acknowledged before WMCore

                // made this request, so insert this request beforehand to keep order in sync.

                break;

            }

        }

        mActiveTransitions.add(insertIdx, active);

    }



    /** Start a new transition directly. */

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/transition/ShellTransitionTests.java

+29 −2
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ import static org.junit.Assert.assertNull; 
import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.ArgumentMatchers.isNull;

import static org.mockito.Mockito.clearInvocations;
@@ -119,8 +120,8 @@ public class ShellTransitionTests extends ShellTestCase { 


    @Before

    public void setUp() {

        doAnswer(invocation -> invocation.getArguments()[1])

                .when(mOrganizer).startTransition(any(), any());

        doAnswer(invocation -> new Binder())

                .when(mOrganizer).startNewTransition(anyInt(), any());

    }



    @Test
@@ -561,6 +562,32 @@ public class ShellTransitionTests extends ShellTestCase { 
        assertEquals(0, mDefaultHandler.activeCount());

    }



    @Test

    public void testTransitionOrderMatchesCore() {

        Transitions transitions = createTestTransitions();

        transitions.replaceDefaultHandlerForTest(mDefaultHandler);



        IBinder transitToken = new Binder();

        IBinder shellInit = transitions.startTransition(TRANSIT_CLOSE,

                new WindowContainerTransaction(), null /* handler */);

        // make sure we are testing the "New" API.

        verify(mOrganizer, times(1)).startNewTransition(eq(TRANSIT_CLOSE), any());

        // WMCore may not receive the new transition before requesting its own.

        transitions.requestStartTransition(transitToken,

                new TransitionRequestInfo(TRANSIT_OPEN, null /* trigger */, null /* remote */));

        verify(mOrganizer, times(1)).startTransition(eq(transitToken), any());



        // At this point, WM is working on its transition (the shell-initialized one is still

        // queued), so continue the transition lifecycle for that.

        TransitionInfo info = new TransitionInfoBuilder(TRANSIT_OPEN)

                .addChange(TRANSIT_OPEN).addChange(TRANSIT_CLOSE).build();

        transitions.onTransitionReady(transitToken, info, mock(SurfaceControl.Transaction.class),

                mock(SurfaceControl.Transaction.class));

        // At this point, if things are not working, we'd get an NPE due to attempting to merge

        // into the shellInit transition which hasn't started yet.

        assertEquals(1, mDefaultHandler.activeCount());

    }



    @Test

    public void testShouldRotateSeamlessly() throws Exception {

        final RunningTaskInfo taskInfo =