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