Fix the ordering of taskevents.

- The ordering was not correct when a task switches to a different organizer.
- TaskVanished event was being sent after the TaskAppeared event and it was
  being sent to the new organizer instead of the previous one.
- Reason for this was that dispose() assumes that the organizer which is being
  disposed-off still has its state in mOrganizerStates. But it was being
  removed from the organizer even before dispose is called.

Fix: 222719752
Test: atest WindowOrganizerTests
Change-Id: Iea6462cada57ee62ec7ed322819dff45c284fa64

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

@@ -67,7 +67,8 @@ import java.util.function.Consumer;
 class TaskOrganizerController extends ITaskOrganizerController.Stub {
     private static final String TAG = "TaskOrganizerController";
 
-    private class DeathRecipient implements IBinder.DeathRecipient {
+    @VisibleForTesting
+    class DeathRecipient implements IBinder.DeathRecipient {
         ITaskOrganizer mTaskOrganizer;
 
         DeathRecipient(ITaskOrganizer organizer) {
@@ -77,7 +78,7 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub {
         @Override
         public void binderDied() {
             synchronized (mGlobalLock) {
-                final TaskOrganizerState state = mTaskOrganizerStates.remove(
+                final TaskOrganizerState state = mTaskOrganizerStates.get(
                         mTaskOrganizer.asBinder());
                 if (state != null) {
                     state.dispose();
@@ -170,7 +171,8 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub {
         }
     }
 
-    private class TaskOrganizerState {
+    @VisibleForTesting
+    class TaskOrganizerState {
         private final TaskOrganizerCallbacks mOrganizer;
         private final DeathRecipient mDeathRecipient;
         private final ArrayList<Task> mOrganizedTasks = new ArrayList<>();
@@ -191,6 +193,11 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub {
             mUid = uid;
         }
 
+        @VisibleForTesting
+        DeathRecipient getDeathRecipient() {
+            return mDeathRecipient;
+        }
+
         /**
          * Register this task with this state, but doesn't trigger the task appeared callback to
          * the organizer.
@@ -265,7 +272,7 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub {
 
             // Remove organizer state after removing tasks so we get a chance to send
             // onTaskVanished.
-            mTaskOrganizerStates.remove(asBinder());
+            mTaskOrganizerStates.remove(mOrganizer.getBinder());
         }
 
         void unlinkDeath() {
@@ -596,7 +603,7 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub {
     private void onTaskVanishedInternal(ITaskOrganizer organizer, Task task) {
         for (int i = mPendingTaskEvents.size() - 1; i >= 0; i--) {
             PendingTaskEvent entry = mPendingTaskEvents.get(i);
-            if (task.mTaskId == entry.mTask.mTaskId) {
+            if (task.mTaskId == entry.mTask.mTaskId && entry.mTaskOrg == organizer) {
                 // This task is vanished so remove all pending event of it.
                 mPendingTaskEvents.remove(i);
                 if (entry.mEventType == PendingTaskEvent.EVENT_APPEARED) {
@@ -693,9 +700,16 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub {
                     }
                     break;
                 case PendingTaskEvent.EVENT_VANISHED:
-                    state = mTaskOrganizerStates.get(event.mTaskOrg.asBinder());
-                    if (state != null) {
-                        state.mOrganizer.onTaskVanished(task);
+                    // TaskOrganizerState cannot be used here because it might have already been
+                    // removed.
+                    // The state is removed when an organizer dies or is unregistered. In order to
+                    // send the pending vanished task events, the mTaskOrg from event is used.
+                    // These events should not ideally be sent and will be removed as part of
+                    // b/224812558.
+                    try {
+                        event.mTaskOrg.onTaskVanished(task.getTaskInfo());
+                    } catch (RemoteException ex) {
+                        Slog.e(TAG, "Exception sending onTaskVanished callback", ex);
                     }
                     mLastSentTaskInfos.remove(task);
                     break;
@@ -1045,4 +1059,9 @@ class TaskOrganizerController extends ITaskOrganizerController.Stub {
         }
         pw.println();
     }
+
+    @VisibleForTesting
+    TaskOrganizerState getTaskOrganizerState(IBinder taskOrganizer) {
+        return mTaskOrganizerStates.get(taskOrganizer);
+    }
 }

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

@@ -308,6 +308,51 @@ public class WindowOrganizerTests extends WindowTestsBase {
         assertTaskVanished(organizer2, true /* expectVanished */, rootTask, rootTask2, rootTask3);
     }
 
+    @Test
+    public void testOrganizerDeathReturnsRegistrationToPrevious() throws RemoteException {
+        final Task rootTask = createRootTask();
+        final Task task = createTask(rootTask);
+        final Task rootTask2 = createRootTask();
+        final Task task2 = createTask(rootTask2);
+        final Task rootTask3 = createRootTask();
+        final Task task3 = createTask(rootTask3);
+        final ArrayList<TaskAppearedInfo> existingTasks = new ArrayList<>();
+        final ITaskOrganizer organizer = registerMockOrganizer(existingTasks);
+        // Ensure events dispatch to organizer.
+        mWm.mAtmService.mTaskOrganizerController.dispatchPendingEvents();
+
+        // verify that tasks are returned and taskAppeared is not called
+        assertContainsTasks(existingTasks, rootTask, rootTask2, rootTask3);
+        verify(organizer, times(0)).onTaskAppeared(any(RunningTaskInfo.class),
+                any(SurfaceControl.class));
+        verify(organizer, times(0)).onTaskVanished(any());
+        assertTrue(rootTask.isOrganized());
+
+        // Now we replace the registration and verify the new organizer receives existing tasks
+        final ArrayList<TaskAppearedInfo> existingTasks2 = new ArrayList<>();
+        final ITaskOrganizer organizer2 = registerMockOrganizer(existingTasks2);
+        // Ensure events dispatch to organizer.
+        mWm.mAtmService.mTaskOrganizerController.dispatchPendingEvents();
+        assertContainsTasks(existingTasks2, rootTask, rootTask2, rootTask3);
+        verify(organizer2, times(0)).onTaskAppeared(any(RunningTaskInfo.class),
+                any(SurfaceControl.class));
+        verify(organizer2, times(0)).onTaskVanished(any());
+        // Removed tasks from the original organizer
+        assertTaskVanished(organizer, true /* expectVanished */, rootTask, rootTask2, rootTask3);
+        assertTrue(rootTask2.isOrganized());
+
+        // Trigger binderDied for second one, the first one should automatically be reregistered
+        // so we verify that it's now seeing changes.
+        mWm.mAtmService.mTaskOrganizerController.getTaskOrganizerState(organizer2.asBinder())
+                .getDeathRecipient().binderDied();
+
+        // Ensure events dispatch to organizer.
+        mWm.mAtmService.mTaskOrganizerController.dispatchPendingEvents();
+        verify(organizer, times(3))
+                .onTaskAppeared(any(RunningTaskInfo.class), any(SurfaceControl.class));
+        assertTaskVanished(organizer2, true /* expectVanished */, rootTask, rootTask2, rootTask3);
+    }
+
     @Test
     public void testRegisterTaskOrganizerWithExistingTasks() throws RemoteException {
         final Task rootTask = createRootTask();