From 8386a0550e4709a8687d2d5fae1aa27a2f5a1438 Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Wed, 12 May 2021 16:53:12 -0400 Subject: [PATCH 1/2] Break circular dependencies in notif code Bug: 186481467 Test: atest Change-Id: Ie5b07bda46c2d217f5eb3be341595a6cb70fdd57 --- .../NotificationLockscreenUserManager.java | 11 ++++ ...NotificationLockscreenUserManagerImpl.java | 11 ++++ .../NotificationEntryManager.java | 30 ++++----- .../notification/NotificationFilter.java | 60 +++++------------ .../collection/NotificationRankingManager.kt | 18 +++-- .../legacy/LegacyNotificationRanker.kt | 34 ++++++++++ .../legacy/LegacyNotificationRankerStub.java | 66 +++++++++++++++++++ .../dagger/NotificationsModule.java | 5 -- .../init/NotificationsControllerImpl.kt | 5 +- ...NotificationLockscreenUserManagerTest.java | 55 ++++++++++++++-- .../NotificationEntryManagerTest.java | 21 +++--- .../notification/NotificationFilterTest.java | 40 ++++++++--- .../NotificationRankingManagerTest.kt | 17 +++-- ...NotificationEntryManagerInflationTest.java | 21 +++--- 14 files changed, 282 insertions(+), 112 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LegacyNotificationRanker.kt create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LegacyNotificationRankerStub.java diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManager.java index ebf7c2d58c2d9..9a1a144924e2f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManager.java @@ -49,6 +49,12 @@ public interface NotificationLockscreenUserManager { /** Adds a listener to be notified when the current user changes. */ void addUserChangedListener(UserChangedListener listener); + /** + * Registers a [KeyguardNotificationSuppressor] that will be consulted during + * {@link #shouldShowOnKeyguard(NotificationEntry)} + */ + void addKeyguardNotificationSuppressor(KeyguardNotificationSuppressor suppressor); + /** * Removes a listener previously registered with * {@link #addUserChangedListener(UserChangedListener)} @@ -88,4 +94,9 @@ public interface NotificationLockscreenUserManager { default void onUserChanged(int userId) {} default void onCurrentProfilesChanged(SparseArray currentProfiles) {} } + + /** Used to hide notifications on the lockscreen */ + interface KeyguardNotificationSuppressor { + boolean shouldSuppressOnKeyguard(NotificationEntry entry); + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java index 7eb921b9e6bbd..e7e9404c9d3e7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java @@ -98,6 +98,7 @@ public class NotificationLockscreenUserManagerImpl implements private LockPatternUtils mLockPatternUtils; protected KeyguardManager mKeyguardManager; private int mState = StatusBarState.SHADE; + private List mKeyguardSuppressors = new ArrayList<>(); protected final BroadcastReceiver mAllUsersReceiver = new BroadcastReceiver() { @Override @@ -343,6 +344,11 @@ public class NotificationLockscreenUserManagerImpl implements Log.wtf(TAG, "mEntryManager was null!", new Throwable()); return false; } + for (int i = 0; i < mKeyguardSuppressors.size(); i++) { + if (mKeyguardSuppressors.get(i).shouldSuppressOnKeyguard(entry)) { + return false; + } + } boolean exceedsPriorityThreshold; if (hideSilentNotificationsOnLockscreen()) { exceedsPriorityThreshold = @@ -620,6 +626,11 @@ public class NotificationLockscreenUserManagerImpl implements mListeners.add(listener); } + @Override + public void addKeyguardNotificationSuppressor(KeyguardNotificationSuppressor suppressor) { + mKeyguardSuppressors.add(suppressor); + } + @Override public void removeUserChangedListener(UserChangedListener listener) { mListeners.remove(listener); 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 13a8661f94dc7..1ab2a9e320b0a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java @@ -46,8 +46,9 @@ import com.android.systemui.statusbar.NotificationRemoteInputManager; import com.android.systemui.statusbar.NotificationRemoveInterceptor; import com.android.systemui.statusbar.NotificationUiAdjustment; import com.android.systemui.statusbar.notification.collection.NotificationEntry; -import com.android.systemui.statusbar.notification.collection.NotificationRankingManager; import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinder; +import com.android.systemui.statusbar.notification.collection.legacy.LegacyNotificationRanker; +import com.android.systemui.statusbar.notification.collection.legacy.LegacyNotificationRankerStub; import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy; import com.android.systemui.statusbar.notification.collection.legacy.VisualStabilityManager; import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection; @@ -138,12 +139,11 @@ public class NotificationEntryManager implements private final LeakDetector mLeakDetector; private final List mNotifCollectionListeners = new ArrayList<>(); - private final KeyguardEnvironment mKeyguardEnvironment; private final NotificationGroupManagerLegacy mGroupManager; - private final Lazy mRankingManager; private final FeatureFlags mFeatureFlags; private final ForegroundServiceDismissalFeatureController mFgsFeatureController; + private LegacyNotificationRanker mRanker = new LegacyNotificationRankerStub(); private NotificationPresenter mPresenter; private RankingMap mLatestRankingMap; @@ -200,8 +200,6 @@ public class NotificationEntryManager implements public NotificationEntryManager( NotificationEntryManagerLogger logger, NotificationGroupManagerLegacy groupManager, - Lazy rankingManager, - KeyguardEnvironment keyguardEnvironment, FeatureFlags featureFlags, Lazy notificationRowBinderLazy, Lazy notificationRemoteInputManagerLazy, @@ -211,8 +209,6 @@ public class NotificationEntryManager implements ) { mLogger = logger; mGroupManager = groupManager; - mRankingManager = rankingManager; - mKeyguardEnvironment = keyguardEnvironment; mFeatureFlags = featureFlags; mNotificationRowBinderLazy = notificationRowBinderLazy; mRemoteInputManagerLazy = notificationRemoteInputManagerLazy; @@ -226,6 +222,10 @@ public class NotificationEntryManager implements notificationListener.addNotificationHandler(mNotifListener); } + public void setRanker(LegacyNotificationRanker ranker) { + mRanker = ranker; + } + /** Adds a {@link NotificationEntryListener}. */ public void addNotificationEntryListener(NotificationEntryListener listener) { mNotificationEntryListeners.add(listener); @@ -419,7 +419,7 @@ public class NotificationEntryManager implements mActiveNotifications.put(entry.getKey(), entry); mGroupManager.onEntryAdded(entry); - updateRankingAndSort(mRankingManager.get().getRankingMap(), "addEntryInternalInternal"); + updateRankingAndSort(mRanker.getRankingMap(), "addEntryInternalInternal"); } /** @@ -698,13 +698,6 @@ public class NotificationEntryManager implements updateNotifications("updateNotificationInternal"); - if (DEBUG) { - // Is this for you? - boolean isForCurrentUser = mKeyguardEnvironment - .isNotificationForCurrentProfiles(notification); - Log.d(TAG, "notification is " + (isForCurrentUser ? "" : "not ") + "for you"); - } - for (NotificationEntryListener listener : mNotificationEntryListeners) { listener.onPostEntryUpdated(entry); } @@ -862,8 +855,7 @@ public class NotificationEntryManager implements final int len = mActiveNotifications.size(); for (int i = 0; i < len; i++) { NotificationEntry entry = mActiveNotifications.valueAt(i); - final StatusBarNotification sbn = entry.getSbn(); - if (!mKeyguardEnvironment.isNotificationForCurrentProfiles(sbn)) { + if (!mRanker.isNotificationForCurrentProfiles(entry)) { continue; } filtered.add(entry); @@ -886,13 +878,13 @@ public class NotificationEntryManager implements /** Resorts / filters the current notification set with the current RankingMap */ public void reapplyFilterAndSort(String reason) { - updateRankingAndSort(mRankingManager.get().getRankingMap(), reason); + updateRankingAndSort(mRanker.getRankingMap(), reason); } /** Calls to NotificationRankingManager and updates mSortedAndFiltered */ private void updateRankingAndSort(@NonNull RankingMap rankingMap, String reason) { mSortedAndFiltered.clear(); - mSortedAndFiltered.addAll(mRankingManager.get().updateRanking( + mSortedAndFiltered.addAll(mRanker.updateRanking( rankingMap, mActiveNotifications.values(), reason)); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationFilter.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationFilter.java index f21771a89c9e3..1940cb2e170fd 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationFilter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationFilter.java @@ -27,14 +27,13 @@ import android.os.RemoteException; import android.service.notification.StatusBarNotification; import com.android.internal.annotations.VisibleForTesting; -import com.android.systemui.Dependency; import com.android.systemui.ForegroundServiceController; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.media.MediaFeatureFlag; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.NotificationLockscreenUserManager; +import com.android.systemui.statusbar.notification.NotificationEntryManager.KeyguardEnvironment; import com.android.systemui.statusbar.notification.collection.NotificationEntry; -import com.android.systemui.statusbar.phone.ShadeController; import javax.inject.Inject; @@ -46,68 +45,43 @@ import javax.inject.Inject; public class NotificationFilter { private final StatusBarStateController mStatusBarStateController; + private final KeyguardEnvironment mKeyguardEnvironment; + private final ForegroundServiceController mForegroundServiceController; + private final NotificationLockscreenUserManager mUserManager; private final Boolean mIsMediaFlagEnabled; - private NotificationEntryManager.KeyguardEnvironment mEnvironment; - private ShadeController mShadeController; - private ForegroundServiceController mFsc; - private NotificationLockscreenUserManager mUserManager; - @Inject public NotificationFilter( StatusBarStateController statusBarStateController, + KeyguardEnvironment keyguardEnvironment, + ForegroundServiceController foregroundServiceController, + NotificationLockscreenUserManager userManager, MediaFeatureFlag mediaFeatureFlag) { mStatusBarStateController = statusBarStateController; + mKeyguardEnvironment = keyguardEnvironment; + mForegroundServiceController = foregroundServiceController; + mUserManager = userManager; mIsMediaFlagEnabled = mediaFeatureFlag.getEnabled(); } - private NotificationEntryManager.KeyguardEnvironment getEnvironment() { - if (mEnvironment == null) { - mEnvironment = Dependency.get(NotificationEntryManager.KeyguardEnvironment.class); - } - return mEnvironment; - } - - private ShadeController getShadeController() { - if (mShadeController == null) { - mShadeController = Dependency.get(ShadeController.class); - } - return mShadeController; - } - - private ForegroundServiceController getFsc() { - if (mFsc == null) { - mFsc = Dependency.get(ForegroundServiceController.class); - } - return mFsc; - } - - private NotificationLockscreenUserManager getUserManager() { - if (mUserManager == null) { - mUserManager = Dependency.get(NotificationLockscreenUserManager.class); - } - return mUserManager; - } - - /** * @return true if the provided notification should NOT be shown right now. */ public boolean shouldFilterOut(NotificationEntry entry) { final StatusBarNotification sbn = entry.getSbn(); - if (!(getEnvironment().isDeviceProvisioned() + if (!(mKeyguardEnvironment.isDeviceProvisioned() || showNotificationEvenIfUnprovisioned(sbn))) { return true; } - if (!getEnvironment().isNotificationForCurrentProfiles(sbn)) { + if (!mKeyguardEnvironment.isNotificationForCurrentProfiles(sbn)) { return true; } - if (getUserManager().isLockscreenPublicMode(sbn.getUserId()) + if (mUserManager.isLockscreenPublicMode(sbn.getUserId()) && (sbn.getNotification().visibility == Notification.VISIBILITY_SECRET - || getUserManager().shouldHideNotifications(sbn.getUserId()) - || getUserManager().shouldHideNotifications(sbn.getKey()))) { + || mUserManager.shouldHideNotifications(sbn.getUserId()) + || mUserManager.shouldHideNotifications(sbn.getKey()))) { return true; } @@ -123,8 +97,8 @@ public class NotificationFilter { return true; } - if (getFsc().isDisclosureNotification(sbn) - && !getFsc().isDisclosureNeededForUser(sbn.getUserId())) { + if (mForegroundServiceController.isDisclosureNotification(sbn) + && !mForegroundServiceController.isDisclosureNeededForUser(sbn.getUserId())) { // this is a foreground-service disclosure for a user that does not need to show one return true; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt index fad0e49b36376..2b620a499d740 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt @@ -23,9 +23,11 @@ import android.service.notification.NotificationListenerService.Ranking import android.service.notification.NotificationListenerService.RankingMap import android.service.notification.StatusBarNotification import com.android.systemui.statusbar.NotificationMediaManager +import com.android.systemui.statusbar.notification.NotificationEntryManager.KeyguardEnvironment import com.android.systemui.statusbar.notification.NotificationEntryManagerLogger import com.android.systemui.statusbar.notification.NotificationFilter import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager +import com.android.systemui.statusbar.notification.collection.legacy.LegacyNotificationRanker import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier @@ -39,7 +41,6 @@ import com.android.systemui.statusbar.policy.HeadsUpManager import dagger.Lazy import java.util.Objects import javax.inject.Inject -import kotlin.Comparator private const val TAG = "NotifRankingManager" @@ -60,10 +61,11 @@ open class NotificationRankingManager @Inject constructor( private val logger: NotificationEntryManagerLogger, private val sectionsFeatureManager: NotificationSectionsFeatureManager, private val peopleNotificationIdentifier: PeopleNotificationIdentifier, - private val highPriorityProvider: HighPriorityProvider -) { + private val highPriorityProvider: HighPriorityProvider, + private val keyguardEnvironment: KeyguardEnvironment +) : LegacyNotificationRanker { - var rankingMap: RankingMap? = null + override var rankingMap: RankingMap? = null protected set private val mediaManager by lazy { mediaManagerLazy.get() @@ -115,7 +117,7 @@ open class NotificationRankingManager @Inject constructor( } } - fun updateRanking( + override fun updateRanking( newRankingMap: RankingMap?, entries: Collection, reason: String @@ -131,6 +133,12 @@ open class NotificationRankingManager @Inject constructor( } } + override fun isNotificationForCurrentProfiles( + entry: NotificationEntry + ): Boolean { + return keyguardEnvironment.isNotificationForCurrentProfiles(entry.sbn) + } + /** Uses the [rankingComparator] to sort notifications which aren't filtered */ private fun filterAndSortLocked( entries: Collection, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LegacyNotificationRanker.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LegacyNotificationRanker.kt new file mode 100644 index 0000000000000..49bc48efda00d --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LegacyNotificationRanker.kt @@ -0,0 +1,34 @@ +/* + * 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.legacy + +import android.service.notification.NotificationListenerService +import com.android.systemui.statusbar.notification.collection.NotificationEntry + +interface LegacyNotificationRanker { + val rankingMap: NotificationListenerService.RankingMap? + + fun updateRanking( + newRankingMap: NotificationListenerService.RankingMap?, + entries: Collection, + reason: String + ): List + + fun isNotificationForCurrentProfiles( + entry: NotificationEntry + ): Boolean +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LegacyNotificationRankerStub.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LegacyNotificationRankerStub.java new file mode 100644 index 0000000000000..12353f80c0d1d --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LegacyNotificationRankerStub.java @@ -0,0 +1,66 @@ +/* + * 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.legacy; + +import android.service.notification.NotificationListenerService.Ranking; +import android.service.notification.NotificationListenerService.RankingMap; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + +import com.android.systemui.statusbar.notification.collection.NotificationEntry; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; + +/** + * Stub implementation that we use until we get passed the "real" one in the form of + * {@link com.android.systemui.statusbar.notification.collection.NotificationRankingManager} + */ +public class LegacyNotificationRankerStub implements LegacyNotificationRanker { + private RankingMap mRankingMap = new RankingMap(new Ranking[] {}); + + @NonNull + @Override + public List updateRanking( + @Nullable RankingMap newRankingMap, + @NonNull Collection entries, + @NonNull String reason) { + if (newRankingMap != null) { + mRankingMap = newRankingMap; + } + List ranked = new ArrayList<>(entries); + ranked.sort(mEntryComparator); + return ranked; + } + + @Nullable + @Override + public RankingMap getRankingMap() { + return mRankingMap; + } + + private final Comparator mEntryComparator = Comparator.comparingLong( + o -> o.getSbn().getNotification().when); + + @Override + public boolean isNotificationForCurrentProfiles(@NonNull NotificationEntry entry) { + return true; + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java index 89bb65278dced..a32b7e3b28360 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java @@ -44,7 +44,6 @@ import com.android.systemui.statusbar.notification.NotificationEntryManagerLogge import com.android.systemui.statusbar.notification.collection.NotifCollection; import com.android.systemui.statusbar.notification.collection.NotifInflaterImpl; import com.android.systemui.statusbar.notification.collection.NotifPipeline; -import com.android.systemui.statusbar.notification.collection.NotificationRankingManager; import com.android.systemui.statusbar.notification.collection.coordinator.VisualStabilityCoordinator; import com.android.systemui.statusbar.notification.collection.inflation.NotifInflater; import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinder; @@ -100,8 +99,6 @@ public interface NotificationsModule { static NotificationEntryManager provideNotificationEntryManager( NotificationEntryManagerLogger logger, NotificationGroupManagerLegacy groupManager, - Lazy rankingManager, - NotificationEntryManager.KeyguardEnvironment keyguardEnvironment, FeatureFlags featureFlags, Lazy notificationRowBinderLazy, Lazy notificationRemoteInputManagerLazy, @@ -111,8 +108,6 @@ public interface NotificationsModule { return new NotificationEntryManager( logger, groupManager, - rankingManager, - keyguardEnvironment, featureFlags, notificationRowBinderLazy, notificationRemoteInputManagerLazy, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt index fd5128a8213ae..88ca86b543f32 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt @@ -16,7 +16,6 @@ package com.android.systemui.statusbar.notification.init -import android.content.Context import android.service.notification.StatusBarNotification import com.android.systemui.dagger.SysUISingleton import com.android.systemui.people.widget.PeopleSpaceWidgetManager @@ -30,6 +29,7 @@ import com.android.systemui.statusbar.notification.NotificationClicker import com.android.systemui.statusbar.notification.NotificationEntryManager import com.android.systemui.statusbar.notification.NotificationListController import com.android.systemui.statusbar.notification.collection.NotifPipeline +import com.android.systemui.statusbar.notification.collection.NotificationRankingManager import com.android.systemui.statusbar.notification.collection.TargetSdkResolver import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinderImpl import com.android.systemui.statusbar.notification.collection.init.NotifPipelineInitializer @@ -59,10 +59,10 @@ import javax.inject.Inject */ @SysUISingleton class NotificationsControllerImpl @Inject constructor( - private val context: Context, private val featureFlags: FeatureFlags, private val notificationListener: NotificationListener, private val entryManager: NotificationEntryManager, + private val legacyRanker: NotificationRankingManager, private val notifPipeline: Lazy, private val targetSdkResolver: TargetSdkResolver, private val newNotifPipeline: Lazy, @@ -128,6 +128,7 @@ class NotificationsControllerImpl @Inject constructor( groupManagerLegacy.get().setHeadsUpManager(headsUpManager) groupAlertTransferHelper.setHeadsUpManager(headsUpManager) + entryManager.setRanker(legacyRanker) entryManager.attach(notificationListener) } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java index 2a5027309aecf..0c65830ea4557 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java @@ -16,9 +16,11 @@ package com.android.systemui.statusbar; +import static android.app.NotificationManager.IMPORTANCE_HIGH; import static android.app.NotificationManager.IMPORTANCE_LOW; import static android.content.Intent.ACTION_USER_SWITCHED; +import static com.android.systemui.statusbar.notification.stack.NotificationSectionsManagerKt.BUCKET_ALERTING; import static com.android.systemui.statusbar.notification.stack.NotificationSectionsManagerKt.BUCKET_MEDIA_CONTROLS; import static com.android.systemui.statusbar.notification.stack.NotificationSectionsManagerKt.BUCKET_PEOPLE; import static com.android.systemui.statusbar.notification.stack.NotificationSectionsManagerKt.BUCKET_SILENT; @@ -55,6 +57,7 @@ import com.android.systemui.Dependency; import com.android.systemui.SysuiTestCase; import com.android.systemui.broadcast.BroadcastDispatcher; import com.android.systemui.plugins.statusbar.StatusBarStateController; +import com.android.systemui.statusbar.NotificationLockscreenUserManager.KeyguardNotificationSuppressor; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; @@ -383,13 +386,44 @@ public class NotificationLockscreenUserManagerTest extends SysuiTestCase { assertTrue(mLockscreenUserManager.shouldShowOnKeyguard(entry)); } + @Test + public void testKeyguardNotificationSuppressors() { + // GIVEN a notification that should be shown on the lockscreen + Settings.Secure.putInt(mContext.getContentResolver(), + Settings.Secure.LOCK_SCREEN_SHOW_NOTIFICATIONS, 1); + final NotificationEntry entry = new NotificationEntryBuilder() + .setImportance(IMPORTANCE_HIGH) + .build(); + entry.setBucket(BUCKET_ALERTING); + + // WHEN a suppressor is added that filters out all entries + FakeKeyguardSuppressor suppressor = new FakeKeyguardSuppressor(); + mLockscreenUserManager.addKeyguardNotificationSuppressor(suppressor); + + // THEN it's filtered out + assertFalse(mLockscreenUserManager.shouldShowOnKeyguard(entry)); + + // WHEN the suppressor no longer filters out entries + suppressor.setShouldSuppress(false); + + // THEN it's no longer filtered out + assertTrue(mLockscreenUserManager.shouldShowOnKeyguard(entry)); + } + private class TestNotificationLockscreenUserManager extends NotificationLockscreenUserManagerImpl { public TestNotificationLockscreenUserManager(Context context) { - super(context, mBroadcastDispatcher, mDevicePolicyManager, mUserManager, - mClickNotifier, NotificationLockscreenUserManagerTest.this.mKeyguardManager, - mStatusBarStateController, Handler.createAsync(Looper.myLooper()), - mDeviceProvisionedController, mKeyguardStateController); + super( + context, + mBroadcastDispatcher, + mDevicePolicyManager, + mUserManager, + mClickNotifier, + NotificationLockscreenUserManagerTest.this.mKeyguardManager, + mStatusBarStateController, + Handler.createAsync(Looper.myLooper()), + mDeviceProvisionedController, + mKeyguardStateController); } public BroadcastReceiver getBaseBroadcastReceiverForTest() { @@ -404,4 +438,17 @@ public class NotificationLockscreenUserManagerTest extends SysuiTestCase { return mSettingsObserver; } } + + private static class FakeKeyguardSuppressor implements KeyguardNotificationSuppressor { + private boolean mShouldSuppress = true; + + @Override + public boolean shouldSuppressOnKeyguard(NotificationEntry entry) { + return mShouldSuppress; + } + + public void setShouldSuppress(boolean shouldSuppress) { + mShouldSuppress = shouldSuppress; + } + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java index 6459c0c9f4413..1be14b66e968a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java @@ -190,16 +190,6 @@ public class NotificationEntryManagerTest extends SysuiTestCase { mEntryManager = new NotificationEntryManager( mLogger, mGroupManager, - () -> new NotificationRankingManager( - () -> mNotificationMediaManager, - mGroupManager, - mHeadsUpManager, - mock(NotificationFilter.class), - mLogger, - mock(NotificationSectionsFeatureManager.class), - mock(PeopleNotificationIdentifier.class), - mock(HighPriorityProvider.class)), - mEnvironment, mFeatureFlags, () -> mNotificationRowBinder, () -> mRemoteInputManager, @@ -207,6 +197,17 @@ public class NotificationEntryManagerTest extends SysuiTestCase { mock(ForegroundServiceDismissalFeatureController.class), mock(IStatusBarService.class) ); + mEntryManager.setRanker( + new NotificationRankingManager( + () -> mNotificationMediaManager, + mGroupManager, + mHeadsUpManager, + mock(NotificationFilter.class), + mLogger, + mock(NotificationSectionsFeatureManager.class), + mock(PeopleNotificationIdentifier.class), + mock(HighPriorityProvider.class), + mEnvironment)); mEntryManager.setUpWithPresenter(mPresenter); mEntryManager.addNotificationEntryListener(mEntryListener); mEntryManager.addNotificationRemoveInterceptor(mRemoveInterceptor); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationFilterTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationFilterTest.java index b493b9a6bd65a..b1eef4b67a6fd 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationFilterTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationFilterTest.java @@ -77,13 +77,16 @@ public class NotificationFilterTest extends SysuiTestCase { mock(StatusBarNotification.class); @Mock - ForegroundServiceController mFsc; + StatusBarStateController mStatusBarStateController; @Mock KeyguardEnvironment mEnvironment; @Mock - MediaFeatureFlag mMediaFeatureFlag; + ForegroundServiceController mFsc; @Mock - StatusBarStateController mStatusBarStateController; + NotificationLockscreenUserManager mUserManager; + @Mock + MediaFeatureFlag mMediaFeatureFlag; + private final IPackageManager mMockPackageManager = mock(IPackageManager.class); private NotificationFilter mNotificationFilter; @@ -127,7 +130,12 @@ public class NotificationFilterTest extends SysuiTestCase { mDependency, TestableLooper.get(this)); mRow = testHelper.createRow(); - mNotificationFilter = new NotificationFilter(mStatusBarStateController, mMediaFeatureFlag); + mNotificationFilter = new NotificationFilter( + mStatusBarStateController, + mEnvironment, + mFsc, + mUserManager, + mMediaFeatureFlag); } @After @@ -195,7 +203,11 @@ public class NotificationFilterTest extends SysuiTestCase { public void shouldFilterOtherNotificationWhenDisabled() { // GIVEN that the media feature is disabled when(mMediaFeatureFlag.getEnabled()).thenReturn(false); - NotificationFilter filter = new NotificationFilter(mStatusBarStateController, + NotificationFilter filter = new NotificationFilter( + mStatusBarStateController, + mEnvironment, + mFsc, + mUserManager, mMediaFeatureFlag); // WHEN the media filter is asked about an entry NotificationEntry otherEntry = new NotificationEntryBuilder().build(); @@ -208,7 +220,11 @@ public class NotificationFilterTest extends SysuiTestCase { public void shouldFilterOtherNotificationWhenEnabled() { // GIVEN that the media feature is enabled when(mMediaFeatureFlag.getEnabled()).thenReturn(true); - NotificationFilter filter = new NotificationFilter(mStatusBarStateController, + NotificationFilter filter = new NotificationFilter( + mStatusBarStateController, + mEnvironment, + mFsc, + mUserManager, mMediaFeatureFlag); // WHEN the media filter is asked about an entry NotificationEntry otherEntry = new NotificationEntryBuilder().build(); @@ -221,7 +237,11 @@ public class NotificationFilterTest extends SysuiTestCase { public void shouldFilterMediaNotificationWhenDisabled() { // GIVEN that the media feature is disabled when(mMediaFeatureFlag.getEnabled()).thenReturn(false); - NotificationFilter filter = new NotificationFilter(mStatusBarStateController, + NotificationFilter filter = new NotificationFilter( + mStatusBarStateController, + mEnvironment, + mFsc, + mUserManager, mMediaFeatureFlag); // WHEN the media filter is asked about a media entry final boolean shouldFilter = filter.shouldFilterOut(mMediaEntry); @@ -233,7 +253,11 @@ public class NotificationFilterTest extends SysuiTestCase { public void shouldFilterMediaNotificationWhenEnabled() { // GIVEN that the media feature is enabled when(mMediaFeatureFlag.getEnabled()).thenReturn(true); - NotificationFilter filter = new NotificationFilter(mStatusBarStateController, + NotificationFilter filter = new NotificationFilter( + mStatusBarStateController, + mEnvironment, + mFsc, + mUserManager, mMediaFeatureFlag); // WHEN the media filter is asked about a media entry final boolean shouldFilter = filter.shouldFilterOut(mMediaEntry); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt index bdd4a01a97663..c51c628c54571 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt @@ -30,9 +30,11 @@ import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.statusbar.NotificationEntryHelper.modifyRanking import com.android.systemui.statusbar.NotificationMediaManager +import com.android.systemui.statusbar.notification.NotificationEntryManager.KeyguardEnvironment import com.android.systemui.statusbar.notification.NotificationEntryManagerLogger import com.android.systemui.statusbar.notification.NotificationFilter import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager +import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.TYPE_FULL_PERSON @@ -42,7 +44,6 @@ import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow import com.android.systemui.statusbar.notification.stack.BUCKET_ALERTING import com.android.systemui.statusbar.notification.stack.BUCKET_FOREGROUND_SERVICE import com.android.systemui.statusbar.notification.stack.BUCKET_SILENT -import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy import com.android.systemui.statusbar.policy.HeadsUpManager import com.google.common.truth.Truth.assertThat import dagger.Lazy @@ -79,9 +80,11 @@ class NotificationRankingManagerTest : SysuiTestCase() { mock(NotificationEntryManagerLogger::class.java), sectionsManager, personNotificationIdentifier, - HighPriorityProvider(personNotificationIdentifier, - mock(NotificationGroupManagerLegacy::class.java)) - ) + HighPriorityProvider( + personNotificationIdentifier, + mock(NotificationGroupManagerLegacy::class.java)), + mock(KeyguardEnvironment::class.java) + ) } @Test @@ -486,7 +489,8 @@ class NotificationRankingManagerTest : SysuiTestCase() { logger: NotificationEntryManagerLogger, sectionsFeatureManager: NotificationSectionsFeatureManager, peopleNotificationIdentifier: PeopleNotificationIdentifier, - highPriorityProvider: HighPriorityProvider + highPriorityProvider: HighPriorityProvider, + keyguardEnvironment: KeyguardEnvironment ) : NotificationRankingManager( mediaManager, groupManager, @@ -495,7 +499,8 @@ class NotificationRankingManagerTest : SysuiTestCase() { logger, sectionsFeatureManager, peopleNotificationIdentifier, - highPriorityProvider + highPriorityProvider, + keyguardEnvironment ) { fun applyTestRankingMap(r: RankingMap) { rankingMap = r diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java index 7b0c06736979e..cea49b71f009f 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java @@ -181,16 +181,6 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase { mEntryManager = new NotificationEntryManager( mock(NotificationEntryManagerLogger.class), mGroupMembershipManager, - () -> new NotificationRankingManager( - () -> mock(NotificationMediaManager.class), - mGroupMembershipManager, - mHeadsUpManager, - mock(NotificationFilter.class), - mock(NotificationEntryManagerLogger.class), - mock(NotificationSectionsFeatureManager.class), - mock(PeopleNotificationIdentifier.class), - mock(HighPriorityProvider.class)), - mEnvironment, mFeatureFlags, () -> mRowBinder, () -> mRemoteInputManager, @@ -198,6 +188,17 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase { mock(ForegroundServiceDismissalFeatureController.class), mock(IStatusBarService.class) ); + mEntryManager.setRanker( + new NotificationRankingManager( + () -> mock(NotificationMediaManager.class), + mGroupMembershipManager, + mHeadsUpManager, + mock(NotificationFilter.class), + mock(NotificationEntryManagerLogger.class), + mock(NotificationSectionsFeatureManager.class), + mock(PeopleNotificationIdentifier.class), + mock(HighPriorityProvider.class), + mEnvironment)); NotifRemoteViewCache cache = new NotifRemoteViewCacheImpl(mEntryManager); NotifBindPipeline pipeline = new NotifBindPipeline( From 72be22bae013381b172dd9082e483f9854f46638 Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Wed, 12 May 2021 16:54:02 -0400 Subject: [PATCH 2/2] Dedupe smartspace and notification content on lockscreen If a notification has been marked as the source of smartspace content, then hide it on the lockscreen. Supports filtering in both the old and new notification pipelines. Bug: 186481467 Test: atest Change-Id: Id3be493038508f0b24f3924936fd57598d7be9e4 --- packages/SystemUI/res/values/flags.xml | 2 + .../systemui/statusbar/FeatureFlags.java | 4 + .../collection/NotifCollection.java | 5 + .../collection/NotifPipeline.java | 11 + .../collection/coordinator/Coordinator.java | 4 +- .../coordinator/NotifCoordinators.java | 8 +- .../SmartspaceDedupingCoordinator.kt | 211 +++++++++ .../listbuilder/pluggable/NotifFilter.java | 4 +- .../SmartspaceDedupingCoordinatorTest.kt | 418 ++++++++++++++++++ 9 files changed, 664 insertions(+), 3 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinatorTest.kt diff --git a/packages/SystemUI/res/values/flags.xml b/packages/SystemUI/res/values/flags.xml index 6104588690e3b..6393147f8b0be 100644 --- a/packages/SystemUI/res/values/flags.xml +++ b/packages/SystemUI/res/values/flags.xml @@ -49,4 +49,6 @@ true false + + true diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/FeatureFlags.java b/packages/SystemUI/src/com/android/systemui/statusbar/FeatureFlags.java index d96e1ba22ecc6..ac6d9e8820ad2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/FeatureFlags.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/FeatureFlags.java @@ -92,4 +92,8 @@ public class FeatureFlags { public boolean isSmartspaceEnabled() { return mFlagReader.isEnabled(R.bool.flag_smartspace); } + + public boolean isSmartspaceDedupingEnabled() { + return isSmartspaceEnabled() && mFlagReader.isEnabled(R.bool.flag_smartspace_deduping); + } } 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 6b96ee4fc8e5e..8ae31cba4cfb2 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 @@ -185,6 +185,11 @@ public class NotifCollection implements Dumpable { mBuildListener = buildListener; } + /** @see NotifPipeline#getEntry(String) () */ + NotificationEntry getEntry(String key) { + return mNotificationSet.get(key); + } + /** @see NotifPipeline#getAllNotifs() */ Collection getAllNotifs() { Assert.isMainThread(); 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 a1844ff5d2211..47939f0579f5d 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 androidx.annotation.Nullable; + import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeFinalizeFilterListener; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeRenderListListener; @@ -89,6 +91,7 @@ public class NotifPipeline implements CommonNotifCollection { * * The returned collection is read-only, unsorted, unfiltered, and ungrouped. */ + @Override public Collection getAllNotifs() { return mNotifCollection.getAllNotifs(); } @@ -98,6 +101,14 @@ public class NotifPipeline implements CommonNotifCollection { mNotifCollection.addCollectionListener(listener); } + /** + * Returns the NotificationEntry associated with [key]. + */ + @Nullable + public NotificationEntry getEntry(String key) { + return mNotifCollection.getEntry(key); + } + /** * Registers a lifetime extender. Lifetime extenders can cause notifications that have been * dismissed or retracted by system server to be temporarily retained in the collection. diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/Coordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/Coordinator.java index c1a11b2f64c83..23570728d27cb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/Coordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/Coordinator.java @@ -16,6 +16,8 @@ package com.android.systemui.statusbar.notification.collection.coordinator; +import androidx.annotation.NonNull; + import com.android.systemui.statusbar.notification.collection.NotifPipeline; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Pluggable; @@ -27,5 +29,5 @@ public interface Coordinator { * Called after the NewNotifPipeline is initialized. * Coordinators should register their listeners and {@link Pluggable}s to the pipeline. */ - void attach(NotifPipeline pipeline); + void attach(@NonNull NotifPipeline pipeline); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java index ded5e46593f84..d80cc082aada9 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java @@ -60,6 +60,7 @@ public class NotifCoordinators implements Dumpable { ConversationCoordinator conversationCoordinator, PreparationCoordinator preparationCoordinator, MediaCoordinator mediaCoordinator, + SmartspaceDedupingCoordinator smartspaceDedupingCoordinator, VisualStabilityCoordinator visualStabilityCoordinator) { dumpManager.registerDumpable(TAG, this); @@ -70,9 +71,14 @@ public class NotifCoordinators implements Dumpable { mCoordinators.add(appOpsCoordinator); mCoordinators.add(deviceProvisionedCoordinator); mCoordinators.add(bubbleCoordinator); - mCoordinators.add(mediaCoordinator); mCoordinators.add(conversationCoordinator); + mCoordinators.add(mediaCoordinator); mCoordinators.add(visualStabilityCoordinator); + + if (featureFlags.isSmartspaceDedupingEnabled()) { + mCoordinators.add(smartspaceDedupingCoordinator); + } + if (featureFlags.isNewNotifPipelineRenderingEnabled()) { mCoordinators.add(headsUpCoordinator); mCoordinators.add(preparationCoordinator); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt new file mode 100644 index 0000000000000..442d9d2bb2054 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt @@ -0,0 +1,211 @@ +/* + * 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.coordinator + +import android.app.smartspace.SmartspaceTarget +import android.os.Parcelable +import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dagger.qualifiers.Main +import com.android.systemui.plugins.statusbar.StatusBarStateController +import com.android.systemui.statusbar.NotificationLockscreenUserManager +import com.android.systemui.statusbar.StatusBarState +import com.android.systemui.statusbar.SysuiStatusBarStateController +import com.android.systemui.statusbar.lockscreen.LockscreenSmartspaceController +import com.android.systemui.statusbar.notification.NotificationEntryManager +import com.android.systemui.statusbar.notification.collection.NotifPipeline +import com.android.systemui.statusbar.notification.collection.NotificationEntry +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter +import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener +import com.android.systemui.util.concurrency.DelayableExecutor +import com.android.systemui.util.time.SystemClock +import java.util.concurrent.TimeUnit.SECONDS +import javax.inject.Inject + +/** + * Hides notifications on the lockscreen if the content of those notifications is also visible + * in smartspace. This ONLY hides the notifications on the lockscreen: if the user pulls the shade + * down or unlocks the device, then the notifications are unhidden. + * + * In addition, notifications that have recently alerted aren't filtered. Tracking this in a way + * that involves the fewest pipeline invalidations requires some unfortunately complex logic. + */ +// This class is a singleton so that the same instance can be accessed by both the old and new +// pipelines +@SysUISingleton +class SmartspaceDedupingCoordinator @Inject constructor( + private val statusBarStateController: SysuiStatusBarStateController, + private val smartspaceController: LockscreenSmartspaceController, + private val notificationEntryManager: NotificationEntryManager, + private val notificationLockscreenUserManager: NotificationLockscreenUserManager, + private val notifPipeline: NotifPipeline, + @Main private val executor: DelayableExecutor, + private val clock: SystemClock +) : Coordinator { + private var isOnLockscreen = false + + private var trackedSmartspaceTargets = mutableMapOf() + + override fun attach(pipeline: NotifPipeline) { + pipeline.addPreGroupFilter(filter) + pipeline.addCollectionListener(collectionListener) + statusBarStateController.addCallback(statusBarStateListener) + smartspaceController.addListener(this::onNewSmartspaceTargets) + + // TODO (b/173126564): Remove this once the old pipeline is no longer necessary + notificationLockscreenUserManager.addKeyguardNotificationSuppressor { entry -> + isDupedWithSmartspaceContent(entry) + } + + recordStatusBarState(statusBarStateController.state) + } + + private fun isDupedWithSmartspaceContent(entry: NotificationEntry): Boolean { + return trackedSmartspaceTargets[entry.key]?.shouldFilter ?: false + } + + private val filter = object : NotifFilter("SmartspaceDedupingFilter") { + override fun shouldFilterOut(entry: NotificationEntry, now: Long): Boolean { + return isOnLockscreen && isDupedWithSmartspaceContent(entry) + } + } + + private val collectionListener = object : NotifCollectionListener { + override fun onEntryAdded(entry: NotificationEntry) { + trackedSmartspaceTargets[entry.key]?.let { + updateFilterStatus(it) + } + } + + override fun onEntryUpdated(entry: NotificationEntry) { + trackedSmartspaceTargets[entry.key]?.let { + updateFilterStatus(it) + } + } + + override fun onEntryRemoved(entry: NotificationEntry, reason: Int) { + trackedSmartspaceTargets[entry.key]?.let { trackedTarget -> + cancelExceptionTimeout(trackedTarget) + } + } + } + + private val statusBarStateListener = object : StatusBarStateController.StateListener { + override fun onStateChanged(newState: Int) { + recordStatusBarState(newState) + } + } + + private fun onNewSmartspaceTargets(targets: List) { + var changed = false + val newMap = mutableMapOf() + val oldMap = trackedSmartspaceTargets + + for (target in targets) { + // For all targets that are SmartspaceTargets and have non-null sourceNotificationKeys + (target as? SmartspaceTarget)?.sourceNotificationKey?.let { key -> + val trackedTarget = oldMap.getOrElse(key) { + TrackedSmartspaceTarget(key) + } + newMap[key] = trackedTarget + changed = changed || updateFilterStatus(trackedTarget) + } + // Currently, only filter out the first target + break + } + + for (prevKey in oldMap.keys) { + if (!newMap.containsKey(prevKey)) { + oldMap[prevKey]?.cancelTimeoutRunnable?.run() + changed = true + } + } + + if (changed) { + filter.invalidateList() + notificationEntryManager.updateNotifications("Smartspace targets changed") + } + + trackedSmartspaceTargets = newMap + } + + /** + * Returns true if the target's alert exception status has changed + */ + private fun updateFilterStatus(target: TrackedSmartspaceTarget): Boolean { + val prevShouldFilter = target.shouldFilter + + val entry = notifPipeline.getEntry(target.key) + if (entry != null) { + updateAlertException(target, entry) + + target.shouldFilter = !hasRecentlyAlerted(entry) + } + + return target.shouldFilter != prevShouldFilter && isOnLockscreen + } + + private fun updateAlertException(target: TrackedSmartspaceTarget, entry: NotificationEntry) { + val now = clock.currentTimeMillis() + val alertExceptionExpires = entry.ranking.lastAudiblyAlertedMillis + ALERT_WINDOW + + if (alertExceptionExpires != target.alertExceptionExpires && + alertExceptionExpires > now) { + // If we got here, the target is subject to a new alert exception window, so we + // should update our timeout to fire at the end of the new window + + target.cancelTimeoutRunnable?.run() + target.alertExceptionExpires = alertExceptionExpires + target.cancelTimeoutRunnable = executor.executeDelayed({ + target.cancelTimeoutRunnable = null + target.shouldFilter = true + filter.invalidateList() + notificationEntryManager.updateNotifications("deduping timeout expired") + }, alertExceptionExpires - now) + } + } + + private fun cancelExceptionTimeout(target: TrackedSmartspaceTarget) { + target.cancelTimeoutRunnable?.run() + target.cancelTimeoutRunnable = null + target.alertExceptionExpires = 0 + } + + private fun recordStatusBarState(newState: Int) { + val wasOnLockscreen = isOnLockscreen + isOnLockscreen = newState == StatusBarState.KEYGUARD + + if (isOnLockscreen != wasOnLockscreen) { + filter.invalidateList() + // No need to call notificationEntryManager.updateNotifications; something else already + // does it for us when the keyguard state changes + } + } + + private fun hasRecentlyAlerted(entry: NotificationEntry): Boolean { + return clock.currentTimeMillis() - entry.ranking.lastAudiblyAlertedMillis <= ALERT_WINDOW + } +} + +private class TrackedSmartspaceTarget( + val key: String +) { + var cancelTimeoutRunnable: Runnable? = null + var alertExceptionExpires: Long = 0 + var shouldFilter = false +} + +private val ALERT_WINDOW = SECONDS.toMillis(30) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifFilter.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifFilter.java index 9edb5fc89d8fa..776c7d5eb7f6b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifFilter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifFilter.java @@ -16,6 +16,8 @@ package com.android.systemui.statusbar.notification.collection.listbuilder.pluggable; +import androidx.annotation.NonNull; + import com.android.systemui.statusbar.notification.collection.NotifPipeline; import com.android.systemui.statusbar.notification.collection.NotificationEntry; @@ -45,5 +47,5 @@ public abstract class NotifFilter extends Pluggable { * various entries against. * @return True if the notif should be removed from the list */ - public abstract boolean shouldFilterOut(NotificationEntry entry, long now); + public abstract boolean shouldFilterOut(@NonNull NotificationEntry entry, long now); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinatorTest.kt new file mode 100644 index 0000000000000..a8db8d7617b7c --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinatorTest.kt @@ -0,0 +1,418 @@ +/* + * 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.coordinator + +import android.app.smartspace.SmartspaceTarget +import android.content.ComponentName +import android.os.UserHandle +import androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.android.systemui.plugins.BcSmartspaceDataPlugin.SmartspaceTargetListener +import com.android.systemui.plugins.statusbar.StatusBarStateController +import com.android.systemui.statusbar.NotificationLockscreenUserManager +import com.android.systemui.statusbar.StatusBarState +import com.android.systemui.statusbar.SysuiStatusBarStateController +import com.android.systemui.statusbar.lockscreen.LockscreenSmartspaceController +import com.android.systemui.statusbar.notification.NotificationEntryManager +import com.android.systemui.statusbar.notification.collection.NotifPipeline +import com.android.systemui.statusbar.notification.collection.NotificationEntry +import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder +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.util.concurrency.FakeExecutor +import com.android.systemui.util.mockito.capture +import com.android.systemui.util.time.FakeSystemClock +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.mockito.ArgumentCaptor +import org.mockito.Captor +import org.mockito.Mock +import org.mockito.Mockito.`when` +import org.mockito.Mockito.anyString +import org.mockito.Mockito.clearInvocations +import org.mockito.Mockito.never +import org.mockito.Mockito.verify +import org.mockito.MockitoAnnotations +import java.util.concurrent.TimeUnit + +@SmallTest +class SmartspaceDedupingCoordinatorTest : SysuiTestCase() { + + @Mock + private lateinit var statusBarStateController: SysuiStatusBarStateController + @Mock + private lateinit var smartspaceController: LockscreenSmartspaceController + @Mock + private lateinit var notificationEntryManager: NotificationEntryManager + @Mock + private lateinit var notificationLockscreenUserManager: NotificationLockscreenUserManager + @Mock + private lateinit var notifPipeline: NotifPipeline + @Mock + private lateinit var pluggableListener: Pluggable.PluggableListener + + @Captor + private lateinit var filterCaptor: ArgumentCaptor + @Captor + private lateinit var collectionListenerCaptor: ArgumentCaptor + @Captor + private lateinit var stateListenerCaptor: ArgumentCaptor + @Captor + private lateinit var smartspaceListenerCaptor: ArgumentCaptor + + private lateinit var filter: NotifFilter + private lateinit var collectionListener: NotifCollectionListener + private lateinit var statusBarListener: StatusBarStateController.StateListener + private lateinit var newTargetListener: SmartspaceTargetListener + + private lateinit var entry1HasRecentlyAlerted: NotificationEntry + private lateinit var entry2HasNotRecentlyAlerted: NotificationEntry + private lateinit var entry3NotAssociatedWithTarget: NotificationEntry + private lateinit var entry4HasNotRecentlyAlerted: NotificationEntry + private lateinit var target1: SmartspaceTarget + private lateinit var target2: SmartspaceTarget + private lateinit var target4: SmartspaceTarget + + private val clock = FakeSystemClock() + private val executor = FakeExecutor(clock) + private val now = clock.currentTimeMillis() + + private lateinit var deduper: SmartspaceDedupingCoordinator + + @Before + fun setUp() { + MockitoAnnotations.initMocks(this) + + // Mock out some behavior + `when`(statusBarStateController.state).thenReturn(StatusBarState.KEYGUARD) + + // Build the deduper + deduper = SmartspaceDedupingCoordinator( + statusBarStateController, + smartspaceController, + notificationEntryManager, + notificationLockscreenUserManager, + notifPipeline, + executor, + clock + ) + + // Attach the deduper and capture the listeners/filters that it registers + deduper.attach(notifPipeline) + + verify(notifPipeline).addPreGroupFilter(filterCaptor.capture()) + filter = filterCaptor.value + filter.setInvalidationListener(pluggableListener) + + verify(notifPipeline).addCollectionListener(capture(collectionListenerCaptor)) + collectionListener = collectionListenerCaptor.value + + verify(statusBarStateController).addCallback(capture(stateListenerCaptor)) + statusBarListener = stateListenerCaptor.value + + verify(smartspaceController).addListener(capture(smartspaceListenerCaptor)) + newTargetListener = smartspaceListenerCaptor.value + + // Initialize some test data + entry1HasRecentlyAlerted = NotificationEntryBuilder() + .setPkg(PACKAGE_1) + .setId(11) + .setLastAudiblyAlertedMs(now - 10000) + .build() + entry2HasNotRecentlyAlerted = NotificationEntryBuilder() + .setPkg(PACKAGE_2) + .setId(22) + .build() + entry3NotAssociatedWithTarget = NotificationEntryBuilder() + .setPkg("com.test.package.3") + .setId(33) + .setLastAudiblyAlertedMs(now - 10000) + .build() + entry4HasNotRecentlyAlerted = NotificationEntryBuilder() + .setPkg(PACKAGE_2) + .setId(44) + .build() + + target1 = buildTargetFor(entry1HasRecentlyAlerted) + target2 = buildTargetFor(entry2HasNotRecentlyAlerted) + target4 = buildTargetFor(entry4HasNotRecentlyAlerted) + } + + @Test + fun testBasicFiltering() { + // GIVEN a few notifications + addEntries( + entry2HasNotRecentlyAlerted, + entry3NotAssociatedWithTarget, + entry4HasNotRecentlyAlerted) + + // WHEN we receive smartspace targets associated with entry 2 and 3 + sendTargets(target2, target4) + + // THEN both pipelines are rerun + verifyPipelinesInvalidated() + + // THEN the first target is filtered out, but the other ones aren't + assertTrue(filter.shouldFilterOut(entry2HasNotRecentlyAlerted, now)) + assertFalse(filter.shouldFilterOut(entry3NotAssociatedWithTarget, now)) + assertFalse(filter.shouldFilterOut(entry4HasNotRecentlyAlerted, now)) + } + + @Test + fun testDoNotFilterRecentlyAlertedNotifs() { + // GIVEN one notif that recently alerted and a second that hasn't + addEntries(entry1HasRecentlyAlerted, entry2HasNotRecentlyAlerted) + + // WHEN they become associated with smartspace targets + sendTargets(target1, target2) + + // THEN neither is filtered (the first because it's recently alerted and the second + // because it's not in the first position + verifyPipelinesNotInvalidated() + assertFalse(filter.shouldFilterOut(entry1HasRecentlyAlerted, now)) + assertFalse(filter.shouldFilterOut(entry2HasNotRecentlyAlerted, now)) + } + + @Test + fun testFilterAlertedButNotRecentNotifs() { + // GIVEN a notification that alerted, but a very long time ago + val entryOldAlert = NotificationEntryBuilder(entry1HasRecentlyAlerted) + .setLastAudiblyAlertedMs(now - 40000) + .build() + addEntries(entryOldAlert) + + // WHEN it becomes part of smartspace + val target = buildTargetFor(entryOldAlert) + sendTargets(target) + + // THEN it's still filtered out (because it's not in the alert window) + verifyPipelinesInvalidated() + assertTrue(filter.shouldFilterOut(entryOldAlert, now)) + } + + @Test + fun testExceptionExpires() { + // GIVEN a recently-alerted notif that is the primary smartspace target + addEntries(entry1HasRecentlyAlerted) + sendTargets(target1) + clearPipelineInvocations() + + // WHEN we go beyond the target's exception window + clock.advanceTime(20000) + + // THEN the pipeline is invalidated + verifyPipelinesInvalidated() + assertExecutorIsClear() + } + + @Test + fun testExceptionIsEventuallyFiltered() { + // GIVEN a notif that has recently alerted + addEntries(entry1HasRecentlyAlerted) + + // WHEN it becomes the primary smartspace target + sendTargets(target1) + + // THEN it isn't filtered out (because it recently alerted) + assertFalse(filter.shouldFilterOut(entry1HasRecentlyAlerted, now)) + + // WHEN we pass the alert window + clock.advanceTime(20000) + + // THEN the notif is once again filtered + assertTrue(filter.shouldFilterOut(entry1HasRecentlyAlerted, clock.uptimeMillis())) + } + + @Test + fun testExceptionIsUpdated() { + // GIVEN a notif that has recently alerted and is the primary smartspace target + addEntries(entry1HasRecentlyAlerted) + sendTargets(target1) + clearPipelineInvocations() + assertFalse(filter.shouldFilterOut(entry1HasRecentlyAlerted, clock.uptimeMillis())) + + // GIVEN the notif is updated with a much more recent alert time + NotificationEntryBuilder(entry1HasRecentlyAlerted) + .setLastAudiblyAlertedMs(clock.currentTimeMillis() - 500) + .apply(entry1HasRecentlyAlerted) + updateEntries(entry1HasRecentlyAlerted) + assertFalse(filter.shouldFilterOut(entry1HasRecentlyAlerted, clock.uptimeMillis())) + + // WHEN we advance beyond the original exception window + clock.advanceTime(25000) + + // THEN the original exception window doesn't fire + verifyPipelinesNotInvalidated() + assertFalse(filter.shouldFilterOut(entry1HasRecentlyAlerted, clock.uptimeMillis())) + + // WHEN we advance beyond the new exception window + clock.advanceTime(4500) + + // THEN the pipelines are invalidated and no more timeouts are scheduled + verifyPipelinesInvalidated() + assertExecutorIsClear() + assertTrue(filter.shouldFilterOut(entry1HasRecentlyAlerted, clock.uptimeMillis())) + } + + @Test + fun testReplacementIsCanceled() { + // GIVEN a single notif and smartspace target + addEntries(entry1HasRecentlyAlerted) + sendTargets(target1) + clearPipelineInvocations() + + // WHEN a higher-ranked target arrives + val newerEntry = NotificationEntryBuilder() + .setPkg(PACKAGE_2) + .setId(55) + .setLastAudiblyAlertedMs(now - 1000) + .build() + val newerTarget = buildTargetFor(newerEntry) + sendTargets(newerTarget, target1) + + // THEN the timeout of the other target is canceled and it is no longer filtered + assertExecutorIsClear() + assertFalse(filter.shouldFilterOut(entry1HasRecentlyAlerted, clock.uptimeMillis())) + verifyPipelinesInvalidated() + clearPipelineInvocations() + + // WHEN the entry associated with the newer target later arrives + addEntries(newerEntry) + + // THEN the entry is not filtered out (because it recently alerted) + assertFalse(filter.shouldFilterOut(newerEntry, clock.uptimeMillis())) + + // WHEN its exception window passes + clock.advanceTime(ALERT_WINDOW) + + // THEN we go back to filtering it + verifyPipelinesInvalidated() + assertExecutorIsClear() + assertTrue(filter.shouldFilterOut(newerEntry, clock.uptimeMillis())) + } + + @Test + fun testRetractedIsCanceled() { + // GIVEN A recently alerted target + addEntries(entry1HasRecentlyAlerted) + sendTargets(target1) + + // WHEN the entry is removed + removeEntries(entry1HasRecentlyAlerted) + + // THEN its pending timeout is canceled + assertExecutorIsClear() + clock.advanceTime(ALERT_WINDOW) + verifyPipelinesNotInvalidated() + } + + @Test + fun testTargetBeforeEntryFunctionsProperly() { + // WHEN targets are added before their entries exist + sendTargets(target2, target1) + + // THEN neither is filtered out + assertFalse(filter.shouldFilterOut(entry2HasNotRecentlyAlerted, now)) + assertFalse(filter.shouldFilterOut(entry1HasRecentlyAlerted, now)) + + // WHEN the entries are later added + addEntries(entry2HasNotRecentlyAlerted, entry1HasRecentlyAlerted) + + // THEN the pipelines are not invalidated (because they're already going to be rerun) + // but the first entry is still filtered out properly. + verifyPipelinesNotInvalidated() + assertTrue(filter.shouldFilterOut(entry2HasNotRecentlyAlerted, now)) + } + + @Test + fun testLockscreenTracking() { + // GIVEN a couple of smartspace targets that haven't alerted recently + addEntries(entry2HasNotRecentlyAlerted, entry4HasNotRecentlyAlerted) + sendTargets(target2, target4) + clearPipelineInvocations() + + assertTrue(filter.shouldFilterOut(entry2HasNotRecentlyAlerted, now)) + + // WHEN we are no longer on the keyguard + statusBarListener.onStateChanged(StatusBarState.SHADE) + + // THEN the new pipeline is invalidated (but the old one isn't because it's not + // necessary) because the notif should no longer be filtered out + verify(pluggableListener).onPluggableInvalidated(filter) + verify(notificationEntryManager, never()).updateNotifications(anyString()) + assertFalse(filter.shouldFilterOut(entry2HasNotRecentlyAlerted, now)) + } + + private fun buildTargetFor(entry: NotificationEntry): SmartspaceTarget { + return SmartspaceTarget + .Builder("test", ComponentName("test", "class"), UserHandle.CURRENT) + .setSourceNotificationKey(entry.key) + .build() + } + + private fun addEntries(vararg entries: NotificationEntry) { + for (entry in entries) { + `when`(notifPipeline.getEntry(entry.key)).thenReturn(entry) + collectionListener.onEntryAdded(entry) + } + } + + private fun updateEntries(vararg entries: NotificationEntry) { + for (entry in entries) { + `when`(notifPipeline.getEntry(entry.key)).thenReturn(entry) + collectionListener.onEntryUpdated(entry) + } + } + + private fun removeEntries(vararg entries: NotificationEntry) { + for (entry in entries) { + `when`(notifPipeline.getEntry(entry.key)).thenReturn(null) + collectionListener.onEntryRemoved(entry, 0) + } + } + + private fun sendTargets(vararg targets: SmartspaceTarget) { + newTargetListener.onSmartspaceTargetsUpdated(targets.toMutableList()) + } + + private fun verifyPipelinesInvalidated() { + verify(pluggableListener).onPluggableInvalidated(filter) + verify(notificationEntryManager).updateNotifications(anyString()) + } + + private fun assertExecutorIsClear() { + assertEquals(0, executor.numPending()) + } + + private fun verifyPipelinesNotInvalidated() { + verify(pluggableListener, never()).onPluggableInvalidated(filter) + verify(notificationEntryManager, never()).updateNotifications(anyString()) + } + + private fun clearPipelineInvocations() { + clearInvocations(pluggableListener) + clearInvocations(notificationEntryManager) + } +} + +private val ALERT_WINDOW = TimeUnit.SECONDS.toMillis(30) +private const val PACKAGE_1 = "com.test.package.1" +private const val PACKAGE_2 = "com.test.package.2"