From 943f798939586f2c47e6d248e0d8628f9b35db9e Mon Sep 17 00:00:00 2001 From: Ibrahim Yilmaz Date: Thu, 2 Feb 2023 21:51:32 +0000 Subject: [PATCH] Throttle Spammy onNotificationChannelModified onNotificationChannelModified events are logged in rapid succession. With this change, we throttle some of them by 1 sec. Test: presubmit && logcat logs && atest NotifCollectionTest Bug: 262010037 Change-Id: I9106bbfda463322136c3e289734ea69d6780747d --- .../collection/NotifCollection.java | 37 +++++++- .../collection/NotifCollectionTest.java | 85 +++++++++++++++++++ 2 files changed, 118 insertions(+), 4 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java index df35c9e6832a7..aa9a6c2c4cc6f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java @@ -164,6 +164,11 @@ public class NotifCollection implements Dumpable, PipelineDumpable { private Queue mEventQueue = new ArrayDeque<>(); + private final Runnable mRebuildListRunnable = () -> { + if (mBuildListener != null) { + mBuildListener.onBuildList(mReadOnlyNotificationSet, "asynchronousUpdate"); + } + }; private boolean mAttached = false; private boolean mAmDispatchingToOtherCode; @@ -458,7 +463,7 @@ public class NotifCollection implements Dumpable, PipelineDumpable { int modificationType) { Assert.isMainThread(); mEventQueue.add(new ChannelChangedEvent(pkgName, user, channel, modificationType)); - dispatchEventsAndRebuildList("onNotificationChannelModified"); + dispatchEventsAndAsynchronouslyRebuildList(); } private void onNotificationsInitialized() { @@ -613,15 +618,39 @@ public class NotifCollection implements Dumpable, PipelineDumpable { private void dispatchEventsAndRebuildList(String reason) { Trace.beginSection("NotifCollection.dispatchEventsAndRebuildList"); + if (mMainHandler.hasCallbacks(mRebuildListRunnable)) { + mMainHandler.removeCallbacks(mRebuildListRunnable); + } + + dispatchEvents(); + + if (mBuildListener != null) { + mBuildListener.onBuildList(mReadOnlyNotificationSet, reason); + } + Trace.endSection(); + } + + private void dispatchEventsAndAsynchronouslyRebuildList() { + Trace.beginSection("NotifCollection.dispatchEventsAndAsynchronouslyRebuildList"); + + dispatchEvents(); + + if (!mMainHandler.hasCallbacks(mRebuildListRunnable)) { + mMainHandler.postDelayed(mRebuildListRunnable, 1000L); + } + + Trace.endSection(); + } + + private void dispatchEvents() { + Trace.beginSection("NotifCollection.dispatchEvents"); + mAmDispatchingToOtherCode = true; while (!mEventQueue.isEmpty()) { mEventQueue.remove().dispatchTo(mNotifCollectionListeners); } mAmDispatchingToOtherCode = false; - if (mBuildListener != null) { - mBuildListener.onBuildList(mReadOnlyNotificationSet, reason); - } Trace.endSection(); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java index 94e3e6cb5f8e1..edb2965eabfa1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java @@ -105,6 +105,7 @@ import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.Spy; +import org.mockito.stubbing.Answer; import java.util.Arrays; import java.util.Collection; @@ -375,6 +376,90 @@ public class NotifCollectionTest extends SysuiTestCase { NOTIFICATION_CHANNEL_OR_GROUP_UPDATED); } + @Test + public void testScheduleBuildNotificationListWhenChannelChanged() { + // GIVEN + final NotificationEntryBuilder neb = buildNotif(TEST_PACKAGE, 48); + final NotificationChannel channel = new NotificationChannel( + "channelId", + "channelName", + NotificationManager.IMPORTANCE_DEFAULT); + neb.setChannel(channel); + + final NotifEvent notif = mNoMan.postNotif(neb); + final NotificationEntry entry = mCollectionListener.getEntry(notif.key); + + when(mMainHandler.hasCallbacks(any())).thenReturn(false); + + clearInvocations(mBuildListener); + + // WHEN + mNotifHandler.onNotificationChannelModified(TEST_PACKAGE, + entry.getSbn().getUser(), channel, NOTIFICATION_CHANNEL_OR_GROUP_UPDATED); + + // THEN + verify(mMainHandler).postDelayed(any(), eq(1000L)); + } + + @Test + public void testCancelScheduledBuildNotificationListEventWhenNotifUpdatedSynchronously() { + // GIVEN + final NotificationEntry entry1 = buildNotif(TEST_PACKAGE, 1) + .setGroup(mContext, "group_1") + .build(); + final NotificationEntry entry2 = buildNotif(TEST_PACKAGE, 2) + .setGroup(mContext, "group_1") + .setContentTitle(mContext, "New version") + .build(); + final NotificationEntry entry3 = buildNotif(TEST_PACKAGE, 3) + .setGroup(mContext, "group_1") + .build(); + + final List entriesToBePosted = Arrays.asList( + new CoalescedEvent(entry1.getKey(), 0, entry1.getSbn(), entry1.getRanking(), null), + new CoalescedEvent(entry2.getKey(), 1, entry2.getSbn(), entry2.getRanking(), null), + new CoalescedEvent(entry3.getKey(), 2, entry3.getSbn(), entry3.getRanking(), null) + ); + + when(mMainHandler.hasCallbacks(any())).thenReturn(true); + + // WHEN + mNotifHandler.onNotificationBatchPosted(entriesToBePosted); + + // THEN + verify(mMainHandler).removeCallbacks(any()); + } + + @Test + public void testBuildNotificationListWhenChannelChanged() { + // GIVEN + final NotificationEntryBuilder neb = buildNotif(TEST_PACKAGE, 48); + final NotificationChannel channel = new NotificationChannel( + "channelId", + "channelName", + NotificationManager.IMPORTANCE_DEFAULT); + neb.setChannel(channel); + + final NotifEvent notif = mNoMan.postNotif(neb); + final NotificationEntry entry = mCollectionListener.getEntry(notif.key); + + when(mMainHandler.hasCallbacks(any())).thenReturn(false); + when(mMainHandler.postDelayed(any(), eq(1000L))).thenAnswer((Answer) invocation -> { + final Runnable runnable = invocation.getArgument(0); + runnable.run(); + return null; + }); + + clearInvocations(mBuildListener); + + // WHEN + mNotifHandler.onNotificationChannelModified(TEST_PACKAGE, + entry.getSbn().getUser(), channel, NOTIFICATION_CHANNEL_OR_GROUP_UPDATED); + + // THEN + verifyBuiltList(List.of(entry)); + } + @Test public void testRankingsAreUpdatedForOtherNotifs() { // GIVEN a collection with one notif