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 ecee00641cd3f..2480ff65d8fc5 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 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 3ddff49974291..e1e5051751bb1 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 filterCaptor = ArgumentCaptor.forClass(NotifFilter.class); - coordinator.attach(mNotifPipeline); + ArgumentCaptor 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(); } }