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

Commit 9050d9a4 authored by Julia Reynolds's avatar Julia Reynolds
Browse files

Some issues with persisted snoozed notifications

- NPEs because they weren't being added to all of the
necessary data structures
- AlarmManager cannot schedule alarms before activitymanager
is ready, so schedule alarms in ALARM_MANAGER_READY phase
instead of NMS startup phase
- persisted notificatoins have persist the time they should return,
not the duration, so don't add elapsedRealtime to their times

Test: atest
Fixes: 149835514
Change-Id: I0fd14936180811f7079d3d1530e3ddb503cdb448
parent 23c9b1f2
Loading
Loading
Loading
Loading
+3 −1
Original line number Diff line number Diff line
@@ -772,7 +772,7 @@ public class NotificationManagerService extends SystemService {
                        parser, mAllowedManagedServicePackages, forRestore, userId);
                migratedManagedServices = true;
            } else if (mSnoozeHelper.XML_TAG_NAME.equals(parser.getName())) {
                mSnoozeHelper.readXml(parser);
                mSnoozeHelper.readXml(parser, System.currentTimeMillis());
            }
            if (LOCKSCREEN_ALLOW_SECURE_NOTIFICATIONS_TAG.equals(parser.getName())) {
                if (forRestore && userId != UserHandle.USER_SYSTEM) {
@@ -2322,6 +2322,8 @@ public class NotificationManagerService extends SystemService {
            mConditionProviders.onBootPhaseAppsCanStart();
            mHistoryManager.onBootPhaseAppsCanStart();
            registerDeviceConfigChange();
        } else if (phase == SystemService.PHASE_ACTIVITY_MANAGER_READY) {
            mSnoozeHelper.scheduleRepostsForPersistedNotifications(System.currentTimeMillis());
        }
    }

+38 −17
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import android.util.Slog;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.logging.MetricsLogger;
import com.android.internal.logging.nano.MetricsProto;
import com.android.internal.util.XmlUtils;

import org.xmlpull.v1.XmlPullParser;
import org.xmlpull.v1.XmlPullParserException;
@@ -43,9 +44,7 @@ import org.xmlpull.v1.XmlSerializer;

import java.io.IOException;
import java.io.PrintWriter;
import java.sql.Array;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
@@ -172,7 +171,7 @@ public class SnoozeHelper {
            for (int i = 0; i < allRecords.size(); i++) {
                NotificationRecord r = allRecords.valueAt(i);
                String currentGroupKey = r.getSbn().getGroup();
                if (currentGroupKey.equals(groupKey)) {
                if (Objects.equals(currentGroupKey, groupKey)) {
                    records.add(r);
                }
            }
@@ -217,7 +216,7 @@ public class SnoozeHelper {

        snooze(record);
        scheduleRepost(pkg, key, userId, duration);
        Long activateAt = System.currentTimeMillis() + duration;
        Long activateAt = SystemClock.elapsedRealtime() + duration;
        synchronized (mPersistedSnoozedNotifications) {
            storeRecord(pkg, key, userId, mPersistedSnoozedNotifications, activateAt);
        }
@@ -244,8 +243,6 @@ public class SnoozeHelper {
        }
        storeRecord(record.getSbn().getPackageName(), record.getKey(),
                userId, mSnoozedNotifications, record);
        mPackages.put(record.getKey(), record.getSbn().getPackageName());
        mUsers.put(record.getKey(), userId);
    }

    private <T> void storeRecord(String pkg, String key, Integer userId,
@@ -258,6 +255,8 @@ public class SnoozeHelper {
        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,
@@ -425,12 +424,34 @@ public class SnoozeHelper {
                PendingIntent.FLAG_UPDATE_CURRENT);
    }

    public void scheduleRepostsForPersistedNotifications(long currentTime) {
        for (ArrayMap<String, Long> snoozed : mPersistedSnoozedNotifications.values()) {
            for (int i = 0; i < snoozed.size(); i++) {
                String key = snoozed.keyAt(i);
                Long time = snoozed.valueAt(i);
                String pkg = mPackages.get(key);
                Integer userId = mUsers.get(key);
                if (time == null || pkg == null || userId == null) {
                    Slog.w(TAG, "data out of sync: " + time + "|" + pkg + "|" + userId);
                    continue;
                }
                if (time != null && time > currentTime) {
                    scheduleRepostAtTime(pkg, key, userId, time);
                }
            }

        }
    }

    private void scheduleRepost(String pkg, String key, int userId, long duration) {
        scheduleRepostAtTime(pkg, key, userId, SystemClock.elapsedRealtime() + duration);
    }

    private void scheduleRepostAtTime(String pkg, String key, int userId, long time) {
        long identity = Binder.clearCallingIdentity();
        try {
            final PendingIntent pi = createPendingIntent(pkg, key, userId);
            mAm.cancel(pi);
            long time = SystemClock.elapsedRealtime() + duration;
            if (DEBUG) Slog.d(TAG, "Scheduling evaluate for " + new Date(time));
            mAm.setExactAndAllowWhileIdle(AlarmManager.ELAPSED_REALTIME_WAKEUP, time, pi);
        } finally {
@@ -496,6 +517,7 @@ public class SnoozeHelper {
    private interface Inserter<T> {
        void insert(T t) throws IOException;
    }

    private <T> void writeXml(XmlSerializer out,
            ArrayMap<String, ArrayMap<String, T>> targets, String tag,
            Inserter<T> attributeInserter)
@@ -503,12 +525,13 @@ public class SnoozeHelper {
        synchronized (targets) {
            final int M = targets.size();
            for (int i = 0; i < M; i++) {
                String userIdPkgKey = targets.keyAt(i);
                // T is a String (snoozed until context) or Long (snoozed until time)
                ArrayMap<String, T> keyToValue = targets.valueAt(i);
                for (int j = 0; j < keyToValue.size(); j++) {
                    String key = keyToValue.keyAt(j);
                    T value = keyToValue.valueAt(j);
                    String pkg = mPackages.get(key);
                    Integer userId = mUsers.get(key);

                    out.startTag(null, tag);

@@ -518,8 +541,7 @@ public class SnoozeHelper {
                            XML_SNOOZED_NOTIFICATION_VERSION);
                    out.attribute(null, XML_SNOOZED_NOTIFICATION_KEY, key);

                    String pkg = mPackages.get(key);
                    int userId = mUsers.get(key);

                    out.attribute(null, XML_SNOOZED_NOTIFICATION_PKG, pkg);
                    out.attribute(null, XML_SNOOZED_NOTIFICATION_USER_ID,
                            String.valueOf(userId));
@@ -530,7 +552,7 @@ public class SnoozeHelper {
        }
    }

    protected void readXml(XmlPullParser parser)
    protected void readXml(XmlPullParser parser, long currentTime)
            throws XmlPullParserException, IOException {
        int type;
        while ((type = parser.next()) != XmlPullParser.END_DOCUMENT) {
@@ -547,16 +569,15 @@ public class SnoozeHelper {
                try {
                    final String key = parser.getAttributeValue(null, XML_SNOOZED_NOTIFICATION_KEY);
                    final String pkg = parser.getAttributeValue(null, XML_SNOOZED_NOTIFICATION_PKG);
                    final int userId = Integer.parseInt(
                            parser.getAttributeValue(null, XML_SNOOZED_NOTIFICATION_USER_ID));
                    final int userId = XmlUtils.readIntAttribute(
                            parser, XML_SNOOZED_NOTIFICATION_USER_ID, UserHandle.USER_ALL);
                    if (tag.equals(XML_SNOOZED_NOTIFICATION)) {
                        final Long time = Long.parseLong(
                                parser.getAttributeValue(null, XML_SNOOZED_NOTIFICATION_TIME));
                        if (time > System.currentTimeMillis()) { //only read new stuff
                        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);
                            }
                            scheduleRepost(pkg, key, userId, time - System.currentTimeMillis());
                        }
                    }
                    if (tag.equals(XML_SNOOZED_NOTIFICATION_CONTEXT)) {
+1 −1
Original line number Diff line number Diff line
@@ -3246,7 +3246,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
                new BufferedInputStream(new ByteArrayInputStream(upgradeXml.getBytes())),
                false,
                UserHandle.USER_ALL);
        verify(mSnoozeHelper, times(1)).readXml(any(XmlPullParser.class));
        verify(mSnoozeHelper, times(1)).readXml(any(XmlPullParser.class), anyLong());
    }

    @Test
+72 −7
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertNull;
import static junit.framework.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyLong;
@@ -96,10 +97,33 @@ public class SnoozeHelperTest extends UiServiceTestCase {
        XmlPullParser parser = Xml.newPullParser();
        parser.setInput(new BufferedInputStream(
                new ByteArrayInputStream(xml_string.getBytes())), null);
        mSnoozeHelper.readXml(parser);
        assertTrue("Should read the notification time from xml and it should be more than zero",
                0 < mSnoozeHelper.getSnoozeTimeForUnpostedNotification(
                        0, "pkg", "key").doubleValue());
        mSnoozeHelper.readXml(parser, 1);
        assertEquals((long) Long.MAX_VALUE, (long) mSnoozeHelper
                .getSnoozeTimeForUnpostedNotification(0, "pkg", "key"));
        verify(mAm, never()).setExactAndAllowWhileIdle(anyInt(), anyLong(), any());
    }

    @Test
    public void testWriteXML_afterReading_noNPE()
            throws XmlPullParserException, IOException {
        final String max_time_str = Long.toString(Long.MAX_VALUE);
        final String xml_string = "<snoozed-notifications>"
                + "<notification version=\"1\" user-id=\"0\" notification=\"notification\" "
                + "pkg=\"pkg\" key=\"key\" time=\"" + max_time_str + "\"/>"
                + "<notification version=\"1\" user-id=\"0\" notification=\"notification\" "
                + "pkg=\"pkg\" key=\"key2\" time=\"" + max_time_str + "\"/>"
                + "</snoozed-notifications>";
        XmlPullParser parser = Xml.newPullParser();
        parser.setInput(new BufferedInputStream(
                new ByteArrayInputStream(xml_string.getBytes())), null);
        mSnoozeHelper.readXml(parser, 1);
        ByteArrayOutputStream baos = new ByteArrayOutputStream();
        XmlSerializer serializer = new FastXmlSerializer();
        serializer.setOutput(new BufferedOutputStream(baos), "utf-8");
        serializer.startDocument(null, true);
        mSnoozeHelper.writeXml(serializer);
        serializer.endDocument();
        serializer.flush();
    }

    @Test
@@ -115,7 +139,7 @@ public class SnoozeHelperTest extends UiServiceTestCase {
        XmlPullParser parser = Xml.newPullParser();
        parser.setInput(new BufferedInputStream(
                new ByteArrayInputStream(xml_string.getBytes())), null);
        mSnoozeHelper.readXml(parser);
        mSnoozeHelper.readXml(parser, 1);
        assertEquals("Should read the notification context from xml and it should be `uri",
                "uri", mSnoozeHelper.getSnoozeContextForUnpostedNotification(
                        0, "pkg", "key"));
@@ -137,7 +161,7 @@ public class SnoozeHelperTest extends UiServiceTestCase {
        XmlPullParser parser = Xml.newPullParser();
        parser.setInput(new BufferedInputStream(
                new ByteArrayInputStream(baos.toByteArray())), "utf-8");
        mSnoozeHelper.readXml(parser);
        mSnoozeHelper.readXml(parser, 1);
        assertTrue("Should read the notification time from xml and it should be more than zero",
                0 < mSnoozeHelper.getSnoozeTimeForUnpostedNotification(
                        0, "pkg", r.getKey()).doubleValue());
@@ -161,7 +185,7 @@ public class SnoozeHelperTest extends UiServiceTestCase {
        XmlPullParser parser = Xml.newPullParser();
        parser.setInput(new BufferedInputStream(
                new ByteArrayInputStream(baos.toByteArray())), "utf-8");
        mSnoozeHelper.readXml(parser);
        mSnoozeHelper.readXml(parser, 2);
        int systemUser = UserHandle.SYSTEM.getIdentifier();
        assertTrue("Should see a past time returned",
                System.currentTimeMillis() >  mSnoozeHelper.getSnoozeTimeForUnpostedNotification(
@@ -194,6 +218,30 @@ public class SnoozeHelperTest extends UiServiceTestCase {
                        "not_my_package", r.getKey()).longValue());
    }

    @Test
    public void testScheduleRepostsForPersistedNotifications() throws Exception {
        final String xml_string = "<snoozed-notifications>"
                + "<notification version=\"1\" user-id=\"0\" notification=\"notification\" "
                + "pkg=\"pkg\" key=\"key\" time=\"" + 10 + "\"/>"
                + "<notification version=\"1\" user-id=\"0\" notification=\"notification\" "
                + "pkg=\"pkg\" key=\"key2\" time=\"" + 15+ "\"/>"
                + "</snoozed-notifications>";
        XmlPullParser parser = Xml.newPullParser();
        parser.setInput(new BufferedInputStream(
                new ByteArrayInputStream(xml_string.getBytes())), null);
        mSnoozeHelper.readXml(parser, 4);

        mSnoozeHelper.scheduleRepostsForPersistedNotifications(5);

        ArgumentCaptor<PendingIntent> captor = ArgumentCaptor.forClass(PendingIntent.class);
        verify(mAm).setExactAndAllowWhileIdle(anyInt(), eq((long) 10), captor.capture());
        assertEquals("key", captor.getValue().getIntent().getStringExtra(EXTRA_KEY));

        ArgumentCaptor<PendingIntent> captor2 = ArgumentCaptor.forClass(PendingIntent.class);
        verify(mAm).setExactAndAllowWhileIdle(anyInt(), eq((long) 15), captor2.capture());
        assertEquals("key2", captor2.getValue().getIntent().getStringExtra(EXTRA_KEY));
    }

    @Test
    public void testSnoozeForTime() throws Exception {
        NotificationRecord r = getNotificationRecord("pkg", 1, "one", UserHandle.SYSTEM);
@@ -413,6 +461,23 @@ public class SnoozeHelperTest extends UiServiceTestCase {
                mSnoozeHelper.getNotifications("pkg", "group", UserHandle.USER_CURRENT).size());
    }

    @Test
    public void testGetSnoozedGroupNotifications_nonGrouped() throws Exception {
        IntArray profileIds = new IntArray();
        profileIds.add(UserHandle.USER_CURRENT);
        when(mUserProfiles.getCurrentProfileIds()).thenReturn(profileIds);
        NotificationRecord r = getNotificationRecord("pkg", 1, "tag",
                UserHandle.CURRENT, "group", true);
        NotificationRecord r2 = getNotificationRecord("pkg", 2, "tag",
                UserHandle.CURRENT, null, true);
        mSnoozeHelper.snooze(r, 1000);
        mSnoozeHelper.snooze(r2, 1000);

        assertEquals(1,
                mSnoozeHelper.getNotifications("pkg", "group", UserHandle.USER_CURRENT).size());
        // and no NPE
    }

    @Test
    public void testGetSnoozedNotificationByKey() throws Exception {
        IntArray profileIds = new IntArray();