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

Commit c0c193d0 authored by Ioana Alexandru's avatar Ioana Alexandru
Browse files

Revert "Avoid side effects in MediaCoordinator notif filter." 

Revert submission 26255133

Reason for revert: this was a code cleanup that shouldn't have caused issues, but it seems to cause jank - b/364549968

Reverted changes: /q/submissionid:26255133

Change-Id: Idefbe7f84808096da696404da93a729d721df230
parent 5bafeec5

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinator.java

+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,

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/MediaCoordinatorTest.java

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