Fix forwarding logic in BitmapLoaders

loadBitmapFromMetadata is a default method in the interface,
but not forwarded to the actual implementation for the two
wrapping BitmapLoaders, preventing any customization of the
method.

PiperOrigin-RevId: 819766711

Author: tonihei

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

@@ -20,18 +20,21 @@
 import android.graphics.Bitmap;
 import android.net.Uri;
 import androidx.annotation.Nullable;
+import androidx.media3.common.MediaMetadata;
 import androidx.media3.common.util.BitmapLoader;
 import androidx.media3.common.util.UnstableApi;
 import com.google.common.util.concurrent.ListenableFuture;
 import java.util.Arrays;
 import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 
 /**
- * A {@link BitmapLoader} that caches the result of the last {@link #decodeBitmap(byte[])} or {@link
- * #loadBitmap(Uri)} request. Requests are fulfilled from the last bitmap load request when the last
- * bitmap is requested from the same {@code data} or the last bitmap is requested from the same
- * {@code uri}. If it's not the above two cases, the request is forwarded to the provided {@link
- * BitmapLoader} and the result is cached.
+ * A {@link BitmapLoader} that caches the result of the last {@link #decodeBitmap(byte[])}, {@link
+ * #loadBitmap(Uri)} or {@link #loadBitmapFromMetadata(MediaMetadata)} request.
+ *
+ * <p>Requests are fulfilled from the last bitmap load request when the last bitmap is requested
+ * from the same {@code data}, the same {@code uri}, or the same {@link MediaMetadata#artworkUri} or
+ * {@link MediaMetadata#artworkData}. If the request doesn't match the previous request, the request
+ * is forwarded to the provided {@link BitmapLoader} and the result is cached.
  */
 @UnstableApi
 public final class CacheBitmapLoader implements BitmapLoader {
@@ -73,6 +76,20 @@ public ListenableFuture<Bitmap> loadBitmap(Uri uri) {
     return future;
   }
 
+  @Nullable
+  @Override
+  public ListenableFuture<Bitmap> loadBitmapFromMetadata(MediaMetadata metadata) {
+    if (lastBitmapLoadRequest != null && lastBitmapLoadRequest.matches(metadata)) {
+      return lastBitmapLoadRequest.getFuture();
+    }
+    ListenableFuture<Bitmap> future = bitmapLoader.loadBitmapFromMetadata(metadata);
+    if (future == null) {
+      return null;
+    }
+    lastBitmapLoadRequest = new BitmapLoadRequest(metadata, future);
+    return future;
+  }
+
   /**
    * Stores the result of a bitmap load request. Requests are identified either by a byte array, if
    * the bitmap is loaded from compressed data, or a URI, if the bitmap was loaded from a URI.
@@ -82,30 +99,48 @@ private static class BitmapLoadRequest {
     @Nullable private final Uri uri;
     @Nullable private final ListenableFuture<Bitmap> future;
 
-    public BitmapLoadRequest(byte[] data, ListenableFuture<Bitmap> future) {
+    /** Creates load request for byte array. */
+    private BitmapLoadRequest(byte[] data, ListenableFuture<Bitmap> future) {
       this.data = data;
       this.uri = null;
       this.future = future;
     }
 
-    public BitmapLoadRequest(Uri uri, ListenableFuture<Bitmap> future) {
+    /** Creates load request for URI. */
+    private BitmapLoadRequest(Uri uri, ListenableFuture<Bitmap> future) {
       this.data = null;
       this.uri = uri;
       this.future = future;
     }
 
+    /** Creates load request for media metadata. */
+    private BitmapLoadRequest(MediaMetadata metadata, ListenableFuture<Bitmap> future) {
+      this.data = metadata.artworkData;
+      this.uri = metadata.artworkUri;
+      this.future = future;
+    }
+
     /** Whether the bitmap load request was performed for {@code data}. */
-    public boolean matches(@Nullable byte[] data) {
+    private boolean matches(byte[] data) {
       return this.data != null && Arrays.equals(this.data, data);
     }
 
     /** Whether the bitmap load request was performed for {@code uri}. */
-    public boolean matches(@Nullable Uri uri) {
+    private boolean matches(Uri uri) {
       return this.uri != null && this.uri.equals(uri);
     }
 
+    /**
+     * Whether the bitmap load request was performed for either {@code metadata.artworkUri} or
+     * {@code metadata.artworkData}.
+     */
+    private boolean matches(MediaMetadata metadata) {
+      return (this.uri != null && this.uri.equals(metadata.artworkUri))
+          || (this.data != null && Arrays.equals(this.data, metadata.artworkData));
+    }
+
     /** Returns the future that set for the bitmap load request. */
-    public ListenableFuture<Bitmap> getFuture() {
+    private ListenableFuture<Bitmap> getFuture() {
       return checkNotNull(future);
     }
   }

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

@@ -20,6 +20,8 @@
 
 import android.graphics.Bitmap;
 import android.net.Uri;
+import androidx.annotation.Nullable;
+import androidx.media3.common.MediaMetadata;
 import androidx.media3.common.util.BitmapLoader;
 import androidx.media3.common.util.UnstableApi;
 import com.google.common.util.concurrent.Futures;
@@ -59,6 +61,15 @@ public ListenableFuture<Bitmap> loadBitmap(Uri uri) {
     return Futures.transform(future, this::scaleIfNecessary, directExecutor());
   }
 
+  @Nullable
+  @Override
+  public ListenableFuture<Bitmap> loadBitmapFromMetadata(MediaMetadata metadata) {
+    ListenableFuture<Bitmap> future = bitmapLoader.loadBitmapFromMetadata(metadata);
+    return future == null
+        ? null
+        : Futures.transform(future, this::scaleIfNecessary, directExecutor());
+  }
+
   private Bitmap scaleIfNecessary(Bitmap bitmap) {
     if (bitmap.getWidth() > maxBitmapSize || bitmap.getHeight() > maxBitmapSize) {
       int width = bitmap.getWidth();

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

@@ -15,6 +15,8 @@
  */
 package androidx.media3.session;
 
+import static androidx.media3.common.MediaMetadata.PICTURE_TYPE_MEDIA;
+import static androidx.media3.test.utils.TestUtil.assertSubclassOverridesAllMethods;
 import static com.google.common.truth.Truth.assertThat;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.junit.Assert.assertThrows;
@@ -25,7 +27,9 @@
 import android.graphics.BitmapFactory;
 import android.graphics.Matrix;
 import android.net.Uri;
+import androidx.media3.common.MediaMetadata;
 import androidx.media3.common.ParserException;
+import androidx.media3.common.util.BitmapLoader;
 import androidx.media3.datasource.DataSourceBitmapLoader;
 import androidx.media3.datasource.HttpDataSource;
 import androidx.media3.test.utils.TestUtil;
@@ -67,7 +71,12 @@ public void setUp() {
   }
 
   @Test
-  public void decodeBitmap_requestWithSameDataTwice_success() throws Exception {
+  public void overridesAllMethods() throws NoSuchMethodException {
+    assertSubclassOverridesAllMethods(BitmapLoader.class, CacheBitmapLoader.class);
+  }
+
+  @Test
+  public void decodeBitmap_requestWithSameDataTwice_returnsCachedFuture() throws Exception {
     CacheBitmapLoader cacheBitmapLoader =
         new CacheBitmapLoader(new DataSourceBitmapLoader(context));
     byte[] imageData = TestUtil.getByteArray(context, TEST_IMAGE_PATH);
@@ -87,7 +96,7 @@ public void decodeBitmap_requestWithSameDataTwice_success() throws Exception {
   }
 
   @Test
-  public void decodeBitmap_requestWithDifferentData_success() throws Exception {
+  public void decodeBitmap_requestWithDifferentData_returnsNewFuture() throws Exception {
     CacheBitmapLoader cacheBitmapLoader =
         new CacheBitmapLoader(new DataSourceBitmapLoader(context));
     byte[] imageData1 = TestUtil.getByteArray(context, TEST_IMAGE_PATH);
@@ -130,7 +139,7 @@ public void decodeBitmap_requestWithSameDataTwice_throwsException() {
   }
 
   @Test
-  public void loadBitmap_httpUri_requestWithSameUriTwice_success() throws Exception {
+  public void loadBitmap_httpUri_requestWithSameUriTwice_returnsCachedFuture() throws Exception {
     CacheBitmapLoader cacheBitmapLoader =
         new CacheBitmapLoader(new DataSourceBitmapLoader(context));
     byte[] imageData = TestUtil.getByteArray(context, TEST_IMAGE_PATH);
@@ -155,7 +164,7 @@ public void loadBitmap_httpUri_requestWithSameUriTwice_success() throws Exceptio
   }
 
   @Test
-  public void loadBitmap_httpUri_requestWithDifferentUri_success() throws Exception {
+  public void loadBitmap_httpUri_requestWithDifferentUri_returnsNewFuture() throws Exception {
     CacheBitmapLoader cacheBitmapLoader =
         new CacheBitmapLoader(new DataSourceBitmapLoader(context));
     byte[] imageData1 = TestUtil.getByteArray(context, TEST_IMAGE_PATH);
@@ -213,6 +222,173 @@ public void loadBitmap_httpUri_requestWithSameUriTwice_throwsException() {
     assertThat(mockWebServer.getRequestCount()).isEqualTo(1);
   }
 
+  @Test
+  public void loadBitmapFromMetadata_requestWithSameDataTwice_returnsCachedFuture()
+      throws Exception {
+    CacheBitmapLoader cacheBitmapLoader =
+        new CacheBitmapLoader(
+            new LoadBitmapFromMetadataOnlyBitmapLoader(new DataSourceBitmapLoader(context)));
+    byte[] imageData = TestUtil.getByteArray(context, TEST_IMAGE_PATH);
+    Bitmap expectedBitmap =
+        apply90DegreeExifRotation(
+            BitmapFactory.decodeByteArray(imageData, /* offset= */ 0, imageData.length));
+    MediaMetadata metadata =
+        new MediaMetadata.Builder().setArtworkData(imageData, PICTURE_TYPE_MEDIA).build();
+
+    // First request, no cached bitmap load request.
+    ListenableFuture<Bitmap> future1 = cacheBitmapLoader.loadBitmapFromMetadata(metadata);
+
+    assertThat(future1.get(10, SECONDS).sameAs(expectedBitmap)).isTrue();
+
+    // Second request, has cached bitmap load request.
+    ListenableFuture<Bitmap> future2 = cacheBitmapLoader.loadBitmapFromMetadata(metadata);
+
+    assertThat(future1).isSameInstanceAs(future2);
+  }
+
+  @Test
+  public void loadBitmapFromMetadata_requestWithDifferentData_returnsNewFuture() throws Exception {
+    CacheBitmapLoader cacheBitmapLoader =
+        new CacheBitmapLoader(
+            new LoadBitmapFromMetadataOnlyBitmapLoader(new DataSourceBitmapLoader(context)));
+    byte[] imageData1 = TestUtil.getByteArray(context, TEST_IMAGE_PATH);
+    byte[] imageData2 = TestUtil.getByteArray(context, SECOND_TEST_IMAGE_PATH);
+    Bitmap expectedBitmap1 =
+        apply90DegreeExifRotation(
+            BitmapFactory.decodeByteArray(imageData1, /* offset= */ 0, imageData1.length));
+    Bitmap expectedBitmap2 =
+        apply90DegreeExifRotation(
+            BitmapFactory.decodeByteArray(imageData2, /* offset= */ 0, imageData2.length));
+    MediaMetadata metadata1 =
+        new MediaMetadata.Builder().setArtworkData(imageData1, PICTURE_TYPE_MEDIA).build();
+    MediaMetadata metadata2 =
+        new MediaMetadata.Builder().setArtworkData(imageData2, PICTURE_TYPE_MEDIA).build();
+
+    // First request.
+    ListenableFuture<Bitmap> future1 = cacheBitmapLoader.loadBitmapFromMetadata(metadata1);
+
+    assertThat(future1.get(10, SECONDS).sameAs(expectedBitmap1)).isTrue();
+
+    // Second request.
+    ListenableFuture<Bitmap> future2 = cacheBitmapLoader.loadBitmapFromMetadata(metadata2);
+
+    assertThat(future2.get(10, SECONDS).sameAs(expectedBitmap2)).isTrue();
+    assertThat(future1).isNotSameInstanceAs(future2);
+  }
+
+  @Test
+  public void loadBitmapFromMetadata_requestWithSameDataTwice_throwsException() {
+    CacheBitmapLoader cacheBitmapLoader =
+        new CacheBitmapLoader(
+            new LoadBitmapFromMetadataOnlyBitmapLoader(new DataSourceBitmapLoader(context)));
+    MediaMetadata metadata =
+        new MediaMetadata.Builder().setArtworkData(new byte[0], PICTURE_TYPE_MEDIA).build();
+
+    // First request, no cached bitmap load request.
+    ListenableFuture<Bitmap> future1 = cacheBitmapLoader.loadBitmapFromMetadata(metadata);
+
+    // Second request, has cached bitmap load request.
+    ListenableFuture<Bitmap> future2 = cacheBitmapLoader.loadBitmapFromMetadata(metadata);
+
+    assertThat(future1).isSameInstanceAs(future2);
+    ExecutionException executionException =
+        assertThrows(ExecutionException.class, () -> future1.get(10, SECONDS));
+    assertThat(executionException).hasCauseThat().isInstanceOf(ParserException.class);
+    assertThat(executionException).hasMessageThat().contains("Could not decode image data");
+  }
+
+  @Test
+  public void loadBitmapFromMetadata_requestWithSameUriTwice_returnsCachedFuture()
+      throws Exception {
+    CacheBitmapLoader cacheBitmapLoader =
+        new CacheBitmapLoader(
+            new LoadBitmapFromMetadataOnlyBitmapLoader(new DataSourceBitmapLoader(context)));
+    byte[] imageData = TestUtil.getByteArray(context, TEST_IMAGE_PATH);
+    Buffer responseBody = new Buffer().write(imageData);
+    MockWebServer mockWebServer = new MockWebServer();
+    mockWebServer.enqueue(new MockResponse().setResponseCode(200).setBody(responseBody));
+    Uri uri = Uri.parse(mockWebServer.url("test_path").toString());
+    Bitmap expectedBitmap =
+        apply90DegreeExifRotation(
+            BitmapFactory.decodeByteArray(imageData, /* offset= */ 0, imageData.length));
+    MediaMetadata metadata = new MediaMetadata.Builder().setArtworkUri(uri).build();
+
+    // First request, no cached bitmap load request.
+    Bitmap bitmap = cacheBitmapLoader.loadBitmapFromMetadata(metadata).get(10, SECONDS);
+
+    assertThat(bitmap.sameAs(expectedBitmap)).isTrue();
+
+    // Second request, has cached bitmap load request.
+    bitmap = cacheBitmapLoader.loadBitmapFromMetadata(metadata).get(10, SECONDS);
+
+    assertThat(bitmap.sameAs(expectedBitmap)).isTrue();
+    assertThat(mockWebServer.getRequestCount()).isEqualTo(1);
+  }
+
+  @Test
+  public void loadBitmapFromMetadata_requestWithDifferentUri_returnsNewFuture() throws Exception {
+    CacheBitmapLoader cacheBitmapLoader =
+        new CacheBitmapLoader(
+            new LoadBitmapFromMetadataOnlyBitmapLoader(new DataSourceBitmapLoader(context)));
+    byte[] imageData1 = TestUtil.getByteArray(context, TEST_IMAGE_PATH);
+    byte[] imageData2 = TestUtil.getByteArray(context, SECOND_TEST_IMAGE_PATH);
+    Buffer responseBody1 = new Buffer().write(imageData1);
+    Buffer responseBody2 = new Buffer().write(imageData2);
+    MockWebServer mockWebServer = new MockWebServer();
+    mockWebServer.enqueue(new MockResponse().setResponseCode(200).setBody(responseBody1));
+    mockWebServer.enqueue(new MockResponse().setResponseCode(200).setBody(responseBody2));
+    Uri uri1 = Uri.parse(mockWebServer.url("test_path_1").toString());
+    Uri uri2 = Uri.parse(mockWebServer.url("test_path_2").toString());
+    Bitmap expectedBitmap1 =
+        apply90DegreeExifRotation(
+            BitmapFactory.decodeByteArray(imageData1, /* offset= */ 0, imageData1.length));
+    Bitmap expectedBitmap2 =
+        apply90DegreeExifRotation(
+            BitmapFactory.decodeByteArray(imageData2, /* offset= */ 0, imageData2.length));
+    MediaMetadata metadata1 = new MediaMetadata.Builder().setArtworkUri(uri1).build();
+    MediaMetadata metadata2 = new MediaMetadata.Builder().setArtworkUri(uri2).build();
+
+    // First request.
+    Bitmap bitmap = cacheBitmapLoader.loadBitmapFromMetadata(metadata1).get(10, SECONDS);
+
+    assertThat(bitmap.sameAs(expectedBitmap1)).isTrue();
+
+    // Second request.
+    bitmap = cacheBitmapLoader.loadBitmapFromMetadata(metadata2).get(10, SECONDS);
+
+    assertThat(bitmap.sameAs(expectedBitmap2)).isTrue();
+    assertThat(mockWebServer.getRequestCount()).isEqualTo(2);
+  }
+
+  @Test
+  public void loadBitmapFromMetadata_requestWithSameUriTwice_throwsException() {
+    CacheBitmapLoader cacheBitmapLoader =
+        new CacheBitmapLoader(
+            new LoadBitmapFromMetadataOnlyBitmapLoader(new DataSourceBitmapLoader(context)));
+    MockWebServer mockWebServer = new MockWebServer();
+    mockWebServer.enqueue(new MockResponse().setResponseCode(404));
+    Uri uri = Uri.parse(mockWebServer.url("test_path").toString());
+    MediaMetadata metadata = new MediaMetadata.Builder().setArtworkUri(uri).build();
+
+    // First request, no cached bitmap load request.
+    ListenableFuture<Bitmap> future1 = cacheBitmapLoader.loadBitmapFromMetadata(metadata);
+
+    // Second request, has cached bitmap load request.
+    ListenableFuture<Bitmap> future2 = cacheBitmapLoader.loadBitmapFromMetadata(metadata);
+
+    ExecutionException executionException1 =
+        assertThrows(ExecutionException.class, () -> future1.get(10, SECONDS));
+    ExecutionException executionException2 =
+        assertThrows(ExecutionException.class, () -> future2.get(10, SECONDS));
+    assertThat(executionException1)
+        .hasCauseThat()
+        .isInstanceOf(HttpDataSource.InvalidResponseCodeException.class);
+    assertThat(executionException2)
+        .hasCauseThat()
+        .isInstanceOf(HttpDataSource.InvalidResponseCodeException.class);
+    assertThat(mockWebServer.getRequestCount()).isEqualTo(1);
+  }
+
   private static Bitmap apply90DegreeExifRotation(Bitmap bitmap) {
     Matrix rotationMatrix = new Matrix();
     rotationMatrix.postRotate(/* degrees= */ 90);
@@ -225,4 +401,32 @@ private static Bitmap apply90DegreeExifRotation(Bitmap bitmap) {
         rotationMatrix,
         /* filter= */ false);
   }
+
+  private static class LoadBitmapFromMetadataOnlyBitmapLoader implements BitmapLoader {
+    private final BitmapLoader bitmapLoader;
+
+    private LoadBitmapFromMetadataOnlyBitmapLoader(BitmapLoader bitmapLoader) {
+      this.bitmapLoader = bitmapLoader;
+    }
+
+    @Override
+    public boolean supportsMimeType(String mimeType) {
+      return true;
+    }
+
+    @Override
+    public ListenableFuture<Bitmap> decodeBitmap(byte[] data) {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public ListenableFuture<Bitmap> loadBitmap(Uri uri) {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public ListenableFuture<Bitmap> loadBitmapFromMetadata(MediaMetadata metadata) {
+      return bitmapLoader.loadBitmapFromMetadata(metadata);
+    }
+  }
 }