Merge "Fix HUN re-show after closing shade" into main

9 files changed, 166 insertions, 28 deletions

diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/AvalancheControllerTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/AvalancheControllerTest.kt
index 495ab61600f9..8f9da3b2e1e3 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/AvalancheControllerTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/AvalancheControllerTest.kt
@@ -180,6 +180,23 @@ class AvalancheControllerTest : SysuiTestCase() {
     }
 
     @Test
+    fun testDelete_untracked_runnableRuns() {
+        val headsUpEntry = createHeadsUpEntry(id = 0)
+
+        // None showing
+        mAvalancheController.headsUpEntryShowing = null
+
+        // Nothing is next
+        mAvalancheController.clearNext()
+
+        // Delete
+        mAvalancheController.delete(headsUpEntry, runnableMock!!, "testLabel")
+
+        // Runnable was run
+        Mockito.verify(runnableMock, Mockito.times(1)).run()
+    }
+
+    @Test
     fun testDelete_isNext_removedFromNext_runnableNotRun() {
         // Entry is next
         val headsUpEntry = createHeadsUpEntry(id = 0)
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/HeadsUpManagerPhoneTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/HeadsUpManagerPhoneTest.kt
index d0ddbffecf9a..5dadc4caf0f6 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/HeadsUpManagerPhoneTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/HeadsUpManagerPhoneTest.kt
@@ -33,6 +33,7 @@ import com.android.systemui.statusbar.StatusBarState
 import com.android.systemui.statusbar.notification.collection.NotificationEntry
 import com.android.systemui.statusbar.notification.collection.provider.VisualStabilityProvider
 import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager
+import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow
 import com.android.systemui.statusbar.notification.shared.NotificationThrottleHun
 import com.android.systemui.statusbar.notification.shared.NotificationsHeadsUpRefactor
 import com.android.systemui.statusbar.phone.ConfigurationControllerImpl
@@ -42,6 +43,7 @@ import com.android.systemui.testKosmos
 import com.android.systemui.util.concurrency.DelayableExecutor
 import com.android.systemui.util.concurrency.mockExecutorHandler
 import com.android.systemui.util.kotlin.JavaAdapter
+import com.android.systemui.util.mockito.mock
 import com.android.systemui.util.settings.GlobalSettings
 import com.android.systemui.util.time.SystemClock
 import junit.framework.Assert
@@ -237,6 +239,34 @@ class HeadsUpManagerPhoneTest(flags: FlagsParameterization) : BaseHeadsUpManager
     }
 
     @Test
+    fun testShowNotification_reorderNotAllowed_notPulsing_seenInShadeTrue() {
+        whenever(mVSProvider.isReorderingAllowed).thenReturn(false)
+        val hmp = createHeadsUpManagerPhone()
+
+        val notifEntry = HeadsUpManagerTestUtil.createEntry(/* id= */ 0, mContext)
+        val row = mock<ExpandableNotificationRow>()
+        whenever(row.showingPulsing()).thenReturn(false)
+        notifEntry.row = row
+
+        hmp.showNotification(notifEntry)
+        Assert.assertTrue(notifEntry.isSeenInShade)
+    }
+
+    @Test
+    fun testShowNotification_reorderAllowed_notPulsing_seenInShadeFalse() {
+        whenever(mVSProvider.isReorderingAllowed).thenReturn(true)
+        val hmp = createHeadsUpManagerPhone()
+
+        val notifEntry = HeadsUpManagerTestUtil.createEntry(/* id= */ 0, mContext)
+        val row = mock<ExpandableNotificationRow>()
+        whenever(row.showingPulsing()).thenReturn(false)
+        notifEntry.row = row
+
+        hmp.showNotification(notifEntry)
+        Assert.assertFalse(notifEntry.isSeenInShade)
+    }
+
+    @Test
     fun shouldHeadsUpBecomePinned_shadeNotExpanded_true() =
         testScope.runTest {
             // GIVEN
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 e48c28d3f3ee..cb133ecadab2 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
@@ -1005,6 +1005,16 @@ public final class NotificationEntry extends ListEntry implements NotificationRo
         mIsMarkedForUserTriggeredMovement = marked;
     }
 
+    private boolean mSeenInShade = false;
+
+    public void setSeenInShade(boolean seen) {
+        mSeenInShade = seen;
+    }
+
+    public boolean isSeenInShade() {
+        return mSeenInShade;
+    }
+
     public void setIsHeadsUpEntry(boolean isHeadsUpEntry) {
         mIsHeadsUpEntry = isHeadsUpEntry;
     }
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/VisualStabilityProvider.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/VisualStabilityProvider.kt
index 5adf31b75fa7..0c7ba15baa92 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/VisualStabilityProvider.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/VisualStabilityProvider.kt
@@ -13,12 +13,18 @@ class VisualStabilityProvider @Inject constructor() {
     /** The subset of active listeners which are temporary (will be removed after called) */
     private val temporaryListeners = ArraySet<OnReorderingAllowedListener>()
 
+    private val banListeners = ListenerSet<OnReorderingBannedListener>()
+
     var isReorderingAllowed = true
         set(value) {
             if (field != value) {
                 field = value
                 if (value) {
                     notifyReorderingAllowed()
+                } else {
+                    banListeners.forEach { listener ->
+                        listener.onReorderingBanned()
+                    }
                 }
             }
         }
@@ -38,6 +44,10 @@ class VisualStabilityProvider @Inject constructor() {
         allListeners.addIfAbsent(listener)
     }
 
+    fun addPersistentReorderingBannedListener(listener: OnReorderingBannedListener) {
+        banListeners.addIfAbsent(listener)
+    }
+
     /** Add a listener which will be removed when it is called. */
     fun addTemporaryReorderingAllowedListener(listener: OnReorderingAllowedListener) {
         // Only add to the temporary set if it was added to the global set
@@ -57,3 +67,7 @@ class VisualStabilityProvider @Inject constructor() {
 fun interface OnReorderingAllowedListener {
     fun onReorderingAllowed()
 }
+
+fun interface OnReorderingBannedListener {
+    fun onReorderingBanned()
+}
\ No newline at end of file
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java
index cec1ef3f1e7b..4a447b7439b8 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java
@@ -4862,14 +4862,20 @@ public class NotificationStackScrollLayout
      * @param isHeadsUp true for appear, false for disappear animations
      */
     public void generateHeadsUpAnimation(ExpandableNotificationRow row, boolean isHeadsUp) {
-        final boolean add = mAnimationsEnabled && (isHeadsUp || mHeadsUpGoingAwayAnimationsAllowed);
+        final boolean closedAndSeenInShade = !mIsExpanded && row.getEntry() != null
+                && row.getEntry().isSeenInShade();
+        final boolean addAnimation = mAnimationsEnabled && !closedAndSeenInShade &&
+                (isHeadsUp || mHeadsUpGoingAwayAnimationsAllowed);
         if (SPEW) {
             Log.v(TAG, "generateHeadsUpAnimation:"
-                    + " willAdd=" + add
-                    + " isHeadsUp=" + isHeadsUp
-                    + " row=" + row.getEntry().getKey());
-        }
-        if (add) {
+                    + " addAnimation=" + addAnimation
+                    + (row.getEntry() == null ? " entry NULL "
+                            : " isSeenInShade=" + row.getEntry().isSeenInShade()
+                                    + " row=" + row.getEntry().getKey())
+                    + " mIsExpanded=" + mIsExpanded
+                    + " isHeadsUp=" + isHeadsUp);
+        }
+        if (addAnimation) {
             // If we're hiding a HUN we just started showing THIS FRAME, then remove that event,
             // and do not add the disappear event either.
             if (!isHeadsUp && mHeadsUpChangeAnimations.remove(new Pair<>(row, true))) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/HeadsUpManagerPhone.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/HeadsUpManagerPhone.java
index a2e44dffb767..8577d48b6679 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/HeadsUpManagerPhone.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/HeadsUpManagerPhone.java
@@ -39,6 +39,7 @@ import com.android.systemui.shade.domain.interactor.ShadeInteractor;
 import com.android.systemui.statusbar.StatusBarState;
 import com.android.systemui.statusbar.notification.collection.NotificationEntry;
 import com.android.systemui.statusbar.notification.collection.provider.OnReorderingAllowedListener;
+import com.android.systemui.statusbar.notification.collection.provider.OnReorderingBannedListener;
 import com.android.systemui.statusbar.notification.collection.provider.VisualStabilityProvider;
 import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager;
 import com.android.systemui.statusbar.notification.data.repository.HeadsUpRepository;
@@ -86,7 +87,7 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements
     private final List<OnHeadsUpPhoneListenerChange> mHeadsUpPhoneListeners = new ArrayList<>();
     private final VisualStabilityProvider mVisualStabilityProvider;
 
-    private final AvalancheController mAvalancheController;
+    private AvalancheController mAvalancheController;
 
     // TODO(b/328393698) move the topHeadsUpRow logic to an interactor
     private final MutableStateFlow<HeadsUpRowRepository> mTopHeadsUpRow =
@@ -173,6 +174,9 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements
         });
         javaAdapter.alwaysCollectFlow(shadeInteractor.isAnyExpanded(),
                     this::onShadeOrQsExpanded);
+        mVisualStabilityProvider.addPersistentReorderingBannedListener(mOnReorderingBannedListener);
+        mVisualStabilityProvider.addPersistentReorderingAllowedListener(
+                mOnReorderingAllowedListener);
     }
 
     public void setAnimationStateHandler(AnimationStateHandler handler) {
@@ -379,6 +383,7 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements
 
     private final OnReorderingAllowedListener mOnReorderingAllowedListener = () -> {
         mAnimationStateHandler.setHeadsUpGoingAwayAnimationsAllowed(false);
+        mAvalancheController.setEnableAtRuntime(true);
         for (NotificationEntry entry : mEntriesToRemoveWhenReorderingAllowed) {
             if (isHeadsUpEntry(entry.getKey())) {
                 // Maybe the heads-up was removed already
@@ -389,6 +394,22 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements
         mAnimationStateHandler.setHeadsUpGoingAwayAnimationsAllowed(true);
     };
 
+    private final OnReorderingBannedListener mOnReorderingBannedListener = () -> {
+        if (mAvalancheController != null) {
+            // In open shade the first HUN is pinned, and visual stability logic prevents us from
+            // unpinning this first HUN as long as the shade remains open. AvalancheController only
+            // shows the next HUN when the currently showing HUN is unpinned, so we must disable
+            // throttling here so that the incoming HUN stream is not forever paused. This is reset
+            // when reorder becomes allowed.
+            mAvalancheController.setEnableAtRuntime(false);
+
+            // Note that we cannot do the above when
+            // 1) The remove runnable runs because its delay means it may not run before shade close
+            // 2) Reordering is allowed again (when shade closes) because the HUN appear animation
+            // will have started by then
+        }
+    };
+
     ///////////////////////////////////////////////////////////////////////////////////////////////
     //  HeadsUpManager utility (protected) methods overrides:
 
@@ -561,18 +582,26 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements
         }
 
         @Override
+        protected void setEntry(@androidx.annotation.NonNull NotificationEntry entry,
+                @androidx.annotation.Nullable Runnable removeRunnable) {
+            super.setEntry(entry, removeRunnable);
+
+            if (!mVisualStabilityProvider.isReorderingAllowed()
+                    // We don't want to allow reordering while pulsing, but headsup need to
+                    // time out anyway
+                    && !entry.showingPulsing()) {
+                mEntriesToRemoveWhenReorderingAllowed.add(entry);
+                entry.setSeenInShade(true);
+            }
+        }
+
+        @Override
         protected Runnable createRemoveRunnable(NotificationEntry entry) {
-            return  () -> {
-                if (!mVisualStabilityProvider.isReorderingAllowed()
-                        // We don't want to allow reordering while pulsing, but headsup need to
-                        // time out anyway
-                        && !entry.showingPulsing()) {
-                    mEntriesToRemoveWhenReorderingAllowed.add(entry);
-                    mVisualStabilityProvider.addTemporaryReorderingAllowedListener(
-                            mOnReorderingAllowedListener);
-                } else if (mTrackingHeadsUp) {
+            return () -> {
+                if (mTrackingHeadsUp) {
                     mEntriesToRemoveAfterExpand.add(entry);
-                } else {
+                } else if (mVisualStabilityProvider.isReorderingAllowed()
+                        || entry.showingPulsing()) {
                     removeEntry(entry.getKey(), "createRemoveRunnable");
                 }
             };
@@ -585,9 +614,6 @@ public class HeadsUpManagerPhone extends BaseHeadsUpManager implements
             if (mEntriesToRemoveAfterExpand.contains(mEntry)) {
                 mEntriesToRemoveAfterExpand.remove(mEntry);
             }
-            if (mEntriesToRemoveWhenReorderingAllowed.contains(mEntry)) {
-                mEntriesToRemoveWhenReorderingAllowed.remove(mEntry);
-            }
         }
 
         @Override
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/AvalancheController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/policy/AvalancheController.kt
index 40799583a7b9..645a3619a7e9 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/AvalancheController.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/AvalancheController.kt
@@ -44,6 +44,16 @@ constructor(dumpManager: DumpManager,
 
     private val tag = "AvalancheController"
     private val debug = Compile.IS_DEBUG && Log.isLoggable(tag, Log.DEBUG)
+    var enableAtRuntime = true
+        set(value) {
+            if (!value) {
+                // Waiting HUNs in AvalancheController are shown in the HUN section in open shade.
+                // Clear them so we don't show them again when the shade closes and reordering is
+                // allowed again.
+                logDroppedHunsInBackground(getWaitingKeys().size)
+                clearNext()
+            }
+        }
 
     // HUN showing right now, in the floating state where full shade is hidden, on launcher or AOD
     @VisibleForTesting var headsUpEntryShowing: HeadsUpEntry? = null
@@ -90,6 +100,10 @@ constructor(dumpManager: DumpManager,
         return getKey(headsUpEntryShowing)
     }
 
+    fun isEnabled() : Boolean {
+        return NotificationThrottleHun.isEnabled && enableAtRuntime
+    }
+
     /** Run or delay Runnable for given HeadsUpEntry */
     fun update(entry: HeadsUpEntry?, runnable: Runnable?, label: String) {
         if (runnable == null) {
@@ -185,7 +199,8 @@ constructor(dumpManager: DumpManager,
             showNext()
             runnable.run()
         } else {
-            log { "$fn => removing untracked ${getKey(entry)}" }
+            log { "$fn => run runnable for untracked shown ${getKey(entry)}" }
+            runnable.run()
         }
         logState("after $fn")
     }
@@ -197,7 +212,7 @@ constructor(dumpManager: DumpManager,
      *    BaseHeadsUpManager.HeadsUpEntry.calculateFinishTime to shorten display duration.
      */
     fun getDurationMs(entry: HeadsUpEntry, autoDismissMs: Int): Int {
-        if (!NotificationThrottleHun.isEnabled) {
+        if (!isEnabled()) {
             // Use default duration, like we did before AvalancheController existed
             return autoDismissMs
         }
@@ -246,7 +261,7 @@ constructor(dumpManager: DumpManager,
 
     /** Return true if entry is waiting to show. */
     fun isWaiting(key: String): Boolean {
-        if (!NotificationThrottleHun.isEnabled) {
+        if (!isEnabled()) {
             return false
         }
         for (entry in nextMap.keys) {
@@ -259,7 +274,7 @@ constructor(dumpManager: DumpManager,
 
     /** Return list of keys for huns waiting */
     fun getWaitingKeys(): MutableList<String> {
-        if (!NotificationThrottleHun.isEnabled) {
+        if (!isEnabled()) {
             return mutableListOf()
         }
         val keyList = mutableListOf<String>()
@@ -270,7 +285,7 @@ constructor(dumpManager: DumpManager,
     }
 
     fun getWaitingEntry(key: String): HeadsUpEntry? {
-        if (!NotificationThrottleHun.isEnabled) {
+        if (!isEnabled()) {
             return null
         }
         for (headsUpEntry in nextMap.keys) {
@@ -282,7 +297,7 @@ constructor(dumpManager: DumpManager,
     }
 
     fun getWaitingEntryList(): List<HeadsUpEntry> {
-        if (!NotificationThrottleHun.isEnabled) {
+        if (!isEnabled()) {
             return mutableListOf()
         }
         return nextMap.keys.toList()
@@ -340,7 +355,7 @@ constructor(dumpManager: DumpManager,
         showNow(headsUpEntryShowing!!, headsUpEntryShowingRunnableList)
     }
 
-    fun logDroppedHunsInBackground(numDropped: Int) {
+    private fun logDroppedHunsInBackground(numDropped: Int) {
         bgHandler.post(Runnable {
             // Do this in the background to avoid missing frames when closing the shade
             for (n in 1..numDropped) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/BaseHeadsUpManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/BaseHeadsUpManager.java
index 220e729625af..a0eb989a57bb 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/BaseHeadsUpManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/BaseHeadsUpManager.java
@@ -756,7 +756,7 @@ public abstract class BaseHeadsUpManager implements HeadsUpManager {
             setEntry(entry, createRemoveRunnable(entry));
         }
 
-        private void setEntry(@NonNull final NotificationEntry entry,
+        protected void setEntry(@NonNull final NotificationEntry entry,
                 @Nullable Runnable removeRunnable) {
             mEntry = entry;
             mRemoveRunnable = removeRunnable;
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java
index a925ccfe174b..c9710370c2c2 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java
@@ -1197,6 +1197,26 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase {
 
     @Test
     @EnableFlags(NotificationsHeadsUpRefactor.FLAG_NAME)
+    public void testGenerateHeadsUpAnimation_isSeenInShade_noAnimation() {
+        // GIVEN NSSL is ready for HUN animations
+        Consumer<Boolean> headsUpAnimatingAwayListener = mock(BooleanConsumer.class);
+        prepareStackScrollerForHunAnimations(headsUpAnimatingAwayListener);
+
+        // Entry was seen in shade
+        NotificationEntry entry = mock(NotificationEntry.class);
+        when(entry.isSeenInShade()).thenReturn(true);
+        ExpandableNotificationRow row = mock(ExpandableNotificationRow.class);
+        when(row.getEntry()).thenReturn(entry);
+
+        // WHEN we generate an add event
+        mStackScroller.generateHeadsUpAnimation(row, /* isHeadsUp = */ true);
+
+        // THEN nothing happens
+        assertThat(mStackScroller.isAddOrRemoveAnimationPending()).isFalse();
+    }
+
+    @Test
+    @EnableFlags(NotificationsHeadsUpRefactor.FLAG_NAME)
     public void testOnChildAnimationsFinished_resetsheadsUpAnimatingAway() {
         // GIVEN NSSL is ready for HUN animations
         Consumer<Boolean> headsUpAnimatingAwayListener = mock(BooleanConsumer.class);