Add CacheBitmapLoader in the session module

* Add `CacheBitmapLoader`.
* Add `CacheBitmapLoaderTest`.
* Remove the `BitmapLoadRequest` and some bitmap caching logic in `DefaultMediaNotificationProvider` since we moved all of them in `CacheBitmapLoader`.
* Modify `DefaultMediaNotificationProviderTest`.

PiperOrigin-RevId: 482787445

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

+/*
+ * Copyright 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package androidx.media3.session;
+
+import static com.google.common.truth.Truth.assertThat;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.Assert.assertThrows;
+
+import android.graphics.Bitmap;
+import android.graphics.BitmapFactory;
+import android.net.Uri;
+import androidx.media3.test.utils.TestUtil;
+import androidx.test.core.app.ApplicationProvider;
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+import com.google.common.util.concurrent.ListenableFuture;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
+import okio.Buffer;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+
+/**
+ * Tests for {@link CacheBitmapLoader}.
+ *
+ * <p>This test needs to run as an androidTest because robolectric's {@link BitmapFactory} is not
+ * fully functional.
+ */
+@RunWith(AndroidJUnit4.class)
+public class CacheBitmapLoaderTest {
+
+  private static final String TEST_IMAGE_PATH = "media/jpeg/non-motion-photo-shortened.jpg";
+
+  private static final String SECOND_TEST_IMAGE_PATH = "media/jpeg/ss-motion-photo-shortened.jpg";
+
+  @Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
+
+  @Test
+  public void decodeBitmap_requestWithSameDataTwice_success() throws Exception {
+    CacheBitmapLoader cacheBitmapLoader = new CacheBitmapLoader(new SimpleBitmapLoader());
+    byte[] imageData =
+        TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), TEST_IMAGE_PATH);
+
+    // First request, no cached bitmap load request.
+    ListenableFuture<Bitmap> future1 = cacheBitmapLoader.decodeBitmap(imageData);
+
+    assertThat(
+            future1
+                .get(10, SECONDS)
+                .sameAs(
+                    BitmapFactory.decodeByteArray(imageData, /* offset= */ 0, imageData.length)))
+        .isTrue();
+
+    // Second request, has cached bitmap load request.
+    ListenableFuture<Bitmap> future2 = cacheBitmapLoader.decodeBitmap(imageData);
+
+    assertThat(future1).isSameInstanceAs(future2);
+  }
+
+  @Test
+  public void decodeBitmap_requestWithDifferentData_success() throws Exception {
+    CacheBitmapLoader cacheBitmapLoader = new CacheBitmapLoader(new SimpleBitmapLoader());
+    byte[] imageData1 =
+        TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), TEST_IMAGE_PATH);
+    byte[] imageData2 =
+        TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), SECOND_TEST_IMAGE_PATH);
+
+    // First request.
+    ListenableFuture<Bitmap> future1 = cacheBitmapLoader.decodeBitmap(imageData1);
+
+    assertThat(
+            future1
+                .get(10, SECONDS)
+                .sameAs(
+                    BitmapFactory.decodeByteArray(imageData1, /* offset= */ 0, imageData1.length)))
+        .isTrue();
+
+    // Second request.
+    ListenableFuture<Bitmap> future2 = cacheBitmapLoader.decodeBitmap(imageData2);
+
+    assertThat(
+            future2
+                .get(10, SECONDS)
+                .sameAs(
+                    BitmapFactory.decodeByteArray(imageData2, /* offset= */ 0, imageData2.length)))
+        .isTrue();
+    assertThat(future1).isNotSameInstanceAs(future2);
+  }
+
+  @Test
+  public void decodeBitmap_requestWithSameDataTwice_throwsException() {
+    CacheBitmapLoader cacheBitmapLoader = new CacheBitmapLoader(new SimpleBitmapLoader());
+
+    // First request, no cached bitmap load request.
+    ListenableFuture<Bitmap> future1 = cacheBitmapLoader.decodeBitmap(new byte[0]);
+
+    // Second request, has cached bitmap load request.
+    ListenableFuture<Bitmap> future2 = cacheBitmapLoader.decodeBitmap(new byte[0]);
+
+    assertThat(future1).isSameInstanceAs(future2);
+    ExecutionException executionException =
+        assertThrows(ExecutionException.class, () -> future1.get(10, SECONDS));
+    assertThat(executionException).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
+    assertThat(executionException).hasMessageThat().contains("Could not decode image data");
+  }
+
+  @Test
+  public void loadBitmap_httpUri_requestWithSameUriTwice_success() throws Exception {
+    CacheBitmapLoader cacheBitmapLoader = new CacheBitmapLoader(new SimpleBitmapLoader());
+    byte[] imageData =
+        TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), 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());
+
+    // First request, no cached bitmap load request.
+    Bitmap bitmap = cacheBitmapLoader.loadBitmap(uri).get(10, SECONDS);
+
+    assertThat(
+            bitmap.sameAs(
+                BitmapFactory.decodeByteArray(imageData, /* offset= */ 0, imageData.length)))
+        .isTrue();
+
+    // Second request, has cached bitmap load request.
+    bitmap = cacheBitmapLoader.loadBitmap(uri).get(10, SECONDS);
+
+    assertThat(
+            bitmap.sameAs(
+                BitmapFactory.decodeByteArray(imageData, /* offset= */ 0, imageData.length)))
+        .isTrue();
+    assertThat(mockWebServer.getRequestCount()).isEqualTo(1);
+  }
+
+  @Test
+  public void loadBitmap_httpUri_requestWithDifferentUri_success() throws Exception {
+    CacheBitmapLoader cacheBitmapLoader = new CacheBitmapLoader(new SimpleBitmapLoader());
+    byte[] imageData1 =
+        TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), TEST_IMAGE_PATH);
+    byte[] imageData2 =
+        TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), 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());
+
+    // First request.
+    Bitmap bitmap = cacheBitmapLoader.loadBitmap(uri1).get(10, SECONDS);
+
+    assertThat(
+            bitmap.sameAs(
+                BitmapFactory.decodeByteArray(imageData1, /* offset= */ 0, imageData1.length)))
+        .isTrue();
+
+    // Second request.
+    bitmap = cacheBitmapLoader.loadBitmap(uri2).get(10, SECONDS);
+
+    assertThat(
+            bitmap.sameAs(
+                BitmapFactory.decodeByteArray(imageData2, /* offset= */ 0, imageData2.length)))
+        .isTrue();
+    assertThat(mockWebServer.getRequestCount()).isEqualTo(2);
+  }
+
+  @Test
+  public void loadBitmap_httpUri_requestWithSameUriTwice_throwsException() throws Exception {
+    CacheBitmapLoader cacheBitmapLoader = new CacheBitmapLoader(new SimpleBitmapLoader());
+    MockWebServer mockWebServer = new MockWebServer();
+    mockWebServer.enqueue(new MockResponse().setResponseCode(404));
+    Uri uri = Uri.parse(mockWebServer.url("test_path").toString());
+
+    // First request, no cached bitmap load request.
+    ListenableFuture<Bitmap> future1 = cacheBitmapLoader.loadBitmap(uri);
+
+    // Second request, has cached bitmap load request.
+    ListenableFuture<Bitmap> future2 = cacheBitmapLoader.loadBitmap(uri);
+
+    ExecutionException executionException1 =
+        assertThrows(ExecutionException.class, () -> future1.get(10, SECONDS));
+    ExecutionException executionException2 =
+        assertThrows(ExecutionException.class, () -> future2.get(10, SECONDS));
+    assertThat(executionException1).hasCauseThat().isInstanceOf(IOException.class);
+    assertThat(executionException2).hasCauseThat().isInstanceOf(IOException.class);
+    assertThat(executionException1).hasMessageThat().contains("Invalid response status");
+    assertThat(executionException2).hasMessageThat().contains("Invalid response status");
+    assertThat(mockWebServer.getRequestCount()).isEqualTo(1);
+  }
+}

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

+/*
+ * Copyright 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package androidx.media3.session;
+
+import static androidx.media3.common.util.Assertions.checkStateNotNull;
+
+import android.graphics.Bitmap;
+import android.net.Uri;
+import androidx.annotation.Nullable;
+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.
+ */
+@UnstableApi
+public final class CacheBitmapLoader implements BitmapLoader {
+
+  private final BitmapLoader bitmapLoader;
+
+  private @MonotonicNonNull BitmapLoadRequest lastBitmapLoadRequest;
+
+  /**
+   * Creates an instance that is able to cache the last bitmap load request to the given bitmap
+   * loader.
+   */
+  public CacheBitmapLoader(BitmapLoader bitmapLoader) {
+    this.bitmapLoader = bitmapLoader;
+  }
+
+  @Override
+  public ListenableFuture<Bitmap> decodeBitmap(byte[] data) {
+    if (lastBitmapLoadRequest != null && lastBitmapLoadRequest.matches(data)) {
+      return lastBitmapLoadRequest.getFuture();
+    }
+    ListenableFuture<Bitmap> future = bitmapLoader.decodeBitmap(data);
+    lastBitmapLoadRequest = new BitmapLoadRequest(data, future);
+    return future;
+  }
+
+  @Override
+  public ListenableFuture<Bitmap> loadBitmap(Uri uri) {
+    if (lastBitmapLoadRequest != null && lastBitmapLoadRequest.matches(uri)) {
+      return lastBitmapLoadRequest.getFuture();
+    }
+    ListenableFuture<Bitmap> future = bitmapLoader.loadBitmap(uri);
+    lastBitmapLoadRequest = new BitmapLoadRequest(uri, 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.
+   */
+  private static class BitmapLoadRequest {
+    @Nullable private final byte[] data;
+    @Nullable private final Uri uri;
+    @Nullable private final ListenableFuture<Bitmap> future;
+
+    public BitmapLoadRequest(byte[] data, ListenableFuture<Bitmap> future) {
+      this.data = data;
+      this.uri = null;
+      this.future = future;
+    }
+
+    public BitmapLoadRequest(Uri uri, ListenableFuture<Bitmap> future) {
+      this.data = null;
+      this.uri = uri;
+      this.future = future;
+    }
+
+    /** Whether the bitmap load request was performed for {@code data}. */
+    public boolean matches(@Nullable 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) {
+      return this.uri != null && this.uri.equals(uri);
+    }
+
+    /** Returns the future that set for the bitmap load request. */
+    public ListenableFuture<Bitmap> getFuture() {
+      return checkStateNotNull(future);
+    }
+  }
+}

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

@@ -32,7 +32,6 @@ import android.app.NotificationChannel;
 import android.app.NotificationManager;
 import android.app.NotificationManager;
 import android.content.Context;
 import android.content.Context;
 import android.graphics.Bitmap;
 import android.graphics.Bitmap;
-import android.net.Uri;
 import android.os.Bundle;
 import android.os.Bundle;
 import android.os.Handler;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.Looper;
@@ -196,15 +195,15 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
     }
     }
 
 
     /**
     /**
-     * Sets the {@link BitmapLoader} used load artwork. By default, a {@link SimpleBitmapLoader}
-     * will be used.
+     * Sets the {@link BitmapLoader} used load artwork. By default, a {@link CacheBitmapLoader} with
+     * a {@link SimpleBitmapLoader} inside will be used.
      *
      *
      * @param bitmapLoader The bitmap loader.
      * @param bitmapLoader The bitmap loader.
      * @return This builder.
      * @return This builder.
      */
      */
     @CanIgnoreReturnValue
     @CanIgnoreReturnValue
     public Builder setBitmapLoader(BitmapLoader bitmapLoader) {
     public Builder setBitmapLoader(BitmapLoader bitmapLoader) {
-      this.bitmapLoader = bitmapLoader;
+      this.bitmapLoader = new CacheBitmapLoader(bitmapLoader);
       return this;
       return this;
     }
     }
 
 
@@ -261,7 +260,6 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
   private final BitmapLoader bitmapLoader;
   private final BitmapLoader bitmapLoader;
   // Cache the last bitmap load request to avoid reloading the bitmap again, particularly useful
   // Cache the last bitmap load request to avoid reloading the bitmap again, particularly useful
   // when 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 BitmapLoadRequest lastBitmapLoadRequest;
   private final Handler mainHandler;
   private final Handler mainHandler;
 
 
   private @MonotonicNonNull OnBitmapLoadedFutureCallback pendingOnBitmapLoadedFutureCallback;
   private @MonotonicNonNull OnBitmapLoadedFutureCallback pendingOnBitmapLoadedFutureCallback;
@@ -272,11 +270,10 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
     this.notificationIdProvider = builder.notificationIdProvider;
     this.notificationIdProvider = builder.notificationIdProvider;
     this.channelId = builder.channelId;
     this.channelId = builder.channelId;
     this.channelNameResourceId = builder.channelNameResourceId;
     this.channelNameResourceId = builder.channelNameResourceId;
-    this.bitmapLoader = builder.bitmapLoader;
+    this.bitmapLoader = new CacheBitmapLoader(builder.bitmapLoader);
     notificationManager =
     notificationManager =
         checkStateNotNull(
         checkStateNotNull(
             (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE));
             (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE));
-    lastBitmapLoadRequest = new BitmapLoadRequest();
     mainHandler = new Handler(Looper.getMainLooper());
     mainHandler = new Handler(Looper.getMainLooper());
     smallIconResourceId = R.drawable.media3_notification_small_icon;
     smallIconResourceId = R.drawable.media3_notification_small_icon;
   }
   }
@@ -590,15 +587,10 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
   @Nullable
   @Nullable
   private ListenableFuture<Bitmap> loadArtworkBitmap(MediaMetadata metadata) {
   private ListenableFuture<Bitmap> loadArtworkBitmap(MediaMetadata metadata) {
     @Nullable ListenableFuture<Bitmap> future;
     @Nullable ListenableFuture<Bitmap> future;
-    if (lastBitmapLoadRequest.matches(metadata.artworkData)
-        || lastBitmapLoadRequest.matches(metadata.artworkUri)) {
-      future = lastBitmapLoadRequest.getFuture();
-    } else if (metadata.artworkData != null) {
+    if (metadata.artworkData != null) {
       future = bitmapLoader.decodeBitmap(metadata.artworkData);
       future = bitmapLoader.decodeBitmap(metadata.artworkData);
-      lastBitmapLoadRequest.setBitmapFuture(metadata.artworkData, future);
     } else if (metadata.artworkUri != null) {
     } else if (metadata.artworkUri != null) {
       future = bitmapLoader.loadBitmap(metadata.artworkUri);
       future = bitmapLoader.loadBitmap(metadata.artworkUri);
-      lastBitmapLoadRequest.setBitmapFuture(metadata.artworkUri, future);
     } else {
     } else {
       future = null;
       future = null;
     }
     }
@@ -654,54 +646,4 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
       }
       }
     }
     }
   }
   }
-
-  /**
-   * 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.
-   */
-  private static class BitmapLoadRequest {
-    @Nullable private byte[] data;
-    @Nullable private Uri uri;
-    @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);
-    }
-
-    /**
-     * 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);
-    }
-
-    /**
-     * 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.bitmapFuture = bitmapFuture;
-      this.uri = null;
-    }
-
-    /**
-     * 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.bitmapFuture = bitmapFuture;
-      this.data = null;
-    }
-  }
 }
 }

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

@@ -399,11 +399,12 @@ public class DefaultMediaNotificationProviderTest {
   }
   }
 
 
   /**
   /**
-   * Tests that the {@link DefaultMediaNotificationProvider} will not request to load the same
-   * artwork bitmap again, if the same bitmap has been requested already.
+   * Tests that the {@link DefaultMediaNotificationProvider} will discard the pending {@link
+   * MediaNotification.Provider.Callback#onNotificationChanged(MediaNotification)}, if there is a
+   * new request.
    */
    */
   @Test
   @Test
-  public void requestsSameBitmap_withPendingRequest_oneRequestOnly() {
+  public void createNotification_withNewRequest_discardPendingCallback() {
     // We will advance the main looper manually in the test.
     // We will advance the main looper manually in the test.
     shadowOf(Looper.getMainLooper()).pause();
     shadowOf(Looper.getMainLooper()).pause();
     // Create a MediaSession whose player returns non-null media metadata so that the
     // Create a MediaSession whose player returns non-null media metadata so that the
@@ -434,7 +435,6 @@ public class DefaultMediaNotificationProviderTest {
         defaultActionFactory,
         defaultActionFactory,
         mockOnNotificationChangedCallback1);
         mockOnNotificationChangedCallback1);
     ShadowLooper.idleMainLooper();
     ShadowLooper.idleMainLooper();
-    verify(mockBitmapLoader).loadBitmap(Uri.parse("http://example.test/image.jpg"));
     verifyNoInteractions(mockOnNotificationChangedCallback1);
     verifyNoInteractions(mockOnNotificationChangedCallback1);
     MediaNotification.Provider.Callback mockOnNotificationChangedCallback2 =
     MediaNotification.Provider.Callback mockOnNotificationChangedCallback2 =
         mock(MediaNotification.Provider.Callback.class);
         mock(MediaNotification.Provider.Callback.class);
@@ -446,8 +446,6 @@ public class DefaultMediaNotificationProviderTest {
     // The bitmap has arrived.
     // The bitmap has arrived.
     bitmapFuture.set(Bitmap.createBitmap(/* width= */ 8, /* height= */ 8, Bitmap.Config.RGB_565));
     bitmapFuture.set(Bitmap.createBitmap(/* width= */ 8, /* height= */ 8, Bitmap.Config.RGB_565));
     ShadowLooper.idleMainLooper();
     ShadowLooper.idleMainLooper();
-
-    verifyNoMoreInteractions(mockBitmapLoader);
     verify(mockOnNotificationChangedCallback2).onNotificationChanged(any());
     verify(mockOnNotificationChangedCallback2).onNotificationChanged(any());
     verifyNoInteractions(mockOnNotificationChangedCallback1);
     verifyNoInteractions(mockOnNotificationChangedCallback1);
   }
   }