Fix notification ordering for groups

- Only request a re-sort on behalf of notifications whose autogroup is
actually changing
- Don't allow notifications that were posted more than HANG_TIME_MS
ago be reconsidered to be intrusive
- Sort notification groups at the position of the group member
with the best authoritative rank, regardless of importance

Bug: 62895856
Bug: 63291402
Test: set AUTOGROUP_AT_COUNT to 2 in GroupHelper, and
run the AttentionManagement cts verifier test;
runtest systemui-notification

Change-Id: Ic433ca827cd01cf166b3cc11d43be639fd7a13b7
(cherry picked from commit bbc469cecac9c50c342902d724bfa68acd140770)

diff --git a/services/core/java/com/android/server/notification/NotificationIntrusivenessExtractor.java b/services/core/java/com/android/server/notification/NotificationIntrusivenessExtractor.java
index 12b29cf..91fee46 100644 (file)
--- a/services/core/java/com/android/server/notification/NotificationIntrusivenessExtractor.java
+++ b/services/core/java/com/android/server/notification/NotificationIntrusivenessExtractor.java
@@ -16,14 +16,14 @@
 
 package com.android.server.notification;
 
-import android.app.Notification;
 import android.app.NotificationManager;
 import android.content.Context;
 import android.net.Uri;
-import android.service.notification.NotificationListenerService;
 import android.util.Log;
 import android.util.Slog;
 
+import com.android.internal.annotations.VisibleForTesting;
+
 /**
  * This {@link com.android.server.notification.NotificationSignalExtractor} notices noisy
  * notifications and marks them to get a temporary ranking bump.
@@ -34,7 +34,8 @@ public class NotificationIntrusivenessExtractor implements NotificationSignalExt
 
     /** Length of time (in milliseconds) that an intrusive or noisy notification will stay at
     the top of the ranking order, before it falls back to its natural position. */
-    private static final long HANG_TIME_MS = 10000;
+    @VisibleForTesting
+    static final long HANG_TIME_MS = 10000;
 
     public void initialize(Context ctx, NotificationUsageStats usageStats) {
         if (DBG) Slog.d(TAG, "Initializing  " + getClass().getSimpleName() + ".");
@@ -46,7 +47,8 @@ public class NotificationIntrusivenessExtractor implements NotificationSignalExt
             return null;
         }
 
-        if (record.getImportance() >= NotificationManager.IMPORTANCE_DEFAULT) {
+        if (record.getFreshnessMs(System.currentTimeMillis()) < HANG_TIME_MS
+                && record.getImportance() >= NotificationManager.IMPORTANCE_DEFAULT) {
             if (record.getSound() != null && record.getSound() != Uri.EMPTY) {
                 record.setRecentlyIntrusive(true);
             }

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index c04124a..a57a8e5 100644 (file)
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -2989,9 +2989,11 @@ public class NotificationManagerService extends SystemService {
         if (r == null) {
             return;
         }
-        addAutoGroupAdjustment(r, GroupHelper.AUTOGROUP_KEY);
-        EventLogTags.writeNotificationAutogrouped(key);
-        mRankingHandler.requestSort();
+        if (r.sbn.getOverrideGroupKey() == null) {
+            addAutoGroupAdjustment(r, GroupHelper.AUTOGROUP_KEY);
+            EventLogTags.writeNotificationAutogrouped(key);
+            mRankingHandler.requestSort();
+        }
     }
 
     @GuardedBy("mNotificationLock")
@@ -3000,9 +3002,11 @@ public class NotificationManagerService extends SystemService {
         if (r == null) {
             return;
         }
-        addAutoGroupAdjustment(r, null);
-        EventLogTags.writeNotificationUnautogrouped(key);
-        mRankingHandler.requestSort();
+        if (r.sbn.getOverrideGroupKey() != null) {
+            addAutoGroupAdjustment(r, null);
+            EventLogTags.writeNotificationUnautogrouped(key);
+            mRankingHandler.requestSort();
+        }
     }
 
     private void addAutoGroupAdjustment(NotificationRecord r, String overrideGroupKey) {

diff --git a/services/core/java/com/android/server/notification/RankingHelper.java b/services/core/java/com/android/server/notification/RankingHelper.java
index 5c2c2b3..f1b83b9 100644 (file)
--- a/services/core/java/com/android/server/notification/RankingHelper.java
+++ b/services/core/java/com/android/server/notification/RankingHelper.java
@@ -419,8 +419,7 @@ public class RankingHelper implements RankingConfig {
                 record.setAuthoritativeRank(i);
                 final String groupKey = record.getGroupKey();
                 NotificationRecord existingProxy = mProxyByGroupTmp.get(groupKey);
-                if (existingProxy == null
-                        || record.getImportance() > existingProxy.getImportance()) {
+                if (existingProxy == null) {
                     mProxyByGroupTmp.put(groupKey, record);
                 }
             }

diff --git a/services/tests/notification/src/com/android/server/notification/NotificationIntrusivenessExtractorTest.java b/services/tests/notification/src/com/android/server/notification/NotificationIntrusivenessExtractorTest.java
index d2f608e..85852f9 100644 (file)
--- a/services/tests/notification/src/com/android/server/notification/NotificationIntrusivenessExtractorTest.java
+++ b/services/tests/notification/src/com/android/server/notification/NotificationIntrusivenessExtractorTest.java
@@ -19,6 +19,9 @@ package com.android.server.notification;
 import static android.app.NotificationManager.IMPORTANCE_DEFAULT;
 import static android.app.NotificationManager.IMPORTANCE_LOW;
 
+import static com.android.server.notification.NotificationIntrusivenessExtractor.HANG_TIME_MS;
+
+import static junit.framework.Assert.assertFalse;
 import static junit.framework.Assert.assertNotNull;
 import static junit.framework.Assert.assertNull;
 
@@ -59,9 +62,29 @@ public class NotificationIntrusivenessExtractorTest extends NotificationTestCase
 
         Notification n = builder.build();
         StatusBarNotification sbn = new StatusBarNotification("", "", 0, "", 0,
-                0, n, UserHandle.ALL, null, System.currentTimeMillis());
+                0, n, UserHandle.ALL, null,
+                System.currentTimeMillis());
         NotificationRecord r = new NotificationRecord(getContext(), sbn, channel);
 
         assertNotNull(new NotificationIntrusivenessExtractor().process(r));
     }
+
+    @Test
+    public void testOldNotificationsNotIntrusive() {
+        NotificationChannel channel = new NotificationChannel("a", "a", IMPORTANCE_DEFAULT);
+        final Notification.Builder builder = new Notification.Builder(getContext())
+                .setContentTitle("foo")
+                .setFullScreenIntent(PendingIntent.getActivity(
+                        getContext(), 0, new Intent(""), 0), true)
+                .setSmallIcon(android.R.drawable.sym_def_app_icon);
+
+        Notification n = builder.build();
+        StatusBarNotification sbn = new StatusBarNotification("", "", 0, "", 0,
+                0, n, UserHandle.ALL, null,
+                System.currentTimeMillis() - HANG_TIME_MS);
+
+        NotificationRecord r = new NotificationRecord(getContext(), sbn, channel);
+        assertNull(new NotificationIntrusivenessExtractor().process(r));
+        assertFalse(r.isRecentlyIntrusive());
+    }
 }

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 ae2253e..bbc03c3 100644 (file)
--- a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -1382,16 +1382,41 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
     }
 
     @Test
-    public void testModifyAutogroup_requestsSort() throws Exception {
+    public void testAddAutogroup_requestsSort() throws Exception {
         RankingHandler rh = mock(RankingHandler.class);
         mNotificationManagerService.setRankingHandler(rh);
 
         final NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);
         mNotificationManagerService.addNotification(r);
         mNotificationManagerService.addAutogroupKeyLocked(r.getKey());
+
+        verify(rh, times(1)).requestSort();
+    }
+
+    @Test
+    public void testRemoveAutogroup_requestsSort() throws Exception {
+        RankingHandler rh = mock(RankingHandler.class);
+        mNotificationManagerService.setRankingHandler(rh);
+
+        final NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);
+        r.setOverrideGroupKey("TEST");
+        mNotificationManagerService.addNotification(r);
         mNotificationManagerService.removeAutogroupKeyLocked(r.getKey());
 
-        verify(rh, times(2)).requestSort();
+        verify(rh, times(1)).requestSort();
+    }
+
+    @Test
+    public void testReaddAutogroup_noSort() throws Exception {
+        RankingHandler rh = mock(RankingHandler.class);
+        mNotificationManagerService.setRankingHandler(rh);
+
+        final NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);
+        r.setOverrideGroupKey("TEST");
+        mNotificationManagerService.addNotification(r);
+        mNotificationManagerService.addAutogroupKeyLocked(r.getKey());
+
+        verify(rh, never()).requestSort();
     }
 
     @Test