Merge "Revert "Avoid side effects in MediaCoordinator notif filter."" into main
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 0bbde21..82ce31b 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
@@ -25,7 +25,6 @@
import androidx.annotation.NonNull;
import com.android.internal.statusbar.IStatusBarService;
-import com.android.systemui.Flags;
import com.android.systemui.media.controls.util.MediaFeatureFlag;
import com.android.systemui.statusbar.notification.InflationException;
import com.android.systemui.statusbar.notification.collection.NotifPipeline;
@@ -61,8 +60,27 @@
return false;
}
- if (!Flags.notificationsBackgroundIcons()) {
- inflateOrUpdateIcons(entry);
+ 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, /* usingCache = */ false);
+ } catch (InflationException e) {
+ reportInflationError(entry, e);
+ mIconsState.put(entry, STATE_ICONS_ERROR);
+ }
+ break;
+ case STATE_ICONS_ERROR:
+ // do nothing
+ break;
}
return true;
@@ -72,19 +90,7 @@
private final NotifCollectionListener mCollectionListener = new NotifCollectionListener() {
@Override
public void onEntryInit(@NonNull NotificationEntry entry) {
- // We default to STATE_ICONS_UNINFLATED anyway, so there's no need to initialize it.
- if (!Flags.notificationsBackgroundIcons()) {
- mIconsState.put(entry, STATE_ICONS_UNINFLATED);
- }
- }
-
- @Override
- public void onEntryAdded(@NonNull NotificationEntry entry) {
- if (Flags.notificationsBackgroundIcons()) {
- if (isMediaNotification(entry.getSbn())) {
- inflateOrUpdateIcons(entry);
- }
- }
+ mIconsState.put(entry, STATE_ICONS_UNINFLATED);
}
@Override
@@ -93,12 +99,6 @@
// The update may have fixed the inflation error, so give it another chance.
mIconsState.put(entry, STATE_ICONS_UNINFLATED);
}
-
- if (Flags.notificationsBackgroundIcons()) {
- if (isMediaNotification(entry.getSbn())) {
- inflateOrUpdateIcons(entry);
- }
- }
}
@Override
@@ -107,31 +107,6 @@
}
};
- private void inflateOrUpdateIcons(NotificationEntry entry) {
- 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, /* usingCache = */ false);
- } catch (InflationException e) {
- reportInflationError(entry, e);
- mIconsState.put(entry, STATE_ICONS_ERROR);
- }
- break;
- case STATE_ICONS_ERROR:
- // do nothing
- break;
- }
- }
-
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,
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 07c29a0..0c65c9c 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
@@ -20,7 +20,6 @@
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.never;
@@ -30,15 +29,12 @@
import android.app.Notification.MediaStyle;
import android.media.session.MediaSession;
-import android.platform.test.annotations.DisableFlags;
-import android.platform.test.annotations.EnableFlags;
import android.service.notification.NotificationListenerService;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.SmallTest;
import com.android.internal.statusbar.IStatusBarService;
-import com.android.systemui.Flags;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.media.controls.util.MediaFeatureFlag;
import com.android.systemui.statusbar.notification.InflationException;
@@ -158,8 +154,7 @@
}
@Test
- @DisableFlags(Flags.FLAG_NOTIFICATIONS_BACKGROUND_ICONS)
- public void inflateMediaNotificationIconsMediaEnabled_old() throws InflationException {
+ public void inflateMediaNotificationIconsMediaEnabled() throws InflationException {
finishSetupWithMediaFeatureFlagEnabled(true);
mListener.onEntryInit(mMediaEntry);
@@ -187,37 +182,7 @@
}
@Test
- @EnableFlags(Flags.FLAG_NOTIFICATIONS_BACKGROUND_ICONS)
- public void inflateMediaNotificationIconsMediaEnabled_new() throws InflationException {
- finishSetupWithMediaFeatureFlagEnabled(true);
-
- mListener.onEntryInit(mMediaEntry);
- mListener.onEntryAdded(mMediaEntry);
- verify(mIconManager).createIcons(eq(mMediaEntry));
- verify(mIconManager, never()).updateIcons(eq(mMediaEntry), anyBoolean());
- clearInvocations(mIconManager);
-
- mFilter.shouldFilterOut(mMediaEntry, 0);
- verify(mIconManager, never()).createIcons(eq(mMediaEntry));
- verify(mIconManager, never()).updateIcons(eq(mMediaEntry), anyBoolean());
-
- mListener.onEntryUpdated(mMediaEntry);
- verify(mIconManager, never()).createIcons(eq(mMediaEntry));
- verify(mIconManager).updateIcons(eq(mMediaEntry), /* usingCache = */ eq(false));
-
- mListener.onEntryRemoved(mMediaEntry, NotificationListenerService.REASON_CANCEL);
- mListener.onEntryCleanUp(mMediaEntry);
- clearInvocations(mIconManager);
-
- mListener.onEntryInit(mMediaEntry);
- mListener.onEntryAdded(mMediaEntry);
- verify(mIconManager).createIcons(eq(mMediaEntry));
- verify(mIconManager, never()).updateIcons(eq(mMediaEntry), anyBoolean());
- }
-
- @Test
- @DisableFlags(Flags.FLAG_NOTIFICATIONS_BACKGROUND_ICONS)
- public void inflationException_old() throws InflationException {
+ public void inflationException() throws InflationException {
finishSetupWithMediaFeatureFlagEnabled(true);
mListener.onEntryInit(mMediaEntry);
@@ -244,31 +209,6 @@
verify(mIconManager, never()).updateIcons(eq(mMediaEntry), anyBoolean());
}
- @Test
- @EnableFlags(Flags.FLAG_NOTIFICATIONS_BACKGROUND_ICONS)
- public void inflationException_new() throws InflationException {
- finishSetupWithMediaFeatureFlagEnabled(true);
-
- doThrow(InflationException.class).when(mIconManager).createIcons(eq(mMediaEntry));
-
- mListener.onEntryInit(mMediaEntry);
- mListener.onEntryAdded(mMediaEntry);
- verify(mIconManager).createIcons(eq(mMediaEntry));
- verify(mIconManager, never()).updateIcons(eq(mMediaEntry), anyBoolean());
- clearInvocations(mIconManager);
-
- mListener.onEntryUpdated(mMediaEntry);
- verify(mIconManager).createIcons(eq(mMediaEntry));
- verify(mIconManager, never()).updateIcons(eq(mMediaEntry), anyBoolean());
- clearInvocations(mIconManager);
-
- doNothing().when(mIconManager).createIcons(eq(mMediaEntry));
-
- mListener.onEntryUpdated(mMediaEntry);
- verify(mIconManager).createIcons(eq(mMediaEntry));
- verify(mIconManager, never()).updateIcons(eq(mMediaEntry), anyBoolean());
- }
-
private void finishSetupWithMediaFeatureFlagEnabled(boolean mediaFeatureFlagEnabled) {
when(mMediaFeatureFlag.getEnabled()).thenReturn(mediaFeatureFlagEnabled);
mCoordinator = new MediaCoordinator(mMediaFeatureFlag, mStatusBarService, mIconManager);