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

Commit 83461490 authored by Alexander Roederer's avatar Alexander Roederer
Browse files

Fix race on setCanceledAfterLifetimeExtension 

We setCancaledAfterLifetimeExtension to avoid multiple cancelation calls
from causing multiple updates being sent to System UI, but were setting
the value on a callback after the post to System UI had occurred.
Unfortunately apps can cancel more quickly than that; these cancelations
will enqueue multiple posts before the callback from systemUI occurs,
and they'll all be sent.

This leads to the same message being posted repeatedly.

So, instead, we move the set to immediately after we've checked
that the record doesn't have the value set, and have committed to the path
where an update will be sent to system UI.

Bug: 371105236
Test: atest NotificationManagerService, flash+test
Flag: android.app.lifetime_extension_refactor
Change-Id: I82d0dfc9940f02108b61f4d8574c9135505efa58
parent dd1ce567

services/core/java/com/android/server/notification/NotificationManagerService.java

+4 −3
Original line number Diff line number Diff line
@@ -12009,6 +12009,10 @@ public class NotificationManagerService extends SystemService { 
        if (record != null && (record.getSbn().getNotification().flags

                & FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY) > 0

                && !record.isCanceledAfterLifetimeExtension()) {

            // Mark that the notification is being updated due to cancelation, so it won't

            // be updated again if the app cancels multiple times.

            record.setCanceledAfterLifetimeExtension(true);















            boolean isAppForeground = pkg != null && packageImportance == IMPORTANCE_FOREGROUND;





























            // Save the original Record's post silently value, so we can restore it after we send









@@ -12024,9 +12028,6 @@ public class NotificationManagerService extends SystemService {













            PostNotificationTracker tracker = mPostNotificationTrackerFactory.newTracker(null);















            tracker.addCleanupRunnable(() -> {















                synchronized (mNotificationLock) {













                    // Mark that the notification has been updated due to cancelation, so it won't













                    // be updated again if the app cancels multiple times.













                    record.setCanceledAfterLifetimeExtension(true);















                    // Set the post silently status to the record's previous value.















                    record.setPostSilently(savedPostSilentlyState);















                    // Remove FLAG_ONLY_ALERT_ONCE if the notification did not previously have it.

























services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java



+0
−1






























Original line number
Diff line number
Diff line








@@ -3206,7 +3206,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {













        // Send two cancelations.















        mBinderService.cancelNotificationWithTag(mPkg, mPkg, sbn.getTag(), sbn.getId(),















                sbn.getUserId());













        waitForIdle();















        mBinderService.cancelNotificationWithTag(mPkg, mPkg, sbn.getTag(), sbn.getId(),















                sbn.getUserId());















        waitForIdle();