diff options
5 files changed, 250 insertions, 12 deletions
diff --git a/core/proto/android/server/peopleservice.proto b/core/proto/android/server/peopleservice.proto index c465233036c4..a96ec41d3708 100644 --- a/core/proto/android/server/peopleservice.proto +++ b/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. diff --git a/services/people/java/com/android/server/people/data/ConversationInfo.java b/services/people/java/com/android/server/people/data/ConversationInfo.java index 16c4c29a798e..6ead44a8cd1b 100644 --- a/services/people/java/com/android/server/people/data/ConversationInfo.java +++ b/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; diff --git a/services/people/java/com/android/server/people/data/DataManager.java b/services/people/java/com/android/server/people/data/DataManager.java index d305fc5d7dc4..693f3a0cf8a0 100644 --- a/services/people/java/com/android/server/people/data/DataManager.java +++ b/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.getLastEventTimestamp() > 0L; + conversationInfo.getParentNotificationChannelId()); } 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); diff --git a/services/tests/servicestests/src/com/android/server/people/data/ConversationInfoTest.java b/services/tests/servicestests/src/com/android/server/people/data/ConversationInfoTest.java index 8139310a4c3d..c90064eaa810 100644 --- a/services/tests/servicestests/src/com/android/server/people/data/ConversationInfoTest.java +++ b/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 + } } diff --git a/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java b/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java index 2a4896a7b041..66c3f0730404 100644 --- a/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java +++ b/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 @@ -1163,6 +1164,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 { mDataManager.onUserUnlocked(USER_ID_PRIMARY); |