Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1ed2e2c7 authored by Ioana Alexandru's avatar Ioana Alexandru Committed by Android (Google) Code Review
Browse files

Merge "Revert "Avoid side effects in MediaCoordinator notif filter."" into main

parents 38146312 c0c193d0
Loading
Loading
Loading
Loading
+22 −47
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ import android.util.ArrayMap;
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 @@ public class MediaCoordinator implements Coordinator {
                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,20 +90,8 @@ public class MediaCoordinator implements Coordinator {
    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);
                }
            }
        }

        @Override
        public void onEntryUpdated(@NonNull NotificationEntry entry) {
@@ -93,12 +99,6 @@ public class MediaCoordinator implements Coordinator {
                // 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 @@ public class MediaCoordinator implements Coordinator {
        }
    };

    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,
+2 −62
Original line number Diff line number Diff line
@@ -20,7 +20,6 @@ import static com.google.common.truth.Truth.assertThat;

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 static org.mockito.Mockito.when;

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 @@ public final class MediaCoordinatorTest extends SysuiTestCase {
    }

    @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 @@ public final class MediaCoordinatorTest extends SysuiTestCase {
    }

    @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 @@ public final class MediaCoordinatorTest extends SysuiTestCase {
        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);