Merge "Fix issue with leaking death recipients" into tm-qpr-dev am: b1f85974bc

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/20889212

Change-Id: Iad87d61427cc5863c8dd7a3a5b0893cd57106d64
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

libs/WindowManager/Shell/src/com/android/wm/shell/common/SingleInstanceRemoteListener.java

@@ -29,6 +29,9 @@ import java.util.function.Consumer;
  * Manages the lifecycle of a single instance of a remote listener, including the clean up if the
  * remote process dies.  All calls on this class should happen on the main shell thread.
  *
+ * Any external interface using this listener should also unregister the listener when it is
+ * invalidated, otherwise it may leak binder death recipients.
+ *
  * @param <C> The controller (must be RemoteCallable)
  * @param <L> The remote listener interface type
  */

libs/WindowManager/Shell/src/com/android/wm/shell/pip/phone/PipController.java

@@ -207,7 +207,8 @@ public class PipController implements PipTransitionController.PipTransitionCallb
 
     private Consumer<Boolean> mOnIsInPipStateChangedListener;
 
-    private interface PipAnimationListener {
+    @VisibleForTesting
+    interface PipAnimationListener {
         /**
          * Notifies the listener that the Pip animation is started.
          */
@@ -434,11 +435,7 @@ public class PipController implements PipTransitionController.PipTransitionCallb
             Optional<OneHandedController> oneHandedController,
             ShellExecutor mainExecutor
     ) {
-        // Ensure that we are the primary user's SystemUI.
+
-        final int processUser = UserManager.get(context).getProcessUserId();
-        if (processUser != UserHandle.USER_SYSTEM) {
-            throw new IllegalStateException("Non-primary Pip component not currently supported.");
-        }
 
         mContext = context;
         mShellCommandHandler = shellCommandHandler;
@@ -872,11 +869,17 @@ public class PipController implements PipTransitionController.PipTransitionCallb
                 animationType == PipAnimationController.ANIM_TYPE_BOUNDS);
     }
 
-    private void setPinnedStackAnimationListener(PipAnimationListener callback) {
+    @VisibleForTesting
+    void setPinnedStackAnimationListener(PipAnimationListener callback) {
         mPinnedStackAnimationRecentsCallback = callback;
         onPipResourceDimensionsChanged();
     }
 
+    @VisibleForTesting
+    boolean hasPinnedStackAnimationListener() {
+        return mPinnedStackAnimationRecentsCallback != null;
+    }
+
     private void onPipResourceDimensionsChanged() {
         if (mPinnedStackAnimationRecentsCallback != null) {
             mPinnedStackAnimationRecentsCallback.onPipResourceDimensionsChanged(
@@ -1166,6 +1169,8 @@ public class PipController implements PipTransitionController.PipTransitionCallb
         @Override
         public void invalidate() {
             mController = null;
+            // Unregister the listener to ensure any registered binder death recipients are unlinked
+            mListener.unregister();
         }
 
         @Override

libs/WindowManager/Shell/src/com/android/wm/shell/recents/RecentTasksController.java

@@ -280,14 +280,21 @@ public class RecentTasksController implements TaskStackListenerCallback,
         }
     }
 
-    private void registerRecentTasksListener(IRecentTasksListener listener) {
+    @VisibleForTesting
+    void registerRecentTasksListener(IRecentTasksListener listener) {
         mListener = listener;
     }
 
-    private void unregisterRecentTasksListener() {
+    @VisibleForTesting
+    void unregisterRecentTasksListener() {
         mListener = null;
     }
 
+    @VisibleForTesting
+    boolean hasRecentTasksListener() {
+        return mListener != null;
+    }
+
     @VisibleForTesting
     ArrayList<GroupedRecentTaskInfo> getRecentTasks(int maxNum, int flags, int userId) {
         // Note: the returned task list is from the most-recent to least-recent order
@@ -442,6 +449,8 @@ public class RecentTasksController implements TaskStackListenerCallback,
         @Override
         public void invalidate() {
             mController = null;
+            // Unregister the listener to ensure any registered binder death recipients are unlinked
+            mListener.unregister();
         }
 
         @Override

libs/WindowManager/Shell/src/com/android/wm/shell/splitscreen/SplitScreenController.java

@@ -954,6 +954,8 @@ public class SplitScreenController implements DragAndDropPolicy.Starter,
         @Override
         public void invalidate() {
             mController = null;
+            // Unregister the listener to ensure any registered binder death recipients are unlinked
+            mListener.unregister();
         }
 
         @Override

libs/WindowManager/Shell/src/com/android/wm/shell/startingsurface/StartingWindowController.java

@@ -39,6 +39,7 @@ import android.window.TaskOrganizer;
 import android.window.TaskSnapshot;
 
 import androidx.annotation.BinderThread;
+import androidx.annotation.VisibleForTesting;
 
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.util.function.TriConsumer;
@@ -138,10 +139,16 @@ public class StartingWindowController implements RemoteCallable<StartingWindowCo
      *
      * @param listener The callback when need a starting window.
      */
+    @VisibleForTesting
     void setStartingWindowListener(TriConsumer<Integer, Integer, Integer> listener) {
         mTaskLaunchingCallback = listener;
     }
 
+    @VisibleForTesting
+    boolean hasStartingWindowListener() {
+        return mTaskLaunchingCallback != null;
+    }
+
     /**
      * Called when a task need a starting window.
      */
@@ -281,6 +288,8 @@ public class StartingWindowController implements RemoteCallable<StartingWindowCo
         @Override
         public void invalidate() {
             mController = null;
+            // Unregister the listener to ensure any registered binder death recipients are unlinked
+            mListener.unregister();
         }
 
         @Override

libs/WindowManager/Shell/src/com/android/wm/shell/sysui/ShellController.java

@@ -164,7 +164,8 @@ public class ShellController {
      * Updates the given bundle with the set of external interfaces, invalidating the old set of
      * binders.
      */
-    private void createExternalInterfaces(Bundle output) {
+    @VisibleForTesting
+    public void createExternalInterfaces(Bundle output) {
         // Invalidate the old binders
         for (int i = 0; i < mExternalInterfaces.size(); i++) {
             mExternalInterfaces.valueAt(i).invalidate();

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/phone/PipControllerTest.java

@@ -18,7 +18,9 @@ package com.android.wm.shell.pip.phone;
 
 import static android.content.pm.PackageManager.FEATURE_PICTURE_IN_PICTURE;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
@@ -38,6 +40,7 @@ import android.content.pm.PackageManager;
 import android.content.res.Configuration;
 import android.graphics.Point;
 import android.graphics.Rect;
+import android.os.Bundle;
 import android.os.RemoteException;
 import android.test.suitebuilder.annotation.SmallTest;
 import android.testing.AndroidTestingRunner;
@@ -62,6 +65,7 @@ import com.android.wm.shell.pip.PipSnapAlgorithm;
 import com.android.wm.shell.pip.PipTaskOrganizer;
 import com.android.wm.shell.pip.PipTransitionController;
 import com.android.wm.shell.pip.PipTransitionState;
+import com.android.wm.shell.recents.IRecentTasksListener;
 import com.android.wm.shell.sysui.ShellCommandHandler;
 import com.android.wm.shell.sysui.ShellController;
 import com.android.wm.shell.sysui.ShellInit;
@@ -187,6 +191,24 @@ public class PipControllerTest extends ShellTestCase {
         verify(mMockDisplayController).addDisplayWindowListener(any());
     }
 
+    @Test
+    public void testInvalidateExternalInterface_unregistersListener() {
+        mPipController.setPinnedStackAnimationListener(new PipController.PipAnimationListener() {
+            @Override
+            public void onPipAnimationStarted() {}
+            @Override
+            public void onPipResourceDimensionsChanged(int cornerRadius, int shadowRadius) {}
+            @Override
+            public void onExpandPip() {}
+        });
+        assertTrue(mPipController.hasPinnedStackAnimationListener());
+        // Create initial interface
+        mShellController.createExternalInterfaces(new Bundle());
+        // Recreate the interface to trigger invalidation of the previous instance
+        mShellController.createExternalInterfaces(new Bundle());
+        assertFalse(mPipController.hasPinnedStackAnimationListener());
+    }
+
     @Test
     public void createPip_notSupported_returnsNull() {
         Context spyContext = spy(mContext);

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

@@ -23,6 +23,7 @@ import static android.app.WindowConfiguration.WINDOWING_MODE_MULTI_WINDOW;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -45,6 +46,7 @@ import android.app.ActivityTaskManager;
 import android.content.Context;
 import android.content.pm.PackageManager;
 import android.graphics.Rect;
+import android.os.Bundle;
 import android.view.SurfaceControl;
 
 import androidx.test.filters.SmallTest;
@@ -87,8 +89,6 @@ public class RecentTasksControllerTest extends ShellTestCase {
     @Mock
     private TaskStackListenerImpl mTaskStackListener;
     @Mock
-    private ShellController mShellController;
-    @Mock
     private ShellCommandHandler mShellCommandHandler;
     @Mock
     private DesktopModeTaskRepository mDesktopModeTaskRepository;
@@ -97,7 +97,9 @@ public class RecentTasksControllerTest extends ShellTestCase {
 
     private ShellTaskOrganizer mShellTaskOrganizer;
     private RecentTasksController mRecentTasksController;
+    private RecentTasksController mRecentTasksControllerReal;
     private ShellInit mShellInit;
+    private ShellController mShellController;
     private TestShellExecutor mMainExecutor;
 
     @Before
@@ -105,9 +107,12 @@ public class RecentTasksControllerTest extends ShellTestCase {
         mMainExecutor = new TestShellExecutor();
         when(mContext.getPackageManager()).thenReturn(mock(PackageManager.class));
         mShellInit = spy(new ShellInit(mMainExecutor));
-        mRecentTasksController = spy(new RecentTasksController(mContext, mShellInit,
+        mShellController = spy(new ShellController(mShellInit, mShellCommandHandler,
+                mMainExecutor));
+        mRecentTasksControllerReal = new RecentTasksController(mContext, mShellInit,
                 mShellController, mShellCommandHandler, mTaskStackListener, mActivityTaskManager,
-                Optional.of(mDesktopModeTaskRepository), mMainExecutor));
+                Optional.of(mDesktopModeTaskRepository), mMainExecutor);
+        mRecentTasksController = spy(mRecentTasksControllerReal);
         mShellTaskOrganizer = new ShellTaskOrganizer(mShellInit, mShellCommandHandler,
                 null /* sizeCompatUI */, Optional.empty(), Optional.of(mRecentTasksController),
                 mMainExecutor);
@@ -131,6 +136,20 @@ public class RecentTasksControllerTest extends ShellTestCase {
                 eq(ShellSharedConstants.KEY_EXTRA_SHELL_RECENT_TASKS), any(), any());
     }
 
+    @Test
+    public void testInvalidateExternalInterface_unregistersListener() {
+        // Note: We have to use the real instance of the controller here since that is the instance
+        // that is passed to ShellController internally, and the instance that the listener will be
+        // unregistered from
+        mRecentTasksControllerReal.registerRecentTasksListener(new IRecentTasksListener.Default());
+        assertTrue(mRecentTasksControllerReal.hasRecentTasksListener());
+        // Create initial interface
+        mShellController.createExternalInterfaces(new Bundle());
+        // Recreate the interface to trigger invalidation of the previous instance
+        mShellController.createExternalInterfaces(new Bundle());
+        assertFalse(mRecentTasksControllerReal.hasRecentTasksListener());
+    }
+
     @Test
     public void testAddRemoveSplitNotifyChange() {
         ActivityManager.RecentTaskInfo t1 = makeTaskInfo(1);

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

@@ -27,11 +27,14 @@ import static com.android.wm.shell.common.split.SplitScreenConstants.SPLIT_POSIT
 import static com.android.wm.shell.common.split.SplitScreenConstants.SPLIT_POSITION_TOP_OR_LEFT;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.ArgumentMatchers.isNull;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
@@ -46,6 +49,7 @@ import android.app.PendingIntent;
 import android.content.ComponentName;
 import android.content.Intent;
 import android.content.pm.ActivityInfo;
+import android.os.Bundle;
 
 import androidx.test.annotation.UiThreadTest;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
@@ -86,7 +90,6 @@ import org.mockito.MockitoAnnotations;
 public class SplitScreenControllerTests extends ShellTestCase {
 
     @Mock ShellInit mShellInit;
-    @Mock ShellController mShellController;
     @Mock ShellCommandHandler mShellCommandHandler;
     @Mock ShellTaskOrganizer mTaskOrganizer;
     @Mock SyncTransactionQueue mSyncQueue;
@@ -103,12 +106,15 @@ public class SplitScreenControllerTests extends ShellTestCase {
     @Mock RecentTasksController mRecentTasks;
     @Captor ArgumentCaptor<Intent> mIntentCaptor;
 
+    private ShellController mShellController;
     private SplitScreenController mSplitScreenController;
 
     @Before
     public void setup() {
         assumeTrue(ActivityTaskManager.supportsSplitScreenMultiWindow(mContext));
         MockitoAnnotations.initMocks(this);
+        mShellController = spy(new ShellController(mShellInit, mShellCommandHandler,
+                mMainExecutor));
         mSplitScreenController = spy(new SplitScreenController(mContext, mShellInit,
                 mShellCommandHandler, mShellController, mTaskOrganizer, mSyncQueue,
                 mRootTDAOrganizer, mDisplayController, mDisplayImeController,
@@ -118,7 +124,7 @@ public class SplitScreenControllerTests extends ShellTestCase {
 
     @Test
     public void instantiateController_addInitCallback() {
-        verify(mShellInit, times(1)).addInitCallback(any(), any());
+        verify(mShellInit, times(1)).addInitCallback(any(), isA(SplitScreenController.class));
     }
 
     @Test
@@ -158,6 +164,19 @@ public class SplitScreenControllerTests extends ShellTestCase {
                 eq(ShellSharedConstants.KEY_EXTRA_SHELL_SPLIT_SCREEN), any(), any());
     }
 
+    @Test
+    public void testInvalidateExternalInterface_unregistersListener() {
+        mSplitScreenController.onInit();
+        mSplitScreenController.registerSplitScreenListener(
+                new SplitScreen.SplitScreenListener() {});
+        verify(mStageCoordinator).registerSplitScreenListener(any());
+        // Create initial interface
+        mShellController.createExternalInterfaces(new Bundle());
+        // Recreate the interface to trigger invalidation of the previous instance
+        mShellController.createExternalInterfaces(new Bundle());
+        verify(mStageCoordinator).unregisterSplitScreenListener(any());
+    }
+
     @Test
     public void testStartIntent_appendsNoUserActionFlag() {
         Intent startIntent = createStartIntent("startActivity");

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

@@ -16,9 +16,12 @@
 
 package com.android.wm.shell.startingsurface;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
@@ -27,16 +30,19 @@ import static org.mockito.Mockito.verify;
 
 import android.content.Context;
 import android.hardware.display.DisplayManager;
+import android.os.Bundle;
 import android.view.Display;
 
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import androidx.test.filters.SmallTest;
 
+import com.android.internal.util.function.TriConsumer;
 import com.android.launcher3.icons.IconProvider;
 import com.android.wm.shell.ShellTaskOrganizer;
 import com.android.wm.shell.ShellTestCase;
 import com.android.wm.shell.common.ShellExecutor;
 import com.android.wm.shell.common.TransactionPool;
+import com.android.wm.shell.sysui.ShellCommandHandler;
 import com.android.wm.shell.sysui.ShellController;
 import com.android.wm.shell.sysui.ShellInit;
 import com.android.wm.shell.sysui.ShellSharedConstants;
@@ -59,7 +65,7 @@ public class StartingWindowControllerTests extends ShellTestCase {
 
     private @Mock Context mContext;
     private @Mock DisplayManager mDisplayManager;
-    private @Mock ShellController mShellController;
+    private @Mock ShellCommandHandler mShellCommandHandler;
     private @Mock ShellTaskOrganizer mTaskOrganizer;
     private @Mock ShellExecutor mMainExecutor;
     private @Mock StartingWindowTypeAlgorithm mTypeAlgorithm;
@@ -67,6 +73,7 @@ public class StartingWindowControllerTests extends ShellTestCase {
     private @Mock TransactionPool mTransactionPool;
     private StartingWindowController mController;
     private ShellInit mShellInit;
+    private ShellController mShellController;
 
     @Before
     public void setUp() {
@@ -74,6 +81,8 @@ public class StartingWindowControllerTests extends ShellTestCase {
         doReturn(mock(Display.class)).when(mDisplayManager).getDisplay(anyInt());
         doReturn(mDisplayManager).when(mContext).getSystemService(eq(DisplayManager.class));
         mShellInit = spy(new ShellInit(mMainExecutor));
+        mShellController = spy(new ShellController(mShellInit, mShellCommandHandler,
+                mMainExecutor));
         mController = new StartingWindowController(mContext, mShellInit, mShellController,
                 mTaskOrganizer, mMainExecutor, mTypeAlgorithm, mIconProvider, mTransactionPool);
         mShellInit.init();
@@ -81,7 +90,7 @@ public class StartingWindowControllerTests extends ShellTestCase {
 
     @Test
     public void instantiateController_addInitCallback() {
-        verify(mShellInit, times(1)).addInitCallback(any(), any());
+        verify(mShellInit, times(1)).addInitCallback(any(), isA(StartingWindowController.class));
     }
 
     @Test
@@ -89,4 +98,18 @@ public class StartingWindowControllerTests extends ShellTestCase {
         verify(mShellController, times(1)).addExternalInterface(
                 eq(ShellSharedConstants.KEY_EXTRA_SHELL_STARTING_WINDOW), any(), any());
     }
+
+    @Test
+    public void testInvalidateExternalInterface_unregistersListener() {
+        mController.setStartingWindowListener(new TriConsumer<Integer, Integer, Integer>() {
+            @Override
+            public void accept(Integer integer, Integer integer2, Integer integer3) {}
+        });
+        assertTrue(mController.hasStartingWindowListener());
+        // Create initial interface
+        mShellController.createExternalInterfaces(new Bundle());
+        // Recreate the interface to trigger invalidation of the previous instance
+        mShellController.createExternalInterfaces(new Bundle());
+        assertFalse(mController.hasStartingWindowListener());
+    }
 }