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

Commit 037e26ab authored by Julia Tuttle's avatar Julia Tuttle
Browse files

Cancel future HUN lifetime extension after action press 

ag/18595495 made sure that action presses *after* a HUN's lifetime was
extended would shorten the lifetime extension to the minimum display
time, but noted that race conditions were possible.

This change makes sure that action presses *before or after* a HUNning
notification's removal will *prevent or end* lifetime extension
entirely.

Bug: 233864220
Bug: 284418902
Test: atest HeadsUpManagerTest HeadsUpCoordinatorTest
Change-Id: Ib751cfe29e994df5582186cb8d9cf91f0977a518
parent c831c399

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt

+10 −5
Original line number Diff line number Diff line
@@ -633,12 +633,17 @@ class HeadsUpCoordinator @Inject constructor( 
        mFSIUpdateCandidates.removeAll(toRemoveForFSI)

    }



    /** When an action is pressed on a notification, end HeadsUp lifetime extension. */

    /**

     * When an action is pressed on a notification, make sure we don't lifetime-extend it in the

     * future by informing the HeadsUpManager, and make sure we don't keep lifetime-extending it if

     * we already are.

     *

     * @see HeadsUpManager.setUserActionMayIndirectlyRemove

     * @see HeadsUpManager.canRemoveImmediately

     */

    private val mActionPressListener = Consumer<NotificationEntry> { entry ->

        if (mNotifsExtendingLifetime.contains(entry)) {

            val removeInMillis = mHeadsUpManager.getEarliestRemovalTime(entry.key)

            mExecutor.executeDelayed({ endNotifLifetimeExtensionIfExtended(entry) }, removeInMillis)

        }

        mHeadsUpManager.setUserActionMayIndirectlyRemove(entry)

        mExecutor.execute { endNotifLifetimeExtensionIfExtended(entry) }

    }



    private val mLifetimeExtender = object : NotifLifetimeExtender {

packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpManager.java

+27 −0
Original line number Diff line number Diff line
@@ -393,6 +393,31 @@ public abstract class HeadsUpManager extends AlertingNotificationManager { 
        }

    }



    /**

     * Notes that the user took an action on an entry that might indirectly cause the system or the

     * app to remove the notification.

     *

     * @param entry the entry that might be indirectly removed by the user's action

     *

     * @see com.android.systemui.statusbar.notification.collection.coordinator.HeadsUpCoordinator#mActionPressListener

     * @see #canRemoveImmediately(String)

     */

    public void setUserActionMayIndirectlyRemove(@NonNull NotificationEntry entry) {

        HeadsUpEntry headsUpEntry = getHeadsUpEntry(entry.getKey());

        if (headsUpEntry != null) {

            headsUpEntry.userActionMayIndirectlyRemove = true;

        }

    }



    @Override

    public boolean canRemoveImmediately(@NonNull String key) {

        HeadsUpEntry headsUpEntry = getHeadsUpEntry(key);

        if (headsUpEntry != null && headsUpEntry.userActionMayIndirectlyRemove) {

            return true;

        }

        return super.canRemoveImmediately(key);

    }



    @NonNull

    @Override

    protected HeadsUpEntry createAlertEntry() {
@@ -421,6 +446,8 @@ public abstract class HeadsUpManager extends AlertingNotificationManager { 
     */

    protected class HeadsUpEntry extends AlertEntry {

        public boolean remoteInputActive;

        public boolean userActionMayIndirectlyRemove;



        protected boolean expanded;

        protected boolean wasUnpinned;

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinatorTest.kt

+23 −4
Original line number Diff line number Diff line
@@ -221,16 +221,35 @@ class HeadsUpCoordinatorTest : SysuiTestCase() { 
    }



    @Test

    fun hunExtensionCancelledWhenHunActionPressed() {

    fun actionPressCancelsExistingLifetimeExtension() {

        whenever(headsUpManager.isSticky(anyString())).thenReturn(true)

        addHUN(entry)



        whenever(headsUpManager.canRemoveImmediately(anyString())).thenReturn(false)

        whenever(headsUpManager.getEarliestRemovalTime(anyString())).thenReturn(1000L)

        assertTrue(notifLifetimeExtender.maybeExtendLifetime(entry, 0))

        assertTrue(notifLifetimeExtender.maybeExtendLifetime(entry, /* reason = */ 0))



        actionPressListener.accept(entry)

        executor.advanceClockToLast()

        executor.runAllReady()

        verify(headsUpManager, times(1)).removeNotification(eq(entry.key), eq(true))

        verify(endLifetimeExtension, times(1)).onEndLifetimeExtension(notifLifetimeExtender, entry)



        collectionListener.onEntryRemoved(entry, /* reason = */ 0)

        verify(headsUpManager, times(1)).removeNotification(eq(entry.key), any())

    }



    @Test

    fun actionPressPreventsFutureLifetimeExtension() {

        whenever(headsUpManager.isSticky(anyString())).thenReturn(true)

        addHUN(entry)



        actionPressListener.accept(entry)

        verify(headsUpManager, times(1)).setUserActionMayIndirectlyRemove(entry)



        whenever(headsUpManager.canRemoveImmediately(anyString())).thenReturn(true)

        assertFalse(notifLifetimeExtender.maybeExtendLifetime(entry, 0))



        collectionListener.onEntryRemoved(entry, /* reason = */ 0)

        verify(headsUpManager, times(1)).removeNotification(eq(entry.key), any())

    }



    @Test

packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/HeadsUpManagerTest.java

+13 −1
Original line number Diff line number Diff line
@@ -20,7 +20,6 @@ import static com.google.common.truth.Truth.assertThat; 


import static junit.framework.Assert.assertEquals;

import static junit.framework.Assert.assertFalse;

import static junit.framework.Assert.assertNotSame;

import static junit.framework.Assert.assertTrue;



import static org.mockito.ArgumentMatchers.anyInt;
@@ -346,4 +345,17 @@ public class HeadsUpManagerTest extends AlertingNotificationManagerTest { 
        assertEquals(HeadsUpManager.NotificationPeekEvent.NOTIFICATION_PEEK.getId(),

                mUiEventLoggerFake.eventId(0));

    }



    @Test

    public void testSetUserActionMayIndirectlyRemove() {

        NotificationEntry notifEntry = new NotificationEntryBuilder()

                .setSbn(createNewNotification(/* id= */ 0))

                .build();



        mHeadsUpManager.showNotification(notifEntry);

        assertFalse(mHeadsUpManager.canRemoveImmediately(notifEntry.getKey()));



        mHeadsUpManager.setUserActionMayIndirectlyRemove(notifEntry);

        assertTrue(mHeadsUpManager.canRemoveImmediately(notifEntry.getKey()));

    }

}