DefaultMediaNotificationProvider: limit requests to load same bitmap

The DefaultMediaNotificationProvider caches the last loaded artwork
bitmap so that the bitmap isn't loaded again when the notification is
updated, e.g., the player is transiting from playing to paused. However,
loading bitmap requests for bitmaps that are already being loaded are
not suppressed. For example, if the notification is updated while the
artwork is still downloading, the same artwork might be downloaded
multiple times.

This change suppresses a bitmap load request if the same artwork is
still being loaded, to avoid additional artwork downloads.

#minor-release

PiperOrigin-RevId: 462572221

libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java

@@ -25,7 +25,6 @@ import static androidx.media3.common.Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM;
 import static androidx.media3.common.Player.COMMAND_STOP;
 import static androidx.media3.common.util.Assertions.checkState;
 import static androidx.media3.common.util.Assertions.checkStateNotNull;
-import static androidx.media3.common.util.Util.castNonNull;
 
 import android.app.Notification;
 import android.app.NotificationChannel;
@@ -45,7 +44,6 @@ import androidx.media.app.NotificationCompat.MediaStyle;
 import androidx.media3.common.C;
 import androidx.media3.common.MediaMetadata;
 import androidx.media3.common.Player;
-import androidx.media3.common.util.Consumer;
 import androidx.media3.common.util.Log;
 import androidx.media3.common.util.UnstableApi;
 import androidx.media3.common.util.Util;
@@ -58,6 +56,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 
 /**
  * The default {@link MediaNotification.Provider}.
@@ -231,12 +230,12 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
   @StringRes private final int channelNameResourceId;
   private final NotificationManager notificationManager;
   private final BitmapLoader bitmapLoader;
-  // Cache the last loaded bitmap to avoid reloading the bitmap again, particularly useful when
+  // Cache the last bitmap load request to avoid reloading the bitmap again, particularly useful
-  // showing a notification for the same item (e.g. when switching from playing to paused).
+  // when showing a notification for the same item (e.g. when switching from playing to paused).
-  private final LoadedBitmapInfo lastLoadedBitmapInfo;
+  private final BitmapLoadRequest lastBitmapLoadRequest;
   private final Handler mainHandler;
 
-  private OnBitmapLoadedFutureCallback pendingOnBitmapLoadedFutureCallback;
+  private @MonotonicNonNull OnBitmapLoadedFutureCallback pendingOnBitmapLoadedFutureCallback;
   @DrawableRes private int smallIconResourceId;
 
   private DefaultMediaNotificationProvider(Builder builder) {
@@ -248,9 +247,8 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
     notificationManager =
         checkStateNotNull(
             (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE));
-    lastLoadedBitmapInfo = new LoadedBitmapInfo();
+    lastBitmapLoadRequest = new BitmapLoadRequest();
     mainHandler = new Handler(Looper.getMainLooper());
-    pendingOnBitmapLoadedFutureCallback = new OnBitmapLoadedFutureCallback(bitmap -> {});
     smallIconResourceId = R.drawable.media3_notification_small_icon;
   }
 
@@ -281,6 +279,9 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
     builder.setContentTitle(metadata.title).setContentText(metadata.artist);
     @Nullable ListenableFuture<Bitmap> bitmapFuture = loadArtworkBitmap(metadata);
     if (bitmapFuture != null) {
+      if (pendingOnBitmapLoadedFutureCallback != null) {
+        pendingOnBitmapLoadedFutureCallback.discardIfPending();
+      }
       if (bitmapFuture.isDone()) {
         try {
           builder.setLargeIcon(Futures.getDone(bitmapFuture));
@@ -288,14 +289,12 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
           Log.w(TAG, "Failed to load bitmap", e);
         }
       } else {
+        pendingOnBitmapLoadedFutureCallback =
+            new OnBitmapLoadedFutureCallback(
+                notificationId, builder, onNotificationChangedCallback);
         Futures.addCallback(
             bitmapFuture,
-            new OnBitmapLoadedFutureCallback(
+            pendingOnBitmapLoadedFutureCallback,
-                bitmap -> {
-                  builder.setLargeIcon(bitmap);
-                  onNotificationChangedCallback.onNotificationChanged(
-                      new MediaNotification(notificationId, builder.build()));
-                }),
             // This callback must be executed on the next looper iteration, after this method has
             // returned a media notification.
             mainHandler::post);
@@ -517,32 +516,19 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
    */
   @Nullable
   private ListenableFuture<Bitmap> loadArtworkBitmap(MediaMetadata metadata) {
-    if (lastLoadedBitmapInfo.matches(metadata.artworkData)
+    @Nullable ListenableFuture<Bitmap> future;
-        || lastLoadedBitmapInfo.matches(metadata.artworkUri)) {
+    if (lastBitmapLoadRequest.matches(metadata.artworkData)
-      return Futures.immediateFuture(lastLoadedBitmapInfo.getBitmap());
+        || lastBitmapLoadRequest.matches(metadata.artworkUri)) {
-    }
+      future = lastBitmapLoadRequest.getFuture();
-
+    } else if (metadata.artworkData != null) {
-    ListenableFuture<Bitmap> future;
-    Consumer<Bitmap> onBitmapLoaded;
-    if (metadata.artworkData != null) {
       future = bitmapLoader.decodeBitmap(metadata.artworkData);
-      onBitmapLoaded =
+      lastBitmapLoadRequest.setBitmapFuture(metadata.artworkData, future);
-          bitmap -> lastLoadedBitmapInfo.setBitmapInfo(castNonNull(metadata.artworkData), bitmap);
     } else if (metadata.artworkUri != null) {
       future = bitmapLoader.loadBitmap(metadata.artworkUri);
-      onBitmapLoaded =
+      lastBitmapLoadRequest.setBitmapFuture(metadata.artworkUri, future);
-          bitmap -> lastLoadedBitmapInfo.setBitmapInfo(castNonNull(metadata.artworkUri), bitmap);
     } else {
-      return null;
+      future = null;
     }
-
-    pendingOnBitmapLoadedFutureCallback.discardIfPending();
-    pendingOnBitmapLoadedFutureCallback = new OnBitmapLoadedFutureCallback(onBitmapLoaded);
-    Futures.addCallback(
-        future,
-        pendingOnBitmapLoadedFutureCallback,
-        // It's ok to run this immediately to update the last loaded bitmap.
-        runnable -> Util.postOrRun(mainHandler, runnable));
     return future;
   }
 
@@ -560,13 +546,19 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
   }
 
   private static class OnBitmapLoadedFutureCallback implements FutureCallback<Bitmap> {
-
+    private final int notificationId;
-    private final Consumer<Bitmap> consumer;
+    private final NotificationCompat.Builder builder;
+    private final Callback onNotificationChangedCallback;
 
     private boolean discarded;
 
-    private OnBitmapLoadedFutureCallback(Consumer<Bitmap> consumer) {
+    public OnBitmapLoadedFutureCallback(
-      this.consumer = consumer;
+        int notificationId,
+        NotificationCompat.Builder builder,
+        Callback onNotificationChangedCallback) {
+      this.notificationId = notificationId;
+      this.builder = builder;
+      this.onNotificationChangedCallback = onNotificationChangedCallback;
     }
 
     public void discardIfPending() {
@@ -576,7 +568,9 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
     @Override
     public void onSuccess(Bitmap result) {
       if (!discarded) {
-        consumer.accept(result);
+        builder.setLargeIcon(result);
+        onNotificationChangedCallback.onNotificationChanged(
+            new MediaNotification(notificationId, builder.build()));
       }
     }
 
@@ -589,35 +583,51 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
   }
 
   /**
-   * Caches the last loaded bitmap. The key to identify a bitmap is either a byte array, if the
+   * Stores the result of a bitmap load request. Requests are identified either by a byte array, if
-   * bitmap is loaded from compressed data, or a URI, if the bitmap was loaded from a URI.
+   * the bitmap is loaded from compressed data, or a URI, if the bitmap was loaded from a URI.
    */
-  private static class LoadedBitmapInfo {
+  private static class BitmapLoadRequest {
     @Nullable private byte[] data;
     @Nullable private Uri uri;
-    @Nullable private Bitmap bitmap;
+    @Nullable private ListenableFuture<Bitmap> bitmapFuture;
 
+    /** Whether the bitmap load request was performed for {@code data}. */
     public boolean matches(@Nullable byte[] data) {
       return this.data != null && data != null && Arrays.equals(this.data, data);
     }
 
+    /** Whether the bitmap load request was performed for {@code uri}. */
     public boolean matches(@Nullable Uri uri) {
       return this.uri != null && this.uri.equals(uri);
     }
 
-    public Bitmap getBitmap() {
+    /**
-      return checkStateNotNull(bitmap);
+     * Returns the future that set for the bitmap load request.
+     *
+     * @see #setBitmapFuture(Uri, ListenableFuture)
+     * @see #setBitmapFuture(byte[], ListenableFuture)
+     */
+    public ListenableFuture<Bitmap> getFuture() {
+      return checkStateNotNull(bitmapFuture);
     }
 
-    public void setBitmapInfo(byte[] data, Bitmap bitmap) {
+    /**
+     * Sets the future result of requesting to {@linkplain BitmapLoader#decodeBitmap(byte[]) decode}
+     * a bitmap from {@code data}.
+     */
+    public void setBitmapFuture(byte[] data, ListenableFuture<Bitmap> bitmapFuture) {
       this.data = data;
-      this.bitmap = bitmap;
+      this.bitmapFuture = bitmapFuture;
       this.uri = null;
     }
 
-    public void setBitmapInfo(Uri uri, Bitmap bitmap) {
+    /**
+     * Sets the future result of requesting {@linkplain BitmapLoader#loadBitmap(Uri) load} a bitmap
+     * from {@code uri}.
+     */
+    public void setBitmapFuture(Uri uri, ListenableFuture<Bitmap> bitmapFuture) {
       this.uri = uri;
-      this.bitmap = bitmap;
+      this.bitmapFuture = bitmapFuture;
       this.data = null;
     }
   }

libraries/session/src/test/java/androidx/media3/session/DefaultMediaNotificationProviderTest.java

@@ -21,14 +21,18 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
+import static org.robolectric.Shadows.shadowOf;
 
 import android.app.NotificationChannel;
 import android.app.NotificationManager;
 import android.content.Context;
+import android.graphics.Bitmap;
 import android.net.Uri;
 import android.os.Bundle;
+import android.os.Looper;
 import androidx.annotation.Nullable;
 import androidx.core.app.NotificationCompat;
 import androidx.media3.common.MediaMetadata;
@@ -37,6 +41,7 @@ import androidx.media3.common.Player.Commands;
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import com.google.common.collect.ImmutableList;
+import com.google.common.util.concurrent.SettableFuture;
 import java.util.List;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -45,6 +50,7 @@ import org.mockito.InOrder;
 import org.mockito.Mockito;
 import org.robolectric.Robolectric;
 import org.robolectric.Shadows;
+import org.robolectric.shadows.ShadowLooper;
 import org.robolectric.shadows.ShadowNotificationManager;
 
 /** Tests for {@link DefaultMediaNotificationProvider}. */
@@ -389,6 +395,60 @@ public class DefaultMediaNotificationProviderTest {
     assertThat(actions.get(0).getExtras().size()).isEqualTo(0);
   }
 
+  /**
+   * Tests that the {@link DefaultMediaNotificationProvider} will not request to load the same
+   * artwork bitmap again, if the same bitmap has been requested already.
+   */
+  @Test
+  public void requestsSameBitmap_withPendingRequest_oneRequestOnly() {
+    // We will advance the main looper manually in the test.
+    shadowOf(Looper.getMainLooper()).pause();
+    // Create a MediaSession whose player returns non-null media metadata so that the
+    // notification provider will request to load artwork bitmaps.
+    MediaSession mockMediaSession =
+        createMockMediaSessionForNotification(
+            new MediaMetadata.Builder()
+                .setArtworkUri(Uri.parse("http://example.test/image.jpg"))
+                .build());
+    DefaultActionFactory defaultActionFactory =
+        new DefaultActionFactory(Robolectric.setupService(TestService.class));
+    BitmapLoader mockBitmapLoader = mock(BitmapLoader.class);
+    SettableFuture<Bitmap> bitmapFuture = SettableFuture.create();
+    when(mockBitmapLoader.loadBitmap(any())).thenReturn(bitmapFuture);
+    DefaultMediaNotificationProvider defaultMediaNotificationProvider =
+        new DefaultMediaNotificationProvider.Builder(ApplicationProvider.getApplicationContext())
+            .setBitmapLoader(mockBitmapLoader)
+            .build();
+
+    // Ask the notification provider to create a notification twice. Use separate callback instances
+    // for each notification so that we can distinguish for which notification we received a
+    // callback.
+    MediaNotification.Provider.Callback mockOnNotificationChangedCallback1 =
+        mock(MediaNotification.Provider.Callback.class);
+    defaultMediaNotificationProvider.createNotification(
+        mockMediaSession,
+        /* customLayout= */ ImmutableList.of(),
+        defaultActionFactory,
+        mockOnNotificationChangedCallback1);
+    ShadowLooper.idleMainLooper();
+    verify(mockBitmapLoader).loadBitmap(Uri.parse("http://example.test/image.jpg"));
+    verifyNoInteractions(mockOnNotificationChangedCallback1);
+    MediaNotification.Provider.Callback mockOnNotificationChangedCallback2 =
+        mock(MediaNotification.Provider.Callback.class);
+    defaultMediaNotificationProvider.createNotification(
+        mockMediaSession,
+        /* customLayout= */ ImmutableList.of(),
+        defaultActionFactory,
+        mockOnNotificationChangedCallback2);
+    // The bitmap has arrived.
+    bitmapFuture.set(Bitmap.createBitmap(/* width= */ 8, /* height= */ 8, Bitmap.Config.RGB_565));
+    ShadowLooper.idleMainLooper();
+
+    verifyNoMoreInteractions(mockBitmapLoader);
+    verify(mockOnNotificationChangedCallback2).onNotificationChanged(any());
+    verifyNoInteractions(mockOnNotificationChangedCallback1);
+  }
+
   @Test
   public void provider_withCustomIds_notificationsUseCustomIds() {
     Context context = ApplicationProvider.getApplicationContext();
@@ -398,7 +458,7 @@ public class DefaultMediaNotificationProviderTest {
             .setChannelId(/* channelId= */ "customChannelId")
             .setChannelName(/* channelNameResourceId= */ R.string.media3_controls_play_description)
             .build();
-    MediaSession mockMediaSession = createMockMediaSessionForNotification();
+    MediaSession mockMediaSession = createMockMediaSessionForNotification(MediaMetadata.EMPTY);
     DefaultActionFactory defaultActionFactory =
         new DefaultActionFactory(Robolectric.setupService(TestService.class));
 
@@ -427,7 +487,7 @@ public class DefaultMediaNotificationProviderTest {
         new DefaultMediaNotificationProvider.Builder(context).build();
     DefaultActionFactory defaultActionFactory =
         new DefaultActionFactory(Robolectric.setupService(TestService.class));
-    MediaSession mockMediaSession = createMockMediaSessionForNotification();
+    MediaSession mockMediaSession = createMockMediaSessionForNotification(MediaMetadata.EMPTY);
 
     MediaNotification notification =
         defaultMediaNotificationProvider.createNotification(
@@ -467,10 +527,10 @@ public class DefaultMediaNotificationProviderTest {
     assertThat(found).isTrue();
   }
 
-  private static MediaSession createMockMediaSessionForNotification() {
+  private static MediaSession createMockMediaSessionForNotification(MediaMetadata mediaMetadata) {
     Player mockPlayer = mock(Player.class);
     when(mockPlayer.getAvailableCommands()).thenReturn(Commands.EMPTY);
-    when(mockPlayer.getMediaMetadata()).thenReturn(MediaMetadata.EMPTY);
+    when(mockPlayer.getMediaMetadata()).thenReturn(mediaMetadata);
     MediaSession mockMediaSession = mock(MediaSession.class);
     when(mockMediaSession.getPlayer()).thenReturn(mockPlayer);
     MediaSessionImpl mockMediaSessionImpl = mock(MediaSessionImpl.class);