Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5171071f authored by Julia Reynolds's avatar Julia Reynolds
Browse files

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 bbc469ce)
parent 0d7ff532
Loading
Loading
Loading
Loading
+6 −4
Original line number Diff line number Diff line
@@ -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);
            }
+10 −6
Original line number Diff line number Diff line
@@ -2989,10 +2989,12 @@ public class NotificationManagerService extends SystemService {
        if (r == null) {
            return;
        }
        if (r.sbn.getOverrideGroupKey() == null) {
            addAutoGroupAdjustment(r, GroupHelper.AUTOGROUP_KEY);
            EventLogTags.writeNotificationAutogrouped(key);
            mRankingHandler.requestSort();
        }
    }

    @GuardedBy("mNotificationLock")
    void removeAutogroupKeyLocked(String key) {
@@ -3000,10 +3002,12 @@ public class NotificationManagerService extends SystemService {
        if (r == null) {
            return;
        }
        if (r.sbn.getOverrideGroupKey() != null) {
            addAutoGroupAdjustment(r, null);
            EventLogTags.writeNotificationUnautogrouped(key);
            mRankingHandler.requestSort();
        }
    }

    private void addAutoGroupAdjustment(NotificationRecord r, String overrideGroupKey) {
        Bundle signals = new Bundle();
+1 −2
Original line number Diff line number Diff line
@@ -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);
                }
            }
+24 −1
Original line number Diff line number Diff line
@@ -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());
    }
}
+27 −2
Original line number Diff line number Diff line
@@ -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