diff options
3 files changed, 46 insertions, 24 deletions
diff --git a/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java b/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java index 484de2b3bc34..6e4df794f8d8 100644 --- a/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java +++ b/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java @@ -425,7 +425,7 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr ProtoLog.v(WM_DEBUG_WINDOW_ORGANIZER, "Register task fragment organizer=%s uid=%d pid=%d", organizer.asBinder(), uid, pid); - if (mTaskFragmentOrganizerState.containsKey(organizer.asBinder())) { + if (isOrganizerRegistered(organizer)) { throw new IllegalStateException( "Replacing existing organizer currently unsupported"); } @@ -502,10 +502,18 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr @WindowManager.TransitionType int transitionType, boolean shouldApplyIndependently) { // Keep the calling identity to avoid unsecure change. synchronized (mGlobalLock) { - applyTransaction(wct, transitionType, shouldApplyIndependently); - final TaskFragmentOrganizerState state = validateAndGetState( - wct.getTaskFragmentOrganizer()); - state.onTransactionFinished(transactionToken); + if (isValidTransaction(wct)) { + applyTransaction(wct, transitionType, shouldApplyIndependently); + } + // Even if the transaction is empty, we still need to invoke #onTransactionFinished + // unless the organizer has been unregistered. + final ITaskFragmentOrganizer organizer = wct.getTaskFragmentOrganizer(); + final TaskFragmentOrganizerState state = organizer != null + ? mTaskFragmentOrganizerState.get(organizer.asBinder()) + : null; + if (state != null) { + state.onTransactionFinished(transactionToken); + } } } @@ -514,7 +522,7 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr @WindowManager.TransitionType int transitionType, boolean shouldApplyIndependently) { // Keep the calling identity to avoid unsecure change. synchronized (mGlobalLock) { - if (wct.isEmpty()) { + if (!isValidTransaction(wct)) { return; } mWindowOrganizerController.applyTaskFragmentTransactionLocked(wct, transitionType, @@ -655,7 +663,7 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr } organizer = organizedTf[0].getTaskFragmentOrganizer(); } - if (!mTaskFragmentOrganizerState.containsKey(organizer.asBinder())) { + if (!isOrganizerRegistered(organizer)) { Slog.w(TAG, "The last TaskFragmentOrganizer no longer exists"); return; } @@ -701,7 +709,7 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr mPendingTaskFragmentEvents.get(event.mTaskFragmentOrg.asBinder()).remove(event); } - boolean isOrganizerRegistered(@NonNull ITaskFragmentOrganizer organizer) { + private boolean isOrganizerRegistered(@NonNull ITaskFragmentOrganizer organizer) { return mTaskFragmentOrganizerState.containsKey(organizer.asBinder()); } @@ -738,6 +746,20 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr return state; } + boolean isValidTransaction(@NonNull WindowContainerTransaction t) { + if (t.isEmpty()) { + return false; + } + final ITaskFragmentOrganizer organizer = t.getTaskFragmentOrganizer(); + if (t.getTaskFragmentOrganizer() == null || !isOrganizerRegistered(organizer)) { + // Transaction from an unregistered organizer should not be applied. This can happen + // when the organizer process died before the transaction is applied. + Slog.e(TAG, "Caller organizer=" + organizer + " is no longer registered"); + return false; + } + return true; + } + /** * A class to store {@link ITaskFragmentOrganizer} and its organized * {@link TaskFragment TaskFragments} with different pending event request. diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java index a15fc1201596..8dc1f0b1fbfa 100644 --- a/services/core/java/com/android/server/wm/WindowOrganizerController.java +++ b/services/core/java/com/android/server/wm/WindowOrganizerController.java @@ -401,9 +401,6 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub */ void applyTaskFragmentTransactionLocked(@NonNull WindowContainerTransaction wct, @WindowManager.TransitionType int type, boolean shouldApplyIndependently) { - if (!isValidTransaction(wct)) { - return; - } enforceTaskFragmentOrganizerPermission("applyTaskFragmentTransaction()", Objects.requireNonNull(wct.getTaskFragmentOrganizer()), Objects.requireNonNull(wct)); @@ -457,7 +454,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub // calls startSyncSet. () -> mTransitionController.moveToCollecting(nextTransition), () -> { - if (isValidTransaction(wct)) { + if (mTaskFragmentOrganizerController.isValidTransaction(wct)) { applyTransaction(wct, -1 /*syncId*/, nextTransition, caller); mTransitionController.requestStartTransition(nextTransition, null /* startTask */, null /* remoteTransition */, @@ -1618,18 +1615,6 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub return (cfgChanges & CONTROLLABLE_CONFIGS) == 0; } - private boolean isValidTransaction(@NonNull WindowContainerTransaction t) { - if (t.getTaskFragmentOrganizer() != null && !mTaskFragmentOrganizerController - .isOrganizerRegistered(t.getTaskFragmentOrganizer())) { - // Transaction from an unregistered organizer should not be applied. This can happen - // when the organizer process died before the transaction is applied. - Slog.e(TAG, "Caller organizer=" + t.getTaskFragmentOrganizer() - + " is no longer registered"); - return false; - } - return true; - } - /** * Makes sure that the transaction only contains operations that are allowed for the * {@link WindowContainerTransaction#getTaskFragmentOrganizer()}. diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java index 426afffdb880..2b493145f854 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java @@ -822,6 +822,21 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { } @Test + public void testOnTransactionHandled_skipTransactionForUnregisterOrganizer() { + mController.unregisterOrganizer(mIOrganizer); + final ActivityRecord ownerActivity = createActivityRecord(mDisplayContent); + final IBinder fragmentToken = new Binder(); + + // Allow organizer to create TaskFragment and start/reparent activity to TaskFragment. + createTaskFragmentFromOrganizer(mTransaction, ownerActivity, fragmentToken); + mController.onTransactionHandled(new Binder(), mTransaction, + getTransitionType(mTransaction), false /* shouldApplyIndependently */); + + // Nothing should happen as the organizer is not registered. + assertNull(mWindowOrganizerController.getTaskFragment(fragmentToken)); + } + + @Test public void testOrganizerRemovedWithPendingEvents() { final TaskFragment tf0 = new TaskFragmentBuilder(mAtm) .setCreateParentTask() |