Merge "Inflate media notification icons in MediaCoordinator" into tm-dev am: e76c9147b6

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/18432858

Change-Id: I9e36e6b89e17a1fd0cb4db98041e40689ce381f5
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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);
     }
 }

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);
+        finishSetupWithMediaFeatureFlagEnabled(false);
-        MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag);
         // WHEN the media filter is asked about an entry
-        NotifFilter filter = captureFilter(coordinator);
+        final boolean shouldFilter = mFilter.shouldFilterOut(mOtherEntry, 0);
-        final boolean shouldFilter = filter.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);
+        finishSetupWithMediaFeatureFlagEnabled(true);
-        MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag);
         // WHEN the media filter is asked about an entry
-        NotifFilter filter = captureFilter(coordinator);
+        final boolean shouldFilter = mFilter.shouldFilterOut(mOtherEntry, 0);
-        final boolean shouldFilter = filter.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);
+        finishSetupWithMediaFeatureFlagEnabled(false);
-        MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag);
         // WHEN the media filter is asked about a media entry
-        NotifFilter filter = captureFilter(coordinator);
+        final boolean shouldFilter = mFilter.shouldFilterOut(mMediaEntry, 0);
-        final boolean shouldFilter = filter.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);
+        finishSetupWithMediaFeatureFlagEnabled(true);
-        MediaCoordinator coordinator = new MediaCoordinator(mMediaFeatureFlag);
         // WHEN the media filter is asked about a media entry
-        NotifFilter filter = captureFilter(coordinator);
+        final boolean shouldFilter = mFilter.shouldFilterOut(mMediaEntry, 0);
-        final boolean shouldFilter = filter.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();
     }
 }