From fac5da2f77035b32e398af4b12db220fc312b344 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Mon, 25 Oct 2021 01:47:29 +0000 Subject: [PATCH] New Pipeline: Remote Input 2/4: Add ability to internally update notifications Fixes: 204127880 Bug: 203938360 Test: atest NotifCollectionTest Merged-In: Ie15f7565b76e7314221d18e540d2ed8a5ce3e4a4 Change-Id: Ie15f7565b76e7314221d18e540d2ed8a5ce3e4a4 --- .../NotificationEntryManager.java | 3 +- .../collection/NotifCollection.java | 53 +++++++++++- .../collection/NotifPipeline.java | 14 ++++ .../notifcollection/InternalNotifUpdater.java | 37 +++++++++ .../NotifCollectionListener.java | 11 +++ .../notifcollection/NotifCollectionLogger.kt | 20 +++++ .../collection/notifcollection/NotifEvent.kt | 5 +- .../ongoingcall/OngoingCallController.kt | 2 +- .../collection/NotifCollectionTest.java | 82 +++++++++++++++++++ 9 files changed, 222 insertions(+), 5 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/InternalNotifUpdater.java diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java index 60f44a0d4fca3..8bc41c20caaff 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java @@ -689,8 +689,9 @@ public class NotificationEntryManager implements for (NotificationEntryListener listener : mNotificationEntryListeners) { listener.onPreEntryUpdated(entry); } + final boolean fromSystem = ranking != null; for (NotifCollectionListener listener : mNotifCollectionListeners) { - listener.onEntryUpdated(entry); + listener.onEntryUpdated(entry, fromSystem); } if (!mFeatureFlags.isNewNotifPipelineRenderingEnabled()) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java index b36b7c903d886..f36f430fc29bb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java @@ -47,6 +47,7 @@ import android.annotation.MainThread; import android.annotation.Nullable; import android.annotation.UserIdInt; import android.app.Notification; +import android.os.Handler; import android.os.RemoteException; import android.os.Trace; import android.os.UserHandle; @@ -62,6 +63,7 @@ import androidx.annotation.NonNull; import com.android.internal.statusbar.IStatusBarService; import com.android.systemui.Dumpable; import com.android.systemui.dagger.SysUISingleton; +import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.dump.DumpManager; import com.android.systemui.dump.LogBufferEulogizer; import com.android.systemui.flags.FeatureFlags; @@ -76,6 +78,7 @@ import com.android.systemui.statusbar.notification.collection.notifcollection.En import com.android.systemui.statusbar.notification.collection.notifcollection.EntryRemovedEvent; import com.android.systemui.statusbar.notification.collection.notifcollection.EntryUpdatedEvent; import com.android.systemui.statusbar.notification.collection.notifcollection.InitEntryEvent; +import com.android.systemui.statusbar.notification.collection.notifcollection.InternalNotifUpdater; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionLogger; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifDismissInterceptor; @@ -131,6 +134,7 @@ public class NotifCollection implements Dumpable { private final SystemClock mClock; private final FeatureFlags mFeatureFlags; private final NotifCollectionLogger mLogger; + private final Handler mMainHandler; private final LogBufferEulogizer mEulogizer; private final Map mNotificationSet = new ArrayMap<>(); @@ -154,6 +158,7 @@ public class NotifCollection implements Dumpable { SystemClock clock, FeatureFlags featureFlags, NotifCollectionLogger logger, + @Main Handler mainHandler, LogBufferEulogizer logBufferEulogizer, DumpManager dumpManager) { Assert.isMainThread(); @@ -161,6 +166,7 @@ public class NotifCollection implements Dumpable { mClock = clock; mFeatureFlags = featureFlags; mLogger = logger; + mMainHandler = mainHandler; mEulogizer = logBufferEulogizer; dumpManager.registerDumpable(TAG, this); @@ -442,7 +448,7 @@ public class NotifCollection implements Dumpable { mEventQueue.add(new BindEntryEvent(entry, sbn)); mLogger.logNotifUpdated(sbn.getKey()); - mEventQueue.add(new EntryUpdatedEvent(entry)); + mEventQueue.add(new EntryUpdatedEvent(entry, true /* fromSystem */)); } } @@ -791,6 +797,51 @@ public class NotifCollection implements Dumpable { private static final String TAG = "NotifCollection"; + /** + * Get an object which can be used to update a notification (internally to the pipeline) + * in response to a user action. + * + * @param name the name of the component that will update notifiations + * @return an updater + */ + public InternalNotifUpdater getInternalNotifUpdater(String name) { + return (sbn, reason) -> mMainHandler.post( + () -> updateNotificationInternally(sbn, name, reason)); + } + + /** + * Provide an updated StatusBarNotification for an existing entry. If no entry exists for the + * given notification key, this method does nothing. + * + * @param sbn the updated notification + * @param name the component which is updating the notification + * @param reason the reason the notification is being updated + */ + private void updateNotificationInternally(StatusBarNotification sbn, String name, + String reason) { + Assert.isMainThread(); + checkForReentrantCall(); + + // Make sure we have the notification to update + NotificationEntry entry = mNotificationSet.get(sbn.getKey()); + if (entry == null) { + mLogger.logNotifInternalUpdateFailed(sbn.getKey(), name, reason); + return; + } + mLogger.logNotifInternalUpdate(sbn.getKey(), name, reason); + + // First do the pieces of postNotification which are not about assuming the notification + // was sent by the app + entry.setSbn(sbn); + mEventQueue.add(new BindEntryEvent(entry, sbn)); + + mLogger.logNotifUpdated(sbn.getKey()); + mEventQueue.add(new EntryUpdatedEvent(entry, false /* fromSystem */)); + + // Skip the applyRanking step and go straight to dispatching the events + dispatchEventsAndRebuildList(); + } + @IntDef(prefix = { "REASON_" }, value = { REASON_NOT_CANCELED, REASON_UNKNOWN, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.java index 577792547c23a..27ba4c23db88a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.java @@ -16,6 +16,8 @@ package com.android.systemui.statusbar.notification.collection; +import android.os.Handler; + import androidx.annotation.Nullable; import com.android.systemui.dagger.SysUISingleton; @@ -30,6 +32,7 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.plugga import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifStabilityManager; import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection; +import com.android.systemui.statusbar.notification.collection.notifcollection.InternalNotifUpdater; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifDismissInterceptor; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender; @@ -222,6 +225,17 @@ public class NotifPipeline implements CommonNotifCollection { mShadeListBuilder.addPreRenderInvalidator(invalidator); } + /** + * Get an object which can be used to update a notification (internally to the pipeline) + * in response to a user action. + * + * @param name the name of the component that will update notifiations + * @return an updater + */ + public InternalNotifUpdater getInternalNotifUpdater(String name) { + return mNotifCollection.getInternalNotifUpdater(name); + } + /** * Returns a read-only view in to the current shade list, i.e. the list of notifications that * are currently present in the shade. If this method is called during pipeline execution it diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/InternalNotifUpdater.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/InternalNotifUpdater.java new file mode 100644 index 0000000000000..5692fb2b523e6 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/InternalNotifUpdater.java @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.notification.collection.notifcollection; + +import android.service.notification.StatusBarNotification; + +/** + * An object that allows Coordinators to update notifications internally to SystemUI. + * This is used when part of the UI involves updating the underlying appearance of a notification + * on behalf of an app, such as to add a spinner or remote input history. + */ +public interface InternalNotifUpdater { + /** + * Called when an already-existing notification needs to be updated to a new temporary + * appearance. + * This update is local to the SystemUI process. + * This has no effect if no notification with the given key exists in the pipeline. + * + * @param sbn a notification to update + * @param reason a debug reason for the update + */ + void onInternalNotificationUpdate(StatusBarNotification sbn, String reason); +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java index db0c1745f5657..68a346f817e1b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java @@ -53,6 +53,17 @@ public interface NotifCollectionListener { default void onEntryAdded(@NonNull NotificationEntry entry) { } + /** + * Called whenever a notification with the same key as an existing notification is posted. By + * the time this listener is called, the entry's SBN and Ranking will already have been updated. + * This delegates to {@link #onEntryUpdated(NotificationEntry)} by default. + * @param fromSystem If true, this update came from the NotificationManagerService. + * If false, the notification update is an internal change within systemui. + */ + default void onEntryUpdated(@NonNull NotificationEntry entry, boolean fromSystem) { + onEntryUpdated(entry); + } + /** * Called whenever a notification with the same key as an existing notification is posted. By * the time this listener is called, the entry's SBN and Ranking will already have been updated. diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt index f8a778d6b1d2e..1ebc66e4c6653 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt @@ -121,6 +121,26 @@ class NotifCollectionLogger @Inject constructor( }) } + fun logNotifInternalUpdate(key: String, name: String, reason: String) { + buffer.log(TAG, INFO, { + str1 = key + str2 = name + str3 = reason + }, { + "UPDATED INTERNALLY $str1 BY $str2 BECAUSE $str3" + }) + } + + fun logNotifInternalUpdateFailed(key: String, name: String, reason: String) { + buffer.log(TAG, INFO, { + str1 = key + str2 = name + str3 = reason + }, { + "FAILED INTERNAL UPDATE $str1 BY $str2 BECAUSE $str3" + }) + } + fun logNoNotificationToRemoveWithKey(key: String) { buffer.log(TAG, ERROR, { str1 = key diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifEvent.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifEvent.kt index 2810b891373ff..179e953284424 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifEvent.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifEvent.kt @@ -64,10 +64,11 @@ data class EntryAddedEvent( } data class EntryUpdatedEvent( - val entry: NotificationEntry + val entry: NotificationEntry, + val fromSystem: Boolean ) : NotifEvent() { override fun dispatchToListener(listener: NotifCollectionListener) { - listener.onEntryUpdated(entry) + listener.onEntryUpdated(entry, fromSystem) } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt index 3806d9a2925c0..31cc823c54adb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt @@ -86,7 +86,7 @@ class OngoingCallController @Inject constructor( // // TODO(b/183229367): Remove this function override when b/178406514 is fixed. override fun onEntryAdded(entry: NotificationEntry) { - onEntryUpdated(entry) + onEntryUpdated(entry, true) } override fun onEntryUpdated(entry: NotificationEntry) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java index ebeb59177397c..f08a74ab13160 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java @@ -35,6 +35,7 @@ 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; @@ -50,6 +51,7 @@ import static java.util.Objects.requireNonNull; import android.annotation.Nullable; import android.app.Notification; +import android.os.Handler; import android.os.RemoteException; import android.service.notification.NotificationListenerService.Ranking; import android.service.notification.NotificationListenerService.RankingMap; @@ -77,6 +79,7 @@ import com.android.systemui.statusbar.notification.collection.coalescer.GroupCoa import com.android.systemui.statusbar.notification.collection.coalescer.GroupCoalescer.BatchableNotificationHandler; import com.android.systemui.statusbar.notification.collection.notifcollection.CollectionReadyForBuildListener; import com.android.systemui.statusbar.notification.collection.notifcollection.DismissedByUserStats; +import com.android.systemui.statusbar.notification.collection.notifcollection.InternalNotifUpdater; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionLogger; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifDismissInterceptor; @@ -107,6 +110,7 @@ public class NotifCollectionTest extends SysuiTestCase { @Mock private FeatureFlags mFeatureFlags; @Mock private NotifCollectionLogger mLogger; @Mock private LogBufferEulogizer mEulogizer; + @Mock private Handler mMainHandler; @Mock private GroupCoalescer mGroupCoalescer; @Spy private RecordingCollectionListener mCollectionListener; @@ -152,6 +156,7 @@ public class NotifCollectionTest extends SysuiTestCase { mClock, mFeatureFlags, mLogger, + mMainHandler, mEulogizer, mock(DumpManager.class)); mCollection.attach(mGroupCoalescer); @@ -1322,6 +1327,78 @@ public class NotifCollectionTest extends SysuiTestCase { verify(mCollectionListener, never()).onEntryRemoved(any(NotificationEntry.class), anyInt()); } + private Runnable getInternalNotifUpdateRunnable(StatusBarNotification sbn) { + InternalNotifUpdater updater = mCollection.getInternalNotifUpdater("Test"); + updater.onInternalNotificationUpdate(sbn, "reason"); + ArgumentCaptor runnableCaptor = ArgumentCaptor.forClass(Runnable.class); + verify(mMainHandler).post(runnableCaptor.capture()); + return runnableCaptor.getValue(); + } + + @Test + public void testGetInternalNotifUpdaterPostsToMainHandler() { + InternalNotifUpdater updater = mCollection.getInternalNotifUpdater("Test"); + updater.onInternalNotificationUpdate(mock(StatusBarNotification.class), "reason"); + verify(mMainHandler).post(any()); + } + + @Test + public void testSecondPostCallsUpdateWithTrue() { + // GIVEN a pipeline with one notification + NotifEvent notifEvent = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 47, "myTag")); + NotificationEntry entry = mCollectionListener.getEntry(notifEvent.key); + + // KNOWING that it already called listener methods once + verify(mCollectionListener).onEntryAdded(eq(entry)); + verify(mCollectionListener).onRankingApplied(); + + // WHEN we update the notification via the system + mNoMan.postNotif(buildNotif(TEST_PACKAGE, 47, "myTag")); + + // 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).onEntryAdded((entry)); + verify(mCollectionListener, times(2)).onRankingApplied(); + } + + @Test + public void testInternalNotifUpdaterCallsUpdate() { + // GIVEN a pipeline with one notification + NotifEvent notifEvent = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 47, "myTag")); + NotificationEntry entry = mCollectionListener.getEntry(notifEvent.key); + + // KNOWING that it will call listener methods once + verify(mCollectionListener).onEntryAdded(eq(entry)); + verify(mCollectionListener).onRankingApplied(); + + // WHEN we update that notification internally + StatusBarNotification sbn = notifEvent.sbn; + getInternalNotifUpdateRunnable(sbn).run(); + + // THEN only entry updated gets called a second time + verify(mCollectionListener).onEntryAdded(eq(entry)); + verify(mCollectionListener).onRankingApplied(); + verify(mCollectionListener).onEntryUpdated(eq(entry)); + verify(mCollectionListener).onEntryUpdated(eq(entry), eq(false)); + } + + @Test + public void testInternalNotifUpdaterIgnoresNew() { + // GIVEN a pipeline without any notifications + StatusBarNotification sbn = buildNotif(TEST_PACKAGE, 47, "myTag").build().getSbn(); + + // WHEN we internally update an unknown notification + getInternalNotifUpdateRunnable(sbn).run(); + + // THEN only entry updated gets called a second time + verify(mCollectionListener, never()).onEntryAdded(any()); + verify(mCollectionListener, never()).onRankingUpdate(any()); + verify(mCollectionListener, never()).onRankingApplied(); + verify(mCollectionListener, never()).onEntryUpdated(any()); + verify(mCollectionListener, never()).onEntryUpdated(any(), anyBoolean()); + } + private static NotificationEntryBuilder buildNotif(String pkg, int id, String tag) { return new NotificationEntryBuilder() .setPkg(pkg) @@ -1371,6 +1448,11 @@ public class NotifCollectionTest extends SysuiTestCase { mLastSeenEntries.put(entry.getKey(), entry); } + @Override + public void onEntryUpdated(NotificationEntry entry, boolean fromSystem) { + onEntryUpdated(entry); + } + @Override public void onEntryRemoved(NotificationEntry entry, int reason) { }