Only try to autogroup if group info changes

Test: runtest systemui-notification
Change-Id: I7ddf622530ee4c452353bea6751b613354917a1b
Fixes: 62788068

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index fbe6d30..48b4c57 100644 (file)
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -1136,7 +1136,7 @@ public class NotificationManagerService extends SystemService {
             NotificationAssistants notificationAssistants, ConditionProviders conditionProviders,
             ICompanionDeviceManager companionManager, SnoozeHelper snoozeHelper,
             NotificationUsageStats usageStats, AtomicFile policyFile,
-            ActivityManager activityManager) {
+            ActivityManager activityManager, GroupHelper groupHelper) {
         Resources resources = getContext().getResources();
         mMaxPackageEnqueueRate = Settings.Global.getFloat(getContext().getContentResolver(),
                 Settings.Global.MAX_NOTIFICATION_ENQUEUE_RATE,
@@ -1193,35 +1193,7 @@ public class NotificationManagerService extends SystemService {
             }
         });
         mSnoozeHelper = snoozeHelper;
-        mGroupHelper = new GroupHelper(new GroupHelper.Callback() {
-            @Override
-            public void addAutoGroup(String key) {
-                synchronized (mNotificationLock) {
-                    addAutogroupKeyLocked(key);
-                }
-                mRankingHandler.requestSort(false);
-            }
-
-            @Override
-            public void removeAutoGroup(String key) {
-                synchronized (mNotificationLock) {
-                    removeAutogroupKeyLocked(key);
-                }
-                mRankingHandler.requestSort(false);
-            }
-
-            @Override
-            public void addAutoGroupSummary(int userId, String pkg, String triggeringKey) {
-                createAutoGroupSummary(userId, pkg, triggeringKey);
-            }
-
-            @Override
-            public void removeAutoGroupSummary(int userId, String pkg) {
-                synchronized (mNotificationLock) {
-                    clearAutogroupSummaryLocked(userId, pkg);
-                }
-            }
-        });
+        mGroupHelper = groupHelper;
 
         // This is a ManagedServices object that keeps track of the listeners.
         mListeners = notificationListeners;
@@ -1337,11 +1309,44 @@ public class NotificationManagerService extends SystemService {
                 new ConditionProviders(getContext(), mUserProfiles, AppGlobals.getPackageManager()),
                 null, snoozeHelper, new NotificationUsageStats(getContext()),
                 new AtomicFile(new File(systemDir, "notification_policy.xml")),
-                (ActivityManager) getContext().getSystemService(Context.ACTIVITY_SERVICE));
+                (ActivityManager) getContext().getSystemService(Context.ACTIVITY_SERVICE),
+                getGroupHelper());
         publishBinderService(Context.NOTIFICATION_SERVICE, mService);
         publishLocalService(NotificationManagerInternal.class, mInternalService);
     }
 
+    private GroupHelper getGroupHelper() {
+        return new GroupHelper(new GroupHelper.Callback() {
+            @Override
+            public void addAutoGroup(String key) {
+                synchronized (mNotificationLock) {
+                    addAutogroupKeyLocked(key);
+                }
+                mRankingHandler.requestSort(false);
+            }
+
+            @Override
+            public void removeAutoGroup(String key) {
+                synchronized (mNotificationLock) {
+                    removeAutogroupKeyLocked(key);
+                }
+                mRankingHandler.requestSort(false);
+            }
+
+            @Override
+            public void addAutoGroupSummary(int userId, String pkg, String triggeringKey) {
+                createAutoGroupSummary(userId, pkg, triggeringKey);
+            }
+
+            @Override
+            public void removeAutoGroupSummary(int userId, String pkg) {
+                synchronized (mNotificationLock) {
+                    clearAutogroupSummaryLocked(userId, pkg);
+                }
+            }
+        });
+    }
+
     private void sendRegisteredOnlyBroadcast(String action) {
         getContext().sendBroadcastAsUser(new Intent(action)
                 .addFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY), UserHandle.ALL, null);
@@ -3323,7 +3328,6 @@ public class NotificationManagerService extends SystemService {
                     (r.mOriginalFlags & ~Notification.FLAG_FOREGROUND_SERVICE);
             mRankingHelper.sort(mNotificationList);
             mListeners.notifyPostedLocked(sbn, sbn /* oldSbn */);
-            mGroupHelper.onNotificationPosted(sbn);
         }
     };
 
@@ -3740,12 +3744,14 @@ public class NotificationManagerService extends SystemService {
                     if (notification.getSmallIcon() != null) {
                         StatusBarNotification oldSbn = (old != null) ? old.sbn : null;
                         mListeners.notifyPostedLocked(n, oldSbn);
-                        mHandler.post(new Runnable() {
-                            @Override
-                            public void run() {
-                                mGroupHelper.onNotificationPosted(n);
-                            }
-                        });
+                        if (oldSbn == null || !Objects.equals(oldSbn.getGroup(), n.getGroup())) {
+                            mHandler.post(new Runnable() {
+                                @Override
+                                public void run() {
+                                    mGroupHelper.onNotificationPosted(n);
+                                }
+                            });
+                        }
                     } else {
                         Slog.e(TAG, "Not posting notification without small icon: " + notification);
                         if (old != null && !old.isCanceled) {

diff --git a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java
index d475d64..8ff4639 100644 (file)
--- a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -118,6 +118,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
     private ManagedServices.ManagedServiceInfo mListener;
     @Mock private ICompanionDeviceManager mCompanionMgr;
     @Mock SnoozeHelper mSnoozeHelper;
+    @Mock GroupHelper mGroupHelper;
 
     // Use a Testable subclass so we can simulate calls from the system without failing.
     private static class TestableNotificationManagerService extends NotificationManagerService {
@@ -175,7 +176,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
             mNotificationManagerService.init(mTestableLooper.getLooper(), mPackageManager,
                     mPackageManagerClient, mockLightsManager, mNotificationListeners,
                     mNotificationAssistants, mConditionProviders, mCompanionMgr,
-                    mSnoozeHelper, mUsageStats, mPolicyFile, mActivityManager);
+                    mSnoozeHelper, mUsageStats, mPolicyFile, mActivityManager, mGroupHelper);
         } catch (SecurityException e) {
             if (!e.getMessage().contains("Permission Denial: not allowed to send broadcast")) {
                 throw e;
@@ -1086,4 +1087,47 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         verify(mNotificationAssistants, never()).setPackageOrComponentEnabled(
                 any(), anyInt(), anyBoolean(), anyBoolean());
     }
+
+    @Test
+    public void testOnlyAutogroupIfGroupChanged_noPriorNoti_autogroups() throws Exception {
+        NotificationRecord r = generateNotificationRecord(mTestNotificationChannel, 0, null, false);
+        mNotificationManagerService.addEnqueuedNotification(r);
+        NotificationManagerService.PostNotificationRunnable runnable =
+                mNotificationManagerService.new PostNotificationRunnable(r.getKey());
+        runnable.run();
+        waitForIdle();
+
+        verify(mGroupHelper, times(1)).onNotificationPosted(any());
+    }
+
+    @Test
+    public void testOnlyAutogroupIfGroupChanged_groupChanged_autogroups()
+            throws Exception {
+        NotificationRecord r = generateNotificationRecord(mTestNotificationChannel, 0, "group", false);
+        mNotificationManagerService.addNotification(r);
+
+        r = generateNotificationRecord(mTestNotificationChannel, 0, null, false);
+        mNotificationManagerService.addEnqueuedNotification(r);
+        NotificationManagerService.PostNotificationRunnable runnable =
+                mNotificationManagerService.new PostNotificationRunnable(r.getKey());
+        runnable.run();
+        waitForIdle();
+
+        verify(mGroupHelper, times(1)).onNotificationPosted(any());
+    }
+
+    @Test
+    public void testOnlyAutogroupIfGroupChanged_noGroupChanged_autogroups()
+            throws Exception {
+        NotificationRecord r = generateNotificationRecord(mTestNotificationChannel, 0, "group", false);
+        mNotificationManagerService.addNotification(r);
+        mNotificationManagerService.addEnqueuedNotification(r);
+
+        NotificationManagerService.PostNotificationRunnable runnable =
+                mNotificationManagerService.new PostNotificationRunnable(r.getKey());
+        runnable.run();
+        waitForIdle();
+
+        verify(mGroupHelper, never()).onNotificationPosted(any());
+    }
 }