summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Julia Tuttle <juliatuttle@google.com> 2022-05-17 12:45:48 -0400
committer Julia Tuttle <juliatuttle@google.com> 2022-05-23 16:15:40 -0400
commit29ce997ade33da886bae381d51e27ef5d56cdb45 (patch)
treeefa9ac42fea931f09b4635c6c2998fd5eb0a057a
parent2274df1efbe32febaaad1a6c34a9dfea4ee6fc13 (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
-rw-r--r--packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinator.java93
-rw-r--r--packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinatorTest.java137
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();
}
}