From 6ba68bb08adc1db3f3aa23ac5f6ba7c25145a720 Mon Sep 17 00:00:00 2001 From: Garfield Tan Date: Thu, 29 Apr 2021 13:47:27 -0700 Subject: Fix 2 LaunchParamsPersister issues The first one is it erroneously accepts tasks with null realActivity. It shouldn't save launch params for such tasks. The second one is when it receives a package removed notification from PackageManagerService, it touches mLaunchParamsMap without holding WM global lock. Bug: 186696426 Test: atest LaunchParamsPersister Change-Id: Ib0159835f177b7a3e9208bf77b3b608920262588 --- .../com/android/server/wm/LaunchParamsPersister.java | 9 +++++++-- .../android/server/wm/LaunchParamsPersisterTests.java | 17 ++++++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/wm/LaunchParamsPersister.java b/services/core/java/com/android/server/wm/LaunchParamsPersister.java index 391e6598dca2..be3ceb8ff8cd 100644 --- a/services/core/java/com/android/server/wm/LaunchParamsPersister.java +++ b/services/core/java/com/android/server/wm/LaunchParamsPersister.java @@ -232,6 +232,9 @@ class LaunchParamsPersister { void saveTask(Task task, DisplayContent display) { final ComponentName name = task.realActivity; + if (name == null) { + return; + } final int userId = task.mUserId; PersistableLaunchParams params; ArrayMap map = mLaunchParamsMap.get(userId); @@ -381,11 +384,13 @@ class LaunchParamsPersister { private class PackageListObserver implements PackageManagerInternal.PackageListObserver { @Override - public void onPackageAdded(String packageName, int uid) { } + public void onPackageAdded(String packageName, int uid) {} @Override public void onPackageRemoved(String packageName, int uid) { - removeRecordForPackage(packageName); + synchronized (mSupervisor.mService.getGlobalLock()) { + removeRecordForPackage(packageName); + } } } diff --git a/services/tests/wmtests/src/com/android/server/wm/LaunchParamsPersisterTests.java b/services/tests/wmtests/src/com/android/server/wm/LaunchParamsPersisterTests.java index b7d8638f06a8..7cb7c79d63a0 100644 --- a/services/tests/wmtests/src/com/android/server/wm/LaunchParamsPersisterTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/LaunchParamsPersisterTests.java @@ -114,10 +114,10 @@ public class LaunchParamsPersisterTests extends WindowTestsBase { when(mRootWindowContainer.getDisplayContent(eq(mDisplayUniqueId))) .thenReturn(mTestDisplay); - Task stack = mTestDisplay.getDefaultTaskDisplayArea() + Task rootTask = mTestDisplay.getDefaultTaskDisplayArea() .createRootTask(TEST_WINDOWING_MODE, ACTIVITY_TYPE_STANDARD, /* onTop */ true); - mTestTask = new TaskBuilder(mSupervisor).setComponent(TEST_COMPONENT).setParentTask(stack) - .build(); + mTestTask = new TaskBuilder(mSupervisor).setComponent(TEST_COMPONENT) + .setParentTask(rootTask).build(); mTestTask.mUserId = TEST_USER_ID; mTestTask.mLastNonFullscreenBounds = TEST_BOUNDS; mTestTask.setHasBeenVisible(true); @@ -157,6 +157,17 @@ public class LaunchParamsPersisterTests extends WindowTestsBase { assertTrue("Default result should be empty.", mResult.isEmpty()); } + @Test + public void testSavingTestWithoutRealActivityWontMakePackageRemovalCrash() { + Task rootTask = mTestDisplay.getDefaultTaskDisplayArea() + .createRootTask(TEST_WINDOWING_MODE, ACTIVITY_TYPE_STANDARD, /* onTop */ true); + assertNull(rootTask.realActivity); + + mTarget.saveTask(rootTask); + + mTarget.removeRecordForPackage(TEST_COMPONENT.getPackageName()); + } + @Test public void testSavesAndRestoresLaunchParamsInSameInstance() { mTarget.saveTask(mTestTask); -- cgit v1.2.3-59-g8ed1b