From 7e2eeb1395f3753414048132bb2e2a747a24775d Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Sat, 24 Sep 2022 00:20:57 +0800 Subject: [PATCH] Make startTransition one-way If shell is starting an existing transition, it doesn't need the returned transition token so it can be an async call then it won't block shell's thread to execute other operations. If shell is starting a new transition, then use the new added 2-way startNewTransition which is the same as the original path. Bug: 248550757 Test: atest ShellTransitionTests Test: CtsWindowManagerDeviceTestCases with shell transition Change-Id: I5f64d19475d5b857a461775dd6f3002567e93ad8 --- .../window/IWindowOrganizerController.aidl | 17 ++++++++------- core/java/android/window/WindowOrganizer.java | 21 +++++++++++++++---- .../wm/shell/transition/Transitions.java | 6 +++--- .../transition/ShellTransitionTests.java | 21 +++++++++++-------- .../server/wm/WindowOrganizerController.java | 17 ++++++++++----- .../android/server/wm/WindowTestsBase.java | 2 +- 6 files changed, 55 insertions(+), 29 deletions(-) diff --git a/core/java/android/window/IWindowOrganizerController.aidl b/core/java/android/window/IWindowOrganizerController.aidl index 3c7cd0254e789..36eaf4966165b 100644 --- a/core/java/android/window/IWindowOrganizerController.aidl +++ b/core/java/android/window/IWindowOrganizerController.aidl @@ -51,16 +51,19 @@ interface IWindowOrganizerController { in IWindowContainerTransactionCallback callback); /** - * Starts a transition. + * Starts a new transition. * @param type The transition type. - * @param transitionToken A token associated with the transition to start. If null, a new - * transition will be created of the provided type. * @param t Operations that are part of the transition. - * @return a token representing the transition. This will just be transitionToken if it was - * non-null. + * @return a token representing the transition. */ - IBinder startTransition(int type, in @nullable IBinder transitionToken, - in @nullable WindowContainerTransaction t); + IBinder startNewTransition(int type, in @nullable WindowContainerTransaction t); + + /** + * Starts the given transition. + * @param transitionToken A token associated with the transition to start. + * @param t Operations that are part of the transition. + */ + oneway void startTransition(IBinder transitionToken, in @nullable WindowContainerTransaction t); /** * Starts a legacy transition. diff --git a/core/java/android/window/WindowOrganizer.java b/core/java/android/window/WindowOrganizer.java index 4ea5ea5694fa6..2a80d021abd69 100644 --- a/core/java/android/window/WindowOrganizer.java +++ b/core/java/android/window/WindowOrganizer.java @@ -84,9 +84,8 @@ public class WindowOrganizer { } /** - * Start a transition. + * Starts a new transition, don't use this to start an already created one. * @param type The type of the transition. This is ignored if a transitionToken is provided. - * @param transitionToken An existing transition to start. If null, a new transition is created. * @param t The set of window operations that are part of this transition. * @return A token identifying the transition. This will be the same as transitionToken if it * was provided. @@ -94,10 +93,24 @@ public class WindowOrganizer { */ @RequiresPermission(android.Manifest.permission.MANAGE_ACTIVITY_TASKS) @NonNull - public IBinder startTransition(int type, @Nullable IBinder transitionToken, + public IBinder startNewTransition(int type, @Nullable WindowContainerTransaction t) { + try { + return getWindowOrganizerController().startNewTransition(type, t); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + /** + * Starts an already created transition. + * @param transitionToken An existing transition to start. + * @hide + */ + @RequiresPermission(android.Manifest.permission.MANAGE_ACTIVITY_TASKS) + public void startTransition(@NonNull IBinder transitionToken, @Nullable WindowContainerTransaction t) { try { - return getWindowOrganizerController().startTransition(type, transitionToken, t); + getWindowOrganizerController().startTransition(transitionToken, t); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java index 29d25bc39223a..d65ac80621df4 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java @@ -716,8 +716,8 @@ public class Transitions implements RemoteCallable { null /* newDisplayAreaInfo */); } } - active.mToken = mOrganizer.startTransition( - request.getType(), transitionToken, wct); + mOrganizer.startTransition(transitionToken, wct != null && wct.isEmpty() ? null : wct); + active.mToken = transitionToken; mActiveTransitions.add(active); } @@ -726,7 +726,7 @@ public class Transitions implements RemoteCallable { @NonNull WindowContainerTransaction wct, @Nullable TransitionHandler handler) { final ActiveTransition active = new ActiveTransition(); active.mHandler = handler; - active.mToken = mOrganizer.startTransition(type, null /* token */, wct); + active.mToken = mOrganizer.startNewTransition(type, wct); mActiveTransitions.add(active); return active.mToken; } diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/transition/ShellTransitionTests.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/transition/ShellTransitionTests.java index c6492bee040e2..db9136d8eacbe 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/transition/ShellTransitionTests.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/transition/ShellTransitionTests.java @@ -45,7 +45,6 @@ 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; @@ -67,10 +66,12 @@ import android.view.SurfaceControl; import android.view.WindowManager; import android.window.IRemoteTransition; import android.window.IRemoteTransitionFinishedCallback; +import android.window.IWindowContainerToken; import android.window.RemoteTransition; import android.window.TransitionFilter; import android.window.TransitionInfo; import android.window.TransitionRequestInfo; +import android.window.WindowContainerToken; import android.window.WindowContainerTransaction; import android.window.WindowOrganizer; @@ -117,7 +118,7 @@ public class ShellTransitionTests extends ShellTestCase { @Before public void setUp() { doAnswer(invocation -> invocation.getArguments()[1]) - .when(mOrganizer).startTransition(anyInt(), any(), any()); + .when(mOrganizer).startTransition(any(), any()); } @Test @@ -136,7 +137,7 @@ public class ShellTransitionTests extends ShellTestCase { IBinder transitToken = new Binder(); transitions.requestStartTransition(transitToken, new TransitionRequestInfo(TRANSIT_OPEN, null /* trigger */, null /* remote */)); - verify(mOrganizer, times(1)).startTransition(eq(TRANSIT_OPEN), eq(transitToken), any()); + verify(mOrganizer, times(1)).startTransition(eq(transitToken), any()); TransitionInfo info = new TransitionInfoBuilder(TRANSIT_OPEN) .addChange(TRANSIT_OPEN).addChange(TRANSIT_CLOSE).build(); transitions.onTransitionReady(transitToken, info, mock(SurfaceControl.Transaction.class), @@ -188,7 +189,7 @@ public class ShellTransitionTests extends ShellTestCase { // Make a request that will be rejected by the testhandler. transitions.requestStartTransition(transitToken, new TransitionRequestInfo(TRANSIT_OPEN, null /* trigger */, null /* remote */)); - verify(mOrganizer, times(1)).startTransition(eq(TRANSIT_OPEN), eq(transitToken), isNull()); + verify(mOrganizer, times(1)).startTransition(eq(transitToken), isNull()); transitions.onTransitionReady(transitToken, open, mock(SurfaceControl.Transaction.class), mock(SurfaceControl.Transaction.class)); assertEquals(1, mDefaultHandler.activeCount()); @@ -199,10 +200,12 @@ public class ShellTransitionTests extends ShellTestCase { // Make a request that will be handled by testhandler but not animated by it. RunningTaskInfo mwTaskInfo = createTaskInfo(1, WINDOWING_MODE_MULTI_WINDOW, ACTIVITY_TYPE_STANDARD); + // Make the wct non-empty. + handlerWCT.setFocusable(new WindowContainerToken(mock(IWindowContainerToken.class)), true); transitions.requestStartTransition(transitToken, new TransitionRequestInfo(TRANSIT_OPEN, mwTaskInfo, null /* remote */)); verify(mOrganizer, times(1)).startTransition( - eq(TRANSIT_OPEN), eq(transitToken), eq(handlerWCT)); + eq(transitToken), eq(handlerWCT)); transitions.onTransitionReady(transitToken, open, mock(SurfaceControl.Transaction.class), mock(SurfaceControl.Transaction.class)); assertEquals(1, mDefaultHandler.activeCount()); @@ -217,8 +220,8 @@ public class ShellTransitionTests extends ShellTestCase { transitions.addHandler(topHandler); transitions.requestStartTransition(transitToken, new TransitionRequestInfo(TRANSIT_CHANGE, mwTaskInfo, null /* remote */)); - verify(mOrganizer, times(1)).startTransition( - eq(TRANSIT_CHANGE), eq(transitToken), eq(handlerWCT)); + verify(mOrganizer, times(2)).startTransition( + eq(transitToken), eq(handlerWCT)); TransitionInfo change = new TransitionInfoBuilder(TRANSIT_CHANGE) .addChange(TRANSIT_CHANGE).build(); transitions.onTransitionReady(transitToken, change, mock(SurfaceControl.Transaction.class), @@ -256,7 +259,7 @@ public class ShellTransitionTests extends ShellTestCase { transitions.requestStartTransition(transitToken, new TransitionRequestInfo(TRANSIT_OPEN, null /* trigger */, new RemoteTransition(testRemote))); - verify(mOrganizer, times(1)).startTransition(eq(TRANSIT_OPEN), eq(transitToken), any()); + verify(mOrganizer, times(1)).startTransition(eq(transitToken), any()); TransitionInfo info = new TransitionInfoBuilder(TRANSIT_OPEN) .addChange(TRANSIT_OPEN).addChange(TRANSIT_CLOSE).build(); transitions.onTransitionReady(transitToken, info, mock(SurfaceControl.Transaction.class), @@ -406,7 +409,7 @@ public class ShellTransitionTests extends ShellTestCase { IBinder transitToken = new Binder(); transitions.requestStartTransition(transitToken, new TransitionRequestInfo(TRANSIT_OPEN, null /* trigger */, null /* remote */)); - verify(mOrganizer, times(1)).startTransition(eq(TRANSIT_OPEN), eq(transitToken), any()); + verify(mOrganizer, times(1)).startTransition(eq(transitToken), any()); TransitionInfo info = new TransitionInfoBuilder(TRANSIT_OPEN) .addChange(TRANSIT_OPEN).addChange(TRANSIT_CLOSE).build(); transitions.onTransitionReady(transitToken, info, mock(SurfaceControl.Transaction.class), diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java index 2e1477ddf0f16..949fa9662258e 100644 --- a/services/core/java/com/android/server/wm/WindowOrganizerController.java +++ b/services/core/java/com/android/server/wm/WindowOrganizerController.java @@ -46,6 +46,7 @@ import static android.window.WindowContainerTransaction.HierarchyOp.HIERARCHY_OP import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_WINDOW_ORGANIZER; import static com.android.server.wm.ActivityTaskManagerService.LAYOUT_REASON_CONFIG_CHANGED; +import static com.android.server.wm.ActivityTaskManagerService.enforceTaskPermission; import static com.android.server.wm.ActivityTaskSupervisor.PRESERVE_WINDOWS; import static com.android.server.wm.Task.FLAG_FORCE_HIDDEN_FOR_PINNED_TASK; import static com.android.server.wm.Task.FLAG_FORCE_HIDDEN_FOR_TASK_ORG; @@ -242,8 +243,18 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub } @Override - public IBinder startTransition(int type, @Nullable IBinder transitionToken, + public IBinder startNewTransition(int type, @Nullable WindowContainerTransaction t) { + return startTransition(type, null /* transitionToken */, t); + } + + @Override + public void startTransition(@NonNull IBinder transitionToken, @Nullable WindowContainerTransaction t) { + startTransition(-1 /* unused type */, transitionToken, t); + } + + private IBinder startTransition(@WindowManager.TransitionType int type, + @Nullable IBinder transitionToken, @Nullable WindowContainerTransaction t) { enforceTaskPermission("startTransition()"); final CallerInfo caller = new CallerInfo(); final long ident = Binder.clearCallingIdentity(); @@ -1557,10 +1568,6 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub return (cfgChanges & CONTROLLABLE_CONFIGS) == 0; } - private void enforceTaskPermission(String func) { - mService.enforceTaskPermission(func); - } - private boolean isValidTransaction(@NonNull WindowContainerTransaction t) { if (t.getTaskFragmentOrganizer() != null && !mTaskFragmentOrganizerController .isOrganizerRegistered(t.getTaskFragmentOrganizer())) { diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java b/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java index ef532f5732eb1..139e44099f425 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java @@ -1727,7 +1727,7 @@ class WindowTestsBase extends SystemServiceTestsBase { } void startTransition() { - mOrganizer.startTransition(mLastRequest.getType(), mLastTransit, null); + mOrganizer.startTransition(mLastTransit, null); } void onTransactionReady(SurfaceControl.Transaction t) {