From a625b94d3f3c8586bd47638d820dcb0713a896c0 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Sat, 15 Feb 2020 09:42:23 -0500 Subject: [PATCH] Clean up settings and history for deleted convos Test: atest Fixes: 149556404 Change-Id: I998ecf39b794b4b3b3daca44875f9754797e0187 --- .../java/android/app/NotificationHistory.java | 20 ++++++++ .../android/app/NotificationHistoryTest.java | 38 ++++++++++++++ .../NotificationHistoryDatabase.java | 50 ++++++++++++++++++- .../NotificationHistoryManager.java | 16 ++++++ .../NotificationManagerService.java | 16 ++++++ .../notification/PreferencesHelper.java | 27 ++++++++++ .../NotificationHistoryDatabaseTest.java | 46 +++++++++++++++++ .../NotificationHistoryManagerTest.java | 14 ++++++ .../notification/PreferencesHelperTest.java | 44 +++++++++++++++- 9 files changed, 269 insertions(+), 2 deletions(-) diff --git a/core/java/android/app/NotificationHistory.java b/core/java/android/app/NotificationHistory.java index d16120dedd24e..8c2cc9492e39e 100644 --- a/core/java/android/app/NotificationHistory.java +++ b/core/java/android/app/NotificationHistory.java @@ -383,6 +383,26 @@ public final class NotificationHistory implements Parcelable { return removed; } + /** + * Removes all notifications from a conversation and regenerates the string pool + */ + public boolean removeConversationFromWrite(String packageName, String conversationId) { + boolean removed = false; + for (int i = mNotificationsToWrite.size() - 1; i >= 0; i--) { + HistoricalNotification hn = mNotificationsToWrite.get(i); + if (packageName.equals(hn.getPackage()) + && conversationId.equals(hn.getConversationId())) { + removed = true; + mNotificationsToWrite.remove(i); + } + } + if (removed) { + poolStringsFromNotifications(); + } + + return removed; + } + /** * Gets pooled strings in order to write them to disk */ diff --git a/core/tests/coretests/src/android/app/NotificationHistoryTest.java b/core/tests/coretests/src/android/app/NotificationHistoryTest.java index 8d8acb75ae03a..0443d3a9e73f0 100644 --- a/core/tests/coretests/src/android/app/NotificationHistoryTest.java +++ b/core/tests/coretests/src/android/app/NotificationHistoryTest.java @@ -288,6 +288,44 @@ public class NotificationHistoryTest { .containsExactlyElementsIn(postRemoveExpectedEntries); } + @Test + public void testRemoveConversationNotificationFromWrite() { + NotificationHistory history = new NotificationHistory(); + + List postRemoveExpectedEntries = new ArrayList<>(); + List postRemoveExpectedStrings = new ArrayList<>(); + for (int i = 1; i <= 10; i++) { + HistoricalNotification n = getHistoricalNotification("pkg", i); + + if (i != 2) { + postRemoveExpectedStrings.add(n.getPackage()); + postRemoveExpectedStrings.add(n.getChannelName()); + postRemoveExpectedStrings.add(n.getChannelId()); + if (n.getConversationId() != null) { + postRemoveExpectedStrings.add(n.getConversationId()); + } + postRemoveExpectedEntries.add(n); + } + + history.addNotificationToWrite(n); + } + // add second notification with the same conversation id that will be removed + history.addNotificationToWrite(getHistoricalNotification("pkg", 2)); + + history.poolStringsFromNotifications(); + + assertThat(history.getNotificationsToWrite().size()).isEqualTo(11); + // 1 package name and 20 unique channel names and ids and 5 conversation ids + assertThat(history.getPooledStringsToWrite().length).isEqualTo(26); + + history.removeConversationFromWrite("pkg", "convo2"); + + // 1 package names and 9 * 2 unique channel names and ids and 4 conversation ids + assertThat(history.getPooledStringsToWrite().length).isEqualTo(23); + assertThat(history.getNotificationsToWrite()) + .containsExactlyElementsIn(postRemoveExpectedEntries); + } + @Test public void testParceling() { NotificationHistory history = new NotificationHistory(); diff --git a/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java b/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java index dc61fb01a246e..dbaf8241ddd58 100644 --- a/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java +++ b/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java @@ -175,6 +175,11 @@ public class NotificationHistoryDatabase { mFileWriteHandler.post(rnr); } + public void deleteConversation(String pkg, String conversationId) { + RemoveConversationRunnable rcr = new RemoveConversationRunnable(pkg, conversationId); + mFileWriteHandler.post(rcr); + } + public void addNotification(final HistoricalNotification notification) { synchronized (mLock) { mBuffer.addNewNotificationToWrite(notification); @@ -396,7 +401,7 @@ public class NotificationHistoryDatabase { @Override public void run() { - if (DEBUG) Slog.d(TAG, "RemovePackageRunnable"); + if (DEBUG) Slog.d(TAG, "RemoveNotificationRunnable"); synchronized (mLock) { // Remove from pending history mBuffer.removeNotificationFromWrite(mPkg, mPostedTime); @@ -422,6 +427,49 @@ public class NotificationHistoryDatabase { } } + final class RemoveConversationRunnable implements Runnable { + private String mPkg; + private String mConversationId; + private NotificationHistory mNotificationHistory; + + public RemoveConversationRunnable(String pkg, String conversationId) { + mPkg = pkg; + mConversationId = conversationId; + } + + @VisibleForTesting + void setNotificationHistory(NotificationHistory nh) { + mNotificationHistory = nh; + } + + @Override + public void run() { + if (DEBUG) Slog.d(TAG, "RemoveConversationRunnable"); + synchronized (mLock) { + // Remove from pending history + mBuffer.removeConversationFromWrite(mPkg, mConversationId); + + Iterator historyFileItr = mHistoryFiles.iterator(); + while (historyFileItr.hasNext()) { + final AtomicFile af = historyFileItr.next(); + try { + NotificationHistory notificationHistory = mNotificationHistory != null + ? mNotificationHistory + : new NotificationHistory(); + readLocked(af, notificationHistory, + new NotificationHistoryFilter.Builder().build()); + if(notificationHistory.removeConversationFromWrite(mPkg, mConversationId)) { + writeLocked(af, notificationHistory); + } + } catch (Exception e) { + Slog.e(TAG, "Cannot clean up file on conversation removal " + + af.getBaseFile().getName(), e); + } + } + } + } + } + public static final class NotificationHistoryFileAttrProvider implements NotificationHistoryDatabase.FileAttrProvider { final static String TAG = "NotifHistoryFileDate"; diff --git a/services/core/java/com/android/server/notification/NotificationHistoryManager.java b/services/core/java/com/android/server/notification/NotificationHistoryManager.java index 9aab0fd912e84..88e0dc6e9cc45 100644 --- a/services/core/java/com/android/server/notification/NotificationHistoryManager.java +++ b/services/core/java/com/android/server/notification/NotificationHistoryManager.java @@ -166,6 +166,22 @@ public class NotificationHistoryManager { } } + public void deleteConversation(String pkg, int uid, String conversationId) { + synchronized (mLock) { + int userId = UserHandle.getUserId(uid); + final NotificationHistoryDatabase userHistory = + getUserHistoryAndInitializeIfNeededLocked(userId); + // TODO: it shouldn't be possible to delete a notification entry while the user is + // locked but we should handle it + if (userHistory == null) { + Slog.w(TAG, "Attempted to remove conversation for locked/gone/disabled user " + + userId); + return; + } + userHistory.deleteConversation(pkg, conversationId); + } + } + // TODO: wire this up to AMS when power button is long pressed public void triggerWriteToDisk() { synchronized (mLock) { diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 2e4a977add4e7..ad5bea59afe54 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -5655,6 +5655,22 @@ public class NotificationManagerService extends SystemService { mHandler.post(new EnqueueNotificationRunnable(userId, r, isAppForeground)); } + public void onConversationRemoved(String pkg, int uid, String conversationId) { + checkCallerIsSystem(); + Preconditions.checkStringNotEmpty(pkg); + Preconditions.checkStringNotEmpty(conversationId); + + mHistoryManager.deleteConversation(pkg, uid, conversationId); + List deletedChannelIds = + mPreferencesHelper.deleteConversation(pkg, uid, conversationId); + for (String channelId : deletedChannelIds) { + cancelAllNotificationsInt(MY_UID, MY_PID, pkg, channelId, 0, 0, true, + UserHandle.getUserId(uid), REASON_CHANNEL_BANNED, + null); + } + handleSavePolicyFile(); + } + @VisibleForTesting protected void fixNotification(Notification notification, String pkg, String tag, int id, int userId) throws NameNotFoundException { diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java index 9f8362cbc0a4f..20c8625d22e49 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -1223,6 +1223,33 @@ public class PreferencesHelper implements RankingConfig { } } + public @NonNull List deleteConversation(String pkg, int uid, String conversationId) { + synchronized (mPackagePreferences) { + List deletedChannelIds = new ArrayList<>(); + PackagePreferences r = getPackagePreferencesLocked(pkg, uid); + if (r == null) { + return deletedChannelIds; + } + int N = r.channels.size(); + for (int i = 0; i < N; i++) { + final NotificationChannel nc = r.channels.valueAt(i); + if (conversationId.equals(nc.getConversationId())) { + nc.setDeleted(true); + LogMaker lm = getChannelLog(nc, pkg); + lm.setType( + com.android.internal.logging.nano.MetricsProto.MetricsEvent.TYPE_CLOSE); + MetricsLogger.action(lm); + + deletedChannelIds.add(nc.getId()); + } + } + if (!deletedChannelIds.isEmpty() && mAreChannelsBypassingDnd) { + updateChannelsBypassingDnd(mContext.getUserId()); + } + return deletedChannelIds; + } + } + @Override public ParceledListSlice getNotificationChannels(String pkg, int uid, boolean includeDeleted) { diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java index cbb760a7a8710..99b4fd98f9a8b 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java @@ -286,6 +286,52 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase { verify(af, never()).startWrite(); } + @Test + public void testRemoveConversationRunnable() throws Exception { + NotificationHistory nh = mock(NotificationHistory.class); + NotificationHistoryDatabase.RemoveConversationRunnable rcr = + mDataBase.new RemoveConversationRunnable("pkg", "convo"); + rcr.setNotificationHistory(nh); + + AtomicFile af = mock(AtomicFile.class); + when(af.getBaseFile()).thenReturn(new File(mRootDir, "af")); + mDataBase.mHistoryFiles.addLast(af); + + when(nh.removeConversationFromWrite("pkg", "convo")).thenReturn(true); + + mDataBase.mBuffer = mock(NotificationHistory.class); + + rcr.run(); + + verify(mDataBase.mBuffer).removeConversationFromWrite("pkg", "convo"); + verify(af).openRead(); + verify(nh).removeConversationFromWrite("pkg", "convo"); + verify(af).startWrite(); + } + + @Test + public void testRemoveConversationRunnable_noChanges() throws Exception { + NotificationHistory nh = mock(NotificationHistory.class); + NotificationHistoryDatabase.RemoveConversationRunnable rcr = + mDataBase.new RemoveConversationRunnable("pkg", "convo"); + rcr.setNotificationHistory(nh); + + AtomicFile af = mock(AtomicFile.class); + when(af.getBaseFile()).thenReturn(new File(mRootDir, "af")); + mDataBase.mHistoryFiles.addLast(af); + + when(nh.removeConversationFromWrite("pkg", "convo")).thenReturn(false); + + mDataBase.mBuffer = mock(NotificationHistory.class); + + rcr.run(); + + verify(mDataBase.mBuffer).removeConversationFromWrite("pkg", "convo"); + verify(af).openRead(); + verify(nh).removeConversationFromWrite("pkg", "convo"); + verify(af, never()).startWrite(); + } + private class TestFileAttrProvider implements NotificationHistoryDatabase.FileAttrProvider { public Map creationDates = new HashMap<>(); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryManagerTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryManagerTest.java index 2c548be185c90..f7c26091a7f41 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryManagerTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryManagerTest.java @@ -302,6 +302,20 @@ public class NotificationHistoryManagerTest extends UiServiceTestCase { verify(userHistory, times(1)).deleteNotificationHistoryItem(pkg, time); } + @Test + public void testDeleteConversation_userUnlocked() { + String pkg = "pkg"; + String convo = "convo"; + NotificationHistoryDatabase userHistory = mock(NotificationHistoryDatabase.class); + + mHistoryManager.onUserUnlocked(USER_SYSTEM); + mHistoryManager.replaceNotificationHistoryDatabase(USER_SYSTEM, userHistory); + + mHistoryManager.deleteConversation(pkg, 1, convo); + + verify(userHistory, times(1)).deleteConversation(pkg, convo); + } + @Test public void testTriggerWriteToDisk() { NotificationHistoryDatabase userHistorySystem = mock(NotificationHistoryDatabase.class); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java index 3f690b105d8b7..0adf15c8b6372 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java @@ -3009,7 +3009,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { NotificationChannel channel2 = new NotificationChannel("B person", "B fabulous person", IMPORTANCE_DEFAULT); - channel2.setConversationId(calls.getId(), channel.getName().toString()); + channel2.setConversationId(calls.getId(), channel2.getName().toString()); mHelper.createNotificationChannel(PKG_O, UID_O, channel2, true, false); Map expected = new HashMap<>(); @@ -3036,4 +3036,46 @@ public class PreferencesHelperTest extends UiServiceTestCase { .isEqualTo(expectedGroup.get(convo.getNotificationChannel().getId())); } } + + @Test + public void testDeleteConversation() { + String convoId = "convo"; + NotificationChannel messages = + new NotificationChannel("messages", "Messages", IMPORTANCE_DEFAULT); + mHelper.createNotificationChannel(PKG_O, UID_O, messages, true, false); + NotificationChannel calls = + new NotificationChannel("calls", "Calls", IMPORTANCE_DEFAULT); + mHelper.createNotificationChannel(PKG_O, UID_O, calls, true, false); + + NotificationChannel channel = + new NotificationChannel("A person msgs", "messages from A", IMPORTANCE_DEFAULT); + channel.setConversationId(messages.getId(), convoId); + mHelper.createNotificationChannel(PKG_O, UID_O, channel, true, false); + + NotificationChannel noMatch = + new NotificationChannel("B person msgs", "messages from B", IMPORTANCE_DEFAULT); + noMatch.setConversationId(messages.getId(), "different convo"); + mHelper.createNotificationChannel(PKG_O, UID_O, noMatch, true, false); + + NotificationChannel channel2 = + new NotificationChannel("A person calls", "calls from A", IMPORTANCE_DEFAULT); + channel2.setConversationId(calls.getId(), convoId); + mHelper.createNotificationChannel(PKG_O, UID_O, channel2, true, false); + + assertEquals(channel, mHelper.getNotificationChannel(PKG_O, UID_O, channel.getId(), false)); + assertEquals(channel2, + mHelper.getNotificationChannel(PKG_O, UID_O, channel2.getId(), false)); + assertEquals(2, mHelper.deleteConversation(PKG_O, UID_O, convoId).size()); + + assertEquals(messages, + mHelper.getNotificationChannel(PKG_O, UID_O, messages.getId(), false)); + assertEquals(noMatch, + mHelper.getNotificationChannel(PKG_O, UID_O, noMatch.getId(), false)); + + assertNull(mHelper.getNotificationChannel(PKG_O, UID_O, channel.getId(), false)); + assertNull(mHelper.getNotificationChannel(PKG_O, UID_O, channel2.getId(), false)); + assertEquals(channel, mHelper.getNotificationChannel(PKG_O, UID_O, channel.getId(), true)); + assertEquals(channel2, + mHelper.getNotificationChannel(PKG_O, UID_O, channel2.getId(), true)); + } }