From f3126140a4cbee18dadb1df8460986cedce7f209 Mon Sep 17 00:00:00 2001 From: Jay Aliomer Date: Mon, 7 Mar 2022 21:24:46 -0500 Subject: [PATCH] Sticky huns dont get cleaned up properly a remove runnable is scheduled for removing the hun if it needs to be at the time where non-sticky huns in case the sticky can be removed. however, if the window is missed, the sticky hun will be life extended until the pipline runs again Fixes: 220826525 Test: HeadsupCoordinatorTest Change-Id: I35f8a19b2fcf234ba18ba082fe50dc5fb2b8430f --- .../coordinator/HeadsUpCoordinator.kt | 19 ++++++------ .../coordinator/HeadsUpCoordinatorTest.kt | 31 +++++++++++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt index 0df2162d33387..da0169bd6dc46 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt @@ -72,10 +72,11 @@ class HeadsUpCoordinator @Inject constructor( private var mEndLifetimeExtension: OnEndLifetimeExtensionCallback? = null private lateinit var mNotifPipeline: NotifPipeline private var mNow: Long = -1 - // notifs we've extended the lifetime for - private val mNotifsExtendingLifetime = ArraySet() private val mPostedEntries = LinkedHashMap() + // notifs we've extended the lifetime for with cancellation callbacks + private val mNotifsExtendingLifetime = ArrayMap() + override fun attach(pipeline: NotifPipeline) { mNotifPipeline = pipeline mHeadsUpManager.addListener(mOnHeadsUpChangedListener) @@ -460,23 +461,20 @@ class HeadsUpCoordinator @Inject constructor( } if (isSticky(entry)) { val removeAfterMillis = mHeadsUpManager.getEarliestRemovalTime(entry.key) - mExecutor.executeDelayed({ - val canStillRemove = mHeadsUpManager.canRemoveImmediately(entry.key) - if (mNotifsExtendingLifetime.contains(entry) && canStillRemove) { - mHeadsUpManager.removeNotification(entry.key, /* releaseImmediately */ true) - } + mNotifsExtendingLifetime[entry] = mExecutor.executeDelayed({ + mHeadsUpManager.removeNotification(entry.key, /* releaseImmediately */ true) }, removeAfterMillis) } else { mExecutor.execute { mHeadsUpManager.removeNotification(entry.key, /* releaseImmediately */ false) } + mNotifsExtendingLifetime[entry] = null } - mNotifsExtendingLifetime.add(entry) return true } override fun cancelLifetimeExtension(entry: NotificationEntry) { - mNotifsExtendingLifetime.remove(entry) + mNotifsExtendingLifetime.remove(entry)?.run() } } @@ -543,7 +541,8 @@ class HeadsUpCoordinator @Inject constructor( mPostedEntries[entry.key]?.calculateShouldBeHeadsUpStrict ?: isAttemptingToShowHun(entry) private fun endNotifLifetimeExtensionIfExtended(entry: NotificationEntry) { - if (mNotifsExtendingLifetime.remove(entry)) { + if (mNotifsExtendingLifetime.contains(entry)) { + mNotifsExtendingLifetime.remove(entry)?.run() mEndLifetimeExtension?.onEndLifetimeExtension(mLifetimeExtender, entry) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorTest.kt index 144eefb17283d..699f77f9b7bb6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorTest.kt @@ -169,12 +169,42 @@ class HeadsUpCoordinatorTest : SysuiTestCase() { verify(mHeadsUpManager, times(1)).removeNotification(anyString(), eq(true)) } + @Test + fun testCancelAndReAddStickyNotification() { + whenever(mHeadsUpManager.isSticky(anyString())).thenReturn(true) + addHUN(mEntry) + whenever(mHeadsUpManager.canRemoveImmediately(anyString())).thenReturn(false, true, false) + whenever(mHeadsUpManager.getEarliestRemovalTime(anyString())).thenReturn(1000L) + assertTrue(mNotifLifetimeExtender.maybeExtendLifetime(mEntry, 0)) + addHUN(mEntry) + assertFalse(mNotifLifetimeExtender.maybeExtendLifetime(mEntry, 0)) + mExecutor.advanceClockToLast() + mExecutor.runAllReady() + assertTrue(mNotifLifetimeExtender.maybeExtendLifetime(mEntry, 0)) + verify(mHeadsUpManager, times(0)).removeNotification(anyString(), eq(false)) + verify(mHeadsUpManager, times(0)).removeNotification(anyString(), eq(true)) + } + + @Test + fun hunNotRemovedWhenExtensionCancelled() { + whenever(mHeadsUpManager.isSticky(anyString())).thenReturn(true) + addHUN(mEntry) + whenever(mHeadsUpManager.canRemoveImmediately(anyString())).thenReturn(false) + whenever(mHeadsUpManager.getEarliestRemovalTime(anyString())).thenReturn(1000L) + assertTrue(mNotifLifetimeExtender.maybeExtendLifetime(mEntry, 0)) + mNotifLifetimeExtender.cancelLifetimeExtension(mEntry) + mExecutor.advanceClockToLast() + mExecutor.runAllReady() + verify(mHeadsUpManager, times(0)).removeNotification(anyString(), any()) + } + @Test fun testCancelUpdatedStickyNotification() { whenever(mHeadsUpManager.isSticky(anyString())).thenReturn(true) addHUN(mEntry) whenever(mHeadsUpManager.getEarliestRemovalTime(anyString())).thenReturn(1000L, 500L) assertTrue(mNotifLifetimeExtender.maybeExtendLifetime(mEntry, 0)) + addHUN(mEntry) mExecutor.advanceClockToLast() mExecutor.runAllReady() verify(mHeadsUpManager, times(0)).removeNotification(anyString(), eq(false)) @@ -305,6 +335,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() { mHuns.add(entry) whenever(mHeadsUpManager.topEntry).thenReturn(entry) mOnHeadsUpChangedListener.onHeadsUpStateChanged(entry, true) + mNotifLifetimeExtender.cancelLifetimeExtension(entry) } @Test