From 048571dd5714340ab37ca5304136dcd083d23c53 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Tue, 23 Mar 2021 17:24:55 -0400 Subject: [PATCH] Remove notification channels from history when they're deleted. To make this possible, this change adds functionality for notification channel removal to NotificationHistory, NotificationHistoryDatabase, and NotificationHistoryManager and corresponding tests. This change does not remove the notifications from deleted channels from the "recently dismissed" list (stored in mArchive); will follow up with that later. Test: atest NotificationHistoryTest, NotificationHistoryDatabaseTest, NotificationManagerServiceTest; manual via creating channels, deleting them and verifying Bug: 169349809 Change-Id: I7b1c137e516d14703665750f06554ac7ca44c90e --- .../java/android/app/NotificationHistory.java | 20 ++++++++ .../android/app/NotificationHistoryTest.java | 44 +++++++++++++++++ .../NotificationHistoryDatabase.java | 48 +++++++++++++++++++ .../NotificationHistoryManager.java | 16 +++++++ .../NotificationManagerService.java | 1 + .../NotificationHistoryDatabaseTest.java | 46 ++++++++++++++++++ .../NotificationHistoryManagerTest.java | 16 ++++++- 7 files changed, 190 insertions(+), 1 deletion(-) diff --git a/core/java/android/app/NotificationHistory.java b/core/java/android/app/NotificationHistory.java index 0c8188b9a51e1..eb9ec869af12a 100644 --- a/core/java/android/app/NotificationHistory.java +++ b/core/java/android/app/NotificationHistory.java @@ -403,6 +403,26 @@ public final class NotificationHistory implements Parcelable { return removed; } + /** + * Removes all notifications from a channel and regenerates the string pool + */ + public boolean removeChannelFromWrite(String packageName, String channelId) { + boolean removed = false; + for (int i = mNotificationsToWrite.size() - 1; i >= 0; i--) { + HistoricalNotification hn = mNotificationsToWrite.get(i); + if (packageName.equals(hn.getPackage()) + && Objects.equals(channelId, hn.getChannelId())) { + 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 3df0a68716399..bd493f41d25b8 100644 --- a/core/tests/coretests/src/android/app/NotificationHistoryTest.java +++ b/core/tests/coretests/src/android/app/NotificationHistoryTest.java @@ -29,6 +29,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -326,6 +328,48 @@ public class NotificationHistoryTest { .containsExactlyElementsIn(postRemoveExpectedEntries); } + @Test + public void testRemoveChannelFromWrite() { + NotificationHistory history = new NotificationHistory(); + + List postRemoveExpectedEntries = new ArrayList<>(); + Set postRemoveExpectedStrings = new HashSet<>(); + for (int i = 1; i <= 10; i++) { + HistoricalNotification n = getHistoricalNotification("pkg", i); + + // Remove channel numbers 5 and 6 + if (i != 5 && i != 6) { + 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 channel id that will also be removed + history.addNotificationToWrite(getHistoricalNotification("pkg", 6)); + + 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.removeChannelFromWrite("pkg", "channelId5"); + history.removeChannelFromWrite("pkg", "channelId6"); + + // 1 package names and 8 * 2 unique channel names and ids and 4 conversation ids + assertThat(history.getPooledStringsToWrite().length).isEqualTo(21); + assertThat(Arrays.asList(history.getPooledStringsToWrite())) + .containsExactlyElementsIn(postRemoveExpectedStrings); + 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 b9984a5c24eea..8bd3b1e0b6ac8 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(rcr); } + public void deleteNotificationChannel(String pkg, String channelId) { + RemoveChannelRunnable rcr = new RemoveChannelRunnable(pkg, channelId); + mFileWriteHandler.post(rcr); + } + public void addNotification(final HistoricalNotification notification) { synchronized (mLock) { mBuffer.addNewNotificationToWrite(notification); @@ -505,4 +510,47 @@ public class NotificationHistoryDatabase { } } } + + final class RemoveChannelRunnable implements Runnable { + private String mPkg; + private String mChannelId; + private NotificationHistory mNotificationHistory; + + RemoveChannelRunnable(String pkg, String channelId) { + mPkg = pkg; + mChannelId = channelId; + } + + @VisibleForTesting + void setNotificationHistory(NotificationHistory nh) { + mNotificationHistory = nh; + } + + @Override + public void run() { + if (DEBUG) Slog.d(TAG, "RemoveChannelRunnable"); + synchronized (mLock) { + // Remove from pending history + mBuffer.removeChannelFromWrite(mPkg, mChannelId); + + 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.removeChannelFromWrite(mPkg, mChannelId)) { + writeLocked(af, notificationHistory); + } + } catch (Exception e) { + Slog.e(TAG, "Cannot clean up file on channel removal " + + af.getBaseFile().getName(), e); + } + } + } + } + } } diff --git a/services/core/java/com/android/server/notification/NotificationHistoryManager.java b/services/core/java/com/android/server/notification/NotificationHistoryManager.java index cf3530bfe7fcb..6da898acdafef 100644 --- a/services/core/java/com/android/server/notification/NotificationHistoryManager.java +++ b/services/core/java/com/android/server/notification/NotificationHistoryManager.java @@ -183,6 +183,22 @@ public class NotificationHistoryManager { } } + public void deleteNotificationChannel(String pkg, int uid, String channelId) { + 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 channel for locked/gone/disabled user " + + userId); + return; + } + userHistory.deleteNotificationChannel(pkg, channelId); + } + } + public void triggerWriteToDisk() { synchronized (mLock) { final int userCount = mUserState.size(); diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 50b91766735e0..02809ff5678cc 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -3623,6 +3623,7 @@ public class NotificationManagerService extends SystemService { cancelAllNotificationsInt(MY_UID, MY_PID, pkg, channelId, 0, 0, true, callingUser, REASON_CHANNEL_REMOVED, null); mPreferencesHelper.deleteNotificationChannel(pkg, callingUid, channelId); + mHistoryManager.deleteNotificationChannel(pkg, callingUid, channelId); mListeners.notifyNotificationChannelChanged(pkg, UserHandle.getUserHandleForUid(callingUid), mPreferencesHelper.getNotificationChannel(pkg, callingUid, channelId, true), 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 f6d6624d7e1c7..809b6d561362a 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java @@ -349,6 +349,52 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase { verify(af, never()).startWrite(); } + @Test + public void testRemoveChannelRunnable() throws Exception { + NotificationHistory nh = mock(NotificationHistory.class); + NotificationHistoryDatabase.RemoveChannelRunnable rcr = + mDataBase.new RemoveChannelRunnable("pkg", "channel"); + rcr.setNotificationHistory(nh); + + AtomicFile af = mock(AtomicFile.class); + when(af.getBaseFile()).thenReturn(new File(mRootDir, "af")); + mDataBase.mHistoryFiles.addLast(af); + + when(nh.removeChannelFromWrite("pkg", "channel")).thenReturn(true); + + mDataBase.mBuffer = mock(NotificationHistory.class); + + rcr.run(); + + verify(mDataBase.mBuffer).removeChannelFromWrite("pkg", "channel"); + verify(af).openRead(); + verify(nh).removeChannelFromWrite("pkg", "channel"); + verify(af).startWrite(); + } + + @Test + public void testRemoveChannelRunnable_noChanges() throws Exception { + NotificationHistory nh = mock(NotificationHistory.class); + NotificationHistoryDatabase.RemoveChannelRunnable rcr = + mDataBase.new RemoveChannelRunnable("pkg", "channel"); + rcr.setNotificationHistory(nh); + + AtomicFile af = mock(AtomicFile.class); + when(af.getBaseFile()).thenReturn(new File(mRootDir, "af")); + mDataBase.mHistoryFiles.addLast(af); + + when(nh.removeChannelFromWrite("pkg", "channel")).thenReturn(false); + + mDataBase.mBuffer = mock(NotificationHistory.class); + + rcr.run(); + + verify(mDataBase.mBuffer).removeChannelFromWrite("pkg", "channel"); + verify(af).openRead(); + verify(nh).removeChannelFromWrite("pkg", "channel"); + verify(af, never()).startWrite(); + } + @Test public void testWriteBufferRunnable() throws Exception { NotificationHistory nh = mock(NotificationHistory.class); 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 a0293b7ad12ad..5892793fdb726 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryManagerTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryManagerTest.java @@ -366,7 +366,7 @@ public class NotificationHistoryManagerTest extends UiServiceTestCase { @Test public void testDeleteConversation_userUnlocked() { String pkg = "pkg"; - Set convos = Set.of("convo", "another"); + Set convos = Set.of("convo", "another"); NotificationHistoryDatabase userHistory = mock(NotificationHistoryDatabase.class); mHistoryManager.onUserUnlocked(USER_SYSTEM); @@ -377,6 +377,20 @@ public class NotificationHistoryManagerTest extends UiServiceTestCase { verify(userHistory, times(1)).deleteConversations(pkg, convos); } + @Test + public void testDeleteNotificationChannel_userUnlocked() { + String pkg = "pkg"; + String channelId = "channelId"; + NotificationHistoryDatabase userHistory = mock(NotificationHistoryDatabase.class); + + mHistoryManager.onUserUnlocked(USER_SYSTEM); + mHistoryManager.replaceNotificationHistoryDatabase(USER_SYSTEM, userHistory); + + mHistoryManager.deleteNotificationChannel(pkg, 1, channelId); + + verify(userHistory, times(1)).deleteNotificationChannel(pkg, channelId); + } + @Test public void testTriggerWriteToDisk() { NotificationHistoryDatabase userHistorySystem = mock(NotificationHistoryDatabase.class);