From 3482ed4c25602cbc7c3e3b76d62f6a804ee0d181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Hern=C3=A1ndez?= Date: Thu, 12 Jan 2023 13:14:13 +0100 Subject: [PATCH] Snooze NMS ManagedServices per user id, not globally Listeners are bound and registered per user, so snoozing should be that way as well. Test: atest ManagedServicesTest (and manually) Fixes: 147075801 Fixes: 167152273 Change-Id: Ia61816518aa00f8a89f22e79ffdc048f3066541d --- core/java/android/util/SparseSetArray.java | 5 ++ core/proto/android/service/notification.proto | 13 +++- .../server/notification/ManagedServices.java | 56 +++++++++++----- .../notification/ManagedServicesTest.java | 65 ++++++++++++++++++- 4 files changed, 119 insertions(+), 20 deletions(-) diff --git a/core/java/android/util/SparseSetArray.java b/core/java/android/util/SparseSetArray.java index b7873b73cb28a..61f29a40ff501 100644 --- a/core/java/android/util/SparseSetArray.java +++ b/core/java/android/util/SparseSetArray.java @@ -139,4 +139,9 @@ public class SparseSetArray { public T valueAt(int intIndex, int valueIndex) { return mData.valueAt(intIndex).valueAt(valueIndex); } + + /** @return The set of values for key at position {@code intIndex}. */ + public ArraySet valuesAt(int intIndex) { + return mData.valueAt(intIndex); + } } diff --git a/core/proto/android/service/notification.proto b/core/proto/android/service/notification.proto index 8e4006aa68614..e029af4f9819c 100644 --- a/core/proto/android/service/notification.proto +++ b/core/proto/android/service/notification.proto @@ -110,11 +110,20 @@ message ManagedServicesProto { // All of this type/caption enabled for current profiles. repeated android.content.ComponentNameProto enabled = 3; - repeated ManagedServiceInfoProto live_services = 4; + // Was: repeated ComponentNameProto, when snoozed services were not per-user-id. + reserved 5; + + message SnoozedServices { + option (android.msg_privacy).dest = DEST_AUTOMATIC; + + optional int32 user_id = 1; + repeated android.content.ComponentNameProto snoozed = 2; + } + // Snoozed for current profiles. - repeated android.content.ComponentNameProto snoozed = 5; + repeated SnoozedServices snoozed = 6; } message RankingHelperProto { diff --git a/services/core/java/com/android/server/notification/ManagedServices.java b/services/core/java/com/android/server/notification/ManagedServices.java index 4d55d4e545eeb..39acaeec89264 100644 --- a/services/core/java/com/android/server/notification/ManagedServices.java +++ b/services/core/java/com/android/server/notification/ManagedServices.java @@ -150,8 +150,9 @@ abstract public class ManagedServices { = new ArraySet<>(); // Just the packages from mEnabledServicesForCurrentProfiles private ArraySet mEnabledServicesPackageNames = new ArraySet<>(); - // List of enabled packages that have nevertheless asked not to be run - private ArraySet mSnoozingForCurrentProfiles = new ArraySet<>(); + // Per user id, list of enabled packages that have nevertheless asked not to be run + private final android.util.SparseSetArray mSnoozing = + new android.util.SparseSetArray<>(); // List of approved packages or components (by user, then by primary/secondary) that are // allowed to be bound as managed services. A package or component appearing in this list does @@ -386,10 +387,15 @@ abstract public class ManagedServices { } } - pw.println(" Snoozed " + getCaption() + "s (" + - mSnoozingForCurrentProfiles.size() + "):"); - for (ComponentName name : mSnoozingForCurrentProfiles) { - pw.println(" " + name.flattenToShortString()); + synchronized (mSnoozing) { + pw.println(" Snoozed " + getCaption() + "s (" + + mSnoozing.size() + "):"); + for (int i = 0; i < mSnoozing.size(); i++) { + pw.println(" User: " + mSnoozing.keyAt(i)); + for (ComponentName name : mSnoozing.valuesAt(i)) { + pw.println(" " + name.flattenToShortString()); + } + } } } @@ -431,8 +437,16 @@ abstract public class ManagedServices { } } - for (ComponentName name : mSnoozingForCurrentProfiles) { - name.dumpDebug(proto, ManagedServicesProto.SNOOZED); + synchronized (mSnoozing) { + for (int i = 0; i < mSnoozing.size(); i++) { + long token = proto.start(ManagedServicesProto.SNOOZED); + proto.write(ManagedServicesProto.SnoozedServices.USER_ID, + mSnoozing.keyAt(i)); + for (ComponentName name : mSnoozing.valuesAt(i)) { + name.dumpDebug(proto, ManagedServicesProto.SnoozedServices.SNOOZED); + } + proto.end(token); + } } } @@ -975,6 +989,9 @@ abstract public class ManagedServices { synchronized (mApproved) { mApproved.remove(user); } + synchronized (mSnoozing) { + mSnoozing.remove(user); + } rebindServices(true, user); } @@ -1066,15 +1083,17 @@ abstract public class ManagedServices { } protected void setComponentState(ComponentName component, int userId, boolean enabled) { - boolean previous = !mSnoozingForCurrentProfiles.contains(component); - if (previous == enabled) { - return; - } + synchronized (mSnoozing) { + boolean previous = !mSnoozing.contains(userId, component); + if (previous == enabled) { + return; + } - if (enabled) { - mSnoozingForCurrentProfiles.remove(component); - } else { - mSnoozingForCurrentProfiles.add(component); + if (enabled) { + mSnoozing.remove(userId, component); + } else { + mSnoozing.add(userId, component); + } } // State changed @@ -1287,7 +1306,10 @@ abstract public class ManagedServices { } final Set add = new HashSet<>(userComponents); - add.removeAll(mSnoozingForCurrentProfiles); + ArraySet snoozed = mSnoozing.get(userId); + if (snoozed != null) { + add.removeAll(snoozed); + } componentsToBind.put(userId, add); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java index 798604306b43f..8b1384ed894fd 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java @@ -1581,6 +1581,55 @@ public class ManagedServicesTest extends UiServiceTestCase { verify(context, never()).unbindService(any()); } + @Test + public void testSetComponentState_differentUsers() throws Exception { + Context context = mock(Context.class); + PackageManager pm = mock(PackageManager.class); + ApplicationInfo ai = new ApplicationInfo(); + ai.targetSdkVersion = Build.VERSION_CODES.CUR_DEVELOPMENT; + + when(context.getPackageName()).thenReturn(mContext.getPackageName()); + when(context.getUserId()).thenReturn(mContext.getUserId()); + when(context.getPackageManager()).thenReturn(pm); + when(pm.getApplicationInfo(anyString(), anyInt())).thenReturn(ai); + + ManagedServices service = new TestManagedServices(context, mLock, mUserProfiles, mIpm, + APPROVAL_BY_COMPONENT); + ComponentName cn = ComponentName.unflattenFromString("a/a"); + + addExpectedServices(service, Arrays.asList("a"), mZero.id); + addExpectedServices(service, Arrays.asList("a"), mTen.id); + when(context.bindServiceAsUser(any(), any(), anyInt(), any())).thenAnswer(invocation -> { + Object[] args = invocation.getArguments(); + ServiceConnection sc = (ServiceConnection) args[1]; + sc.onServiceConnected(cn, mock(IBinder.class)); + return true; + }); + service.addApprovedList("a/a", 0, true); + service.addApprovedList("a/a", 10, false); + + service.registerService(cn, mZero.id); + assertTrue(service.isBound(cn, mZero.id)); + + service.onUserSwitched(mTen.id); + assertFalse(service.isBound(cn, mZero.id)); + service.registerService(cn, mTen.id); + assertTrue(service.isBound(cn, mTen.id)); + + service.setComponentState(cn, mTen.id, false); + assertFalse(service.isBound(cn, mZero.id)); + assertFalse(service.isBound(cn, mTen.id)); + + // Service should be rebound on user 0, since it was only disabled for user 10. + service.onUserSwitched(mZero.id); + assertTrue(service.isBound(cn, mZero.id)); + assertFalse(service.isBound(cn, mTen.id)); + + // Service should stay unbound on going back to user 10. + service.onUserSwitched(mTen.id); + assertFalse(service.isBound(cn, mZero.id)); + assertFalse(service.isBound(cn, mTen.id)); + } @Test public void testOnPackagesChanged_nullValuesPassed_noNullPointers() { for (int approvalLevel : new int[] {APPROVAL_BY_COMPONENT, APPROVAL_BY_PACKAGE}) { @@ -1847,7 +1896,7 @@ public class ManagedServicesTest extends UiServiceTestCase { } private void addExpectedServices(final ManagedServices service, final List packages, - int userId) { + int userId) throws Exception { ManagedServices.Config config = service.getConfig(); when(mPm.queryIntentServicesAsUser(any(), anyInt(), eq(userId))). thenAnswer(new Answer>() { @@ -1876,6 +1925,20 @@ public class ManagedServicesTest extends UiServiceTestCase { return new ArrayList<>(); } }); + + when(mIpm.getServiceInfo(any(), anyLong(), anyInt())).thenAnswer( + (Answer) invocation -> { + ComponentName invocationCn = invocation.getArgument(0); + if (invocationCn != null && packages.contains(invocationCn.getPackageName())) { + ServiceInfo serviceInfo = new ServiceInfo(); + serviceInfo.packageName = invocationCn.getPackageName(); + serviceInfo.name = invocationCn.getClassName(); + serviceInfo.permission = service.getConfig().bindPermission; + return serviceInfo; + } + return null; + } + ); } private List stringToList(String list) {