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

Commit 6cab5ff8 authored by Julia Reynolds's avatar Julia Reynolds
Browse files

Fix some more snoozing bugs 

- Make all time based snoozing use the same time frame
(current millis, not elapsed walltime)
- Add synchronization to snoozing
- Update when entries are and are not removed from data structures
to avoid NPEs

Test: atest, and manual, repeated snooze and unsnooze
Fixes: 150377159
Change-Id: Ic91ff7ac0bd1b3635b35267bdbc64d3ee614c8ab
parent 6503162d

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

+0 −1
Original line number Diff line number Diff line
@@ -8069,7 +8069,6 @@ public class NotificationManagerService extends SystemService { 
        if (DBG) {

            Slog.d(TAG, String.format("unsnooze event(%s, %s)", key, listenerName));

        }

        mSnoozeHelper.cleanupPersistedContext(key);

        mSnoozeHelper.repost(key, muteOnReturn);

        handleSavePolicyFile();

    }

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

+293 −233
Original line number Diff line number Diff line
@@ -106,6 +106,8 @@ public class SnoozeHelper { 
    private ArrayMap<String, Integer> mUsers = new ArrayMap<>();

    private Callback mCallback;



    private final Object mLock = new Object();



    public SnoozeHelper(Context context, Callback callback,

            ManagedServices.UserProfiles userProfiles) {

        mContext = context;
@@ -122,42 +124,53 @@ public class SnoozeHelper { 
    }



    void cleanupPersistedContext(String key){

        synchronized (mLock) {

            int userId = mUsers.get(key);

            String pkg = mPackages.get(key);

        synchronized (mPersistedSnoozedNotificationsWithContext) {

            removeRecord(pkg, key, userId, mPersistedSnoozedNotificationsWithContext);

            removeRecordLocked(pkg, key, userId, mPersistedSnoozedNotificationsWithContext);

        }

    }



    //This function has a side effect of removing the time from the list of persisted notifications.

    //IT IS NOT IDEMPOTENT!

    @NonNull

    protected Long getSnoozeTimeForUnpostedNotification(int userId, String pkg, String key) {

        Long time;

        synchronized (mPersistedSnoozedNotifications) {

            time = removeRecord(pkg, key, userId, mPersistedSnoozedNotifications);

        Long time = null;

        synchronized (mLock) {

           ArrayMap<String, Long> snoozed =

                   mPersistedSnoozedNotifications.get(getPkgKey(userId, pkg));

           if (snoozed != null) {

               time = snoozed.get(key);

           }

        }

        if (time == null) {

            return 0L;

            time = 0L;

        }

        return time;

    }



    protected String getSnoozeContextForUnpostedNotification(int userId, String pkg, String key) {

        synchronized (mPersistedSnoozedNotificationsWithContext) {

            return removeRecord(pkg, key, userId, mPersistedSnoozedNotificationsWithContext);

        synchronized (mLock) {

            ArrayMap<String, String> snoozed =

                    mPersistedSnoozedNotificationsWithContext.get(getPkgKey(userId, pkg));

            if (snoozed != null) {

                return snoozed.get(key);

            }

        }

        return null;

    }



    protected boolean isSnoozed(int userId, String pkg, String key) {

        synchronized (mLock) {

            return mSnoozedNotifications.containsKey(getPkgKey(userId, pkg))

                    && mSnoozedNotifications.get(getPkgKey(userId, pkg)).containsKey(key);

        }

    }



    protected Collection<NotificationRecord> getSnoozed(int userId, String pkg) {

        synchronized (mLock) {

            if (mSnoozedNotifications.containsKey(getPkgKey(userId, pkg))) {

                return mSnoozedNotifications.get(getPkgKey(userId, pkg)).values();

            }

        }

        return Collections.EMPTY_LIST;

    }
@@ -165,6 +178,7 @@ public class SnoozeHelper { 
    ArrayList<NotificationRecord> getNotifications(String pkg,

            String groupKey, Integer userId) {

        ArrayList<NotificationRecord> records =  new ArrayList<>();

        synchronized (mLock) {

            ArrayMap<String, NotificationRecord> allRecords =

                    mSnoozedNotifications.get(getPkgKey(userId, pkg));

            if (allRecords != null) {
@@ -176,10 +190,12 @@ public class SnoozeHelper { 
                    }

                }

            }

        }

        return records;

    }



    protected NotificationRecord getNotification(String key) {

        synchronized (mLock) {

            if (!mUsers.containsKey(key) || !mPackages.containsKey(key)) {

                Slog.w(TAG, "Snoozed data sets no longer agree for " + key);

                return null;
@@ -193,8 +209,10 @@ public class SnoozeHelper { 
            }

            return snoozed.get(key);

        }

    }



    protected @NonNull List<NotificationRecord> getSnoozed() {

        synchronized (mLock) {

            // caller filters records based on the current user profiles and listener access, so just

            // return everything

            List<NotificationRecord> snoozed = new ArrayList<>();
@@ -205,6 +223,7 @@ public class SnoozeHelper { 
            }

            return snoozed;

        }

    }



    /**

     * Snoozes a notification and schedules an alarm to repost at that time.
@@ -216,9 +235,9 @@ public class SnoozeHelper { 


        snooze(record);

        scheduleRepost(pkg, key, userId, duration);

        Long activateAt = SystemClock.elapsedRealtime() + duration;

        synchronized (mPersistedSnoozedNotifications) {

            storeRecord(pkg, key, userId, mPersistedSnoozedNotifications, activateAt);

        Long activateAt = System.currentTimeMillis() + duration;

        synchronized (mLock) {

            storeRecordLocked(pkg, key, userId, mPersistedSnoozedNotifications, activateAt);

        }

    }
@@ -228,8 +247,8 @@ public class SnoozeHelper { 
    protected void snooze(NotificationRecord record, String contextId) {

        int userId = record.getUser().getIdentifier();

        if (contextId != null) {

            synchronized (mPersistedSnoozedNotificationsWithContext) {

                storeRecord(record.getSbn().getPackageName(), record.getKey(),

            synchronized (mLock) {

                storeRecordLocked(record.getSbn().getPackageName(), record.getKey(),

                        userId, mPersistedSnoozedNotificationsWithContext, contextId);

            }

        }
@@ -241,25 +260,26 @@ public class SnoozeHelper { 
        if (DEBUG) {

            Slog.d(TAG, "Snoozing " + record.getKey());

        }

        storeRecord(record.getSbn().getPackageName(), record.getKey(),

        synchronized (mLock) {

            storeRecordLocked(record.getSbn().getPackageName(), record.getKey(),

                    userId, mSnoozedNotifications, record);

        }

    }



    private <T> void storeRecord(String pkg, String key, Integer userId,

    private <T> void storeRecordLocked(String pkg, String key, Integer userId,

            ArrayMap<String, ArrayMap<String, T>> targets, T object) {



        mPackages.put(key, pkg);

        mUsers.put(key, userId);

        ArrayMap<String, T> keyToValue = targets.get(getPkgKey(userId, pkg));

        if (keyToValue == null) {

            keyToValue = new ArrayMap<>();

        }

        keyToValue.put(key, object);

        targets.put(getPkgKey(userId, pkg), keyToValue);



        mPackages.put(key, pkg);

        mUsers.put(key, userId);

    }



    private <T> T removeRecord(String pkg, String key, Integer userId,

    private <T> T removeRecordLocked(String pkg, String key, Integer userId,

            ArrayMap<String, ArrayMap<String, T>> targets) {

        T object = null;

        ArrayMap<String, T> keyToValue = targets.get(getPkgKey(userId, pkg));
@@ -274,6 +294,7 @@ public class SnoozeHelper { 
    }



    protected boolean cancel(int userId, String pkg, String tag, int id) {

        synchronized (mLock) {

            ArrayMap<String, NotificationRecord> recordsForPkg =

                    mSnoozedNotifications.get(getPkgKey(userId, pkg));

            if (recordsForPkg != null) {
@@ -286,10 +307,12 @@ public class SnoozeHelper { 
                    }

                }

            }

        }

        return false;

    }



    protected void cancel(int userId, boolean includeCurrentProfiles) {

        synchronized (mLock) {

            if (mSnoozedNotifications.size() == 0) {

                return;

            }
@@ -306,8 +329,10 @@ public class SnoozeHelper { 
                }

            }

        }

    }



    protected boolean cancel(int userId, String pkg) {

        synchronized (mLock) {

            ArrayMap<String, NotificationRecord> records =

                    mSnoozedNotifications.get(getPkgKey(userId, pkg));

            if (records == null) {
@@ -319,11 +344,13 @@ public class SnoozeHelper { 
            }

            return true;

        }

    }



    /**

     * Updates the notification record so the most up to date information is shown on re-post.

     */

    protected void update(int userId, NotificationRecord record) {

        synchronized (mLock) {

            ArrayMap<String, NotificationRecord> records =

                    mSnoozedNotifications.get(getPkgKey(userId, record.getSbn().getPackageName()));

            if (records == null) {
@@ -331,27 +358,36 @@ public class SnoozeHelper { 
            }

            records.put(record.getKey(), record);

        }

    }



    protected void repost(String key, boolean muteOnReturn) {

        synchronized (mLock) {

            Integer userId = mUsers.get(key);

            if (userId != null) {

                repost(key, userId, muteOnReturn);

            }

        }

    }



    protected void repost(String key, int userId, boolean muteOnReturn) {

        NotificationRecord record;

        synchronized (mLock) {

            final String pkg = mPackages.remove(key);

            mUsers.remove(key);

            removeRecordLocked(pkg, key, userId, mPersistedSnoozedNotifications);

            removeRecordLocked(pkg, key, userId, mPersistedSnoozedNotificationsWithContext);

            ArrayMap<String, NotificationRecord> records =

                    mSnoozedNotifications.get(getPkgKey(userId, pkg));

            if (records == null) {

                return;

            }

        final NotificationRecord record = records.remove(key);

        mPackages.remove(key);

        mUsers.remove(key);

            record = records.remove(key);



        }



        if (record != null && !record.isCanceled) {

            final PendingIntent pi = createPendingIntent(pkg, record.getKey(), userId);

            final PendingIntent pi = createPendingIntent(

                    record.getSbn().getPackageName(), record.getKey(), userId);

            mAm.cancel(pi);

            MetricsLogger.action(record.getLogMaker()

                    .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED)
@@ -361,6 +397,7 @@ public class SnoozeHelper { 
    }



    protected void repostGroupSummary(String pkg, int userId, String groupKey) {

        synchronized (mLock) {

            ArrayMap<String, NotificationRecord> recordsByKey

                    = mSnoozedNotifications.get(getPkgKey(userId, pkg));

            if (recordsByKey == null) {
@@ -385,15 +422,20 @@ public class SnoozeHelper { 
                mUsers.remove(groupSummaryKey);



                if (record != null && !record.isCanceled) {

                    Runnable runnable = () -> {

                        MetricsLogger.action(record.getLogMaker()

                                .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED)

                                .setType(MetricsProto.MetricsEvent.TYPE_OPEN));

                        mCallback.repost(userId, record, false);

                    };

                    runnable.run();

                }

            }

        }

    }



    protected void clearData(int userId, String pkg) {

        synchronized (mLock) {

            ArrayMap<String, NotificationRecord> records =

                    mSnoozedNotifications.get(getPkgKey(userId, pkg));

            if (records == null) {
@@ -404,11 +446,15 @@ public class SnoozeHelper { 
                if (r != null) {

                    mPackages.remove(r.getKey());

                    mUsers.remove(r.getKey());

                    Runnable runnable = () -> {

                        final PendingIntent pi = createPendingIntent(pkg, r.getKey(), userId);

                        mAm.cancel(pi);

                        MetricsLogger.action(r.getLogMaker()

                                .setCategory(MetricsProto.MetricsEvent.NOTIFICATION_SNOOZED)

                                .setType(MetricsProto.MetricsEvent.TYPE_DISMISS));

                    };

                    runnable.run();

                }

            }

        }

    }
@@ -425,6 +471,7 @@ public class SnoozeHelper { 
    }



    public void scheduleRepostsForPersistedNotifications(long currentTime) {

        synchronized (mLock) {

            for (ArrayMap<String, Long> snoozed : mPersistedSnoozedNotifications.values()) {

                for (int i = 0; i < snoozed.size(); i++) {

                    String key = snoozed.keyAt(i);
@@ -439,27 +486,31 @@ public class SnoozeHelper { 
                        scheduleRepostAtTime(pkg, key, userId, time);

                    }

                }



            }

        }

    }



    private void scheduleRepost(String pkg, String key, int userId, long duration) {

        scheduleRepostAtTime(pkg, key, userId, SystemClock.elapsedRealtime() + duration);

        scheduleRepostAtTime(pkg, key, userId, System.currentTimeMillis() + duration);

    }



    private void scheduleRepostAtTime(String pkg, String key, int userId, long time) {

        Runnable runnable = () -> {

            long identity = Binder.clearCallingIdentity();

            try {

                final PendingIntent pi = createPendingIntent(pkg, key, userId);

                mAm.cancel(pi);

                if (DEBUG) Slog.d(TAG, "Scheduling evaluate for " + new Date(time));

            mAm.setExactAndAllowWhileIdle(AlarmManager.ELAPSED_REALTIME_WAKEUP, time, pi);

                mAm.setExactAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, time, pi);

            } finally {

                Binder.restoreCallingIdentity(identity);

            }

        };

        runnable.run();

    }



    public void dump(PrintWriter pw, NotificationManagerService.DumpFilter filter) {

        synchronized (mLock) {

            pw.println("\n  Snoozed notifications:");

            for (String userPkgKey : mSnoozedNotifications.keySet()) {

                pw.print(INDENT);
@@ -494,8 +545,10 @@ public class SnoozeHelper { 
                }

            }

        }

    }



    protected void writeXml(XmlSerializer out) throws IOException {

        synchronized (mLock) {

            final long currentTime = System.currentTimeMillis();

            out.startTag(null, XML_TAG_NAME);

            writeXml(out, mPersistedSnoozedNotifications, XML_SNOOZED_NOTIFICATION,
@@ -506,13 +559,15 @@ public class SnoozeHelper { 
                        out.attribute(null, XML_SNOOZED_NOTIFICATION_TIME,

                                value.toString());

                    });

        writeXml(out, mPersistedSnoozedNotificationsWithContext, XML_SNOOZED_NOTIFICATION_CONTEXT,

            writeXml(out, mPersistedSnoozedNotificationsWithContext,

                    XML_SNOOZED_NOTIFICATION_CONTEXT,

                    value -> {

                        out.attribute(null, XML_SNOOZED_NOTIFICATION_CONTEXT_ID,

                                value);

                    });

            out.endTag(null, XML_TAG_NAME);

        }

    }



    private interface Inserter<T> {

        void insert(T t) throws IOException;
@@ -522,7 +577,6 @@ public class SnoozeHelper { 
            ArrayMap<String, ArrayMap<String, T>> targets, String tag,

            Inserter<T> attributeInserter)

            throws IOException {

        synchronized (targets) {

        final int M = targets.size();

        for (int i = 0; i < M; i++) {

            // T is a String (snoozed until context) or Long (snoozed until time)
@@ -533,6 +587,11 @@ public class SnoozeHelper { 
                String pkg = mPackages.get(key);

                Integer userId = mUsers.get(key);



                if (pkg == null || userId == null) {

                    Slog.w(TAG, "pkg " + pkg + " or user " + userId + " missing for " + key);

                    continue;

                }



                out.startTag(null, tag);



                attributeInserter.insert(value);
@@ -550,7 +609,6 @@ public class SnoozeHelper { 
            }

        }

    }

    }



    protected void readXml(XmlPullParser parser, long currentTime)

            throws XmlPullParserException, IOException {
@@ -575,16 +633,18 @@ public class SnoozeHelper { 
                        final Long time = XmlUtils.readLongAttribute(

                                parser, XML_SNOOZED_NOTIFICATION_TIME, 0);

                        if (time > currentTime) { //only read new stuff

                            synchronized (mPersistedSnoozedNotifications) {

                                storeRecord(pkg, key, userId, mPersistedSnoozedNotifications, time);

                            synchronized (mLock) {

                                storeRecordLocked(

                                        pkg, key, userId, mPersistedSnoozedNotifications, time);

                            }

                        }

                    }

                    if (tag.equals(XML_SNOOZED_NOTIFICATION_CONTEXT)) {

                        final String creationId = parser.getAttributeValue(

                                null, XML_SNOOZED_NOTIFICATION_CONTEXT_ID);

                        synchronized (mPersistedSnoozedNotificationsWithContext) {

                            storeRecord(pkg, key, userId, mPersistedSnoozedNotificationsWithContext,

                        synchronized (mLock) {

                            storeRecordLocked(

                                    pkg, key, userId, mPersistedSnoozedNotificationsWithContext,

                                    creationId);

                        }

                    }

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

+1 −1
Original line number Diff line number Diff line
@@ -250,7 +250,7 @@ public class SnoozeHelperTest extends UiServiceTestCase { 
        ArgumentCaptor<Long> captor = ArgumentCaptor.forClass(Long.class);

        verify(mAm, times(1)).setExactAndAllowWhileIdle(

                anyInt(), captor.capture(), any(PendingIntent.class));

        long actualSnoozedUntilDuration = captor.getValue() - SystemClock.elapsedRealtime();

        long actualSnoozedUntilDuration = captor.getValue() - System.currentTimeMillis();

        assertTrue(Math.abs(actualSnoozedUntilDuration - 1000) < 250);

        assertTrue(mSnoozeHelper.isSnoozed(

                UserHandle.USER_SYSTEM, r.getSbn().getPackageName(), r.getKey()));