From effd216b8f6c1b78c87ef547079e4058b4aae834 Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Thu, 19 Mar 2020 15:19:09 -0400 Subject: [PATCH] Move inactive user filtering to its own filter Previously it was being lumped in with the keyguard filter, which made it difficult to tell in logs why something was being filtered. Bug: 112656837 Test: atest Change-Id: Ibfbfa017811952cd5a873419d3e003b70b65763e --- .../HideNotifsForOtherUsersCoordinator.java | 69 ++++++++++++ .../coordinator/KeyguardCoordinator.java | 5 - .../coordinator/NotifCoordinators.java | 4 +- ...ideNotifsForOtherUsersCoordinatorTest.java | 106 ++++++++++++++++++ .../coordinator/KeyguardCoordinatorTest.java | 13 --- 5 files changed, 178 insertions(+), 19 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinator.java create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinatorTest.java diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinator.java new file mode 100644 index 0000000000000..261ae079a2c8c --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinator.java @@ -0,0 +1,69 @@ +/* + * Copyright (C) 2020 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.content.pm.UserInfo; +import android.util.SparseArray; + +import com.android.systemui.statusbar.NotificationLockscreenUserManager; +import com.android.systemui.statusbar.NotificationLockscreenUserManager.UserChangedListener; +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 javax.inject.Inject; + +/** + * A coordinator that filters out notifications for other users + * + * The NotifCollection contains the notifs for ALL users, so we need to remove any notifications + * that have been posted specifically to other users. Note that some system notifications are not + * posted to any particular user, and so must be shown to everyone. + * + * TODO: The NotificationLockscreenUserManager currently maintains the list of active user profiles. + * We should spin that off into a standalone section at some point. + */ +public class HideNotifsForOtherUsersCoordinator implements Coordinator { + private final NotificationLockscreenUserManager mLockscreenUserManager; + + @Inject + public HideNotifsForOtherUsersCoordinator( + NotificationLockscreenUserManager lockscreenUserManager) { + mLockscreenUserManager = lockscreenUserManager; + } + + @Override + public void attach(NotifPipeline pipeline) { + pipeline.addPreGroupFilter(mFilter); + mLockscreenUserManager.addUserChangedListener(mUserChangedListener); + } + + private final NotifFilter mFilter = new NotifFilter("NotCurrentUserFilter") { + @Override + public boolean shouldFilterOut(NotificationEntry entry, long now) { + return !mLockscreenUserManager + .isCurrentProfile(entry.getSbn().getUser().getIdentifier()); + } + }; + + private final UserChangedListener mUserChangedListener = new UserChangedListener() { + @Override + public void onCurrentProfilesChanged(SparseArray currentProfiles) { + mFilter.invalidateList(); + } + }; +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java index aaf71f58cb5c1..b7738569ad3cf 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java @@ -95,11 +95,6 @@ public class KeyguardCoordinator implements Coordinator { public boolean shouldFilterOut(NotificationEntry entry, long now) { final StatusBarNotification sbn = entry.getSbn(); - // FILTER OUT the notification when the notification isn't for the current profile - if (!mLockscreenUserManager.isCurrentProfile(sbn.getUserId())) { - return true; - } - // FILTER OUT the notification when the keyguard is showing and... if (mKeyguardStateController.isShowing()) { // ... user settings or the device policy manager doesn't allow lockscreen 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 98104f84f30e6..03c0ae6fde501 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 @@ -49,15 +49,17 @@ public class NotifCoordinators implements Dumpable { public NotifCoordinators( DumpManager dumpManager, FeatureFlags featureFlags, - HeadsUpCoordinator headsUpCoordinator, + HideNotifsForOtherUsersCoordinator hideNotifsForOtherUsersCoordinator, KeyguardCoordinator keyguardCoordinator, RankingCoordinator rankingCoordinator, ForegroundCoordinator foregroundCoordinator, DeviceProvisionedCoordinator deviceProvisionedCoordinator, BubbleCoordinator bubbleCoordinator, + HeadsUpCoordinator headsUpCoordinator, PreparationCoordinator preparationCoordinator) { dumpManager.registerDumpable(TAG, this); mCoordinators.add(new HideLocallyDismissedNotifsCoordinator()); + mCoordinators.add(hideNotifsForOtherUsersCoordinator); mCoordinators.add(keyguardCoordinator); mCoordinators.add(rankingCoordinator); mCoordinators.add(foregroundCoordinator); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinatorTest.java new file mode 100644 index 0000000000000..87fc02062ad4d --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/HideNotifsForOtherUsersCoordinatorTest.java @@ -0,0 +1,106 @@ +/* + * Copyright (C) 2020 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 static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.testing.AndroidTestingRunner; +import android.util.SparseArray; + +import androidx.test.filters.SmallTest; + +import com.android.systemui.SysuiTestCase; +import com.android.systemui.statusbar.NotificationLockscreenUserManager; +import com.android.systemui.statusbar.NotificationLockscreenUserManager.UserChangedListener; +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.PluggableListener; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +@SmallTest +@RunWith(AndroidTestingRunner.class) +public class HideNotifsForOtherUsersCoordinatorTest extends SysuiTestCase { + + @Mock private NotificationLockscreenUserManager mLockscreenUserManager; + @Mock private NotifPipeline mNotifPipeline; + @Mock private PluggableListener mInvalidationListener; + + @Captor private ArgumentCaptor mUserChangedListenerCaptor; + @Captor private ArgumentCaptor mNotifFilterCaptor; + + private UserChangedListener mCapturedUserChangeListener; + private NotifFilter mCapturedNotifFilter; + + private NotificationEntry mEntry = new NotificationEntryBuilder().build(); + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + + HideNotifsForOtherUsersCoordinator coordinator = + new HideNotifsForOtherUsersCoordinator(mLockscreenUserManager); + coordinator.attach(mNotifPipeline); + + verify(mLockscreenUserManager).addUserChangedListener(mUserChangedListenerCaptor.capture()); + verify(mNotifPipeline).addPreGroupFilter(mNotifFilterCaptor.capture()); + + mCapturedUserChangeListener = mUserChangedListenerCaptor.getValue(); + mCapturedNotifFilter = mNotifFilterCaptor.getValue(); + + mCapturedNotifFilter.setInvalidationListener(mInvalidationListener); + } + + @Test + public void testFilterOutNotifsFromOtherProfiles() { + // GIVEN that all notifs are NOT for the current user + when(mLockscreenUserManager.isCurrentProfile(anyInt())).thenReturn(false); + + // THEN they should all be filtered out + assertTrue(mCapturedNotifFilter.shouldFilterOut(mEntry, 0)); + } + + @Test + public void testPreserveNotifsFromThisProfile() { + // GIVEN that all notifs ARE for the current user + when(mLockscreenUserManager.isCurrentProfile(anyInt())).thenReturn(true); + + // THEN none should be filtered out + assertFalse(mCapturedNotifFilter.shouldFilterOut(mEntry, 0)); + } + + @Test + public void testFilterIsInvalidatedWhenProfilesChange() { + // WHEN the current user profiles change + mCapturedUserChangeListener.onCurrentProfilesChanged(new SparseArray<>()); + + // THEN the filter is invalidated + verify(mInvalidationListener).onPluggableInvalidated(mCapturedNotifFilter); + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.java index c4f3a1611afc6..4f481081855fc 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.java @@ -101,16 +101,6 @@ public class KeyguardCoordinatorTest extends SysuiTestCase { assertFalse(mKeyguardFilter.shouldFilterOut(mEntry, 0)); } - @Test - public void notificationNotForCurrentProfile() { - // GIVEN the notification isn't for the given user - setupUnfilteredState(mEntry); - when(mLockscreenUserManager.isCurrentProfile(NOTIF_USER_ID)).thenReturn(false); - - // THEN filter out the entry - assertTrue(mKeyguardFilter.shouldFilterOut(mEntry, 0)); - } - @Test public void keyguardNotShowing() { // GIVEN the lockscreen isn't showing @@ -229,9 +219,6 @@ public class KeyguardCoordinatorTest extends SysuiTestCase { * KeyguardNotificationCoordinator when the keyguard is showing. */ private void setupUnfilteredState(NotificationEntry entry) { - // notification is for current profile - when(mLockscreenUserManager.isCurrentProfile(NOTIF_USER_ID)).thenReturn(true); - // keyguard is showing when(mKeyguardStateController.isShowing()).thenReturn(true);