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

Commit b4012458 authored by Omar Elmekkawy's avatar Omar Elmekkawy
Browse files

Use CopyOnWriteArrayList for various listeners in ShellTaskOrganizer 

When calling tasks listeners from ShellTaskOrganizer, task listeners that
remove themselves could remove themselves from the list of listeners,
modifying the Collection while iterating on it, this causes undefined
behavior and NullPointerException.

This CL updates this behavior to use CopyOnWriteArrayList instead of
ArraySet to avoid modifying the set while iterating on it.

Flag: EXEMPT, persist.wm.debug.desktop_experience_devopts
Test: EXEMPT, changing private logic, tested on device.
Bug: 416150006
Change-Id: I2dd6872c36faa4a424f9e3aa86eb14cd4c612efd
parent 3d3000cc

libs/WindowManager/Shell/src/com/android/wm/shell/ShellTaskOrganizer.java

+11 −8
Original line number Diff line number Diff line
@@ -41,7 +41,6 @@ import android.graphics.Rect; 
import android.os.Binder;

import android.os.IBinder;

import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.Log;

import android.util.SparseArray;

import android.view.SurfaceControl;
@@ -72,6 +71,7 @@ import java.util.Arrays; 
import java.util.List;

import java.util.Objects;

import java.util.Optional;

import java.util.concurrent.CopyOnWriteArrayList;



/**

 * Unified task organizer for all components in the shell.
@@ -173,12 +173,15 @@ public class ShellTaskOrganizer extends TaskOrganizer { 
    private final SparseArray<LocusId> mVisibleTasksWithLocusId = new SparseArray<>();



    /** @see #addLocusIdListener */

    private final ArraySet<LocusIdListener> mLocusIdListeners = new ArraySet<>();

    private final CopyOnWriteArrayList<LocusIdListener> mLocusIdListeners =

            new CopyOnWriteArrayList<>();



    private final ArraySet<FocusListener> mFocusListeners = new ArraySet<>();

    private final CopyOnWriteArrayList<FocusListener> mFocusListeners =

            new CopyOnWriteArrayList<>();



    // Listeners that should be notified when a task is removed

    private final ArraySet<TaskVanishedListener> mTaskVanishedListeners = new ArraySet<>();

    private final CopyOnWriteArrayList<TaskVanishedListener> mTaskVanishedListeners =

            new CopyOnWriteArrayList<>();



    private final Object mLock = new Object();

    private StartingWindowController mStartingWindow;
@@ -627,8 +630,8 @@ public class ShellTaskOrganizer extends TaskOrganizer { 
                    || mLastFocusedTaskInfo.getWindowingMode() != taskInfo.getWindowingMode())

                    && isFocusedOrHome;

            if (focusTaskChanged) {

                for (int i = 0; i < mFocusListeners.size(); i++) {

                    mFocusListeners.valueAt(i).onFocusTaskChanged(taskInfo);

                for (FocusListener focusListener : mFocusListeners) {

                    focusListener.onFocusTaskChanged(taskInfo);

                }

                mLastFocusedTaskInfo = taskInfo;

            }
@@ -781,8 +784,8 @@ public class ShellTaskOrganizer extends TaskOrganizer { 
    }



    private void notifyLocusIdChange(int taskId, LocusId locus, boolean visible) {

        for (int i = 0; i < mLocusIdListeners.size(); i++) {

            mLocusIdListeners.valueAt(i).onVisibilityChanged(taskId, locus, visible);

        for (LocusIdListener l : mLocusIdListeners) {

            l.onVisibilityChanged(taskId, locus, visible);

        }

    }

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

+35 −0
Original line number Diff line number Diff line
@@ -82,6 +82,7 @@ import org.mockito.MockitoAnnotations; 


import java.util.ArrayList;

import java.util.Optional;

import java.util.concurrent.atomic.AtomicInteger;



/**

 * Tests for the shell task organizer.
@@ -672,6 +673,29 @@ public class ShellTaskOrganizerTests extends ShellTestCase { 
        assertEquals(vanishedTasks[0], task1);

    }



    @Test

    public void testSelfRemovingVanishedTaskListenersCallback() {

        RunningTaskInfo task1 = createTaskInfo(/* taskId= */ 1, WINDOWING_MODE_FULLSCREEN);

        mOrganizer.onTaskAppeared(task1, /* leash= */ null);



        AtomicInteger calledListenerCount = new AtomicInteger(0);

        ShellTaskOrganizer.TaskVanishedListener listener1 = getSelfRemovingVanishedListener(

                mOrganizer, calledListenerCount);

        ShellTaskOrganizer.TaskVanishedListener listener2 = getSelfRemovingVanishedListener(

                mOrganizer, calledListenerCount);

        mOrganizer.addTaskVanishedListener(listener1);

        mOrganizer.addTaskVanishedListener(listener2);

        mOrganizer.onTaskVanished(task1);



        assertEquals(2, calledListenerCount.get());



        mOrganizer.onTaskAppeared(task1, /* leash= */ null);

        mOrganizer.onTaskVanished(task1);



        // Count should remain the same if no new vanished listeners are added.

        assertEquals(2, calledListenerCount.get());

    }



    @Test

    public void testHomeTaskOnDefaultDisplay() {

        RunningTaskInfo taskInfo = createTaskInfo(
@@ -715,6 +739,17 @@ public class ShellTaskOrganizerTests extends ShellTestCase { 
        assertEquals(mOrganizer.getHomeTaskSurface(/* displayId= */ 2), taskLeash);

    }



    private static ShellTaskOrganizer.TaskVanishedListener getSelfRemovingVanishedListener(

            ShellTaskOrganizer shellTaskOrganizer, AtomicInteger taskVanishedCalls) {

        return new ShellTaskOrganizer.TaskVanishedListener() {

            @Override

            public void onTaskVanished(RunningTaskInfo taskInfo) {

                shellTaskOrganizer.removeTaskVanishedListener(this);

                taskVanishedCalls.incrementAndGet();

            }

        };

    }



    private static RunningTaskInfo createTaskInfo(int taskId, int windowingMode) {

        RunningTaskInfo taskInfo = new RunningTaskInfo();

        taskInfo.taskId = taskId;