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

Commit 409051c0 authored by Yining Liu's avatar Yining Liu
Browse files

Fix forced auto-grouping mark notifications as unseen issue 

The force auto-grouping will update the notifications in the
NotifCollection, which makes such notifications unseen. There was an
existing parameter, fromSystem, in onEntryUpdated() method, however,
this parameter is not accurate enough for us to distinguish an update
from the system server(eg. auto-grouping) and a real update from App,
(because that is also made through system server).

This change replaced the fromSystem parameter with a Enum class:
UpdateSource, which distinguishes system server, app, and systemui
updates. We are not going to mark an existing notification unseen again
if it's updated by a source other than App.

Fix: 391443182
Test: RemoteInputCoordinatorTest, BubblesTest
Flag: com.android.server.notification.notification_minimalism
Change-Id: I8d95dc6c3878465071c7ed2cb1de28d7a5d6ec43
parent 27045609

libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/Bubbles.java

+2 −1
Original line number Diff line number Diff line
@@ -214,7 +214,8 @@ public interface Bubbles { 
     *

     * @param entry          the {@link BubbleEntry} by the notification.

     * @param shouldBubbleUp {@code true} if this notification should bubble up.

     * @param fromSystem     {@code true} if this update is from NotificationManagerService.

     * @param fromSystem     {@code true} if this update is from NotificationManagerService or App,

     *                                   false means this update is from SystemUi

     */

    void onEntryUpdated(BubbleEntry entry, boolean shouldBubbleUp, boolean fromSystem);

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

+6 −7
Original line number Diff line number Diff line
@@ -41,7 +41,6 @@ import static org.junit.Assert.assertNotEquals; 
import static org.junit.Assert.assertNotNull;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.clearInvocations;
@@ -64,7 +63,6 @@ import android.app.NotificationChannel; 
import android.app.NotificationManager;

import android.os.Handler;

import android.os.RemoteException;

import android.platform.test.annotations.EnableFlags;

import android.service.notification.NotificationListenerService.Ranking;

import android.service.notification.NotificationListenerService.RankingMap;

import android.service.notification.StatusBarNotification;
@@ -78,7 +76,6 @@ import androidx.test.filters.SmallTest; 


import com.android.internal.statusbar.IStatusBarService;

import com.android.internal.statusbar.NotificationVisibility;

import com.android.systemui.Flags;

import com.android.systemui.SysuiTestCase;

import com.android.systemui.dump.DumpManager;

import com.android.systemui.dump.LogBufferEulogizer;
@@ -96,6 +93,7 @@ import com.android.systemui.statusbar.notification.collection.notifcollection.No 
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionLogger;

import com.android.systemui.statusbar.notification.collection.notifcollection.NotifDismissInterceptor;

import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender;

import com.android.systemui.statusbar.notification.collection.notifcollection.UpdateSource;

import com.android.systemui.statusbar.notification.collection.provider.NotificationDismissibilityProvider;

import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;

import com.android.systemui.util.concurrency.FakeExecutor;
@@ -1587,7 +1585,7 @@ public class NotifCollectionTest extends SysuiTestCase { 


        // THEN entry updated gets called, added does not, and ranking is called again

        verify(mCollectionListener).onEntryUpdated(eq(entry));

        verify(mCollectionListener).onEntryUpdated(eq(entry), eq(true));

        verify(mCollectionListener).onEntryUpdated(eq(entry), eq(UpdateSource.App));

        verify(mCollectionListener).onEntryAdded((entry));

        verify(mCollectionListener, times(2)).onRankingApplied();

    }
@@ -1610,7 +1608,7 @@ public class NotifCollectionTest extends SysuiTestCase { 
        verify(mCollectionListener).onEntryAdded(eq(entry));

        verify(mCollectionListener).onRankingApplied();

        verify(mCollectionListener).onEntryUpdated(eq(entry));

        verify(mCollectionListener).onEntryUpdated(eq(entry), eq(false));

        verify(mCollectionListener).onEntryUpdated(eq(entry), eq(UpdateSource.SystemUi));

    }



    @Test
@@ -1626,7 +1624,7 @@ public class NotifCollectionTest extends SysuiTestCase { 
        verify(mCollectionListener, never()).onRankingUpdate(any());

        verify(mCollectionListener, never()).onRankingApplied();

        verify(mCollectionListener, never()).onEntryUpdated(any());

        verify(mCollectionListener, never()).onEntryUpdated(any(), anyBoolean());

        verify(mCollectionListener, never()).onEntryUpdated(any(), any());

    }



    @Test
@@ -1786,6 +1784,7 @@ public class NotifCollectionTest extends SysuiTestCase { 


    private static NotificationEntryBuilder buildNotif(String pkg, int id, String tag) {

        return new NotificationEntryBuilder()

                .setPostTime(System.currentTimeMillis())

                .setPkg(pkg)

                .setId(id)

                .setTag(tag);
@@ -1838,7 +1837,7 @@ public class NotifCollectionTest extends SysuiTestCase { 
        }



        @Override

        public void onEntryUpdated(NotificationEntry entry, boolean fromSystem) {

        public void onEntryUpdated(NotificationEntry entry, UpdateSource source) {

            onEntryUpdated(entry);

        }

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

+2 −1
Original line number Diff line number Diff line
@@ -42,6 +42,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntryB 
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter

import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Pluggable

import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener

import com.android.systemui.statusbar.notification.collection.notifcollection.UpdateSource

import com.android.systemui.statusbar.notification.domain.interactor.SeenNotificationsInteractor

import com.android.systemui.statusbar.notification.domain.interactor.lockScreenShowOnlyUnseenNotificationsSetting

import com.android.systemui.statusbar.notification.domain.interactor.seenNotificationsInteractor
@@ -586,7 +587,7 @@ class OriginalUnseenKeyguardCoordinatorTest(flags: FlagsParameterization) : Sysu 
            testScheduler.runCurrent()



            // WHEN: the notification is updated

            collectionListener.onEntryUpdated(entry)

            collectionListener.onEntryUpdated(entry, UpdateSource.App)

            testScheduler.runCurrent()



            // WHEN: four more seconds have passed

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

+5 −4
Original line number Diff line number Diff line
@@ -39,6 +39,7 @@ import com.android.systemui.statusbar.notification.collection.notifcollection.In 
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener

import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender

import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender.OnEndLifetimeExtensionCallback

import com.android.systemui.statusbar.notification.collection.notifcollection.UpdateSource

import com.android.systemui.util.mockito.any

import com.android.systemui.util.mockito.captureMany

import com.android.systemui.util.mockito.withArgCaptor
@@ -191,7 +192,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, UpdateSource.App) }



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

    }
@@ -208,7 +209,7 @@ class RemoteInputCoordinatorTest : SysuiTestCase() { 
                .build()

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

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

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

        collectionListeners.forEach { it.onEntryUpdated(entry, UpdateSource.App) }



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

        verify(smartReplyController, times(1)).stopSending(entry)
@@ -226,7 +227,7 @@ class RemoteInputCoordinatorTest : SysuiTestCase() { 
                .build()

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

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

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

        collectionListeners.forEach { it.onEntryUpdated(entry, UpdateSource.App) }



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

    }
@@ -246,7 +247,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, UpdateSource.App) }



        assertThat(entry.remoteInputs).isNull()

    }

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

+9 −2
Original line number Diff line number Diff line
@@ -96,6 +96,7 @@ import com.android.systemui.statusbar.notification.collection.notifcollection.No 
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender;

import com.android.systemui.statusbar.notification.collection.notifcollection.RankingAppliedEvent;

import com.android.systemui.statusbar.notification.collection.notifcollection.RankingUpdatedEvent;

import com.android.systemui.statusbar.notification.collection.notifcollection.UpdateSource;

import com.android.systemui.statusbar.notification.collection.provider.NotificationDismissibilityProvider;

import com.android.systemui.util.Assert;

import com.android.systemui.util.NamedListenerSet;
@@ -547,6 +548,12 @@ public class NotifCollection implements Dumpable, PipelineDumpable { 
            // TODO: If a coalesced event ever gets here, it's possible to lose track of children,

            //  since their rankings might have been updated earlier (and thus we may no longer

            //  think a child is associated with this locally-dismissed entry).

            // If the postTime remains the same, we can assume the update is from SystemServer, not

            // the app.

            long lastUpdateTime = entry.getSbn().getPostTime();

            UpdateSource source = sbn.getPostTime() == lastUpdateTime

                    ? UpdateSource.SystemServer

                    : UpdateSource.App;

            cancelLocalDismissal(entry);

            cancelLifetimeExtension(entry);

            cancelDismissInterception(entry);
@@ -556,7 +563,7 @@ public class NotifCollection implements Dumpable, PipelineDumpable { 
            mEventQueue.add(new BindEntryEvent(entry, sbn));



            mLogger.logNotifUpdated(entry);

            mEventQueue.add(new EntryUpdatedEvent(entry, true /* fromSystem */));

            mEventQueue.add(new EntryUpdatedEvent(entry, source));

        }

    }
@@ -1045,7 +1052,7 @@ public class NotifCollection implements Dumpable, PipelineDumpable { 
        mEventQueue.add(new BindEntryEvent(entry, sbn));



        mLogger.logNotifUpdated(entry);

        mEventQueue.add(new EntryUpdatedEvent(entry, false /* fromSystem */));

        mEventQueue.add(new EntryUpdatedEvent(entry, UpdateSource.SystemUi));



        // Skip the applyRanking step and go straight to dispatching the events

        dispatchEventsAndRebuildList("updateNotificationInternally");