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

Commit b908ca8a authored by Alexander Roederer's avatar Alexander Roederer
Browse files

RemoteInputCoordinator to update notif on reply 

Modifies RemoteInputCoordinator to always update the notification on
direct reply. Before, we'd check "isSpinning" to see if the update came
while the notification was in the sending state, which would mean a
reply occured for the first time and should be appended. If an update
occured and the notification wasn't "spinning," it was a duplicate
update caused by the app repeatedly canceling.

The problem with this approach is that NotificationContentView _also_
updates the notification, as part of onNotificationUpdated calls
applyRemoteInputAndSmartReply, which resets isSpinning. Thus, we have a
race condition, where if the update is processed by
NotificationContentView before RemoteInputCoordinator, the direct reply
never gets appended to the notification.

After ag/28913159, we no longer send a special system update beyond the first
cancelation of a lifetime extended notification by the app. So, repeated
cancelations don't send multiple updates, which means we no longer need
to check isSpinning to know if we should update the notification; we
should always update.

Bug: 360090006
Test: atest NotificationManagerServiceTest, atest
RemoteInputCoordinatorTest, atest NotificationRemoteInput
Flag: android.app.lifetime_extension_refactor

Change-Id: I993cd9fae05f131852d313ba4e5662cbffd49ff6
parent 670246f5

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

+8 −19
Original line number Diff line number Diff line
@@ -117,18 +117,12 @@ constructor( 
                            (entry.getSbn().getNotification().flags and

                                FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY) > 0

                        ) {

                            // If we've received an update from the system and the entry is marked

                            // as lifetime extended, that means system server has received a

                            // cancelation in response to a direct reply, and sent an update to

                            // let system ui know that it should rebuild the notification with

                            // that direct reply.

                            if (

                                mNotificationRemoteInputManager.shouldKeepForRemoteInputHistory(

                                    entry

                                )

                            ) {

                                val newSbn = mRebuilder.rebuildForRemoteInputReply(entry)

                                entry.onRemoteInputInserted()

                                mNotifUpdater.onInternalNotificationUpdate(

                                    newSbn,

                                    "Extending lifetime of notification with remote input",

                                )

                            } else if (

                                mNotificationRemoteInputManager.shouldKeepForSmartReplyHistory(

                                    entry

                                )
@@ -140,16 +134,11 @@ constructor( 
                                    "Extending lifetime of notification with smart reply",

                                )

                            } else {

                                // The app may have re-cancelled a notification after it had already

                                // been lifetime extended.

                                // Rebuild the notification with the replies it already had to

                                // ensure

                                // those replies continue to be displayed.

                                val newSbn = mRebuilder.rebuildWithExistingReplies(entry)

                                val newSbn = mRebuilder.rebuildForRemoteInputReply(entry)

                                entry.onRemoteInputInserted()

                                mNotifUpdater.onInternalNotificationUpdate(

                                    newSbn,

                                    "Extending lifetime of notification that has already been " +

                                        "lifetime extended.",

                                    "Extending lifetime of notification with remote input",

                                )

                            }

                        } else {

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

+30 −31
Original line number Diff line number Diff line
@@ -15,8 +15,8 @@ 
 */

package com.android.systemui.statusbar.notification.collection.coordinator



import android.app.Flags.lifetimeExtensionRefactor

import android.app.Flags.FLAG_LIFETIME_EXTENSION_REFACTOR

import android.app.Flags.lifetimeExtensionRefactor

import android.app.Notification

import android.app.RemoteInputHistoryItem

import android.os.Handler
@@ -47,10 +47,10 @@ import org.junit.Before 
import org.junit.Test

import org.junit.runner.RunWith

import org.mockito.Mock

import org.mockito.Mockito.`when`

import org.mockito.Mockito.never

import org.mockito.Mockito.times

import org.mockito.Mockito.verify

import org.mockito.Mockito.`when`

import org.mockito.MockitoAnnotations.initMocks



@SmallTest
@@ -78,21 +78,20 @@ class RemoteInputCoordinatorTest : SysuiTestCase() { 
    @Before

    fun setUp() {

        initMocks(this)

        coordinator = RemoteInputCoordinator(

        coordinator =

            RemoteInputCoordinator(

                dumpManager,

                rebuilder,

                remoteInputManager,

                mainHandler,

                smartReplyController

                smartReplyController,

            )

        `when`(pipeline.addNotificationLifetimeExtender(any())).thenAnswer {

            (it.arguments[0] as NotifLifetimeExtender).setCallback(lifetimeExtensionCallback)

        }

        `when`(pipeline.getInternalNotifUpdater(any())).thenReturn(notifUpdater)

        coordinator.attach(pipeline)

        listener = withArgCaptor {

            verify(remoteInputManager).setRemoteInputListener(capture())

        }

        listener = withArgCaptor { verify(remoteInputManager).setRemoteInputListener(capture()) }

        entry1 = NotificationEntryBuilder().setId(1).build()

        entry2 = NotificationEntryBuilder().setId(2).build()

        `when`(rebuilder.rebuildForCanceledSmartReplies(any())).thenReturn(sbn)
@@ -101,13 +100,17 @@ class RemoteInputCoordinatorTest : SysuiTestCase() { 
        `when`(rebuilder.rebuildWithExistingReplies(any())).thenReturn(sbn)

    }



    val remoteInputActiveExtender get() = coordinator.mRemoteInputActiveExtender

    val remoteInputHistoryExtender get() = coordinator.mRemoteInputHistoryExtender

    val smartReplyHistoryExtender get() = coordinator.mSmartReplyHistoryExtender

    val remoteInputActiveExtender

        get() = coordinator.mRemoteInputActiveExtender



    val collectionListeners get() = captureMany {

        verify(pipeline, times(1)).addCollectionListener(capture())

    }

    val remoteInputHistoryExtender

        get() = coordinator.mRemoteInputHistoryExtender



    val smartReplyHistoryExtender

        get() = coordinator.mSmartReplyHistoryExtender



    val collectionListeners

        get() = captureMany { verify(pipeline, times(1)).addCollectionListener(capture()) }



    @Test

    fun testRemoteInputActive() {
@@ -179,7 +182,8 @@ class RemoteInputCoordinatorTest : SysuiTestCase() { 
    @EnableFlags(FLAG_LIFETIME_EXTENSION_REFACTOR)

    fun testRemoteInputLifetimeExtensionListenerTrigger() {

        // Create notification with LIFETIME_EXTENDED_BY_DIRECT_REPLY flag.

        val entry = NotificationEntryBuilder()

        val entry =

            NotificationEntryBuilder()

                .setId(3)

                .setTag("entry")

                .setFlag(mContext, Notification.FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY, true)
@@ -187,9 +191,7 @@ class RemoteInputCoordinatorTest : SysuiTestCase() { 
        `when`(remoteInputManager.shouldKeepForRemoteInputHistory(entry)).thenReturn(true)

        `when`(remoteInputManager.shouldKeepForSmartReplyHistory(entry)).thenReturn(false)



        collectionListeners.forEach {

            it.onEntryUpdated(entry, true)

        }

        collectionListeners.forEach { it.onEntryUpdated(entry, true) }



        verify(rebuilder, times(1)).rebuildForRemoteInputReply(entry)

    }
@@ -198,16 +200,15 @@ class RemoteInputCoordinatorTest : SysuiTestCase() { 
    @EnableFlags(FLAG_LIFETIME_EXTENSION_REFACTOR)

    fun testSmartReplyLifetimeExtensionListenerTrigger() {

        // Create notification with LIFETIME_EXTENDED_BY_DIRECT_REPLY flag.

        val entry = NotificationEntryBuilder()

        val entry =

            NotificationEntryBuilder()

                .setId(3)

                .setTag("entry")

                .setFlag(mContext, Notification.FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY, true)

                .build()

        `when`(remoteInputManager.shouldKeepForRemoteInputHistory(entry)).thenReturn(false)

        `when`(remoteInputManager.shouldKeepForSmartReplyHistory(entry)).thenReturn(true)

        collectionListeners.forEach {

            it.onEntryUpdated(entry, true)

        }

        collectionListeners.forEach { it.onEntryUpdated(entry, true) }



        verify(rebuilder, times(1)).rebuildForCanceledSmartReplies(entry)

        verify(smartReplyController, times(1)).stopSending(entry)
@@ -217,25 +218,25 @@ class RemoteInputCoordinatorTest : SysuiTestCase() { 
    @EnableFlags(FLAG_LIFETIME_EXTENSION_REFACTOR)

    fun testRepeatedUpdateTriggersRebuild() {

        // Create notification with LIFETIME_EXTENDED_BY_DIRECT_REPLY flag.

        val entry = NotificationEntryBuilder()

        val entry =

            NotificationEntryBuilder()

                .setId(3)

                .setTag("entry")

                .setFlag(mContext, Notification.FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY, true)

                .build()

        `when`(remoteInputManager.shouldKeepForRemoteInputHistory(entry)).thenReturn(false)

        `when`(remoteInputManager.shouldKeepForSmartReplyHistory(entry)).thenReturn(false)

        collectionListeners.forEach {

            it.onEntryUpdated(entry, true)

        }

        collectionListeners.forEach { it.onEntryUpdated(entry, true) }



        verify(rebuilder, times(1)).rebuildWithExistingReplies(entry)

        verify(rebuilder, times(1)).rebuildForRemoteInputReply(entry)

    }



    @Test

    @EnableFlags(FLAG_LIFETIME_EXTENSION_REFACTOR)

    fun testLifetimeExtensionListenerClearsRemoteInputs() {

        // Create notification with LIFETIME_EXTENDED_BY_DIRECT_REPLY flag.

        val entry = NotificationEntryBuilder()

        val entry =

            NotificationEntryBuilder()

                .setId(3)

                .setTag("entry")

                .setFlag(mContext, Notification.FLAG_LIFETIME_EXTENDED_BY_DIRECT_REPLY, false)
@@ -245,9 +246,7 @@ class RemoteInputCoordinatorTest : SysuiTestCase() { 
        `when`(remoteInputManager.shouldKeepForRemoteInputHistory(entry)).thenReturn(false)

        `when`(remoteInputManager.shouldKeepForSmartReplyHistory(entry)).thenReturn(false)



        collectionListeners.forEach {

            it.onEntryUpdated(entry, true)

        }

        collectionListeners.forEach { it.onEntryUpdated(entry, true) }



        assertThat(entry.remoteInputs).isNull()

    }