From 4bf10e42b91a8bd9f2d75f62232a79ea24e294e9 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Mon, 7 Mar 2022 16:40:12 -0500 Subject: [PATCH] Expand notif activity starter lifetime extension Another animation is played while launching the intent for a notification. This CL moves the onFinishLaunchNotifActivity invocation until after that animation has ended (or after checking that it won't be played). Fixes: 220725343 Test: 1. Install Notify.apk (go/notify-apk) 2. In the Notify app: Check off Delayed 5000 and Auto Cancel 3. Click "Add" and spam click "Update" (5-10 times should suffice) 4. Immediately tap the Notify heads up notification when it gets posted Observe: no crashing Change-Id: I13312b4c42b4147fb061db617302cde4b45829ff --- .../NotificationLaunchAnimatorController.kt | 13 ++++++++++--- .../StatusBarNotificationActivityStarter.java | 19 +++++++++++-------- ...otificationLaunchAnimatorControllerTest.kt | 15 ++++++++++----- ...tusBarNotificationActivityStarterTest.java | 18 +++++++++++++----- 4 files changed, 44 insertions(+), 21 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorController.kt index 2c1296f34a42b..7fbb0f1182c07 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorController.kt @@ -22,15 +22,18 @@ class NotificationLaunchAnimatorControllerProvider @Inject constructor( private val headsUpManager: HeadsUpManagerPhone, private val jankMonitor: InteractionJankMonitor ) { + @JvmOverloads fun getAnimatorController( - notification: ExpandableNotificationRow + notification: ExpandableNotificationRow, + onFinishAnimationCallback: Runnable = Runnable {} ): NotificationLaunchAnimatorController { return NotificationLaunchAnimatorController( notificationShadeWindowViewController, notificationListContainer, headsUpManager, notification, - jankMonitor + jankMonitor, + onFinishAnimationCallback ) } } @@ -45,7 +48,8 @@ class NotificationLaunchAnimatorController( private val notificationListContainer: NotificationListContainer, private val headsUpManager: HeadsUpManagerPhone, private val notification: ExpandableNotificationRow, - private val jankMonitor: InteractionJankMonitor + private val jankMonitor: InteractionJankMonitor, + private val onFinishAnimationCallback: Runnable ) : ActivityLaunchAnimator.Controller { companion object { @@ -119,6 +123,7 @@ class NotificationLaunchAnimatorController( if (!willAnimate) { removeHun(animate = true) + onFinishAnimationCallback.run() } } @@ -137,6 +142,7 @@ class NotificationLaunchAnimatorController( notificationShadeWindowViewController.setExpandAnimationRunning(false) notificationEntry.isExpandAnimationRunning = false removeHun(animate = true) + onFinishAnimationCallback.run() } override fun onLaunchAnimationStart(isExpandingFullyAbove: Boolean) { @@ -156,6 +162,7 @@ class NotificationLaunchAnimatorController( notificationListContainer.setExpandingNotification(null) applyParams(null) removeHun(animate = false) + onFinishAnimationCallback.run() } private fun applyParams(params: ExpandAnimationParameters?) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java index 108d98a129b2d..637e4bee8948d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java @@ -370,6 +370,8 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte mLogger.logExpandingBubble(notificationKey); removeHunAfterClick(row); expandBubbleStackOnMainThread(entry); + mMainThreadHandler.post( + () -> mLaunchEventsEmitter.notifyFinishLaunchNotifActivity(entry)); } else { startNotificationIntent(intent, fillInIntent, entry, row, animate, isActivityIntent); } @@ -395,7 +397,6 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte mMainThreadHandler.post(() -> { final Runnable removeNotification = () -> { mOnUserInteractionCallback.onDismiss(entry, REASON_CLICK, summaryToRemove); - mLaunchEventsEmitter.notifyFinishLaunchNotifActivity(entry); }; if (mPresenter.isCollapsing()) { // To avoid lags we're only performing the remove @@ -405,9 +406,6 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte removeNotification.run(); } }); - } else { - mMainThreadHandler.post( - () -> mLaunchEventsEmitter.notifyFinishLaunchNotifActivity(entry)); } mIsCollapsingToShowActivityOverLockscreen = false; @@ -483,14 +481,19 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte boolean isActivityIntent) { mLogger.logStartNotificationIntent(entry.getKey(), intent); try { + Runnable onFinishAnimationCallback = + () -> mLaunchEventsEmitter.notifyFinishLaunchNotifActivity(entry); ActivityLaunchAnimator.Controller animationController = new StatusBarLaunchAnimatorController( - mNotificationAnimationProvider.getAnimatorController(row), + mNotificationAnimationProvider + .getAnimatorController(row, onFinishAnimationCallback), mCentralSurfaces, isActivityIntent); - - mActivityLaunchAnimator.startPendingIntentWithAnimation(animationController, - animate, intent.getCreatorPackage(), (adapter) -> { + mActivityLaunchAnimator.startPendingIntentWithAnimation( + animationController, + animate, + intent.getCreatorPackage(), + (adapter) -> { long eventTime = row.getAndResetLastActionUpTime(); Bundle options = eventTime > 0 ? getActivityOptions( diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorControllerTest.kt index 3a60c049b3f3e..9f82a5673c6e2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorControllerTest.kt @@ -31,6 +31,7 @@ class NotificationLaunchAnimatorControllerTest : SysuiTestCase() { @Mock lateinit var notificationListContainer: NotificationListContainer @Mock lateinit var headsUpManager: HeadsUpManagerPhone @Mock lateinit var jankMonitor: InteractionJankMonitor + @Mock lateinit var onFinishAnimationCallback: Runnable private lateinit var notificationTestHelper: NotificationTestHelper private lateinit var notification: ExpandableNotificationRow @@ -52,7 +53,8 @@ class NotificationLaunchAnimatorControllerTest : SysuiTestCase() { notificationListContainer, headsUpManager, notification, - jankMonitor + jankMonitor, + onFinishAnimationCallback ) } @@ -61,7 +63,7 @@ class NotificationLaunchAnimatorControllerTest : SysuiTestCase() { } @Test - fun testHunIsRemovedIfWeDontAnimateLaunch() { + fun testHunIsRemovedAndCallbackIsInvokedIfWeDontAnimateLaunch() { flagNotificationAsHun() controller.onIntentStarted(willAnimate = false) @@ -69,10 +71,11 @@ class NotificationLaunchAnimatorControllerTest : SysuiTestCase() { assertFalse(notification.entry.isExpandAnimationRunning) verify(headsUpManager).removeNotification( notificationKey, true /* releaseImmediately */, true /* animate */) + verify(onFinishAnimationCallback).run() } @Test - fun testHunIsRemovedWhenAnimationIsCancelled() { + fun testHunIsRemovedAndCallbackIsInvokedWhenAnimationIsCancelled() { flagNotificationAsHun() controller.onLaunchAnimationCancelled() @@ -80,10 +83,11 @@ class NotificationLaunchAnimatorControllerTest : SysuiTestCase() { assertFalse(notification.entry.isExpandAnimationRunning) verify(headsUpManager).removeNotification( notificationKey, true /* releaseImmediately */, true /* animate */) + verify(onFinishAnimationCallback).run() } @Test - fun testHunIsRemovedWhenAnimationEnds() { + fun testHunIsRemovedAndCallbackIsInvokedWhenAnimationEnds() { flagNotificationAsHun() controller.onLaunchAnimationEnd(isExpandingFullyAbove = true) @@ -91,6 +95,7 @@ class NotificationLaunchAnimatorControllerTest : SysuiTestCase() { assertFalse(notification.entry.isExpandAnimationRunning) verify(headsUpManager).removeNotification( notificationKey, true /* releaseImmediately */, false /* animate */) + verify(onFinishAnimationCallback).run() } @Test @@ -99,4 +104,4 @@ class NotificationLaunchAnimatorControllerTest : SysuiTestCase() { assertTrue(notification.entry.isExpandAnimationRunning) } -} \ No newline at end of file +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java index 27179fd7be92b..d48ce8c6803e3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java @@ -85,6 +85,7 @@ import com.android.systemui.wmshell.BubblesManager; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.Mockito; @@ -188,10 +189,11 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase { when(mNotifPipelineFlags.isNewPipelineEnabled()).thenReturn(false); when(mOnUserInteractionCallback.getGroupSummaryToDismiss(mNotificationRow.getEntry())) .thenReturn(null); - when(mVisibilityProvider.obtain(anyString(), anyBoolean())).thenAnswer( - invocation-> NotificationVisibility.obtain(invocation.getArgument(0), 0, 1, false)); - when(mVisibilityProvider.obtain(any(NotificationEntry.class), anyBoolean())).thenAnswer( - invocation-> NotificationVisibility.obtain( + when(mVisibilityProvider.obtain(anyString(), anyBoolean())) + .thenAnswer(invocation -> NotificationVisibility.obtain( + invocation.getArgument(0), 0, 1, false)); + when(mVisibilityProvider.obtain(any(NotificationEntry.class), anyBoolean())) + .thenAnswer(invocation -> NotificationVisibility.obtain( invocation.getArgument(0).getKey(), 0, 1, false)); HeadsUpManagerPhone headsUpManager = mock(HeadsUpManagerPhone.class); @@ -431,12 +433,18 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase { } @Test - public void testNotifActivityStarterEventSourceFinishEvent_postPanelCollapse() { + public void testNotifActivityStarterEventSourceFinishEvent_postPanelCollapse() + throws Exception { NotifActivityLaunchEvents.Listener listener = mock(NotifActivityLaunchEvents.Listener.class); mLaunchEventsEmitter.registerListener(listener); mNotificationActivityStarter .onNotificationClicked(mNotificationRow.getEntry().getSbn(), mNotificationRow); + ArgumentCaptor controllerCaptor = + ArgumentCaptor.forClass(ActivityLaunchAnimator.Controller.class); + verify(mActivityLaunchAnimator).startPendingIntentWithAnimation( + controllerCaptor.capture(), anyBoolean(), any(), any()); + controllerCaptor.getValue().onIntentStarted(false); verify(listener).onFinishLaunchNotifActivity(mNotificationRow.getEntry()); } }