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

Commit f9c0e0b2 authored by Matías Hernández's avatar Matías Hernández
Browse files

Fix "apply NAS adjustment" when a notification is enqueued multiple times

Because the Adjustment object is modified when applying it (for some keys), applying it to multiple NotificationRecords would not work correctly starting from the second enqueued copy. And because the last one wins, this effectively resulted in the adjustment not being applied at all.

Fixes: 429948005
Test: atest NotificationManagerServiceTest
Flag: EXEMPT simple bugfix
Change-Id: Ic111fa1d85e79e34cbe6f54dc3e0c2e08cdb1182
parent 31dc5673
Loading
Loading
Loading
Loading
+10 −0
Original line number Diff line number Diff line
@@ -279,6 +279,16 @@ public final class Adjustment implements Parcelable {
        mUser = userHandle.getIdentifier();
    }

    /**
     * Create a deep copy of a {@link Adjustment}
     * @hide
     */
    public Adjustment(Adjustment src) {
        this(src.mPackage, src.mKey, src.mSignals != null ? src.mSignals.deepCopy() : new Bundle(),
                src.mExplanation, src.mUser);
        this.mIssuer = src.mIssuer;
    }

    /**
     * @hide
     */
+23 −13
Original line number Diff line number Diff line
@@ -7277,27 +7277,37 @@ public class NotificationManagerService extends SystemService {
        @Override
        public void applyEnqueuedAdjustmentFromAssistant(INotificationListener token,
                Adjustment adjustment) {
            boolean foundEnqueued = false;
            final long identity = Binder.clearCallingIdentity();
            try {
                synchronized (mNotificationLock) {
                    ManagedServiceInfo info = mAssistants.checkServiceTokenLocked(token);
                    int N = mEnqueuedNotifications.size();
                    for (int i = 0; i < N; i++) {
                    mAssistants.checkServiceTokenLocked(token);
                    ArrayList<NotificationRecord> enqueuedRecords = new ArrayList<>();
                    for (int i = 0; i < mEnqueuedNotifications.size(); i++) {
                        final NotificationRecord r = mEnqueuedNotifications.get(i);
                        if (Objects.equals(adjustment.getKey(), r.getKey())
                                && Objects.equals(adjustment.getUser(), r.getUserId())
                                && adjustment.getUser() == r.getUserId()
                                && mAssistants.isSameUser(token, r.getUserId())) {
                            applyAdjustmentLocked(r, adjustment, false);
                            enqueuedRecords.add(r);
                        }
                    }
                    for (NotificationRecord r : enqueuedRecords) {
                        // Adjustment might be tweaked when applying, and is also associated
                        // with the particular NotificationRecord, so don't share them.
                        Adjustment recordAdjustment = enqueuedRecords.size() > 1
                                ? new Adjustment(adjustment)
                                : adjustment;
                        applyAdjustmentLocked(r, recordAdjustment, false);
                        r.applyAdjustments();
                        // importance is checked at the beginning of the
                        // PostNotificationRunnable, before the signal extractors are run, so
                        // calculate the final importance here
                        r.calculateImportance();
                            foundEnqueued = true;
                    }
                    }
                    if (!foundEnqueued) {
                    if (enqueuedRecords.isEmpty()) {
                        // Notification was posted before the NAS came with the adjustment, so
                        // adjust that one.
                        applyAdjustmentsFromAssistant(token, List.of(adjustment));
                    }
                }
+3 −1
Original line number Diff line number Diff line
@@ -817,7 +817,9 @@ public final class NotificationRecord {
                if (android.service.notification.Flags.notificationClassification()) {
                    if (signals.containsKey(Adjustment.KEY_TYPE)) {
                        // Store original channel visibility before re-assigning channel
                        if (!NotificationChannel.SYSTEM_RESERVED_IDS.contains(mChannel.getId())) {
                            setOriginalChannelVisibility(mChannel.getLockscreenVisibility());
                        }
                        updateNotificationChannel(signals.getParcelable(Adjustment.KEY_TYPE,
                                NotificationChannel.class));
                        EventLogTags.writeNotificationAdjusted(getKey(),
+39 −0
Original line number Diff line number Diff line
@@ -18337,6 +18337,45 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
        assertThat(r.getChannel().getId()).isEqualTo(RECS_ID);
    }
    @Test
    @EnableFlags(android.service.notification.Flags.FLAG_NOTIFICATION_CLASSIFICATION)
    public void applyAdjustment_classify_withUpdatesEnqueued_appliesChannel() throws Exception {
        when(mAssistants.isClassificationTypeAllowed(anyInt(), anyInt())).thenReturn(true);
        when(mAssistants.isAdjustmentAllowedForPackage(anyInt(), anyString(),
                anyString())).thenReturn(true);
        when(mAssistants.isSameUser(any(), anyInt())).thenReturn(true);
        NotificationRecord sample = generateNotificationRecord(mTestNotificationChannel);
        // App rapidly posts the same notification multiple times
        int times = 2;
        for (int i = 0; i < times; i++) {
            NotificationRecord copy = generateNotificationRecord(mTestNotificationChannel);
            mService.addEnqueuedNotification(copy);
        }
        // Assistant gets all the enqueues, so classifies the same notification multiple times too,
        // before actual post has occurred.
        for (int i = 0; i < times; i++) {
            Bundle signals = new Bundle();
            signals.putInt(KEY_TYPE, TYPE_NEWS);
            Adjustment adjustment = new Adjustment(
                    sample.getSbn().getPackageName(), sample.getKey(), signals, "",
                    sample.getUser().getIdentifier());
            mBinderService.applyEnqueuedAdjustmentFromAssistant(null, adjustment);
        }
        // Process and post all enqueues.
        for (int i = 0; i < times; i++) {
            mService.new PostNotificationRunnable(sample.getKey(), sample.getSbn().getPackageName(),
                    sample.getUid(), mPostNotificationTrackerFactory.newTracker(null)).run();
        }
        waitForIdle();
        assertThat(mService.mEnqueuedNotifications).isEmpty();
        NotificationRecord posted = mService.mNotificationsByKey.get(sample.getKey());
        assertThat(posted).isNotNull();
        assertThat(posted.getChannel().getId()).isEqualTo(NEWS_ID);
        assertThat(mService.mNotificationList).hasSize(2); // Bundle header + notif
    }
    @Test
    @EnableFlags({android.service.notification.Flags.FLAG_NOTIFICATION_CLASSIFICATION,
            android.app.Flags.FLAG_NOTIFICATION_CLASSIFICATION_UI})