From 546aa10dbf29c9ec144bdd94017b5786c645f06a Mon Sep 17 00:00:00 2001 From: Beverly Date: Mon, 15 Jun 2020 15:25:47 -0400 Subject: [PATCH] Track non-foreground service notifs for appOps Previously we were only updating appOps for notifications with standard layouts that were associated with a foreground services. However, non-foreground service notifications can be tagged with appOps, so we make sure to update these notifications' appOps whenever appOps are changed. (We do this by tracking all notifications with standard layouts instead of just the foreground service notifications) Test: atest AppOpsCoordinatorTest ForegroundServiceControllerTest Test: manual (use camera on whatsApp, receive message with app op, quit app, see that app op is gone on the notification) Fixes: 158585352 Change-Id: I674fc42441c2847a030df03516484ee6cd9217ac Change-Id: I0527b8596277e53bea5a68aa57e0726aea9d14ac --- .../systemui/ForegroundServiceController.java | 52 ++++---- ...ForegroundServiceNotificationListener.java | 26 ++-- .../systemui/ForegroundServicesUserState.java | 18 ++- ...oordinator.java => AppOpsCoordinator.java} | 61 +++++---- .../coordinator/NotifCoordinators.java | 4 +- .../ForegroundServiceControllerTest.java | 125 +++++++++++------- ...orTest.java => AppOpsCoordinatorTest.java} | 123 +++++++++++++---- 7 files changed, 264 insertions(+), 145 deletions(-) rename packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/{ForegroundCoordinator.java => AppOpsCoordinator.java} (84%) rename packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/{ForegroundCoordinatorTest.java => AppOpsCoordinatorTest.java} (71%) diff --git a/packages/SystemUI/src/com/android/systemui/ForegroundServiceController.java b/packages/SystemUI/src/com/android/systemui/ForegroundServiceController.java index 82e665bdf5ac8..2deeb1230f09d 100644 --- a/packages/SystemUI/src/com/android/systemui/ForegroundServiceController.java +++ b/packages/SystemUI/src/com/android/systemui/ForegroundServiceController.java @@ -29,6 +29,8 @@ import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.util.Assert; +import java.util.Set; + import javax.inject.Inject; import javax.inject.Singleton; @@ -62,7 +64,7 @@ public class ForegroundServiceController { /** * @return true if this user has services missing notifications and therefore needs a - * disclosure notification. + * disclosure notification for running a foreground service. */ public boolean isDisclosureNeededForUser(int userId) { synchronized (mMutex) { @@ -74,26 +76,26 @@ public class ForegroundServiceController { /** * @return true if this user/pkg has a missing or custom layout notification and therefore needs - * a disclosure notification for system alert windows. + * a disclosure notification showing the user which appsOps the app is using. */ public boolean isSystemAlertWarningNeeded(int userId, String pkg) { synchronized (mMutex) { final ForegroundServicesUserState services = mUserServices.get(userId); if (services == null) return false; - return services.getStandardLayoutKey(pkg) == null; + return services.getStandardLayoutKeys(pkg) == null; } } /** - * Returns the key of the foreground service from this package using the standard template, - * if one exists. + * Returns the keys for notifications from this package using the standard template, + * if they exist. */ @Nullable - public String getStandardLayoutKey(int userId, String pkg) { + public ArraySet getStandardLayoutKeys(int userId, String pkg) { synchronized (mMutex) { final ForegroundServicesUserState services = mUserServices.get(userId); if (services == null) return null; - return services.getStandardLayoutKey(pkg); + return services.getStandardLayoutKeys(pkg); } } @@ -140,25 +142,27 @@ public class ForegroundServiceController { } // TODO: (b/145659174) remove when moving to NewNotifPipeline. Replaced by - // ForegroundCoordinator - // Update appOp if there's an associated pending or visible notification: - final String foregroundKey = getStandardLayoutKey(userId, packageName); - if (foregroundKey != null) { - final NotificationEntry entry = mEntryManager.getPendingOrActiveNotif(foregroundKey); - if (entry != null - && uid == entry.getSbn().getUid() - && packageName.equals(entry.getSbn().getPackageName())) { - boolean changed; - synchronized (entry.mActiveAppOps) { - if (active) { - changed = entry.mActiveAppOps.add(appOpCode); - } else { - changed = entry.mActiveAppOps.remove(appOpCode); + // AppOpsCoordinator + // Update appOps if there are associated pending or visible notifications + final Set notificationKeys = getStandardLayoutKeys(userId, packageName); + if (notificationKeys != null) { + boolean changed = false; + for (String key : notificationKeys) { + final NotificationEntry entry = mEntryManager.getPendingOrActiveNotif(key); + if (entry != null + && uid == entry.getSbn().getUid() + && packageName.equals(entry.getSbn().getPackageName())) { + synchronized (entry.mActiveAppOps) { + if (active) { + changed |= entry.mActiveAppOps.add(appOpCode); + } else { + changed |= entry.mActiveAppOps.remove(appOpCode); + } } } - if (changed) { - mEntryManager.updateNotifications("appOpChanged pkg=" + packageName); - } + } + if (changed) { + mEntryManager.updateNotifications("appOpChanged pkg=" + packageName); } } } diff --git a/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java b/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java index 650b9a7f9c0cd..bb445832da93d 100644 --- a/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java +++ b/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java @@ -163,31 +163,31 @@ public class ForegroundServiceNotificationListener { userState.addImportantNotification(sbn.getPackageName(), sbn.getKey()); } - final Notification.Builder builder = - Notification.Builder.recoverBuilder( - mContext, sbn.getNotification()); - if (builder.usesStandardHeader()) { - userState.addStandardLayoutNotification( - sbn.getPackageName(), sbn.getKey()); - } + } + final Notification.Builder builder = + Notification.Builder.recoverBuilder( + mContext, sbn.getNotification()); + if (builder.usesStandardHeader()) { + userState.addStandardLayoutNotification( + sbn.getPackageName(), sbn.getKey()); } } - tagForeground(entry); + tagAppOps(entry); return true; }, true /* create if not found */); } // TODO: (b/145659174) remove when moving to NewNotifPipeline. Replaced by - // ForegroundCoordinator - private void tagForeground(NotificationEntry entry) { + // AppOpsCoordinator + private void tagAppOps(NotificationEntry entry) { final StatusBarNotification sbn = entry.getSbn(); ArraySet activeOps = mForegroundServiceController.getAppOps( sbn.getUserId(), sbn.getPackageName()); - if (activeOps != null) { - synchronized (entry.mActiveAppOps) { - entry.mActiveAppOps.clear(); + synchronized (entry.mActiveAppOps) { + entry.mActiveAppOps.clear(); + if (activeOps != null) { entry.mActiveAppOps.addAll(activeOps); } } diff --git a/packages/SystemUI/src/com/android/systemui/ForegroundServicesUserState.java b/packages/SystemUI/src/com/android/systemui/ForegroundServicesUserState.java index 2ef46dca317a2..5c2950f1365de 100644 --- a/packages/SystemUI/src/com/android/systemui/ForegroundServicesUserState.java +++ b/packages/SystemUI/src/com/android/systemui/ForegroundServicesUserState.java @@ -30,9 +30,11 @@ public class ForegroundServicesUserState { private String[] mRunning = null; private long mServiceStartTime = 0; - // package -> sufficiently important posted notification keys + + // package -> sufficiently important posted notification keys that signal an app is + // running a foreground service private ArrayMap> mImportantNotifications = new ArrayMap<>(1); - // package -> standard layout posted notification keys + // package -> standard layout posted notification keys that can display appOps private ArrayMap> mStandardLayoutNotifications = new ArrayMap<>(1); // package -> app ops @@ -110,6 +112,11 @@ public class ForegroundServicesUserState { return found; } + /** + * System disclosures for foreground services are required if an app has a foreground service + * running AND the app hasn't posted its own notification signalling it is running a + * foreground service + */ public boolean isDisclosureNeeded() { if (mRunning != null && System.currentTimeMillis() - mServiceStartTime @@ -129,12 +136,15 @@ public class ForegroundServicesUserState { return mAppOps.get(pkg); } - public String getStandardLayoutKey(String pkg) { + /** + * Gets the notifications with standard layouts associated with this package + */ + public ArraySet getStandardLayoutKeys(String pkg) { final ArraySet set = mStandardLayoutNotifications.get(pkg); if (set == null || set.size() == 0) { return null; } - return set.valueAt(0); + return set; } @Override diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ForegroundCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/AppOpsCoordinator.java similarity index 84% rename from packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ForegroundCoordinator.java rename to packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/AppOpsCoordinator.java index b5b756d6ed9b7..4b244bb189755 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ForegroundCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/AppOpsCoordinator.java @@ -39,8 +39,8 @@ import javax.inject.Inject; import javax.inject.Singleton; /** - * Handles ForegroundService interactions with notifications. - * Tags notifications with appOps. + * Handles ForegroundService and AppOp interactions with notifications. + * Tags notifications with appOps * Lifetime extends notifications associated with an ongoing ForegroundService. * Filters out notifications that represent foreground services that are no longer running * @@ -48,12 +48,10 @@ import javax.inject.Singleton; * frameworks/base/packages/SystemUI/src/com/android/systemui/ForegroundServiceController * frameworks/base/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener * frameworks/base/packages/SystemUI/src/com/android/systemui/ForegroundServiceLifetimeExtender - * - * TODO: AppOps stuff should be spun off into its own coordinator */ @Singleton -public class ForegroundCoordinator implements Coordinator { - private static final String TAG = "ForegroundCoordinator"; +public class AppOpsCoordinator implements Coordinator { + private static final String TAG = "AppOpsCoordinator"; private final ForegroundServiceController mForegroundServiceController; private final AppOpsController mAppOpsController; @@ -62,7 +60,7 @@ public class ForegroundCoordinator implements Coordinator { private NotifPipeline mNotifPipeline; @Inject - public ForegroundCoordinator( + public AppOpsCoordinator( ForegroundServiceController foregroundServiceController, AppOpsController appOpsController, @Main DelayableExecutor mainExecutor) { @@ -89,18 +87,22 @@ public class ForegroundCoordinator implements Coordinator { } /** - * Filters out notifications that represent foreground services that are no longer running. + * Filters out notifications that represent foreground services that are no longer running or + * that already have an app notification with the appOps tagged to */ private final NotifFilter mNotifFilter = new NotifFilter(TAG) { @Override public boolean shouldFilterOut(NotificationEntry entry, long now) { StatusBarNotification sbn = entry.getSbn(); + + // Filters out system-posted disclosure notifications when unneeded if (mForegroundServiceController.isDisclosureNotification(sbn) && !mForegroundServiceController.isDisclosureNeededForUser( sbn.getUser().getIdentifier())) { return true; } + // Filters out system alert notifications when unneeded if (mForegroundServiceController.isSystemAlertNotification(sbn)) { final String[] apps = sbn.getNotification().extras.getStringArray( Notification.EXTRA_FOREGROUND_APPS); @@ -179,23 +181,24 @@ public class ForegroundCoordinator implements Coordinator { private NotifCollectionListener mNotifCollectionListener = new NotifCollectionListener() { @Override public void onEntryAdded(NotificationEntry entry) { - tagForeground(entry); + tagAppOps(entry); } @Override public void onEntryUpdated(NotificationEntry entry) { - tagForeground(entry); + tagAppOps(entry); } - private void tagForeground(NotificationEntry entry) { + private void tagAppOps(NotificationEntry entry) { final StatusBarNotification sbn = entry.getSbn(); // note: requires that the ForegroundServiceController is updating their appOps first ArraySet activeOps = mForegroundServiceController.getAppOps( sbn.getUser().getIdentifier(), sbn.getPackageName()); + + entry.mActiveAppOps.clear(); if (activeOps != null) { - entry.mActiveAppOps.clear(); entry.mActiveAppOps.addAll(activeOps); } } @@ -218,24 +221,26 @@ public class ForegroundCoordinator implements Coordinator { int userId = UserHandle.getUserId(uid); - // Update appOp if there's an associated posted notification: - final String foregroundKey = mForegroundServiceController.getStandardLayoutKey(userId, - packageName); - if (foregroundKey != null) { - final NotificationEntry entry = findNotificationEntryWithKey(foregroundKey); - if (entry != null - && uid == entry.getSbn().getUid() - && packageName.equals(entry.getSbn().getPackageName())) { - boolean changed; - if (active) { - changed = entry.mActiveAppOps.add(code); - } else { - changed = entry.mActiveAppOps.remove(code); - } - if (changed) { - mNotifFilter.invalidateList(); + // Update appOps of the app's posted notifications with standard layouts + final ArraySet notifKeys = + mForegroundServiceController.getStandardLayoutKeys(userId, packageName); + if (notifKeys != null) { + boolean changed = false; + for (int i = 0; i < notifKeys.size(); i++) { + final NotificationEntry entry = findNotificationEntryWithKey(notifKeys.valueAt(i)); + if (entry != null + && uid == entry.getSbn().getUid() + && packageName.equals(entry.getSbn().getPackageName())) { + if (active) { + changed |= entry.mActiveAppOps.add(code); + } else { + changed |= entry.mActiveAppOps.remove(code); + } } } + if (changed) { + mNotifFilter.invalidateList(); + } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java index ac42964395079..99e822c66a8f1 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java @@ -52,7 +52,7 @@ public class NotifCoordinators implements Dumpable { HideNotifsForOtherUsersCoordinator hideNotifsForOtherUsersCoordinator, KeyguardCoordinator keyguardCoordinator, RankingCoordinator rankingCoordinator, - ForegroundCoordinator foregroundCoordinator, + AppOpsCoordinator appOpsCoordinator, DeviceProvisionedCoordinator deviceProvisionedCoordinator, BubbleCoordinator bubbleCoordinator, HeadsUpCoordinator headsUpCoordinator, @@ -64,7 +64,7 @@ public class NotifCoordinators implements Dumpable { mCoordinators.add(hideNotifsForOtherUsersCoordinator); mCoordinators.add(keyguardCoordinator); mCoordinators.add(rankingCoordinator); - mCoordinators.add(foregroundCoordinator); + mCoordinators.add(appOpsCoordinator); mCoordinators.add(deviceProvisionedCoordinator); mCoordinators.add(bubbleCoordinator); if (featureFlags.isNewNotifPipelineRenderingEnabled()) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java index 805254cda1757..60f0cd9da5f28 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java @@ -406,60 +406,76 @@ public class ForegroundServiceControllerTest extends SysuiTestCase { } @Test - public void testStdLayoutBasic() { - final String PKG1 = "com.example.app0"; + public void testNoNotifsNorAppOps_noSystemAlertWarningRequired() { + // no notifications nor app op signals that this package/userId requires system alert + // warning + assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_ONE, "any")); + } - StatusBarNotification sbn_user1_app1 = makeMockFgSBN(USERID_ONE, PKG1, 0, true); - sbn_user1_app1.getNotification().flags = 0; - StatusBarNotification sbn_user1_app1_fg = makeMockFgSBN(USERID_ONE, PKG1, 1, true); - entryAdded(sbn_user1_app1, NotificationManager.IMPORTANCE_MIN); // not fg - assertTrue(mFsc.isSystemAlertWarningNeeded(USERID_ONE, PKG1)); // should be required! - entryAdded(sbn_user1_app1_fg, NotificationManager.IMPORTANCE_MIN); - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_ONE, PKG1)); // app1 has got it covered - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_TWO, "otherpkg")); - // let's take out the non-fg notification and see what happens. - entryRemoved(sbn_user1_app1); - // still covered by sbn_user1_app1_fg - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_ONE, PKG1)); - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_TWO, "anyPkg")); + @Test + public void testCustomLayouts_systemAlertWarningRequired() { + // GIVEN a notification with a custom layout + final String pkg = "com.example.app0"; + StatusBarNotification customLayoutNotif = makeMockSBN(USERID_ONE, pkg, 0, + false); - // let's attempt to downgrade the notification from FLAG_FOREGROUND and see what we get - StatusBarNotification sbn_user1_app1_fg_sneaky = makeMockFgSBN(USERID_ONE, PKG1, 1, true); - sbn_user1_app1_fg_sneaky.getNotification().flags = 0; - entryUpdated(sbn_user1_app1_fg_sneaky, NotificationManager.IMPORTANCE_MIN); - assertTrue(mFsc.isSystemAlertWarningNeeded(USERID_ONE, PKG1)); // should be required! - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_TWO, "anything")); - // ok, ok, we'll put it back - sbn_user1_app1_fg_sneaky.getNotification().flags = Notification.FLAG_FOREGROUND_SERVICE; - entryUpdated(sbn_user1_app1_fg, NotificationManager.IMPORTANCE_MIN); - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_ONE, PKG1)); - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_TWO, "whatever")); + // WHEN the custom layout entry is added + entryAdded(customLayoutNotif, NotificationManager.IMPORTANCE_MIN); - entryRemoved(sbn_user1_app1_fg_sneaky); - assertTrue(mFsc.isSystemAlertWarningNeeded(USERID_ONE, PKG1)); // should be required! - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_TWO, "a")); + // THEN a system alert warning is required since there aren't any notifications that can + // display the app ops + assertTrue(mFsc.isSystemAlertWarningNeeded(USERID_ONE, pkg)); + } - // let's try a custom layout - sbn_user1_app1_fg_sneaky = makeMockFgSBN(USERID_ONE, PKG1, 1, false); - entryUpdated(sbn_user1_app1_fg_sneaky, NotificationManager.IMPORTANCE_MIN); - assertTrue(mFsc.isSystemAlertWarningNeeded(USERID_ONE, PKG1)); // should be required! - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_TWO, "anything")); - // now let's test an upgrade (non fg to fg) - entryAdded(sbn_user1_app1, NotificationManager.IMPORTANCE_MIN); - assertTrue(mFsc.isSystemAlertWarningNeeded(USERID_ONE, PKG1)); - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_TWO, "b")); - sbn_user1_app1.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; - entryUpdated(sbn_user1_app1, - NotificationManager.IMPORTANCE_MIN); // this is now a fg notification + @Test + public void testStandardLayoutExists_noSystemAlertWarningRequired() { + // GIVEN two notifications (one with a custom layout, the other with a standard layout) + final String pkg = "com.example.app0"; + StatusBarNotification customLayoutNotif = makeMockSBN(USERID_ONE, pkg, 0, + false); + StatusBarNotification standardLayoutNotif = makeMockSBN(USERID_ONE, pkg, 1, true); - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_TWO, PKG1)); - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_ONE, PKG1)); + // WHEN the entries are added + entryAdded(customLayoutNotif, NotificationManager.IMPORTANCE_MIN); + entryAdded(standardLayoutNotif, NotificationManager.IMPORTANCE_MIN); - // remove it, make sure we're out of compliance again - entryRemoved(sbn_user1_app1); // was fg, should return true - entryRemoved(sbn_user1_app1); - assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_TWO, PKG1)); - assertTrue(mFsc.isSystemAlertWarningNeeded(USERID_ONE, PKG1)); + // THEN no system alert warning is required, since there is at least one notification + // with a standard layout that can display the app ops on the notification + assertFalse(mFsc.isSystemAlertWarningNeeded(USERID_ONE, pkg)); + } + + @Test + public void testStandardLayoutRemoved_systemAlertWarningRequired() { + // GIVEN two notifications (one with a custom layout, the other with a standard layout) + final String pkg = "com.example.app0"; + StatusBarNotification customLayoutNotif = makeMockSBN(USERID_ONE, pkg, 0, + false); + StatusBarNotification standardLayoutNotif = makeMockSBN(USERID_ONE, pkg, 1, true); + + // WHEN the entries are added and then the standard layout notification is removed + entryAdded(customLayoutNotif, NotificationManager.IMPORTANCE_MIN); + entryAdded(standardLayoutNotif, NotificationManager.IMPORTANCE_MIN); + entryRemoved(standardLayoutNotif); + + // THEN a system alert warning is required since there aren't any notifications that can + // display the app ops + assertTrue(mFsc.isSystemAlertWarningNeeded(USERID_ONE, pkg)); + } + + @Test + public void testStandardLayoutUpdatedToCustomLayout_systemAlertWarningRequired() { + // GIVEN a standard layout notification and then an updated version with a customLayout + final String pkg = "com.example.app0"; + StatusBarNotification standardLayoutNotif = makeMockSBN(USERID_ONE, pkg, 1, true); + StatusBarNotification updatedToCustomLayoutNotif = makeMockSBN(USERID_ONE, pkg, 1, false); + + // WHEN the entries is added and then updated to a custom layout + entryAdded(standardLayoutNotif, NotificationManager.IMPORTANCE_MIN); + entryUpdated(updatedToCustomLayoutNotif, NotificationManager.IMPORTANCE_MIN); + + // THEN a system alert warning is required since there aren't any notifications that can + // display the app ops + assertTrue(mFsc.isSystemAlertWarningNeeded(USERID_ONE, pkg)); } private StatusBarNotification makeMockSBN(int userId, String pkg, int id, String tag, @@ -483,6 +499,19 @@ public class ForegroundServiceControllerTest extends SysuiTestCase { return sbn; } + private StatusBarNotification makeMockSBN(int uid, String pkg, int id, + boolean usesStdLayout) { + StatusBarNotification sbn = makeMockSBN(uid, pkg, id, "foo", 0); + if (usesStdLayout) { + sbn.getNotification().contentView = null; + sbn.getNotification().headsUpContentView = null; + sbn.getNotification().bigContentView = null; + } else { + sbn.getNotification().contentView = mock(RemoteViews.class); + } + return sbn; + } + private StatusBarNotification makeMockFgSBN(int uid, String pkg, int id, boolean usesStdLayout) { StatusBarNotification sbn = diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ForegroundCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/AppOpsCoordinatorTest.java similarity index 71% rename from packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ForegroundCoordinatorTest.java rename to packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/AppOpsCoordinatorTest.java index 407e1e671f861..314b19140e7a3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ForegroundCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/AppOpsCoordinatorTest.java @@ -43,6 +43,7 @@ import com.android.systemui.statusbar.notification.collection.NotifPipeline; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; +import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender; import com.android.systemui.util.concurrency.FakeExecutor; import com.android.systemui.util.time.FakeSystemClock; @@ -51,7 +52,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -60,7 +60,7 @@ import java.util.List; @SmallTest @RunWith(AndroidTestingRunner.class) @TestableLooper.RunWithLooper -public class ForegroundCoordinatorTest extends SysuiTestCase { +public class AppOpsCoordinatorTest extends SysuiTestCase { private static final String TEST_PKG = "test_pkg"; private static final int NOTIF_USER_ID = 0; @@ -68,12 +68,11 @@ public class ForegroundCoordinatorTest extends SysuiTestCase { @Mock private AppOpsController mAppOpsController; @Mock private NotifPipeline mNotifPipeline; - @Captor private ArgumentCaptor mAppOpsCaptor; - private NotificationEntry mEntry; private Notification mNotification; - private ForegroundCoordinator mForegroundCoordinator; + private AppOpsCoordinator mAppOpsCoordinator; private NotifFilter mForegroundFilter; + private NotifCollectionListener mNotifCollectionListener; private AppOpsController.Callback mAppOpsCallback; private NotifLifetimeExtender mForegroundNotifLifetimeExtender; @@ -85,8 +84,8 @@ public class ForegroundCoordinatorTest extends SysuiTestCase { MockitoAnnotations.initMocks(this); allowTestableLooperAsMainThread(); - mForegroundCoordinator = - new ForegroundCoordinator( + mAppOpsCoordinator = + new AppOpsCoordinator( mForegroundServiceController, mAppOpsController, mExecutor); @@ -97,19 +96,32 @@ public class ForegroundCoordinatorTest extends SysuiTestCase { .setNotification(mNotification) .build(); + mAppOpsCoordinator.attach(mNotifPipeline); + + // capture filter ArgumentCaptor filterCaptor = ArgumentCaptor.forClass(NotifFilter.class); + verify(mNotifPipeline, times(1)).addPreGroupFilter(filterCaptor.capture()); + mForegroundFilter = filterCaptor.getValue(); + + // capture lifetime extender ArgumentCaptor lifetimeExtenderCaptor = ArgumentCaptor.forClass(NotifLifetimeExtender.class); - - mForegroundCoordinator.attach(mNotifPipeline); - verify(mNotifPipeline, times(1)).addPreGroupFilter(filterCaptor.capture()); verify(mNotifPipeline, times(1)).addNotificationLifetimeExtender( lifetimeExtenderCaptor.capture()); - verify(mAppOpsController).addCallback(any(int[].class), mAppOpsCaptor.capture()); - - mForegroundFilter = filterCaptor.getValue(); mForegroundNotifLifetimeExtender = lifetimeExtenderCaptor.getValue(); - mAppOpsCallback = mAppOpsCaptor.getValue(); + + // capture notifCollectionListener + ArgumentCaptor notifCollectionCaptor = + ArgumentCaptor.forClass(NotifCollectionListener.class); + verify(mNotifPipeline, times(1)).addCollectionListener( + notifCollectionCaptor.capture()); + mNotifCollectionListener = notifCollectionCaptor.getValue(); + + // capture app ops callback + ArgumentCaptor appOpsCaptor = + ArgumentCaptor.forClass(AppOpsController.Callback.class); + verify(mAppOpsController).addCallback(any(int[].class), appOpsCaptor.capture()); + mAppOpsCallback = appOpsCaptor.getValue(); } @Test @@ -199,15 +211,14 @@ public class ForegroundCoordinatorTest extends SysuiTestCase { .setNotification(mNotification) .build(); - // THEN don't extend the lifetime because the extended time exceeds - // ForegroundCoordinator.MIN_FGS_TIME_MS + // THEN don't extend the lifetime because the extended time exceeds MIN_FGS_TIME_MS assertFalse(mForegroundNotifLifetimeExtender .shouldExtendLifetime(mEntry, NotificationListenerService.REASON_CLICK)); } @Test - public void testAppOpsAreApplied() { - // GIVEN Three current notifications, two with the same key but from different users + public void testAppOpsUpdateOnlyAppliedToRelevantNotificationWithStandardLayout() { + // GIVEN three current notifications, two with the same key but from different users NotificationEntry entry1 = new NotificationEntryBuilder() .setUser(new UserHandle(NOTIF_USER_ID)) .setPkg(TEST_PKG) @@ -218,16 +229,16 @@ public class ForegroundCoordinatorTest extends SysuiTestCase { .setPkg(TEST_PKG) .setId(2) .build(); - NotificationEntry entry2Other = new NotificationEntryBuilder() + NotificationEntry entry3_diffUser = new NotificationEntryBuilder() .setUser(new UserHandle(NOTIF_USER_ID + 1)) .setPkg(TEST_PKG) .setId(2) .build(); - when(mNotifPipeline.getAllNotifs()).thenReturn(List.of(entry1, entry2, entry2Other)); + when(mNotifPipeline.getAllNotifs()).thenReturn(List.of(entry1, entry2, entry3_diffUser)); - // GIVEN that entry2 is currently associated with a foreground service - when(mForegroundServiceController.getStandardLayoutKey(0, TEST_PKG)) - .thenReturn(entry2.getKey()); + // GIVEN that only entry2 has a standard layout + when(mForegroundServiceController.getStandardLayoutKeys(NOTIF_USER_ID, TEST_PKG)) + .thenReturn(new ArraySet<>(List.of(entry2.getKey()))); // WHEN a new app ops code comes in mAppOpsCallback.onActiveStateChanged(47, NOTIF_USER_ID, TEST_PKG, true); @@ -242,7 +253,46 @@ public class ForegroundCoordinatorTest extends SysuiTestCase { entry2.mActiveAppOps); assertEquals( new ArraySet<>(), - entry2Other.mActiveAppOps); + entry3_diffUser.mActiveAppOps); + } + + @Test + public void testAppOpsUpdateAppliedToAllNotificationsWithStandardLayouts() { + // GIVEN three notifications with standard layouts + NotificationEntry entry1 = new NotificationEntryBuilder() + .setUser(new UserHandle(NOTIF_USER_ID)) + .setPkg(TEST_PKG) + .setId(1) + .build(); + NotificationEntry entry2 = new NotificationEntryBuilder() + .setUser(new UserHandle(NOTIF_USER_ID)) + .setPkg(TEST_PKG) + .setId(2) + .build(); + NotificationEntry entry3 = new NotificationEntryBuilder() + .setUser(new UserHandle(NOTIF_USER_ID)) + .setPkg(TEST_PKG) + .setId(3) + .build(); + when(mNotifPipeline.getAllNotifs()).thenReturn(List.of(entry1, entry2, entry3)); + when(mForegroundServiceController.getStandardLayoutKeys(NOTIF_USER_ID, TEST_PKG)) + .thenReturn(new ArraySet<>(List.of(entry1.getKey(), entry2.getKey(), + entry3.getKey()))); + + // WHEN a new app ops code comes in + mAppOpsCallback.onActiveStateChanged(47, NOTIF_USER_ID, TEST_PKG, true); + mExecutor.runAllReady(); + + // THEN all entries get updated + assertEquals( + new ArraySet<>(List.of(47)), + entry1.mActiveAppOps); + assertEquals( + new ArraySet<>(List.of(47)), + entry2.mActiveAppOps); + assertEquals( + new ArraySet<>(List.of(47)), + entry3.mActiveAppOps); } @Test @@ -254,8 +304,8 @@ public class ForegroundCoordinatorTest extends SysuiTestCase { .setId(2) .build(); when(mNotifPipeline.getAllNotifs()).thenReturn(List.of(entry)); - when(mForegroundServiceController.getStandardLayoutKey(0, TEST_PKG)) - .thenReturn(entry.getKey()); + when(mForegroundServiceController.getStandardLayoutKeys(0, TEST_PKG)) + .thenReturn(new ArraySet<>(List.of(entry.getKey()))); // GIVEN that the notification's app ops are already [47, 33] mAppOpsCallback.onActiveStateChanged(47, NOTIF_USER_ID, TEST_PKG, true); @@ -274,4 +324,25 @@ public class ForegroundCoordinatorTest extends SysuiTestCase { new ArraySet<>(List.of(33)), entry.mActiveAppOps); } + + @Test + public void testNullAppOps() { + // GIVEN one notification with app ops + NotificationEntry entry = new NotificationEntryBuilder() + .setUser(new UserHandle(NOTIF_USER_ID)) + .setPkg(TEST_PKG) + .setId(2) + .build(); + entry.mActiveAppOps.clear(); + entry.mActiveAppOps.addAll(List.of(47, 33)); + + // WHEN the notification is updated and the foreground service controller returns null for + // this notification + when(mForegroundServiceController.getAppOps(entry.getSbn().getUser().getIdentifier(), + entry.getSbn().getPackageName())).thenReturn(null); + mNotifCollectionListener.onEntryUpdated(entry); + + // THEN the entry's active app ops is updated to empty + assertTrue(entry.mActiveAppOps.isEmpty()); + } }