From 65a6a0e18aa9f16fa4c2cb77cabd778343e00d33 Mon Sep 17 00:00:00 2001 From: Jordan Demeulenaere Date: Fri, 4 Jun 2021 13:04:38 +0200 Subject: [PATCH] Fix HUN launch animation Bug: 189884891 Fixes: 189884891 Test: Post an auto-cancellable notification with MAX priority and click it after > 1s. Change-Id: I301633e5487cd30b23c0b7b5fa22bcbd14b63625 --- .../AlertingNotificationManager.java | 6 ++ .../NotificationLaunchAnimatorController.kt | 16 ++- .../collection/NotificationEntry.java | 11 +++ .../notification/stack/ViewState.java | 2 +- .../StatusBarLaunchAnimatorController.kt | 2 +- .../StatusBarNotificationActivityStarter.java | 20 ++-- .../statusbar/policy/HeadsUpUtil.java | 2 +- ...otificationLaunchAnimatorControllerTest.kt | 99 +++++++++++++++++++ 8 files changed, 138 insertions(+), 20 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorControllerTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/AlertingNotificationManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/AlertingNotificationManager.java index 0df69a0a1f43b..9fa460928e130 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/AlertingNotificationManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/AlertingNotificationManager.java @@ -204,6 +204,12 @@ public abstract class AlertingNotificationManager implements NotificationLifetim return; } NotificationEntry entry = alertEntry.mEntry; + + // If the notification is animating, we will remove it at the end of the animation. + if (entry != null && entry.isExpandAnimationRunning()) { + return; + } + mAlertEntries.remove(key); onAlertEntryRemoved(alertEntry); entry.sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED); 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 bce39ce80d356..c248670c48dbc 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorController.kt @@ -7,6 +7,7 @@ import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow import com.android.systemui.statusbar.notification.stack.NotificationListContainer import com.android.systemui.statusbar.phone.HeadsUpManagerPhone import com.android.systemui.statusbar.phone.NotificationShadeWindowViewController +import com.android.systemui.statusbar.policy.HeadsUpUtil import kotlin.math.ceil import kotlin.math.max @@ -22,8 +23,8 @@ class NotificationLaunchAnimatorControllerProvider( return NotificationLaunchAnimatorController( notificationShadeWindowViewController, notificationListContainer, - notification, - headsUpManager + headsUpManager, + notification ) } } @@ -36,10 +37,11 @@ class NotificationLaunchAnimatorControllerProvider( class NotificationLaunchAnimatorController( private val notificationShadeWindowViewController: NotificationShadeWindowViewController, private val notificationListContainer: NotificationListContainer, - private val notification: ExpandableNotificationRow, - private val headsUpManager: HeadsUpManagerPhone + private val headsUpManager: HeadsUpManagerPhone, + private val notification: ExpandableNotificationRow ) : ActivityLaunchAnimator.Controller { - private val notificationKey = notification.entry.sbn.key + private val notificationEntry = notification.entry + private val notificationKey = notificationEntry.sbn.key override var launchContainer: ViewGroup get() = notification.rootView as ViewGroup @@ -82,6 +84,7 @@ class NotificationLaunchAnimatorController( override fun onIntentStarted(willAnimate: Boolean) { notificationShadeWindowViewController.setExpandAnimationRunning(willAnimate) + notificationEntry.isExpandAnimationRunning = willAnimate if (!willAnimate) { removeHun(animate = true) @@ -93,6 +96,7 @@ class NotificationLaunchAnimatorController( return } + HeadsUpUtil.setNeedsHeadsUpDisappearAnimationAfterClick(notification, animate) headsUpManager.removeNotification(notificationKey, true /* releaseImmediately */, animate) } @@ -100,6 +104,7 @@ class NotificationLaunchAnimatorController( // TODO(b/184121838): Should we call InteractionJankMonitor.cancel if the animation started // here? notificationShadeWindowViewController.setExpandAnimationRunning(false) + notificationEntry.isExpandAnimationRunning = false removeHun(animate = true) } @@ -116,6 +121,7 @@ class NotificationLaunchAnimatorController( notification.isExpandAnimationRunning = false notificationShadeWindowViewController.setExpandAnimationRunning(false) + notificationEntry.isExpandAnimationRunning = false notificationListContainer.setExpandingNotification(null) applyParams(null) removeHun(animate = false) 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 5f93f4807c73b..9f82152eb5ed9 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 @@ -179,6 +179,7 @@ public final class NotificationEntry extends ListEntry { private boolean mIsAlerting; public boolean mRemoteEditImeVisible; + private boolean mExpandAnimationRunning; /** * @param sbn the StatusBarNotification from system server @@ -952,6 +953,16 @@ public final class NotificationEntry extends ListEntry { return mIsAlerting; } + /** Set whether this notification is currently used to animate a launch. */ + public void setExpandAnimationRunning(boolean expandAnimationRunning) { + mExpandAnimationRunning = expandAnimationRunning; + } + + /** Whether this notification is currently used to animate a launch. */ + public boolean isExpandAnimationRunning() { + return mExpandAnimationRunning; + } + /** Information about a suggestion that is being edited. */ public static class EditedSuggestionInfo { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ViewState.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ViewState.java index df86e2012564d..6d82a45313d11 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ViewState.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ViewState.java @@ -591,7 +591,7 @@ public class ViewState implements Dumpable { animator.addListener(new AnimatorListenerAdapter() { @Override public void onAnimationEnd(Animator animation) { - HeadsUpUtil.setIsClickedHeadsUpNotification(child, false); + HeadsUpUtil.setNeedsHeadsUpDisappearAnimationAfterClick(child, false); child.setTag(TAG_ANIMATOR_TRANSLATION_Y, null); child.setTag(TAG_START_TRANSLATION_Y, null); child.setTag(TAG_END_TRANSLATION_Y, null); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarLaunchAnimatorController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarLaunchAnimatorController.kt index b2ab30785b4e4..14e513a0556d7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarLaunchAnimatorController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarLaunchAnimatorController.kt @@ -28,8 +28,8 @@ class StatusBarLaunchAnimatorController( override fun onLaunchAnimationEnd(isExpandingFullyAbove: Boolean) { delegate.onLaunchAnimationEnd(isExpandingFullyAbove) - statusBar.onLaunchAnimationEnd(isExpandingFullyAbove) statusBar.notificationPanelViewController.setIsLaunchAnimationRunning(false) + statusBar.onLaunchAnimationEnd(isExpandingFullyAbove) } override fun onLaunchAnimationProgress( 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 f5dd19552b7f2..7b7c17d08eda9 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java @@ -282,15 +282,6 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit boolean showOverLockscreen) { mLogger.logHandleClickAfterKeyguardDismissed(entry.getKey()); - // TODO: Some of this code may be able to move to NotificationEntryManager. - String key = row.getEntry().getSbn().getKey(); - if (mHeadsUpManager != null && mHeadsUpManager.isAlerting(key)) { - // Release the HUN notification to the shade. - if (mPresenter.isPresenterFullyCollapsed()) { - HeadsUpUtil.setIsClickedHeadsUpNotification(row, true); - } - } - final Runnable runnable = () -> handleNotificationClickAfterPanelCollapsed( entry, row, controller, intent, isActivityIntent, animate); @@ -337,7 +328,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit // bypass work challenge if (mStatusBarRemoteInputCallback.startWorkChallengeIfNecessary(userId, intent.getIntentSender(), notificationKey)) { - removeHUN(row); + removeHunAfterClick(row); // Show work challenge, do not run PendingIntent and // remove notification collapseOnMainThread(); @@ -357,7 +348,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit final boolean canBubble = entry.canBubble(); if (canBubble) { mLogger.logExpandingBubble(notificationKey); - removeHUN(row); + removeHunAfterClick(row); expandBubbleStackOnMainThread(entry); } else { startNotificationIntent(intent, fillInIntent, entry, row, animate, isActivityIntent); @@ -508,9 +499,14 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit }, null, false /* afterKeyguardGone */); } - private void removeHUN(ExpandableNotificationRow row) { + private void removeHunAfterClick(ExpandableNotificationRow row) { String key = row.getEntry().getSbn().getKey(); if (mHeadsUpManager != null && mHeadsUpManager.isAlerting(key)) { + // Release the HUN notification to the shade. + if (mPresenter.isPresenterFullyCollapsed()) { + HeadsUpUtil.setNeedsHeadsUpDisappearAnimationAfterClick(row, true); + } + // In most cases, when FLAG_AUTO_CANCEL is set, the notification will // become canceled shortly by NoMan, but we can't assume that. mHeadsUpManager.removeNotification(key, true /* releaseImmediately */); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpUtil.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpUtil.java index 1e3c123cfbc6c..1212585d98295 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpUtil.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpUtil.java @@ -31,7 +31,7 @@ public final class HeadsUpUtil { * @param view The view to be set the flag to. * @param clicked True to set as clicked. False to not-clicked. */ - public static void setIsClickedHeadsUpNotification(View view, boolean clicked) { + public static void setNeedsHeadsUpDisappearAnimationAfterClick(View view, boolean clicked) { view.setTag(TAG_CLICKED_NOTIFICATION, clicked ? true : null); } 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 new file mode 100644 index 0000000000000..c74437f5ad945 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationLaunchAnimatorControllerTest.kt @@ -0,0 +1,99 @@ +package com.android.systemui.statusbar.notification + +import android.testing.AndroidTestingRunner +import android.testing.TestableLooper +import android.testing.TestableLooper.RunWithLooper +import androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow +import com.android.systemui.statusbar.notification.row.NotificationTestHelper +import com.android.systemui.statusbar.notification.stack.NotificationListContainer +import com.android.systemui.statusbar.phone.HeadsUpManagerPhone +import com.android.systemui.statusbar.phone.NotificationShadeWindowViewController +import com.android.systemui.statusbar.policy.HeadsUpUtil +import junit.framework.Assert.assertFalse +import junit.framework.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.Mockito.`when` +import org.mockito.Mockito.verify +import org.mockito.junit.MockitoJUnit + +@SmallTest +@RunWith(AndroidTestingRunner::class) +@RunWithLooper +class NotificationLaunchAnimatorControllerTest : SysuiTestCase() { + @Mock lateinit var notificationShadeWindowViewController: NotificationShadeWindowViewController + @Mock lateinit var notificationListContainer: NotificationListContainer + @Mock lateinit var headsUpManager: HeadsUpManagerPhone + + private lateinit var notificationTestHelper: NotificationTestHelper + private lateinit var notification: ExpandableNotificationRow + private lateinit var controller: NotificationLaunchAnimatorController + + private val notificationKey: String + get() = notification.entry.sbn.key + + @get:Rule val rule = MockitoJUnit.rule() + + @Before + fun setUp() { + allowTestableLooperAsMainThread() + notificationTestHelper = + NotificationTestHelper(mContext, mDependency, TestableLooper.get(this)) + notification = notificationTestHelper.createRow() + controller = NotificationLaunchAnimatorController( + notificationShadeWindowViewController, + notificationListContainer, + headsUpManager, + notification + ) + } + + private fun flagNotificationAsHun() { + `when`(headsUpManager.isAlerting(notificationKey)).thenReturn(true) + } + + @Test + fun testHunIsRemovedIfWeDontAnimateLaunch() { + flagNotificationAsHun() + controller.onIntentStarted(willAnimate = false) + + assertTrue(HeadsUpUtil.isClickedHeadsUpNotification(notification)) + assertFalse(notification.entry.isExpandAnimationRunning) + verify(headsUpManager).removeNotification( + notificationKey, true /* releaseImmediately */, true /* animate */) + } + + @Test + fun testHunIsRemovedWhenAnimationIsCancelled() { + flagNotificationAsHun() + controller.onLaunchAnimationCancelled() + + assertTrue(HeadsUpUtil.isClickedHeadsUpNotification(notification)) + assertFalse(notification.entry.isExpandAnimationRunning) + verify(headsUpManager).removeNotification( + notificationKey, true /* releaseImmediately */, true /* animate */) + } + + @Test + fun testHunIsRemovedWhenAnimationEnds() { + flagNotificationAsHun() + controller.onLaunchAnimationEnd(isExpandingFullyAbove = true) + + assertFalse(HeadsUpUtil.isClickedHeadsUpNotification(notification)) + assertFalse(notification.entry.isExpandAnimationRunning) + verify(headsUpManager).removeNotification( + notificationKey, true /* releaseImmediately */, false /* animate */) + } + + @Test + fun testNotificationIsExpandingDuringAnimation() { + controller.onIntentStarted(willAnimate = true) + + assertTrue(notification.entry.isExpandAnimationRunning) + } +} \ No newline at end of file