Merge "Fix unbounded number of uncached shortcuts" into tm-qpr-dev

core/proto/android/server/peopleservice.proto

@@ -62,7 +62,10 @@ message ConversationInfoProto {
   // The timestamp of the last event in millis.
   optional int64 last_event_timestamp = 9;
 
-  // Next tag: 10
+  // The timestamp this conversation was created in millis.
+  optional int64 creation_timestamp = 10;
+
+  // Next tag: 11
 }
 
 // On disk data of events.

services/people/java/com/android/server/people/data/ConversationInfo.java

@@ -26,6 +26,7 @@ import android.content.pm.ShortcutInfo;
 import android.content.pm.ShortcutInfo.ShortcutFlags;
 import android.net.Uri;
 import android.text.TextUtils;
+import android.util.Log;
 import android.util.Slog;
 import android.util.proto.ProtoInputStream;
 import android.util.proto.ProtoOutputStream;
@@ -37,6 +38,7 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
+import java.io.EOFException;
 import java.io.IOException;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -50,6 +52,11 @@ import java.util.Objects;
  * Represents a conversation that is provided by the app based on {@link ShortcutInfo}.
  */
 public class ConversationInfo {
+    private static final boolean DEBUG = false;
+
+    // Schema version for the backup payload. Must be incremented whenever fields are added in
+    // backup payload.
+    private static final int VERSION = 1;
 
     private static final String TAG = ConversationInfo.class.getSimpleName();
 
@@ -100,6 +107,8 @@ public class ConversationInfo {
 
     private long mLastEventTimestamp;
 
+    private long mCreationTimestamp;
+
     @ShortcutFlags
     private int mShortcutFlags;
 
@@ -116,6 +125,7 @@ public class ConversationInfo {
         mNotificationChannelId = builder.mNotificationChannelId;
         mParentNotificationChannelId = builder.mParentNotificationChannelId;
         mLastEventTimestamp = builder.mLastEventTimestamp;
+        mCreationTimestamp = builder.mCreationTimestamp;
         mShortcutFlags = builder.mShortcutFlags;
         mConversationFlags = builder.mConversationFlags;
         mCurrStatuses = builder.mCurrStatuses;
@@ -170,6 +180,13 @@ public class ConversationInfo {
         return mLastEventTimestamp;
     }
 
+    /**
+     * Timestamp of the creation of the conversationInfo.
+     */
+    long getCreationTimestamp() {
+        return mCreationTimestamp;
+    }
+
     /** Whether the shortcut for this conversation is set long-lived by the app. */
     public boolean isShortcutLongLived() {
         return hasShortcutFlags(ShortcutInfo.FLAG_LONG_LIVED);
@@ -241,6 +258,7 @@ public class ConversationInfo {
                 && Objects.equals(mNotificationChannelId, other.mNotificationChannelId)
                 && Objects.equals(mParentNotificationChannelId, other.mParentNotificationChannelId)
                 && Objects.equals(mLastEventTimestamp, other.mLastEventTimestamp)
+                && mCreationTimestamp == other.mCreationTimestamp
                 && mShortcutFlags == other.mShortcutFlags
                 && mConversationFlags == other.mConversationFlags
                 && Objects.equals(mCurrStatuses, other.mCurrStatuses);
@@ -250,7 +268,7 @@ public class ConversationInfo {
     public int hashCode() {
         return Objects.hash(mShortcutId, mLocusId, mContactUri, mContactPhoneNumber,
                 mNotificationChannelId, mParentNotificationChannelId, mLastEventTimestamp,
-                mShortcutFlags, mConversationFlags, mCurrStatuses);
+                mCreationTimestamp, mShortcutFlags, mConversationFlags, mCurrStatuses);
     }
 
     @Override
@@ -264,6 +282,7 @@ public class ConversationInfo {
         sb.append(", notificationChannelId=").append(mNotificationChannelId);
         sb.append(", parentNotificationChannelId=").append(mParentNotificationChannelId);
         sb.append(", lastEventTimestamp=").append(mLastEventTimestamp);
+        sb.append(", creationTimestamp=").append(mCreationTimestamp);
         sb.append(", statuses=").append(mCurrStatuses);
         sb.append(", shortcutFlags=0x").append(Integer.toHexString(mShortcutFlags));
         sb.append(" [");
@@ -329,6 +348,7 @@ public class ConversationInfo {
                     mParentNotificationChannelId);
         }
         protoOutputStream.write(ConversationInfoProto.LAST_EVENT_TIMESTAMP, mLastEventTimestamp);
+        protoOutputStream.write(ConversationInfoProto.CREATION_TIMESTAMP, mCreationTimestamp);
         protoOutputStream.write(ConversationInfoProto.SHORTCUT_FLAGS, mShortcutFlags);
         protoOutputStream.write(ConversationInfoProto.CONVERSATION_FLAGS, mConversationFlags);
         if (mContactPhoneNumber != null) {
@@ -352,6 +372,8 @@ public class ConversationInfo {
             out.writeUTF(mContactPhoneNumber != null ? mContactPhoneNumber : "");
             out.writeUTF(mParentNotificationChannelId != null ? mParentNotificationChannelId : "");
             out.writeLong(mLastEventTimestamp);
+            out.writeInt(VERSION);
+            out.writeLong(mCreationTimestamp);
             // ConversationStatus is a transient object and not persisted
         } catch (IOException e) {
             Slog.e(TAG, "Failed to write fields to backup payload.", e);
@@ -399,6 +421,9 @@ public class ConversationInfo {
                     builder.setLastEventTimestamp(protoInputStream.readLong(
                             ConversationInfoProto.LAST_EVENT_TIMESTAMP));
                     break;
+                case (int) ConversationInfoProto.CREATION_TIMESTAMP:
+                    builder.setCreationTimestamp(protoInputStream.readLong(
+                            ConversationInfoProto.CREATION_TIMESTAMP));
                 case (int) ConversationInfoProto.SHORTCUT_FLAGS:
                     builder.setShortcutFlags(protoInputStream.readInt(
                             ConversationInfoProto.SHORTCUT_FLAGS));
@@ -448,6 +473,10 @@ public class ConversationInfo {
                 builder.setParentNotificationChannelId(parentNotificationChannelId);
             }
             builder.setLastEventTimestamp(in.readLong());
+            int payloadVersion = maybeReadVersion(in);
+            if (payloadVersion == 1) {
+                builder.setCreationTimestamp(in.readLong());
+            }
         } catch (IOException e) {
             Slog.e(TAG, "Failed to read conversation info fields from backup payload.", e);
             return null;
@@ -455,6 +484,16 @@ public class ConversationInfo {
         return builder.build();
     }
 
+    private static int maybeReadVersion(DataInputStream in) throws IOException {
+        try {
+            return in.readInt();
+        } catch (EOFException eofException) {
+            // EOF is expected if using old backup payload protocol.
+            if (DEBUG) Log.d(TAG, "Eof reached for data stream, missing version number");
+            return 0;
+        }
+    }
+
     /**
      * Builder class for {@link ConversationInfo} objects.
      */
@@ -479,6 +518,8 @@ public class ConversationInfo {
 
         private long mLastEventTimestamp;
 
+        private long mCreationTimestamp;
+
         @ShortcutFlags
         private int mShortcutFlags;
 
@@ -502,6 +543,7 @@ public class ConversationInfo {
             mNotificationChannelId = conversationInfo.mNotificationChannelId;
             mParentNotificationChannelId = conversationInfo.mParentNotificationChannelId;
             mLastEventTimestamp = conversationInfo.mLastEventTimestamp;
+            mCreationTimestamp = conversationInfo.mCreationTimestamp;
             mShortcutFlags = conversationInfo.mShortcutFlags;
             mConversationFlags = conversationInfo.mConversationFlags;
             mCurrStatuses = conversationInfo.mCurrStatuses;
@@ -542,6 +584,11 @@ public class ConversationInfo {
             return this;
         }
 
+        Builder setCreationTimestamp(long creationTimestamp) {
+            mCreationTimestamp = creationTimestamp;
+            return this;
+        }
+
         Builder setShortcutFlags(@ShortcutFlags int shortcutFlags) {
             mShortcutFlags = shortcutFlags;
             return this;

services/people/java/com/android/server/people/data/DataManager.java

@@ -816,10 +816,18 @@ public class DataManager {
     }
 
     private boolean isCachedRecentConversation(ConversationInfo conversationInfo) {
+        return isEligibleForCleanUp(conversationInfo)
+                && conversationInfo.getLastEventTimestamp() > 0L;
+    }
+
+    /**
+     * Conversations that are cached and not customized are eligible for clean-up, even if they
+     * don't have an associated notification event with them.
+     */
+    private boolean isEligibleForCleanUp(ConversationInfo conversationInfo) {
         return conversationInfo.isShortcutCachedForNotification()
                 && Objects.equals(conversationInfo.getNotificationChannelId(),
-                conversationInfo.getParentNotificationChannelId())
+                conversationInfo.getParentNotificationChannelId());
-                && conversationInfo.getLastEventTimestamp() > 0L;
     }
 
     private boolean hasActiveNotifications(String packageName, @UserIdInt int userId,
@@ -842,14 +850,14 @@ public class DataManager {
         }
         // pair of <package name, conversation info>
         List<Pair<String, ConversationInfo>> cachedConvos = new ArrayList<>();
-        userData.forAllPackages(packageData ->
+        userData.forAllPackages(packageData -> {
                 packageData.forAllConversations(conversationInfo -> {
-                    if (isCachedRecentConversation(conversationInfo)) {
+                    if (isEligibleForCleanUp(conversationInfo)) {
                         cachedConvos.add(
                                 Pair.create(packageData.getPackageName(), conversationInfo));
                     }
-                })
+                });
-        );
+        });
         if (cachedConvos.size() <= targetCachedCount) {
             return;
         }
@@ -858,7 +866,9 @@ public class DataManager {
         PriorityQueue<Pair<String, ConversationInfo>> maxHeap = new PriorityQueue<>(
                 numToUncache + 1,
                 Comparator.comparingLong((Pair<String, ConversationInfo> pair) ->
-                        pair.second.getLastEventTimestamp()).reversed());
+                        Math.max(
+                            pair.second.getLastEventTimestamp(),
+                            pair.second.getCreationTimestamp())).reversed());
         for (Pair<String, ConversationInfo> cached : cachedConvos) {
             if (hasActiveNotifications(cached.first, userId, cached.second.getShortcutId())) {
                 continue;
@@ -893,7 +903,7 @@ public class DataManager {
         }
         ConversationInfo.Builder builder = oldConversationInfo != null
                 ? new ConversationInfo.Builder(oldConversationInfo)
-                : new ConversationInfo.Builder();
+                : new ConversationInfo.Builder().setCreationTimestamp(System.currentTimeMillis());
 
         builder.setShortcutId(shortcutInfo.getId());
         builder.setLocusId(shortcutInfo.getLocusId());
@@ -1326,7 +1336,8 @@ public class DataManager {
         }
     }
 
-    private void updateConversationStoreThenNotifyListeners(ConversationStore cs,
+    @VisibleForTesting
+    void updateConversationStoreThenNotifyListeners(ConversationStore cs,
             ConversationInfo modifiedConv,
             String packageName, int userId) {
         cs.addOrUpdate(modifiedConv);

services/tests/servicestests/src/com/android/server/people/data/ConversationInfoTest.java

@@ -58,6 +58,7 @@ public final class ConversationInfoTest {
                 .setNotificationChannelId(NOTIFICATION_CHANNEL_ID)
                 .setParentNotificationChannelId(PARENT_NOTIFICATION_CHANNEL_ID)
                 .setLastEventTimestamp(100L)
+                .setCreationTimestamp(200L)
                 .setShortcutFlags(ShortcutInfo.FLAG_LONG_LIVED
                         | ShortcutInfo.FLAG_CACHED_NOTIFICATIONS)
                 .setImportant(true)
@@ -79,6 +80,7 @@ public final class ConversationInfoTest {
         assertEquals(PARENT_NOTIFICATION_CHANNEL_ID,
                 conversationInfo.getParentNotificationChannelId());
         assertEquals(100L, conversationInfo.getLastEventTimestamp());
+        assertEquals(200L, conversationInfo.getCreationTimestamp());
         assertTrue(conversationInfo.isShortcutLongLived());
         assertTrue(conversationInfo.isShortcutCachedForNotification());
         assertTrue(conversationInfo.isImportant());
@@ -105,6 +107,7 @@ public final class ConversationInfoTest {
         assertNull(conversationInfo.getNotificationChannelId());
         assertNull(conversationInfo.getParentNotificationChannelId());
         assertEquals(0L, conversationInfo.getLastEventTimestamp());
+        assertEquals(0L, conversationInfo.getCreationTimestamp());
         assertFalse(conversationInfo.isShortcutLongLived());
         assertFalse(conversationInfo.isShortcutCachedForNotification());
         assertFalse(conversationInfo.isImportant());
@@ -131,6 +134,7 @@ public final class ConversationInfoTest {
                 .setNotificationChannelId(NOTIFICATION_CHANNEL_ID)
                 .setParentNotificationChannelId(PARENT_NOTIFICATION_CHANNEL_ID)
                 .setLastEventTimestamp(100L)
+                .setCreationTimestamp(200L)
                 .setShortcutFlags(ShortcutInfo.FLAG_LONG_LIVED)
                 .setImportant(true)
                 .setNotificationSilenced(true)
@@ -154,6 +158,7 @@ public final class ConversationInfoTest {
         assertEquals(NOTIFICATION_CHANNEL_ID, destination.getNotificationChannelId());
         assertEquals(PARENT_NOTIFICATION_CHANNEL_ID, destination.getParentNotificationChannelId());
         assertEquals(100L, destination.getLastEventTimestamp());
+        assertEquals(200L, destination.getCreationTimestamp());
         assertTrue(destination.isShortcutLongLived());
         assertFalse(destination.isImportant());
         assertTrue(destination.isNotificationSilenced());
@@ -164,4 +169,105 @@ public final class ConversationInfoTest {
         assertThat(destination.getStatuses()).contains(cs);
         assertThat(destination.getStatuses()).contains(cs2);
     }
+
+    @Test
+    public void testBuildFromAnotherConversation_identicalConversation() {
+        ConversationStatus cs = new ConversationStatus.Builder("id", ACTIVITY_ANNIVERSARY).build();
+        ConversationStatus cs2 = new ConversationStatus.Builder("id2", ACTIVITY_GAME).build();
+
+        ConversationInfo source = new ConversationInfo.Builder()
+                .setShortcutId(SHORTCUT_ID)
+                .setLocusId(LOCUS_ID)
+                .setContactUri(CONTACT_URI)
+                .setContactPhoneNumber(PHONE_NUMBER)
+                .setNotificationChannelId(NOTIFICATION_CHANNEL_ID)
+                .setParentNotificationChannelId(PARENT_NOTIFICATION_CHANNEL_ID)
+                .setLastEventTimestamp(100L)
+                .setCreationTimestamp(200L)
+                .setShortcutFlags(ShortcutInfo.FLAG_LONG_LIVED)
+                .setImportant(true)
+                .setNotificationSilenced(true)
+                .setBubbled(true)
+                .setPersonImportant(true)
+                .setPersonBot(true)
+                .setContactStarred(true)
+                .addOrUpdateStatus(cs)
+                .addOrUpdateStatus(cs2)
+                .build();
+
+        ConversationInfo destination = new ConversationInfo.Builder(source).build();
+
+        assertEquals(SHORTCUT_ID, destination.getShortcutId());
+        assertEquals(LOCUS_ID, destination.getLocusId());
+        assertEquals(CONTACT_URI, destination.getContactUri());
+        assertEquals(PHONE_NUMBER, destination.getContactPhoneNumber());
+        assertEquals(NOTIFICATION_CHANNEL_ID, destination.getNotificationChannelId());
+        assertEquals(PARENT_NOTIFICATION_CHANNEL_ID, destination.getParentNotificationChannelId());
+        assertEquals(100L, destination.getLastEventTimestamp());
+        assertEquals(200L, destination.getCreationTimestamp());
+        assertTrue(destination.isShortcutLongLived());
+        assertTrue(destination.isImportant());
+        assertTrue(destination.isNotificationSilenced());
+        assertTrue(destination.isBubbled());
+        assertTrue(destination.isPersonImportant());
+        assertTrue(destination.isPersonBot());
+        assertTrue(destination.isContactStarred());
+        assertThat(destination.getStatuses()).contains(cs);
+        assertThat(destination.getStatuses()).contains(cs2);
+        // Also check equals() implementation
+        assertTrue(source.equals(destination));
+        assertTrue(destination.equals(source));
+    }
+
+    @Test
+    public void testBuildFromBackupPayload() {
+        ConversationStatus cs = new ConversationStatus.Builder("id", ACTIVITY_ANNIVERSARY).build();
+        ConversationStatus cs2 = new ConversationStatus.Builder("id2", ACTIVITY_GAME).build();
+
+        ConversationInfo conversationInfo = new ConversationInfo.Builder()
+                .setShortcutId(SHORTCUT_ID)
+                .setLocusId(LOCUS_ID)
+                .setContactUri(CONTACT_URI)
+                .setContactPhoneNumber(PHONE_NUMBER)
+                .setNotificationChannelId(NOTIFICATION_CHANNEL_ID)
+                .setParentNotificationChannelId(PARENT_NOTIFICATION_CHANNEL_ID)
+                .setLastEventTimestamp(100L)
+                .setCreationTimestamp(200L)
+                .setShortcutFlags(ShortcutInfo.FLAG_LONG_LIVED
+                        | ShortcutInfo.FLAG_CACHED_NOTIFICATIONS)
+                .setImportant(true)
+                .setNotificationSilenced(true)
+                .setBubbled(true)
+                .setDemoted(true)
+                .setPersonImportant(true)
+                .setPersonBot(true)
+                .setContactStarred(true)
+                .addOrUpdateStatus(cs)
+                .addOrUpdateStatus(cs2)
+                .build();
+
+        ConversationInfo conversationInfoFromBackup =
+                ConversationInfo.readFromBackupPayload(conversationInfo.getBackupPayload());
+
+        assertEquals(SHORTCUT_ID, conversationInfoFromBackup.getShortcutId());
+        assertEquals(LOCUS_ID, conversationInfoFromBackup.getLocusId());
+        assertEquals(CONTACT_URI, conversationInfoFromBackup.getContactUri());
+        assertEquals(PHONE_NUMBER, conversationInfoFromBackup.getContactPhoneNumber());
+        assertEquals(
+                NOTIFICATION_CHANNEL_ID, conversationInfoFromBackup.getNotificationChannelId());
+        assertEquals(PARENT_NOTIFICATION_CHANNEL_ID,
+                conversationInfoFromBackup.getParentNotificationChannelId());
+        assertEquals(100L, conversationInfoFromBackup.getLastEventTimestamp());
+        assertEquals(200L, conversationInfoFromBackup.getCreationTimestamp());
+        assertTrue(conversationInfoFromBackup.isShortcutLongLived());
+        assertTrue(conversationInfoFromBackup.isShortcutCachedForNotification());
+        assertTrue(conversationInfoFromBackup.isImportant());
+        assertTrue(conversationInfoFromBackup.isNotificationSilenced());
+        assertTrue(conversationInfoFromBackup.isBubbled());
+        assertTrue(conversationInfoFromBackup.isDemoted());
+        assertTrue(conversationInfoFromBackup.isPersonImportant());
+        assertTrue(conversationInfoFromBackup.isPersonBot());
+        assertTrue(conversationInfoFromBackup.isContactStarred());
+        // ConversationStatus is a transient object and not persisted
+    }
 }

services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java

@@ -291,7 +291,8 @@ public final class DataManagerTest {
                 mShortcutChangeCallbackCaptor.capture());
         mShortcutChangeCallback = mShortcutChangeCallbackCaptor.getValue();
 
-        verify(mContext, times(2)).registerReceiver(any(), any());
+        verify(mContext, times(1)).registerReceiver(any(), any());
+        verify(mContext, times(1)).registerReceiver(any(), any(), anyInt());
     }
 
     @After
@@ -1162,6 +1163,76 @@ public final class DataManagerTest {
         }
     }
 
+    @Test
+    public void testUncacheOldestCachedShortcut_missingNotificationEvents() {
+        mDataManager.onUserUnlocked(USER_ID_PRIMARY);
+
+        for (int i = 0; i < DataManager.MAX_CACHED_RECENT_SHORTCUTS + 1; i++) {
+            String shortcutId = TEST_SHORTCUT_ID + i;
+            ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, shortcutId,
+                    buildPerson());
+            shortcut.setCached(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS);
+            mShortcutChangeCallback.onShortcutsAddedOrUpdated(
+                    TEST_PKG_NAME,
+                    Collections.singletonList(shortcut),
+                    UserHandle.of(USER_ID_PRIMARY));
+            mLooper.dispatchAll();
+        }
+
+        // Only the shortcut #0 is uncached, all the others are not.
+        verify(mShortcutServiceInternal).uncacheShortcuts(
+                anyInt(), any(), eq(TEST_PKG_NAME),
+                eq(Collections.singletonList(TEST_SHORTCUT_ID + 0)), eq(USER_ID_PRIMARY),
+                eq(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS));
+        for (int i = 1; i < DataManager.MAX_CACHED_RECENT_SHORTCUTS + 1; i++) {
+            verify(mShortcutServiceInternal, never()).uncacheShortcuts(
+                    anyInt(), anyString(), anyString(),
+                    eq(Collections.singletonList(TEST_SHORTCUT_ID + i)), anyInt(),
+                    eq(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS));
+        }
+    }
+
+    @Test
+    public void testUncacheOldestCachedShortcut_legacyConversation() {
+        mDataManager.onUserUnlocked(USER_ID_PRIMARY);
+
+        // Add an extra conversation with a legacy type (no creationTime)
+        ConversationStore conversationStore = mDataManager
+                .getUserDataForTesting(USER_ID_PRIMARY)
+                .getOrCreatePackageData(TEST_PKG_NAME)
+                .getConversationStore();
+        ConversationInfo.Builder builder = new ConversationInfo.Builder();
+        builder.setShortcutId(TEST_SHORTCUT_ID + 0);
+        builder.setShortcutFlags(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS);
+        mDataManager.updateConversationStoreThenNotifyListeners(
+                conversationStore,
+                builder.build(),
+                TEST_PKG_NAME, USER_ID_PRIMARY);
+        for (int i = 1; i < DataManager.MAX_CACHED_RECENT_SHORTCUTS + 1; i++) {
+            String shortcutId = TEST_SHORTCUT_ID + i;
+            ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, shortcutId,
+                    buildPerson());
+            shortcut.setCached(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS);
+            mShortcutChangeCallback.onShortcutsAddedOrUpdated(
+                    TEST_PKG_NAME,
+                    Collections.singletonList(shortcut),
+                    UserHandle.of(USER_ID_PRIMARY));
+            mLooper.dispatchAll();
+        }
+
+        // Only the shortcut #0 is uncached, all the others are not.
+        verify(mShortcutServiceInternal).uncacheShortcuts(
+                anyInt(), any(), eq(TEST_PKG_NAME),
+                eq(Collections.singletonList(TEST_SHORTCUT_ID + 0)), eq(USER_ID_PRIMARY),
+                eq(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS));
+        for (int i = 1; i < DataManager.MAX_CACHED_RECENT_SHORTCUTS + 1; i++) {
+            verify(mShortcutServiceInternal, never()).uncacheShortcuts(
+                    anyInt(), anyString(), anyString(),
+                    eq(Collections.singletonList(TEST_SHORTCUT_ID + i)), anyInt(),
+                    eq(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS));
+        }
+    }
+
     @Test
     public void testBackupAndRestoration()
             throws IntentFilter.MalformedMimeTypeException {