From 95fdfad44b6a5da64eda65933bf424086bff270b Mon Sep 17 00:00:00 2001 From: Beverly Date: Fri, 15 May 2020 10:15:42 -0400 Subject: [PATCH] Reset initalization state of notif entries When the notifications views are filtered out, reset their initialization time, so now we can rely on the initialization time to determine whether the notification was recently shown in the shade. Fixes: 153885122 Test: atest NotificationRankingManagerTest Test: manual 1. post grouped notifications 2. swipe down notification shade to view notifications 3. toggle on/off DND and see notifications are not ungrouped when they're unfiltered Change-Id: Ib9b45cd13ead51d7df2484e91d159cd268799e9a --- .../NotificationViewHierarchyManager.java | 3 +- .../collection/NotificationEntry.java | 10 ++++-- .../collection/NotificationRankingManager.kt | 11 ++++++- .../collection/ShadeListBuilder.java | 4 +++ .../NotificationRankingManagerTest.kt | 31 ++++++++++++++++++- .../collection/ShadeListBuilderTest.java | 25 +++++++++++++++ 6 files changed, 78 insertions(+), 6 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index 765f85aab3192..3dda15b5ce397 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java @@ -186,8 +186,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle boolean groupChangesAllowed = mVisualStabilityManager.areGroupChangesAllowed() // user isn't looking at notifs - || !ent.hasFinishedInitialization() // notif recently added - || !mListContainer.containsView(ent.getRow()); // notif recently unfiltered + || !ent.hasFinishedInitialization(); // notif recently added NotificationEntry parent = mGroupManager.getGroupSummary(ent.getSbn()); if (!groupChangesAllowed) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java index 22ac1a2e5cf99..1eadd9ebfd7f7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java @@ -151,6 +151,8 @@ public final class NotificationEntry extends ListEntry { public CharSequence headsUpStatusBarText; public CharSequence headsUpStatusBarTextPublic; + // indicates when this entry's view was first attached to a window + // this value will reset when the view is completely removed from the shade (ie: filtered out) private long initializationTime = -1; /** @@ -473,8 +475,8 @@ public final class NotificationEntry extends ListEntry { } public boolean hasFinishedInitialization() { - return initializationTime == -1 - || SystemClock.elapsedRealtime() > initializationTime + INITIALIZATION_DELAY; + return initializationTime != -1 + && SystemClock.elapsedRealtime() > initializationTime + INITIALIZATION_DELAY; } public int getContrastedColor(Context context, boolean isLowPriority, @@ -565,6 +567,10 @@ public final class NotificationEntry extends ListEntry { return false; } + public void resetInitializationTime() { + initializationTime = -1; + } + public void setInitializationTime(long time) { if (initializationTime == -1) { initializationTime = time; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt index cbf680c5b7825..dc0b802129fcf 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt @@ -134,13 +134,22 @@ open class NotificationRankingManager @Inject constructor( ): List { logger.logFilterAndSort(reason) val filtered = entries.asSequence() - .filterNot(notifFilter::shouldFilterOut) + .filterNot(this::filter) .sortedWith(rankingComparator) .toList() entries.forEach { it.bucket = getBucketForEntry(it) } return filtered } + private fun filter(entry: NotificationEntry): Boolean { + val filtered = notifFilter.shouldFilterOut(entry) + if (filtered) { + // notification is removed from the list, so we reset its initialization time + entry.resetInitializationTime() + } + return filtered + } + @PriorityBucket private fun getBucketForEntry(entry: NotificationEntry): Int { val isHeadsUp = entry.isRowHeadsUp diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java index 0a3b02c1a7678..4cec383776b21 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java @@ -713,6 +713,10 @@ public class ShadeListBuilder implements Dumpable { private boolean applyFilters(NotificationEntry entry, long now, List filters) { final NotifFilter filter = findRejectingFilter(entry, now, filters); entry.getAttachState().setExcludingFilter(filter); + if (filter != null) { + // notification is removed from the list, so we reset its initialization time + entry.resetInitializationTime(); + } return filter != null; } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt index a83de139bbcaf..9f47f4a71ca97 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt @@ -21,6 +21,7 @@ import android.app.NotificationChannel import android.app.NotificationManager.IMPORTANCE_DEFAULT import android.app.NotificationManager.IMPORTANCE_HIGH import android.app.NotificationManager.IMPORTANCE_LOW +import android.os.SystemClock import android.service.notification.NotificationListenerService.RankingMap import android.testing.AndroidTestingRunner import androidx.test.filters.SmallTest @@ -58,17 +59,19 @@ class NotificationRankingManagerTest : SysuiTestCase() { private lateinit var personNotificationIdentifier: PeopleNotificationIdentifier private lateinit var rankingManager: TestableNotificationRankingManager private lateinit var sectionsManager: NotificationSectionsFeatureManager + private lateinit var notificationFilter: NotificationFilter @Before fun setup() { personNotificationIdentifier = mock(PeopleNotificationIdentifier::class.java) sectionsManager = mock(NotificationSectionsFeatureManager::class.java) + notificationFilter = mock(NotificationFilter::class.java) rankingManager = TestableNotificationRankingManager( lazyMedia, mock(NotificationGroupManager::class.java), mock(HeadsUpManager::class.java), - mock(NotificationFilter::class.java), + notificationFilter, mock(NotificationEntryManagerLogger::class.java), sectionsManager, personNotificationIdentifier, @@ -324,6 +327,32 @@ class NotificationRankingManagerTest : SysuiTestCase() { assertEquals(e.bucket, BUCKET_SILENT) } + @Test + fun testFilter_resetsInitalizationTime() { + // GIVEN an entry that was initialized 1 second ago + val notif = Notification.Builder(mContext, "test") .build() + + val e = NotificationEntryBuilder() + .setPkg("pkg") + .setOpPkg("pkg") + .setTag("tag") + .setNotification(notif) + .setUser(mContext.user) + .setChannel(NotificationChannel("test", "", IMPORTANCE_DEFAULT)) + .setOverrideGroupKey("") + .build() + + e.setInitializationTime(SystemClock.elapsedRealtime() - 1000) + assertEquals(true, e.hasFinishedInitialization()) + + // WHEN we update ranking and filter out the notification entry + whenever(notificationFilter.shouldFilterOut(e)).thenReturn(true) + rankingManager.updateRanking(RankingMap(arrayOf(e.ranking)), listOf(e), "test") + + // THEN the initialization time for the entry is reset + assertEquals(false, e.hasFinishedInitialization()) + } + internal class TestableNotificationRankingManager( mediaManager: Lazy, groupManager: NotificationGroupManager, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java index 3adc3d0455f69..a93b54ac10098 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java @@ -19,8 +19,10 @@ package com.android.systemui.statusbar.notification.collection; import static com.android.systemui.statusbar.notification.collection.ListDumper.dumpTree; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyLong; @@ -33,6 +35,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import android.os.SystemClock; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.util.ArrayMap; @@ -474,6 +477,28 @@ public class ShadeListBuilderTest extends SysuiTestCase { assertEquals(filter1, mEntrySet.get(0).getExcludingFilter()); } + @Test + public void testFilter_resetsInitalizationTime() { + // GIVEN a NotifFilter that filters out a specific package + NotifFilter filter1 = spy(new PackageFilter(PACKAGE_1)); + mListBuilder.addFinalizeFilter(filter1); + + // GIVEN a notification that was initialized 1 second ago that will be filtered out + final NotificationEntry entry = new NotificationEntryBuilder() + .setPkg(PACKAGE_1) + .setId(nextId(PACKAGE_1)) + .setRank(nextRank()) + .build(); + entry.setInitializationTime(SystemClock.elapsedRealtime() - 1000); + assertTrue(entry.hasFinishedInitialization()); + + // WHEN the pipeline is kicked off + mReadyForBuildListener.onBuildList(Arrays.asList(entry)); + + // THEN the entry's initialization time is reset + assertFalse(entry.hasFinishedInitialization()); + } + @Test public void testNotifFiltersCanBePreempted() { // GIVEN two notif filters