Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 96f3d03b authored by Lyn's avatar Lyn Committed by lyn
Browse files

Fix HUN re-show after closing shade 

The removeRunnable that we create for every entry
adds the removed entry to mEntriesToRemoveWhenReorderingAllowed.
We remove from this list on entry update.

Problem: if the HUN is seen in shade, and we close the shade
BEFORE the removeRunnable runs OR after entry UPDATE,
the HUN is no longer in this list and we re-show the HUN.

Solution: record the HUN as “seen” immediately when shown in shade,
instead of on removal/update.

In this change ---------------

NotificationEntry.seenInShade
- New field set true when HUN shows when shade is open

HeadsUpmanager.OnReorderingBannedListener
- Disables AvalancheController when shade is open

HeadsUpManagerPhone.setEntry, if shade is open:
- Set "seenInShade" true
- Save for removal on shade close

HeadsUpManagerPhone.updateEntry
- Do NOT remove entry from mEntriesToRemoveWhenReorderingAllowed

  When HUNs show in shade, we immediately call updateEntry for
  them (from updatePostTime and scheduleAutoRemovalCallback) which
  should not change the fact that they should be demoted from HUN
  when shade closes

HeadsUpManagerPhone.OnReorderingAllowedListener
- Add this as one persistent listener instead of a temporary one for
  each HUN

NSSL.generateHeadsUpAnimation
- Do NOT generate animation if notification seenInShade==true

AvalancheController.delete
- Runs runnable for untracked but shown HUN so that HUN is removed
  (without animation) once shade closes

Full CUJ walkthrough --------------

Open shade
- HeadsUpManagerPhone.OnReorderingBannedListener clears waiting HUNs
  and disables AvalancheController

HUN arrives; the disabled AvalancheController runs the following ASAP:
- HeadsUpManagerPhone.setEntry
  - adds entry to mEntriesToRemoveWhenReorderingAllowed
  - sets "seenInShade" true
- BaseHeadsUpManager.showNotification: runs the appear animation
  - HeadsUpNotificationViewBinder.bindHeadsUpNotifications calls
    NSSL.generateHeadsUpAnimation, which adds an item to
    mHeadsUpChangeAnimations
  - requestChildrenUpdate calls generateAllAnimationEvents, calls
    generateHeadsUpAnimationEvents, with this new HeadsUpChangeAnimation
- BaseHeadsUpManager.updateEntry (twice, from updatePostEntry and
  scheduleAutoRemovalCallback)
  - HUN now stays in mEntriesToRemoveWhenReorderingAllowed

Shade closes:
- OnReorderingAllowedListener calls removeEntry and enables
AvalancheController
- AvalancheController runs removeEntry ASAP for the untracked HUN,
which is untracked because it was never added to the queue while
AvalancheController was disabled during shade open.
- removeEntry results in HeadsUpNotificationViewBinder calling
NSSL.generateHeadsUpAnimation, which DOES NOT generate the animation
since the HUN was seen and shade is closed; thus the HUN does not
re-show.

Bug: 350797731
Test: AvalancheControllerTest
Test: NotificationStackScrollLayoutTest
Test: HeadsUpManagerPhoneTest
Test: Close shade/exit lockscreen before HUN timeout
      => HUNs do not re-show
      Close shade/exit lockscreen after HUN timeout
      => HUNs do not re-show
      Inline reply from shade/fullscreen quick settings,
      then close shade
      => HUNs do not re-show
      Send floating HUN with shade closed
      => No change to throttled behavior

Flag: com.android.systemui.notification_avalanche_throttle_hun
Change-Id: I0b1d36582025b61d7c917ca02e7deac18ce87f23
parent 3a93b970

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/AvalancheControllerTest.kt

+17 −0
Original line number Original line Diff line number Diff line
@@ -179,6 +179,23 @@ class AvalancheControllerTest : SysuiTestCase() { 
        assertThat(mAvalancheController.nextMap.containsKey(headsUpEntry)).isTrue()

        assertThat(mAvalancheController.nextMap.containsKey(headsUpEntry)).isTrue()

    }

    }





    @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

    @Test

    fun testDelete_isNext_removedFromNext_runnableNotRun() {

    fun testDelete_isNext_removedFromNext_runnableNotRun() {

        // Entry is next

        // Entry is next

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/policy/HeadsUpManagerPhoneTest.kt

+30 −0
Original line number Original line Diff line number Diff line
@@ -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.NotificationEntry

import com.android.systemui.statusbar.notification.collection.provider.VisualStabilityProvider

import com.android.systemui.statusbar.notification.collection.provider.VisualStabilityProvider

import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager

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.NotificationThrottleHun

import com.android.systemui.statusbar.notification.shared.NotificationsHeadsUpRefactor

import com.android.systemui.statusbar.notification.shared.NotificationsHeadsUpRefactor

import com.android.systemui.statusbar.phone.ConfigurationControllerImpl

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.DelayableExecutor

import com.android.systemui.util.concurrency.mockExecutorHandler

import com.android.systemui.util.concurrency.mockExecutorHandler

import com.android.systemui.util.kotlin.JavaAdapter

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.settings.GlobalSettings

import com.android.systemui.util.time.SystemClock

import com.android.systemui.util.time.SystemClock

import junit.framework.Assert

import junit.framework.Assert
@@ -236,6 +238,34 @@ class HeadsUpManagerPhoneTest(flags: FlagsParameterization) : BaseHeadsUpManager 
        Assert.assertTrue(hmp.isHeadsUpEntry(entry.key))

        Assert.assertTrue(hmp.isHeadsUpEntry(entry.key))

    }

    }





    @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

    @Test

    fun shouldHeadsUpBecomePinned_shadeNotExpanded_true() =

    fun shouldHeadsUpBecomePinned_shadeNotExpanded_true() =

        testScope.runTest {

        testScope.runTest {

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java

+10 −0
Original line number Original line Diff line number Diff line
@@ -1005,6 +1005,16 @@ public final class NotificationEntry extends ListEntry implements NotificationRo 
        mIsMarkedForUserTriggeredMovement = marked;

        mIsMarkedForUserTriggeredMovement = marked;

    }

    }





    private boolean mSeenInShade = false;



    public void setSeenInShade(boolean seen) {

        mSeenInShade = seen;

    }



    public boolean isSeenInShade() {

        return mSeenInShade;

    }



    public void setIsHeadsUpEntry(boolean isHeadsUpEntry) {

    public void setIsHeadsUpEntry(boolean isHeadsUpEntry) {

        mIsHeadsUpEntry = isHeadsUpEntry;

        mIsHeadsUpEntry = isHeadsUpEntry;

    }

    }

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/VisualStabilityProvider.kt

+14 −0
Original line number Original line Diff line number Diff line
@@ -13,12 +13,18 @@ class VisualStabilityProvider @Inject constructor() { 
    /** The subset of active listeners which are temporary (will be removed after called) */

    /** The subset of active listeners which are temporary (will be removed after called) */

    private val temporaryListeners = ArraySet<OnReorderingAllowedListener>()

    private val temporaryListeners = ArraySet<OnReorderingAllowedListener>()





    private val banListeners = ListenerSet<OnReorderingBannedListener>()



    var isReorderingAllowed = true

    var isReorderingAllowed = true

        set(value) {

        set(value) {

            if (field != value) {

            if (field != value) {

                field = value

                field = value

                if (value) {

                if (value) {

                    notifyReorderingAllowed()

                    notifyReorderingAllowed()

                } else {

                    banListeners.forEach { listener ->

                        listener.onReorderingBanned()

                    }

                }

                }

            }

            }

        }

        }
@@ -38,6 +44,10 @@ class VisualStabilityProvider @Inject constructor() { 
        allListeners.addIfAbsent(listener)

        allListeners.addIfAbsent(listener)

    }

    }





    fun addPersistentReorderingBannedListener(listener: OnReorderingBannedListener) {

        banListeners.addIfAbsent(listener)

    }



    /** Add a listener which will be removed when it is called. */

    /** Add a listener which will be removed when it is called. */

    fun addTemporaryReorderingAllowedListener(listener: OnReorderingAllowedListener) {

    fun addTemporaryReorderingAllowedListener(listener: OnReorderingAllowedListener) {

        // Only add to the temporary set if it was added to the global set

        // 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 interface OnReorderingAllowedListener {

    fun onReorderingAllowed()

    fun onReorderingAllowed()

}

}



fun interface OnReorderingBannedListener {

    fun onReorderingBanned()

}
 
 No newline at end of file

packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java

+12 −6
Original line number Original line Diff line number Diff line
@@ -4857,14 +4857,20 @@ public class NotificationStackScrollLayout 
     * @param isHeadsUp true for appear, false for disappear animations

     * @param isHeadsUp true for appear, false for disappear animations

     */

     */

    public void generateHeadsUpAnimation(ExpandableNotificationRow row, boolean isHeadsUp) {

    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) {

        if (SPEW) {

            Log.v(TAG, "generateHeadsUpAnimation:"

            Log.v(TAG, "generateHeadsUpAnimation:"

                    + " willAdd=" + add

                    + " addAnimation=" + addAnimation

                    + " isHeadsUp=" + isHeadsUp

                    + (row.getEntry() == null ? " entry NULL "

                    + " row=" + row.getEntry().getKey());

                            : " isSeenInShade=" + row.getEntry().isSeenInShade()

        }

                                    + " row=" + row.getEntry().getKey())

        if (add) {

                    + " mIsExpanded=" + mIsExpanded

                    + " isHeadsUp=" + isHeadsUp);

        }

        if (addAnimation) {

            // If we're hiding a HUN we just started showing THIS FRAME, then remove that event,

            // If we're hiding a HUN we just started showing THIS FRAME, then remove that event,

            // and do not add the disappear event either.

            // and do not add the disappear event either.

            if (!isHeadsUp && mHeadsUpChangeAnimations.remove(new Pair<>(row, true))) {

            if (!isHeadsUp && mHeadsUpChangeAnimations.remove(new Pair<>(row, true))) {