Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix issue with leaking death recipients

Browse Source 
- When invalidating an existing external interface, we also need to
  unregister any listeners to ensure that binder death recipients are
  unlinked.  Otherwise, a crash in the process for the old listener
  can cause the current listener to be unregistered.

Bug: 242377577
Test: atest WMShellUnitTests
Change-Id: Ibb77ac19e4ebc616b8977db70d6269e3b930848e
This commit is contained in:
Winson Chung
2023-01-06 08:38:40 +00:00
parent 12f428cb7c
commit 9a81477e86
10 changed files with 130 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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
*/

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

@@ -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

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

@@ -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

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

@@ -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

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

@@ -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

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

@@ -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();

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

@@ -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);

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

@@ -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);

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

@@ -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");

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

@@ -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());
}
}