From 49541aa3c58f4a820769940e826b327fbafd7558 Mon Sep 17 00:00:00 2001 From: Robert Snoeberger Date: Wed, 3 Jun 2020 16:54:40 -0400 Subject: [PATCH] Move media filtering from NVHM to NotificationFilter In addition, using the same inspection code as used by MediaDataManager to determine when an SBN is a media notification. Bug: 156928028 Test: manual - played audio with GPM and checked that notification didn't appear. Test: manual - played Support V7 Demo app and checked that notification didn't appear. Test: manual - `adb shell dumpsys activity service com.android.systemui/.SystemUIService NotifListBuilderImpl` and check that GPM notification doesn't appear in list. Change-Id: Ie670ac7c6d5f50489a8fc5670258d4eac030e41d --- .../NotificationViewHierarchyManager.java | 3 - .../notification/NotificationFilter.java | 13 +- .../coordinator/MediaCoordinator.java | 52 ++++++++ .../coordinator/NotifCoordinators.java | 4 +- .../notification/NotificationFilterTest.java | 73 +++++++++- .../coordinator/MediaCoordinatorTest.java | 126 ++++++++++++++++++ 6 files changed, 265 insertions(+), 6 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinator.java create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinatorTest.java diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index 3dda15b5ce397..a8c03243c1174 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java @@ -42,7 +42,6 @@ import com.android.systemui.statusbar.notification.stack.NotificationListContain import com.android.systemui.statusbar.phone.KeyguardBypassController; import com.android.systemui.statusbar.phone.NotificationGroupManager; import com.android.systemui.util.Assert; -import com.android.systemui.util.Utils; import java.util.ArrayList; import java.util.HashMap; @@ -150,9 +149,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle final int N = activeNotifications.size(); for (int i = 0; i < N; i++) { NotificationEntry ent = activeNotifications.get(i); - boolean hideMedia = Utils.useQsMediaPlayer(mContext); if (ent.isRowDismissed() || ent.isRowRemoved() - || (ent.isMediaNotification() && hideMedia) || mBubbleController.isBubbleNotificationSuppressedFromShade(ent) || mFgsSectionController.hasEntry(ent)) { // we don't want to update removed notifications because they could 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 3afd6235b2873..6335a09cde2b8 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationFilter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationFilter.java @@ -16,6 +16,8 @@ package com.android.systemui.statusbar.notification; +import static com.android.systemui.media.MediaDataManagerKt.isMediaNotification; + import android.Manifest; import android.app.AppGlobals; import android.app.Notification; @@ -27,6 +29,7 @@ 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.media.MediaFeatureFlag; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.NotificationLockscreenUserManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; @@ -46,6 +49,7 @@ public class NotificationFilter { private final NotificationGroupManager mGroupManager = Dependency.get( NotificationGroupManager.class); private final StatusBarStateController mStatusBarStateController; + private final Boolean mIsMediaFlagEnabled; private NotificationEntryManager.KeyguardEnvironment mEnvironment; private ShadeController mShadeController; @@ -53,8 +57,11 @@ public class NotificationFilter { private NotificationLockscreenUserManager mUserManager; @Inject - public NotificationFilter(StatusBarStateController statusBarStateController) { + public NotificationFilter( + StatusBarStateController statusBarStateController, + MediaFeatureFlag mediaFeatureFlag) { mStatusBarStateController = statusBarStateController; + mIsMediaFlagEnabled = mediaFeatureFlag.getEnabled(); } private NotificationEntryManager.KeyguardEnvironment getEnvironment() { @@ -133,6 +140,10 @@ public class NotificationFilter { } } } + + if (mIsMediaFlagEnabled && isMediaNotification(sbn)) { + return true; + } return false; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinator.java new file mode 100644 index 0000000000000..026a3ffb73cd3 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinator.java @@ -0,0 +1,52 @@ +/* + * 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 com.android.systemui.media.MediaDataManagerKt.isMediaNotification; + +import com.android.systemui.media.MediaFeatureFlag; +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; + +/** + * Coordinates hiding (filtering) of media notifications. + */ +public class MediaCoordinator implements Coordinator { + private static final String TAG = "MediaCoordinator"; + + private final Boolean mIsMediaFeatureEnabled; + + private final NotifFilter mMediaFilter = new NotifFilter(TAG) { + @Override + public boolean shouldFilterOut(NotificationEntry entry, long now) { + return mIsMediaFeatureEnabled && isMediaNotification(entry.getSbn()); + } + }; + + @Inject + public MediaCoordinator(MediaFeatureFlag featureFlag) { + mIsMediaFeatureEnabled = featureFlag.getEnabled(); + } + + @Override + public void attach(NotifPipeline pipeline) { + pipeline.addFinalizeFilter(mMediaFilter); + } +} 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 2b279bbd553ac..ac42964395079 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 @@ -57,7 +57,8 @@ public class NotifCoordinators implements Dumpable { BubbleCoordinator bubbleCoordinator, HeadsUpCoordinator headsUpCoordinator, ConversationCoordinator conversationCoordinator, - PreparationCoordinator preparationCoordinator) { + PreparationCoordinator preparationCoordinator, + MediaCoordinator mediaCoordinator) { dumpManager.registerDumpable(TAG, this); mCoordinators.add(new HideLocallyDismissedNotifsCoordinator()); mCoordinators.add(hideNotifsForOtherUsersCoordinator); @@ -72,6 +73,7 @@ public class NotifCoordinators implements Dumpable { mCoordinators.add(preparationCoordinator); } // TODO: add new Coordinators here! (b/112656837) + mCoordinators.add(mediaCoordinator); // TODO: add the sections in a particular ORDER (HeadsUp < People < Alerting) for (Coordinator c : mCoordinators) { 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 595ba89ca3b6f..5a81d36ea7445 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 @@ -27,8 +27,10 @@ import static org.mockito.Mockito.when; import android.Manifest; import android.app.Notification; +import android.app.Notification.MediaStyle; import android.content.pm.IPackageManager; import android.content.pm.PackageManager; +import android.media.session.MediaSession; import android.os.Bundle; import android.service.notification.StatusBarNotification; import android.testing.AndroidTestingRunner; @@ -40,6 +42,7 @@ import androidx.test.filters.SmallTest; import com.android.systemui.ForegroundServiceController; import com.android.systemui.SysuiTestCase; +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; @@ -51,6 +54,7 @@ import com.android.systemui.statusbar.notification.row.NotificationTestHelper; import com.android.systemui.statusbar.phone.NotificationGroupManager; import com.android.systemui.statusbar.phone.ShadeController; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -73,10 +77,16 @@ public class NotificationFilterTest extends SysuiTestCase { ForegroundServiceController mFsc; @Mock KeyguardEnvironment mEnvironment; + @Mock + MediaFeatureFlag mMediaFeatureFlag; + @Mock + StatusBarStateController mStatusBarStateController; private final IPackageManager mMockPackageManager = mock(IPackageManager.class); private NotificationFilter mNotificationFilter; private ExpandableNotificationRow mRow; + private NotificationEntry mMediaEntry; + private MediaSession mMediaSession; @Before public void setUp() throws Exception { @@ -84,6 +94,12 @@ public class NotificationFilterTest extends SysuiTestCase { MockitoAnnotations.initMocks(this); when(mMockStatusBarNotification.getUid()).thenReturn(UID_NORMAL); + mMediaSession = new MediaSession(mContext, "TEST_MEDIA_SESSION"); + NotificationEntryBuilder builder = new NotificationEntryBuilder(); + builder.modifyNotification(mContext).setStyle( + new MediaStyle().setMediaSession(mMediaSession.getSessionToken())); + mMediaEntry = builder.build(); + when(mMockPackageManager.checkUidPermission( eq(Manifest.permission.NOTIFICATION_DURING_SETUP), eq(UID_NORMAL))) @@ -107,7 +123,12 @@ public class NotificationFilterTest extends SysuiTestCase { mDependency, TestableLooper.get(this)); mRow = testHelper.createRow(); - mNotificationFilter = new NotificationFilter(mock(StatusBarStateController.class)); + mNotificationFilter = new NotificationFilter(mStatusBarStateController, mMediaFeatureFlag); + } + + @After + public void tearDown() { + mMediaSession.release(); } @Test @@ -218,6 +239,56 @@ public class NotificationFilterTest extends SysuiTestCase { assertFalse(mNotificationFilter.shouldFilterOut(entry)); } + @Test + public void shouldFilterOtherNotificationWhenDisabled() { + // GIVEN that the media feature is disabled + when(mMediaFeatureFlag.getEnabled()).thenReturn(false); + NotificationFilter filter = new NotificationFilter(mStatusBarStateController, + mMediaFeatureFlag); + // WHEN the media filter is asked about an entry + NotificationEntry otherEntry = new NotificationEntryBuilder().build(); + final boolean shouldFilter = filter.shouldFilterOut(otherEntry); + // THEN it shouldn't be filtered + assertFalse(shouldFilter); + } + + @Test + public void shouldFilterOtherNotificationWhenEnabled() { + // GIVEN that the media feature is enabled + when(mMediaFeatureFlag.getEnabled()).thenReturn(true); + NotificationFilter filter = new NotificationFilter(mStatusBarStateController, + mMediaFeatureFlag); + // WHEN the media filter is asked about an entry + NotificationEntry otherEntry = new NotificationEntryBuilder().build(); + final boolean shouldFilter = filter.shouldFilterOut(otherEntry); + // THEN it shouldn't be filtered + assertFalse(shouldFilter); + } + + @Test + public void shouldFilterMediaNotificationWhenDisabled() { + // GIVEN that the media feature is disabled + when(mMediaFeatureFlag.getEnabled()).thenReturn(false); + NotificationFilter filter = new NotificationFilter(mStatusBarStateController, + mMediaFeatureFlag); + // WHEN the media filter is asked about a media entry + final boolean shouldFilter = filter.shouldFilterOut(mMediaEntry); + // THEN it shouldn't be filtered + assertFalse(shouldFilter); + } + + @Test + public void shouldFilterMediaNotificationWhenEnabled() { + // GIVEN that the media feature is enabled + when(mMediaFeatureFlag.getEnabled()).thenReturn(true); + NotificationFilter filter = new NotificationFilter(mStatusBarStateController, + mMediaFeatureFlag); + // WHEN the media filter is asked about a media entry + final boolean shouldFilter = filter.shouldFilterOut(mMediaEntry); + // THEN it should be filtered + assertTrue(shouldFilter); + } + private void initStatusBarNotification(boolean allowDuringSetup) { Bundle bundle = new Bundle(); bundle.putBoolean(Notification.EXTRA_ALLOW_DURING_SETUP, allowDuringSetup); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinatorTest.java new file mode 100644 index 0000000000000..c5dc2b4d4f035 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinatorTest.java @@ -0,0 +1,126 @@ +/* + * 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 com.google.common.truth.Truth.assertThat; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.app.Notification.MediaStyle; +import android.media.session.MediaSession; +import android.testing.AndroidTestingRunner; + +import androidx.test.filters.SmallTest; + +import com.android.systemui.SysuiTestCase; +import com.android.systemui.media.MediaFeatureFlag; +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 org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +@SmallTest +@RunWith(AndroidTestingRunner.class) +public final class MediaCoordinatorTest extends SysuiTestCase { + + private MediaSession mMediaSession; + private NotificationEntry mOtherEntry; + private NotificationEntry mMediaEntry; + + @Mock private NotifPipeline mNotifPipeline; + @Mock private MediaFeatureFlag mMediaFeatureFlag; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mOtherEntry = new NotificationEntryBuilder().build(); + mMediaSession = new MediaSession(mContext, "TEST_MEDIA_SESSION"); + NotificationEntryBuilder builder = new NotificationEntryBuilder(); + builder.modifyNotification(mContext).setStyle( + new MediaStyle().setMediaSession(mMediaSession.getSessionToken())); + mMediaEntry = builder.build(); + } + + @After + public void tearDown() { + mMediaSession.release(); + } + + @Test + public void shouldFilterOtherNotificationWhenDisabled() { + // GIVEN that the media feature is disabled + when(mMediaFeatureFlag.getEnabled()).thenReturn(false); + MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag); + // WHEN the media filter is asked about an entry + NotifFilter filter = captureFilter(coordinator); + final boolean shouldFilter = filter.shouldFilterOut(mOtherEntry, 0); + // THEN it shouldn't be filtered + assertThat(shouldFilter).isFalse(); + } + + @Test + public void shouldFilterOtherNotificationWhenEnabled() { + // GIVEN that the media feature is enabled + when(mMediaFeatureFlag.getEnabled()).thenReturn(true); + MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag); + // WHEN the media filter is asked about an entry + NotifFilter filter = captureFilter(coordinator); + final boolean shouldFilter = filter.shouldFilterOut(mOtherEntry, 0); + // THEN it shouldn't be filtered + assertThat(shouldFilter).isFalse(); + } + + @Test + public void shouldFilterMediaNotificationWhenDisabled() { + // GIVEN that the media feature is disabled + when(mMediaFeatureFlag.getEnabled()).thenReturn(false); + MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag); + // WHEN the media filter is asked about a media entry + NotifFilter filter = captureFilter(coordinator); + final boolean shouldFilter = filter.shouldFilterOut(mMediaEntry, 0); + // THEN it shouldn't be filtered + assertThat(shouldFilter).isFalse(); + } + + @Test + public void shouldFilterMediaNotificationWhenEnabled() { + // GIVEN that the media feature is enabled + when(mMediaFeatureFlag.getEnabled()).thenReturn(true); + MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag); + // WHEN the media filter is asked about a media entry + NotifFilter filter = captureFilter(coordinator); + final boolean shouldFilter = filter.shouldFilterOut(mMediaEntry, 0); + // THEN it should be filtered + assertThat(shouldFilter).isTrue(); + } + + private NotifFilter captureFilter(MediaCoordinator coordinator) { + ArgumentCaptor filterCaptor = ArgumentCaptor.forClass(NotifFilter.class); + coordinator.attach(mNotifPipeline); + verify(mNotifPipeline).addFinalizeFilter(filterCaptor.capture()); + return filterCaptor.getValue(); + } +}