Merge "Fix unbounded number of uncached shortcuts" into tm-qpr-dev (00825873) · Commits · e / os / android_frameworks_base

core/proto/android/server/peopleservice.proto

+4 −1

Original line number	Diff line number	Diff line
		@@ -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

+48 −1

Original line number	Diff line number	Diff line
		@@ -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

+20 −9

Original line number	Diff line number	Diff line
		@@ -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);

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

+106 −0

Original line number	Diff line number	Diff line
		@@ -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

+72 −1

Original line number	Diff line number	Diff line
		@@ -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 {