Cache children when subscribing

Childrens returned by the legacy service when a Media3
browser connects are cached and returned with the first
`getChildren` call in case the same `paranetid` is
requested.

In any other case the cache is immediately cleared.

#cherrypick

PiperOrigin-RevId: 686157511

RELEASENOTES.md

@@ -139,6 +139,9 @@
     *   Fix bug where a `MediaBrowser` connected to a legacy browser service,
         didn't receive an error sent by the service after the browser has
         subscribed to a `parentid`.
+    *   Improve interoperability behavior, so that a Media3 browser that is
+        connected to a legacy `MediaBrowserService` doesn't request the children
+        of a `parentId` twice when subscribing to a parent.
 *   UI:
     *   Make the stretched/cropped video in
         `PlayerView`-in-Compose-`AndroidView` workaround opt-in, due to issues

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

@@ -199,12 +199,54 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization;
       return Futures.immediateFuture(LibraryResult.ofError(ERROR_SESSION_DISCONNECTED));
     }
 
-    SettableFuture<LibraryResult<ImmutableList<MediaItem>>> future = SettableFuture.create();
     Bundle options = createOptionsWithPagingInfo(params, page, pageSize);
-    browserCompat.subscribe(parentId, options, new GetChildrenCallback(future, parentId));
+    SettableFuture<LibraryResult<ImmutableList<MediaItem>>> future = SettableFuture.create();
+    // Try to get the cached children in case this is the first call right after subscribing.
+    List<MediaBrowserCompat.MediaItem> childrenFromCache =
+        getChildrenFromSubscription(parentId, page);
+    // Always evict to avoid memory leaks. We've done what we can.
+    evictChildrenFromSubscription(parentId);
+    if (childrenFromCache != null) {
+      future.set(
+          LibraryResult.ofItemList(
+              LegacyConversions.convertBrowserItemListToMediaItemList(childrenFromCache),
+              new LibraryParams.Builder().setExtras(options).build()));
+    } else {
+      GetChildrenCallback getChildrenCallback = new GetChildrenCallback(future, parentId);
+      browserCompat.subscribe(parentId, options, getChildrenCallback);
+    }
     return future;
   }
 
+  @Nullable
+  private List<MediaBrowserCompat.MediaItem> getChildrenFromSubscription(
+      String parentId, int page) {
+    List<SubscribeCallback> callbacks = subscribeCallbacks.get(parentId);
+    if (callbacks == null) {
+      return null;
+    }
+    for (int i = 0; i < callbacks.size(); i++) {
+      if (callbacks.get(i).canServeGetChildrenRequest(parentId, page)) {
+        return callbacks.get(i).receivedChildren;
+      }
+    }
+    return null;
+  }
+
+  private void evictChildrenFromSubscription(String parentId) {
+    List<SubscribeCallback> callbacks = subscribeCallbacks.get(parentId);
+    if (callbacks == null) {
+      return;
+    }
+    for (int i = 0; i < callbacks.size(); i++) {
+      if (callbacks.get(i).receivedChildren != null) {
+        // Evict the first cached children we find.
+        callbacks.get(i).receivedChildren = null;
+        return;
+      }
+    }
+  }
+
   @Override
   public ListenableFuture<LibraryResult<MediaItem>> getItem(String mediaId) {
     if (!getInstance().isSessionCommandAvailable(SessionCommand.COMMAND_CODE_LIBRARY_GET_ITEM)) {
@@ -471,6 +513,8 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization;
     private final Bundle subscriptionOptions;
     private final SettableFuture<LibraryResult<Void>> future;
 
+    @Nullable private List<MediaBrowserCompat.MediaItem> receivedChildren;
+
     public SubscribeCallback(
         String subscriptionParentId,
         Bundle subscriptionOptions,
@@ -535,24 +579,43 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization;
         // Browser is closed.
         return;
       }
-      int itemCount;
+      if (children == null) {
-      if (children != null) {
+        // Note this doesn't happen except possibly when someone is using a very old OS (change
-        itemCount = children.size();
+        // landed in Android tree at 2016-02-23). Recent Android versions turn children=null into an
-      } else {
+        // error and `onError` of this `SubscriptionCallback` is called instead of
-        // Currently no way to tell failures in MediaBrowser#subscribe().
+        // `onChildrenLoaded` (see `MediaBrowser.onLoadChildren`). We should do the same here to be
+        // consistent again.
+        onError(subscriptionParentId, subscriptionOptions);
         return;
       }
 
       LibraryParams params =
           LegacyConversions.convertToLibraryParams(
               context, browserCompat.getNotifyChildrenChangedOptions());
+      receivedChildren = children;
       getInstance()
           .notifyBrowserListener(
               listener -> {
-                listener.onChildrenChanged(getInstance(), parentId, itemCount, params);
+                listener.onChildrenChanged(getInstance(), parentId, children.size(), params);
               });
       future.set(LibraryResult.ofVoid());
     }
+
+    /**
+     * Returns true if the cached children can be served for a request for the given parent ID and
+     * paging options.
+     *
+     * @param parentId The media ID of the parent of the requested children.
+     * @param pageIndex The requested page index.
+     * @return True if the request can be served with the cached children of the subscription
+     *     callback.
+     */
+    public boolean canServeGetChildrenRequest(String parentId, int pageIndex) {
+      return subscriptionParentId.equals(parentId)
+          && receivedChildren != null
+          && pageIndex
+              == subscriptionOptions.getInt(MediaBrowserCompat.EXTRA_PAGE, /* defaultValue= */ 0);
+    }
   }
 
   private class GetChildrenCallback extends SubscriptionCallback {

libraries/test_session_common/src/main/java/androidx/media3/test/session/common/MediaBrowserServiceCompatConstants.java

@@ -25,6 +25,8 @@ public class MediaBrowserServiceCompatConstants {
   public static final String TEST_GET_CHILDREN = "getChildren_correctMetadataExtras";
   public static final String TEST_GET_CHILDREN_WITH_NULL_LIST =
       "onChildrenChanged_withNullChildrenListInLegacyService_convertedToSessionError";
+  public static final String TEST_GET_CHILDREN_INCREASE_NUMBER_OF_CHILDREN_WITH_EACH_CALL =
+      "onChildrenChanged_cacheChildrenOfSubscribeCall_serviceCalledOnceOnly";
   public static final String TEST_GET_CHILDREN_FATAL_AUTHENTICATION_ERROR =
       "getLibraryRoot_fatalAuthenticationError_receivesPlaybackException";
   public static final String TEST_GET_CHILDREN_NON_FATAL_AUTHENTICATION_ERROR =

libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserListenerWithMediaBrowserServiceCompatTest.java

@@ -29,6 +29,7 @@ import static androidx.media3.test.session.common.MediaBrowserConstants.ROOT_ID_
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_CONNECT_REJECTED;
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN;
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN_FATAL_AUTHENTICATION_ERROR;
+import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN_INCREASE_NUMBER_OF_CHILDREN_WITH_EACH_CALL;
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN_NON_FATAL_AUTHENTICATION_ERROR;
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN_WITH_NULL_LIST;
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_LIBRARY_ROOT;
@@ -380,6 +381,60 @@ public class MediaBrowserListenerWithMediaBrowserServiceCompatTest {
         .isEqualTo(SessionError.DEFAULT_ERROR_MESSAGE);
   }
 
+  @Test
+  public void onChildrenChanged_cacheChildrenOfSubscribeCall_serviceCalledOnceOnly()
+      throws Exception {
+    String testParentId = TEST_GET_CHILDREN_INCREASE_NUMBER_OF_CHILDREN_WITH_EACH_CALL;
+    remoteService.setProxyForTest(TEST_GET_CHILDREN_INCREASE_NUMBER_OF_CHILDREN_WITH_EACH_CALL);
+    CountDownLatch latch = new CountDownLatch(/* count= */ 1);
+    MediaBrowser browser =
+        createBrowser(
+            new MediaBrowser.Listener() {
+              @Override
+              public void onChildrenChanged(
+                  MediaBrowser browser,
+                  String parentId,
+                  int itemCount,
+                  @Nullable LibraryParams params) {
+                latch.countDown();
+              }
+            });
+    // Subscribing causes the first call to the legacy `onLoadChildren()` that we want to cache.
+    LibraryResult<Void> resultForSubscribe =
+        threadTestRule
+            .getHandler()
+            .postAndSync(() -> browser.subscribe(testParentId, null))
+            .get(TIMEOUT_MS, MILLISECONDS);
+    assertThat(resultForSubscribe.resultCode).isEqualTo(RESULT_SUCCESS);
+    assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
+
+    LibraryResult<ImmutableList<MediaItem>> resultGetChildren =
+        threadTestRule
+            .getHandler()
+            .postAndSync(
+                () ->
+                    browser.getChildren(
+                        testParentId, /* page= */ 0, /* pageSize= */ 12, /* params= */ null))
+            .get(TIMEOUT_MS, MILLISECONDS);
+
+    assertThat(resultGetChildren.resultCode).isEqualTo(RESULT_SUCCESS);
+    // If caching in `MediaBrowserImplLegacy` doesn't work, the children would be delivered by a
+    // second call to the service which would have two items.
+    assertThat(resultGetChildren.value).hasSize(1);
+
+    // Cache is cleared after delivery. We call the service again that now delivers two items.
+    LibraryResult<ImmutableList<MediaItem>> resultGetChildrenAgain =
+        threadTestRule
+            .getHandler()
+            .postAndSync(
+                () ->
+                    browser.getChildren(
+                        testParentId, /* page= */ 0, /* pageSize= */ 12, /* params= */ null))
+            .get(TIMEOUT_MS, MILLISECONDS);
+
+    assertThat(resultGetChildrenAgain.value).hasSize(2);
+  }
+
   @Test
   public void getLibraryRoot_correctExtraKeyAndValue() throws Exception {
     remoteService.setProxyForTest(TEST_GET_LIBRARY_ROOT);

libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaBrowserServiceCompat.java

@@ -29,6 +29,7 @@ import static androidx.media3.test.session.common.MediaBrowserConstants.ROOT_ID_
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_CONNECT_REJECTED;
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN;
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN_FATAL_AUTHENTICATION_ERROR;
+import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN_INCREASE_NUMBER_OF_CHILDREN_WITH_EACH_CALL;
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN_NON_FATAL_AUTHENTICATION_ERROR;
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN_WITH_NULL_LIST;
 import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_LIBRARY_ROOT;
@@ -197,6 +198,7 @@ public class MockMediaBrowserServiceCompat extends MediaBrowserServiceCompat {
         return;
       }
     }
+    super.onLoadChildren(parentId, result, Bundle.EMPTY);
   }
 
   @Override
@@ -302,6 +304,9 @@ public class MockMediaBrowserServiceCompat extends MediaBrowserServiceCompat {
         case TEST_SUBSCRIBE_THEN_REJECT_ON_LOAD_CHILDREN:
           setProxyForSubscribeAndRejectGetChildren();
           break;
+        case TEST_GET_CHILDREN_INCREASE_NUMBER_OF_CHILDREN_WITH_EACH_CALL:
+          setProxyForGetChildrenIncreaseNumberOfChildrenWithEachCall();
+          break;
         default:
           throw new IllegalArgumentException("Unknown testName: " + testName);
       }
@@ -598,6 +603,31 @@ public class MockMediaBrowserServiceCompat extends MediaBrowserServiceCompat {
             }
           });
     }
+
+    private void setProxyForGetChildrenIncreaseNumberOfChildrenWithEachCall() {
+      setMediaBrowserServiceProxy(
+          new MockMediaBrowserServiceCompat.Proxy() {
+            private int callCount;
+
+            @Override
+            public BrowserRoot onGetRoot(
+                String clientPackageName, int clientUid, @Nullable Bundle rootHints) {
+              return new BrowserRoot(ROOT_ID, ROOT_EXTRAS);
+            }
+
+            @Override
+            public void onLoadChildren(String parentId, Result<List<MediaItem>> result) {
+              super.onLoadChildren(parentId, result, Bundle.EMPTY);
+            }
+
+            @Override
+            public void onLoadChildren(
+                String parentId, Result<List<MediaItem>> result, Bundle options) {
+              result.sendResult(MediaTestUtils.createBrowserItems(callCount + 1));
+              callCount++;
+            }
+          });
+    }
   }
 
   private static ImmutableList<MediaItem> createMediaItems() {