Merge "Track non-foreground service notifs for appOps" into rvc-dev

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<String> 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<String> 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);
             }
         }
     }

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<Integer> 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);
             }
         }

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<String, ArraySet<String>> mImportantNotifications = new ArrayMap<>(1);
-    // package -> standard layout posted notification keys
+    // package -> standard layout posted notification keys that can display appOps
     private ArrayMap<String, ArraySet<String>> 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<String> getStandardLayoutKeys(String pkg) {
         final ArraySet<String> set = mStandardLayoutNotifications.get(pkg);
         if (set == null || set.size() == 0) {
             return null;
         }
-        return set.valueAt(0);
+        return set;
     }
 
     @Override

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

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()) {

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 =

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<AppOpsController.Callback> 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<NotifFilter> filterCaptor = ArgumentCaptor.forClass(NotifFilter.class);
+        verify(mNotifPipeline, times(1)).addPreGroupFilter(filterCaptor.capture());
+        mForegroundFilter = filterCaptor.getValue();
+
+        // capture lifetime extender
         ArgumentCaptor<NotifLifetimeExtender> 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<NotifCollectionListener> notifCollectionCaptor =
+                ArgumentCaptor.forClass(NotifCollectionListener.class);
+        verify(mNotifPipeline, times(1)).addCollectionListener(
+                notifCollectionCaptor.capture());
+        mNotifCollectionListener = notifCollectionCaptor.getValue();
+
+        // capture app ops callback
+        ArgumentCaptor<AppOpsController.Callback> 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());
+    }
 }