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

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

Merge changes Icfac69ec,Ifa7cd2b3 into main

* changes:
  Avoid side effects in MediaCoordinator notif filter.
  Add bugfix flag for bg icon updates for media notif
parents 8945ab69 b7ce749a
Loading
Loading
Loading
Loading
+10 −0
Original line number Diff line number Diff line
@@ -66,6 +66,16 @@ flag {
    bug: "308623704"
}

flag {
    name: "notifications_background_media_icons"
    namespace: "systemui"
    description: "Updates icons for media notifications in the background."
    bug: "315143160"
    metadata {
        purpose: PURPOSE_BUGFIX
    }
}

flag {
    name: "nssl_falsing_fix"
    namespace: "systemui"
+53 −26
Original line number Diff line number Diff line
@@ -22,7 +22,10 @@ import android.os.RemoteException;
import android.service.notification.StatusBarNotification;
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;
@@ -53,11 +56,58 @@ public class MediaCoordinator implements Coordinator {

    private final NotifFilter mMediaFilter = new NotifFilter(TAG) {
        @Override
        public boolean shouldFilterOut(NotificationEntry entry, long now) {
        public boolean shouldFilterOut(@NonNull NotificationEntry entry, long now) {
            if (!mIsMediaFeatureEnabled || !isMediaNotification(entry.getSbn())) {
                return false;
            }

            if (!Flags.notificationsBackgroundMediaIcons()) {
                inflateOrUpdateIcons(entry);
            }

            return true;
        }
    };

    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.notificationsBackgroundMediaIcons()) {
                mIconsState.put(entry, STATE_ICONS_UNINFLATED);
            }
        }

        @Override
        public void onEntryAdded(@NonNull NotificationEntry entry) {
            if (Flags.notificationsBackgroundMediaIcons()) {
                if (isMediaNotification(entry.getSbn())) {
                    inflateOrUpdateIcons(entry);
                }
            }
        }

        @Override
        public void onEntryUpdated(@NonNull 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);
            }

            if (Flags.notificationsBackgroundMediaIcons()) {
                if (isMediaNotification(entry.getSbn())) {
                    inflateOrUpdateIcons(entry);
                }
            }
        }

        @Override
        public void onEntryCleanUp(@NonNull NotificationEntry entry) {
            mIconsState.remove(entry);
        }
    };

    private void inflateOrUpdateIcons(NotificationEntry entry) {
        switch (mIconsState.getOrDefault(entry, STATE_ICONS_UNINFLATED)) {
            case STATE_ICONS_UNINFLATED:
                try {
@@ -80,30 +130,7 @@ public class MediaCoordinator implements Coordinator {
                // 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
+62 −2
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ 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.clearInvocations;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.never;
@@ -28,12 +29,15 @@ 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 android.testing.AndroidTestingRunner;

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;
@@ -153,7 +157,8 @@ public final class MediaCoordinatorTest extends SysuiTestCase {
    }

    @Test
    public void inflateMediaNotificationIconsMediaEnabled() throws InflationException {
    @DisableFlags(Flags.FLAG_NOTIFICATIONS_BACKGROUND_MEDIA_ICONS)
    public void inflateMediaNotificationIconsMediaEnabled_old() throws InflationException {
        finishSetupWithMediaFeatureFlagEnabled(true);

        mListener.onEntryInit(mMediaEntry);
@@ -181,7 +186,37 @@ public final class MediaCoordinatorTest extends SysuiTestCase {
    }

    @Test
    public void inflationException() throws InflationException {
    @EnableFlags(Flags.FLAG_NOTIFICATIONS_BACKGROUND_MEDIA_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));
        clearInvocations(mIconManager);

        mFilter.shouldFilterOut(mMediaEntry, 0);
        verify(mIconManager, never()).createIcons(eq(mMediaEntry));
        verify(mIconManager, never()).updateIcons(eq(mMediaEntry));

        mListener.onEntryUpdated(mMediaEntry);
        verify(mIconManager, never()).createIcons(eq(mMediaEntry));
        verify(mIconManager).updateIcons(eq(mMediaEntry));

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

    @Test
    @DisableFlags(Flags.FLAG_NOTIFICATIONS_BACKGROUND_MEDIA_ICONS)
    public void inflationException_old() throws InflationException {
        finishSetupWithMediaFeatureFlagEnabled(true);

        mListener.onEntryInit(mMediaEntry);
@@ -208,6 +243,31 @@ public final class MediaCoordinatorTest extends SysuiTestCase {
        verify(mIconManager, never()).updateIcons(eq(mMediaEntry));
    }

    @Test
    @EnableFlags(Flags.FLAG_NOTIFICATIONS_BACKGROUND_MEDIA_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));
        clearInvocations(mIconManager);

        mListener.onEntryUpdated(mMediaEntry);
        verify(mIconManager).createIcons(eq(mMediaEntry));
        verify(mIconManager, never()).updateIcons(eq(mMediaEntry));
        clearInvocations(mIconManager);

        doNothing().when(mIconManager).createIcons(eq(mMediaEntry));

        mListener.onEntryUpdated(mMediaEntry);
        verify(mIconManager).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);