From 5ac04e358cb88c0c0708bd46278662954e27e367 Mon Sep 17 00:00:00 2001 From: Joanne Chung Date: Wed, 5 Jul 2023 10:15:06 +0000 Subject: [PATCH 1/2] Fix EphemeralTest#testGetSearchableInfo The failed root cause is the PackageMonitorCallbackHelper misses handling instant cases. Add instantUserIds checking to fix the issue. Bug: 289797357 Test: atest EphemeralTest#testGetSearchableInfo Test: atest PackageMonitorCallbackHelperTest Change-Id: Ib5d4869c906cc996620b52e9c137c12a106d03f3 --- .../server/pm/PackageManagerService.java | 10 ++++++---- .../pm/PackageMonitorCallbackHelper.java | 20 +++++++++++++------ .../server/pm/SuspendPackageHelper.java | 3 ++- .../pm/PackageMonitorCallbackHelperTest.java | 16 +++++++++------ 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index af6c1a25967cc..477e120695e6b 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -2998,12 +2998,14 @@ public class PackageManagerService implements PackageSender, TestUtilityService // action. When the targetPkg is set, it sends the broadcast to specific app, e.g. // installer app or null for registered apps. The callback only need to send back to the // registered apps so we check the null condition here. - notifyPackageMonitor(action, pkg, extras, userIds); + notifyPackageMonitor(action, pkg, extras, userIds, instantUserIds); } } - void notifyPackageMonitor(String action, String pkg, Bundle extras, int[] userIds) { - mPackageMonitorCallbackHelper.notifyPackageMonitor(action, pkg, extras, userIds); + void notifyPackageMonitor(String action, String pkg, Bundle extras, int[] userIds, + int[] instantUserIds) { + mPackageMonitorCallbackHelper.notifyPackageMonitor(action, pkg, extras, userIds, + instantUserIds); } void notifyResourcesChanged(boolean mediaStatus, boolean replacing, @@ -4053,7 +4055,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService packageName, dontKillApp, componentNames, packageUid, reason, userIds, instantUserIds, broadcastAllowList)); mPackageMonitorCallbackHelper.notifyPackageChanged(packageName, dontKillApp, componentNames, - packageUid, reason, userIds); + packageUid, reason, userIds, instantUserIds); } /** diff --git a/services/core/java/com/android/server/pm/PackageMonitorCallbackHelper.java b/services/core/java/com/android/server/pm/PackageMonitorCallbackHelper.java index c582321da1eca..55823cfe41104 100644 --- a/services/core/java/com/android/server/pm/PackageMonitorCallbackHelper.java +++ b/services/core/java/com/android/server/pm/PackageMonitorCallbackHelper.java @@ -78,7 +78,7 @@ class PackageMonitorCallbackHelper { extras.putInt(Intent.EXTRA_UID, uid); extras.putInt(PackageInstaller.EXTRA_DATA_LOADER_TYPE, dataLoaderType); notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, packageName, extras , - userIds /* userIds */); + userIds /* userIds */, instantUserIds); } public void notifyResourcesChanged(boolean mediaStatus, boolean replacing, @@ -91,11 +91,13 @@ class PackageMonitorCallbackHelper { } String action = mediaStatus ? Intent.ACTION_EXTERNAL_APPLICATIONS_AVAILABLE : Intent.ACTION_EXTERNAL_APPLICATIONS_UNAVAILABLE; - notifyPackageMonitor(action, null /* pkg */, extras, null /* userIds */); + notifyPackageMonitor(action, null /* pkg */, extras, null /* userIds */, + null /* instantUserIds */); } public void notifyPackageChanged(String packageName, boolean dontKillApp, - ArrayList componentNames, int packageUid, String reason, int[] userIds) { + ArrayList componentNames, int packageUid, String reason, int[] userIds, + int[] instantUserIds) { Bundle extras = new Bundle(4); extras.putString(Intent.EXTRA_CHANGED_COMPONENT_NAME, componentNames.get(0)); String[] nameList = new String[componentNames.size()]; @@ -106,11 +108,12 @@ class PackageMonitorCallbackHelper { if (reason != null) { extras.putString(Intent.EXTRA_REASON, reason); } - notifyPackageMonitor(Intent.ACTION_PACKAGE_CHANGED, packageName, extras, userIds); + notifyPackageMonitor(Intent.ACTION_PACKAGE_CHANGED, packageName, extras, userIds, + instantUserIds); } public void notifyPackageMonitor(String action, String pkg, Bundle extras, - int[] userIds) { + int[] userIds, int[] instantUserIds) { if (!isAllowedCallbackAction(action)) { return; } @@ -122,7 +125,12 @@ class PackageMonitorCallbackHelper { } else { resolvedUserIds = userIds; } - doNotifyCallbacks(action, pkg, extras, resolvedUserIds); + + if (ArrayUtils.isEmpty(instantUserIds)) { + doNotifyCallbacks(action, pkg, extras, resolvedUserIds); + } else { + doNotifyCallbacks(action, pkg, extras, instantUserIds); + } } catch (RemoteException e) { // do nothing } diff --git a/services/core/java/com/android/server/pm/SuspendPackageHelper.java b/services/core/java/com/android/server/pm/SuspendPackageHelper.java index 89aff9eec4cf5..893bc11adf5c0 100644 --- a/services/core/java/com/android/server/pm/SuspendPackageHelper.java +++ b/services/core/java/com/android/server/pm/SuspendPackageHelper.java @@ -633,7 +633,8 @@ public final class SuspendPackageHelper { (callingUid, intentExtras) -> BroadcastHelper.filterExtrasChangedPackageList( mPm.snapshotComputer(), callingUid, intentExtras), options)); - mPm.notifyPackageMonitor(intent, null /* pkg */, extras, new int[]{userId}); + mPm.notifyPackageMonitor(intent, null /* pkg */, extras, new int[]{userId}, + null /* instantUserIds */); } /** diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/PackageMonitorCallbackHelperTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/PackageMonitorCallbackHelperTest.java index 2c2b1f52a8443..6f2cca530f04f 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/PackageMonitorCallbackHelperTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/PackageMonitorCallbackHelperTest.java @@ -77,7 +77,8 @@ public class PackageMonitorCallbackHelperTest { IRemoteCallback callback = createMockPackageMonitorCallback(); mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, - FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */); + FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */, + null /* instantUserIds */); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).never()).sendResult(any()); } @@ -88,14 +89,15 @@ public class PackageMonitorCallbackHelperTest { mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */); mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, - FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0}); + FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0}, null /* instantUserIds */); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).times(1)).sendResult(any()); reset(callback); mPackageMonitorCallbackHelper.unregisterPackageMonitorCallback(callback); mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, - FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */); + FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */, + null /* instantUserIds */); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).never()).sendResult(any()); } @@ -106,7 +108,8 @@ public class PackageMonitorCallbackHelperTest { mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */); mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, - FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */); + FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */, + null /* instantUserIds */); ArgumentCaptor bundleCaptor = ArgumentCaptor.forClass(Bundle.class); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).times(1)).sendResult( @@ -128,7 +131,8 @@ public class PackageMonitorCallbackHelperTest { mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */); // Notify for user 10 mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, - FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{10} /* userIds */); + FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{10} /* userIds */, + null /* instantUserIds */); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).never()).sendResult(any()); } @@ -143,7 +147,7 @@ public class PackageMonitorCallbackHelperTest { mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */); mPackageMonitorCallbackHelper.notifyPackageChanged(FAKE_PACKAGE_NAME, false /* dontKillApp */, components, FAKE_PACKAGE_UID, null /* reason */, - new int[]{0} /* userIds */); + new int[]{0} /* userIds */, null /* instantUserIds */); ArgumentCaptor bundleCaptor = ArgumentCaptor.forClass(Bundle.class); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).times(1)).sendResult( From 897c83693f828b9a65f4b9508876d4ae9d8ea4f5 Mon Sep 17 00:00:00 2001 From: Joanne Chung Date: Wed, 5 Jul 2023 10:48:31 +0000 Subject: [PATCH 2/2] Run EphemeralTest#testGetSearchableInfo in presubmit when changing PackageMonitorCallbackHelper Does't catch the issue in presubmit. Because this may be impacted by the logic in PackageMonitorCallbackHelper. For someone who doesn't familiar with the topic may miss it. Try to add into presubmit. The running time is short, it should be acceptable to add into presubmit. Bug: 289797357 Test: TH result Change-Id: I887686a7699965361758c189f3a6f87e0b866c66 --- services/core/java/com/android/server/pm/TEST_MAPPING | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/services/core/java/com/android/server/pm/TEST_MAPPING b/services/core/java/com/android/server/pm/TEST_MAPPING index a622d0743bbb0..01e0fbdd0fee7 100644 --- a/services/core/java/com/android/server/pm/TEST_MAPPING +++ b/services/core/java/com/android/server/pm/TEST_MAPPING @@ -88,6 +88,15 @@ "include-filter": "android.uidmigration.cts" } ] + }, + { + "name": "CtsAppSecurityHostTestCases", + "file_patterns": ["(/|^)PackageMonitorCallbackHelper\\.java"], + "options": [ + { + "include-filter": "android.appsecurity.cts.EphemeralTest#testGetSearchableInfo" + } + ] } ], "presubmit-large": [