diff options
| author | 2022-05-17 12:45:48 -0400 | |
|---|---|---|
| committer | 2022-05-23 16:15:40 -0400 | |
| commit | 29ce997ade33da886bae381d51e27ef5d56cdb45 (patch) | |
| tree | efa9ac42fea931f09b4635c6c2998fd5eb0a057a | |
| parent | 2274df1efbe32febaaad1a6c34a9dfea4ee6fc13 (diff) | |
Inflate media notification icons in MediaCoordinator
In the old pipeline, notifications were inflated before the media
notification(s) were filtered out and used to populate the quick
settings, lockscreen, or AOD media views.
In the new pipeline, MediaCoordinator filters out media notifications
before PreparationCoordinator inflates them, so they're never inflated.
This is mostly okay, except that the AOD (and maybe lockscreen?) media
views use one of the media notification's icons to label the media
metadata, and that icon was no longer getting inflated.
Therefore, have MediaCoordinator do the necessary subset of the work of
PreparationCoordinator for the media notifications it filters out:
create and update icons for notifications it filters out, report
inflation errors back to StatusBarService, and do the bookkeeping
necessary to do the right thing (create, update, or nothing) each time
we see a notification.
Bug: 228397680
Test: atest MediaCoordinatorTest (added some test cases)
Change-Id: Ic42c276e5a973cdd260a8c4729a5abdf53987199
2 files changed, 209 insertions, 21 deletions
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 index ecee00641cd3..2480ff65d8fc 100644 --- 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 @@ -18,11 +18,19 @@ package com.android.systemui.statusbar.notification.collection.coordinator; import static com.android.systemui.media.MediaDataManagerKt.isMediaNotification; +import android.os.RemoteException; +import android.service.notification.StatusBarNotification; +import android.util.ArrayMap; + +import com.android.internal.statusbar.IStatusBarService; import com.android.systemui.media.MediaFeatureFlag; +import com.android.systemui.statusbar.notification.InflationException; import com.android.systemui.statusbar.notification.collection.NotifPipeline; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; +import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener; +import com.android.systemui.statusbar.notification.icon.IconManager; import javax.inject.Inject; @@ -34,21 +42,102 @@ public class MediaCoordinator implements Coordinator { private static final String TAG = "MediaCoordinator"; private final Boolean mIsMediaFeatureEnabled; + private final IStatusBarService mStatusBarService; + private final IconManager mIconManager; + + private static final int STATE_ICONS_UNINFLATED = 0; + private static final int STATE_ICONS_INFLATED = 1; + private static final int STATE_ICONS_ERROR = 2; + + private final ArrayMap<NotificationEntry, Integer> mIconsState = new ArrayMap<>(); private final NotifFilter mMediaFilter = new NotifFilter(TAG) { @Override public boolean shouldFilterOut(NotificationEntry entry, long now) { - return mIsMediaFeatureEnabled && isMediaNotification(entry.getSbn()); + if (!mIsMediaFeatureEnabled || !isMediaNotification(entry.getSbn())) { + return false; + } + + switch (mIconsState.getOrDefault(entry, STATE_ICONS_UNINFLATED)) { + case STATE_ICONS_UNINFLATED: + try { + mIconManager.createIcons(entry); + mIconsState.put(entry, STATE_ICONS_INFLATED); + } catch (InflationException e) { + reportInflationError(entry, e); + mIconsState.put(entry, STATE_ICONS_ERROR); + } + break; + case STATE_ICONS_INFLATED: + try { + mIconManager.updateIcons(entry); + } catch (InflationException e) { + reportInflationError(entry, e); + mIconsState.put(entry, STATE_ICONS_ERROR); + } + break; + case STATE_ICONS_ERROR: + // do nothing + break; + } + + return true; + } + }; + + private final NotifCollectionListener mCollectionListener = new NotifCollectionListener() { + @Override + public void onEntryInit(NotificationEntry entry) { + mIconsState.put(entry, STATE_ICONS_UNINFLATED); + } + + @Override + public void onEntryUpdated(NotificationEntry entry) { + if (mIconsState.getOrDefault(entry, STATE_ICONS_UNINFLATED) == STATE_ICONS_ERROR) { + // The update may have fixed the inflation error, so give it another chance. + mIconsState.put(entry, STATE_ICONS_UNINFLATED); + } + } + + @Override + public void onEntryCleanUp(NotificationEntry entry) { + mIconsState.remove(entry); } }; + private void reportInflationError(NotificationEntry entry, Exception e) { + // This is the same logic as in PreparationCoordinator; it doesn't handle media + // notifications when the media feature is enabled since they aren't displayed in the shade, + // so we have to handle inflating the icons (for AOD, at the very least) and reporting any + // errors ourselves. + try { + final StatusBarNotification sbn = entry.getSbn(); + // report notification inflation errors back up + // to notification delegates + mStatusBarService.onNotificationError( + sbn.getPackageName(), + sbn.getTag(), + sbn.getId(), + sbn.getUid(), + sbn.getInitialPid(), + e.getMessage(), + sbn.getUser().getIdentifier()); + } catch (RemoteException ex) { + // System server is dead, nothing to do about that + } + } + @Inject - public MediaCoordinator(MediaFeatureFlag featureFlag) { + public MediaCoordinator(MediaFeatureFlag featureFlag, IStatusBarService statusBarService, + IconManager iconManager) { mIsMediaFeatureEnabled = featureFlag.getEnabled(); + mStatusBarService = statusBarService; + mIconManager = iconManager; } @Override public void attach(NotifPipeline pipeline) { pipeline.addPreGroupFilter(mMediaFilter); + pipeline.addCollectionListener(mCollectionListener); } } 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 index 3ddff4997429..e1e5051751bb 100644 --- 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 @@ -18,21 +18,31 @@ package com.android.systemui.statusbar.notification.collection.coordinator; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.Notification.MediaStyle; import android.media.session.MediaSession; +import android.service.notification.NotificationListenerService; import android.testing.AndroidTestingRunner; import androidx.test.filters.SmallTest; +import com.android.internal.statusbar.IStatusBarService; import com.android.systemui.SysuiTestCase; import com.android.systemui.media.MediaFeatureFlag; +import com.android.systemui.statusbar.notification.InflationException; 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.notifcollection.NotifCollectionListener; +import com.android.systemui.statusbar.notification.icon.IconManager; import org.junit.After; import org.junit.Before; @@ -52,6 +62,12 @@ public final class MediaCoordinatorTest extends SysuiTestCase { @Mock private NotifPipeline mNotifPipeline; @Mock private MediaFeatureFlag mMediaFeatureFlag; + @Mock private IStatusBarService mStatusBarService; + @Mock private IconManager mIconManager; + + private MediaCoordinator mCoordinator; + private NotifFilter mFilter; + private NotifCollectionListener mListener; @Before public void setUp() { @@ -72,11 +88,9 @@ public final class MediaCoordinatorTest extends SysuiTestCase { @Test public void shouldFilterOtherNotificationWhenDisabled() { // GIVEN that the media feature is disabled - when(mMediaFeatureFlag.getEnabled()).thenReturn(false); - MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag); + finishSetupWithMediaFeatureFlagEnabled(false); // WHEN the media filter is asked about an entry - NotifFilter filter = captureFilter(coordinator); - final boolean shouldFilter = filter.shouldFilterOut(mOtherEntry, 0); + final boolean shouldFilter = mFilter.shouldFilterOut(mOtherEntry, 0); // THEN it shouldn't be filtered assertThat(shouldFilter).isFalse(); } @@ -84,11 +98,9 @@ public final class MediaCoordinatorTest extends SysuiTestCase { @Test public void shouldFilterOtherNotificationWhenEnabled() { // GIVEN that the media feature is enabled - when(mMediaFeatureFlag.getEnabled()).thenReturn(true); - MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag); + finishSetupWithMediaFeatureFlagEnabled(true); // WHEN the media filter is asked about an entry - NotifFilter filter = captureFilter(coordinator); - final boolean shouldFilter = filter.shouldFilterOut(mOtherEntry, 0); + final boolean shouldFilter = mFilter.shouldFilterOut(mOtherEntry, 0); // THEN it shouldn't be filtered assertThat(shouldFilter).isFalse(); } @@ -96,11 +108,9 @@ public final class MediaCoordinatorTest extends SysuiTestCase { @Test public void shouldFilterMediaNotificationWhenDisabled() { // GIVEN that the media feature is disabled - when(mMediaFeatureFlag.getEnabled()).thenReturn(false); - MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag); + finishSetupWithMediaFeatureFlagEnabled(false); // WHEN the media filter is asked about a media entry - NotifFilter filter = captureFilter(coordinator); - final boolean shouldFilter = filter.shouldFilterOut(mMediaEntry, 0); + final boolean shouldFilter = mFilter.shouldFilterOut(mMediaEntry, 0); // THEN it shouldn't be filtered assertThat(shouldFilter).isFalse(); } @@ -108,19 +118,108 @@ public final class MediaCoordinatorTest extends SysuiTestCase { @Test public void shouldFilterMediaNotificationWhenEnabled() { // GIVEN that the media feature is enabled - when(mMediaFeatureFlag.getEnabled()).thenReturn(true); - MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag); + finishSetupWithMediaFeatureFlagEnabled(true); // WHEN the media filter is asked about a media entry - NotifFilter filter = captureFilter(coordinator); - final boolean shouldFilter = filter.shouldFilterOut(mMediaEntry, 0); + final boolean shouldFilter = mFilter.shouldFilterOut(mMediaEntry, 0); // THEN it should be filtered assertThat(shouldFilter).isTrue(); } - private NotifFilter captureFilter(MediaCoordinator coordinator) { + @Test + public void inflateNotificationIconsMediaDisabled() throws InflationException { + finishSetupWithMediaFeatureFlagEnabled(false); + + mListener.onEntryInit(mOtherEntry); + mFilter.shouldFilterOut(mOtherEntry, 0); + verify(mIconManager, never()).createIcons(eq(mMediaEntry)); + } + + @Test + public void inflateNotificationIconsMediaEnabled() throws InflationException { + finishSetupWithMediaFeatureFlagEnabled(true); + + mListener.onEntryInit(mOtherEntry); + mFilter.shouldFilterOut(mOtherEntry, 0); + verify(mIconManager, never()).createIcons(eq(mMediaEntry)); + } + + @Test + public void inflateMediaNotificationIconsMediaDisabled() throws InflationException { + finishSetupWithMediaFeatureFlagEnabled(false); + + mListener.onEntryInit(mMediaEntry); + mFilter.shouldFilterOut(mMediaEntry, 0); + verify(mIconManager, never()).createIcons(eq(mMediaEntry)); + } + + @Test + public void inflateMediaNotificationIconsMediaEnabled() throws InflationException { + finishSetupWithMediaFeatureFlagEnabled(true); + + mListener.onEntryInit(mMediaEntry); + mListener.onEntryAdded(mMediaEntry); + verify(mIconManager, never()).createIcons(eq(mMediaEntry)); + verify(mIconManager, never()).updateIcons(eq(mMediaEntry)); + + mFilter.shouldFilterOut(mMediaEntry, 0); + verify(mIconManager, times(1)).createIcons(eq(mMediaEntry)); + verify(mIconManager, never()).updateIcons(eq(mMediaEntry)); + + mFilter.shouldFilterOut(mMediaEntry, 0); + verify(mIconManager, times(1)).createIcons(eq(mMediaEntry)); + verify(mIconManager, times(1)).updateIcons(eq(mMediaEntry)); + + mListener.onEntryRemoved(mMediaEntry, NotificationListenerService.REASON_CANCEL); + mListener.onEntryCleanUp(mMediaEntry); + mListener.onEntryInit(mMediaEntry); + verify(mIconManager, times(1)).createIcons(eq(mMediaEntry)); + verify(mIconManager, times(1)).updateIcons(eq(mMediaEntry)); + + mFilter.shouldFilterOut(mMediaEntry, 0); + verify(mIconManager, times(2)).createIcons(eq(mMediaEntry)); + verify(mIconManager, times(1)).updateIcons(eq(mMediaEntry)); + } + + @Test + public void inflationException() throws InflationException { + finishSetupWithMediaFeatureFlagEnabled(true); + + mListener.onEntryInit(mMediaEntry); + mListener.onEntryAdded(mMediaEntry); + verify(mIconManager, never()).createIcons(eq(mMediaEntry)); + verify(mIconManager, never()).updateIcons(eq(mMediaEntry)); + + doThrow(InflationException.class).when(mIconManager).createIcons(eq(mMediaEntry)); + mFilter.shouldFilterOut(mMediaEntry, 0); + verify(mIconManager, times(1)).createIcons(eq(mMediaEntry)); + verify(mIconManager, never()).updateIcons(eq(mMediaEntry)); + + mFilter.shouldFilterOut(mMediaEntry, 0); + verify(mIconManager, times(1)).createIcons(eq(mMediaEntry)); + verify(mIconManager, never()).updateIcons(eq(mMediaEntry)); + + mListener.onEntryUpdated(mMediaEntry); + verify(mIconManager, times(1)).createIcons(eq(mMediaEntry)); + verify(mIconManager, never()).updateIcons(eq(mMediaEntry)); + + doNothing().when(mIconManager).createIcons(eq(mMediaEntry)); + mFilter.shouldFilterOut(mMediaEntry, 0); + verify(mIconManager, times(2)).createIcons(eq(mMediaEntry)); + verify(mIconManager, never()).updateIcons(eq(mMediaEntry)); + } + + private void finishSetupWithMediaFeatureFlagEnabled(boolean mediaFeatureFlagEnabled) { + when(mMediaFeatureFlag.getEnabled()).thenReturn(mediaFeatureFlagEnabled); + mCoordinator = new MediaCoordinator(mMediaFeatureFlag, mStatusBarService, mIconManager); + ArgumentCaptor<NotifFilter> filterCaptor = ArgumentCaptor.forClass(NotifFilter.class); - coordinator.attach(mNotifPipeline); + ArgumentCaptor<NotifCollectionListener> listenerCaptor = + ArgumentCaptor.forClass(NotifCollectionListener.class); + mCoordinator.attach(mNotifPipeline); verify(mNotifPipeline).addPreGroupFilter(filterCaptor.capture()); - return filterCaptor.getValue(); + verify(mNotifPipeline).addCollectionListener(listenerCaptor.capture()); + + mFilter = filterCaptor.getValue(); + mListener = listenerCaptor.getValue(); } } |