Fix broadcasting notifyChildrenChanged for legacy controllers

When broadcasting a notifyChildrenChanged event, the task for legacy controllers was sent to the broadcasting callback. This would technically work, but because the subscription list is maintained with specific controllers, the broadcast controller isn't subscribed and hence the call wasn't executed. This change calls the overloaded method for a specific controller for each connected controller. Making sure (only) subscribed controllers are notified. Issue: androidx/media#644 PiperOrigin-RevId: 590904037
DiceTechnology · Dec 14, 2023 · 4974f96 · 4974f96
1 parent 8447181
commit 4974f96
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 7 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -100,6 +100,8 @@
         `MediaMetadataCompat`
         ([#756](https://github.com/androidx/media/issues/756),
         [#802](https://github.com/androidx/media/issues/802)).
+    *   Fix broadcasting `notifyChildrenChanged` for legacy controllers
+        ([#644](https://github.com/androidx/media/issues/644)).
 *   UI:
     *   Fix issue where forward and rewind buttons are not visible when used
         with Material Design in a BottomSheetDialogFragment

diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaLibrarySessionImpl.java b/libraries/session/src/main/java/androidx/media3/session/MediaLibrarySessionImpl.java
@@ -256,12 +256,10 @@ public ListenableFuture<LibraryResult<Void>> onUnsubscribeOnHandler(
 
   public void notifyChildrenChanged(
       String parentId, int itemCount, @Nullable LibraryParams params) {
-    dispatchRemoteControllerTaskWithoutReturn(
-        (callback, seq) -> {
-          if (isSubscribed(callback, parentId)) {
-            callback.onChildrenChanged(seq, parentId, itemCount, params);
-          }
-        });
+    List<ControllerInfo> connectedControllers = instance.getConnectedControllers();
+    for (int i = 0; i < connectedControllers.size(); i++) {
+      notifyChildrenChanged(connectedControllers.get(i), parentId, itemCount, params);
+    }
   }
 
   public void notifyChildrenChanged(

diff --git a/...droidTest/java/androidx/media3/session/MediaBrowserCompatWithMediaLibraryServiceTest.java b/...droidTest/java/androidx/media3/session/MediaBrowserCompatWithMediaLibraryServiceTest.java
@@ -19,6 +19,7 @@
 import static androidx.media3.session.MediaConstants.EXTRAS_KEY_COMPLETION_STATUS;
 import static androidx.media3.session.MediaConstants.EXTRAS_VALUE_COMPLETION_STATUS_PARTIALLY_PLAYED;
 import static androidx.media3.session.MockMediaLibraryService.CONNECTION_HINTS_CUSTOM_LIBRARY_ROOT;
+import static androidx.media3.session.MockMediaLibraryService.createNotifyChildrenChangedBundle;
 import static androidx.media3.test.session.common.CommonConstants.METADATA_ARTWORK_URI;
 import static androidx.media3.test.session.common.CommonConstants.METADATA_DESCRIPTION;
 import static androidx.media3.test.session.common.CommonConstants.METADATA_EXTRA_KEY;
@@ -52,6 +53,7 @@
 import static androidx.media3.test.session.common.MediaBrowserConstants.SEARCH_QUERY_LONG_LIST;
 import static androidx.media3.test.session.common.MediaBrowserConstants.SEARCH_RESULT;
 import static androidx.media3.test.session.common.MediaBrowserConstants.SEARCH_RESULT_COUNT;
+import static androidx.media3.test.session.common.MediaBrowserConstants.SUBSCRIBE_PARENT_ID_2;
 import static androidx.media3.test.session.common.TestUtils.LONG_TIMEOUT_MS;
 import static androidx.media3.test.session.common.TestUtils.NO_RESPONSE_TIMEOUT_MS;
 import static androidx.media3.test.session.common.TestUtils.SERVICE_CONNECTION_TIMEOUT_MS;
@@ -481,6 +483,70 @@ public void onChildrenLoaded(String parentId, List<MediaItem> children, Bundle o
     assertThat(onChildrenLoadedWithBundleCalled.get()).isFalse();
   }
 
+  @Test
+  public void getChildren_browserNotifyChildrenChanged_callsOnChildrenLoadedTwice()
+      throws Exception {
+    String testParentId = SUBSCRIBE_PARENT_ID_2;
+    connectAndWait(/* rootHints= */ Bundle.EMPTY);
+    CountDownLatch latch = new CountDownLatch(2);
+    List<String> parentIds = new ArrayList<>();
+    List<List<MediaItem>> childrenList = new ArrayList<>();
+    Bundle requestNotifyChildrenWithDelayBundle =
+        createNotifyChildrenChangedBundle(
+            testParentId, /* itemCount= */ 12, /* delayMs= */ 100L, /* broadcast= */ false);
+    requestNotifyChildrenWithDelayBundle.putInt(MediaBrowserCompat.EXTRA_PAGE_SIZE, 12);
+
+    browserCompat.subscribe(
+        testParentId,
+        requestNotifyChildrenWithDelayBundle,
+        new SubscriptionCallback() {
+          @Override
+          public void onChildrenLoaded(String parentId, List<MediaItem> children, Bundle options) {
+            parentIds.add(parentId);
+            childrenList.add(children);
+            latch.countDown();
+          }
+        });
+
+    assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
+    assertThat(parentIds).containsExactly(testParentId, testParentId);
+    assertThat(childrenList).hasSize(2);
+    assertThat(childrenList.get(0)).hasSize(12);
+    assertThat(childrenList.get(1)).hasSize(12);
+  }
+
+  @Test
+  public void getChildren_broadcastNotifyChildrenChanged_callsOnChildrenLoadedTwice()
+      throws Exception {
+    String testParentId = SUBSCRIBE_PARENT_ID_2;
+    connectAndWait(/* rootHints= */ Bundle.EMPTY);
+    CountDownLatch latch = new CountDownLatch(2);
+    List<String> parentIds = new ArrayList<>();
+    List<List<MediaItem>> childrenList = new ArrayList<>();
+    Bundle requestNotifyChildrenWithDelayBundle =
+        createNotifyChildrenChangedBundle(
+            testParentId, /* itemCount= */ 12, /* delayMs= */ 100L, /* broadcast= */ true);
+    requestNotifyChildrenWithDelayBundle.putInt(MediaBrowserCompat.EXTRA_PAGE_SIZE, 12);
+
+    browserCompat.subscribe(
+        testParentId,
+        requestNotifyChildrenWithDelayBundle,
+        new SubscriptionCallback() {
+          @Override
+          public void onChildrenLoaded(String parentId, List<MediaItem> children, Bundle options) {
+            parentIds.add(parentId);
+            childrenList.add(children);
+            latch.countDown();
+          }
+        });
+
+    assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
+    assertThat(parentIds).containsExactly(testParentId, testParentId);
+    assertThat(childrenList).hasSize(2);
+    assertThat(childrenList.get(0)).hasSize(12);
+    assertThat(childrenList.get(1)).hasSize(12);
+  }
+
   @Test
   public void getChildren_commandGetChildrenNotAvailable_reportsError() throws Exception {
     Bundle rootHints = new Bundle();

diff --git a/...s/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java b/...s/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java
@@ -337,7 +337,8 @@ public ListenableFuture<LibraryResult<ImmutableList<MediaItem>>> onGetChildren(
       assertLibraryParams(params);
       if (Objects.equals(parentId, PARENT_ID_NO_CHILDREN)) {
         return Futures.immediateFuture(LibraryResult.ofItemList(ImmutableList.of(), params));
-      } else if (Objects.equals(parentId, PARENT_ID)) {
+      } else if (Objects.equals(parentId, PARENT_ID)
+          || Objects.equals(parentId, SUBSCRIBE_PARENT_ID_2)) {
         return Futures.immediateFuture(
             LibraryResult.ofItemList(
                 getPaginatedResult(GET_CHILDREN_RESULT, page, pageSize), params));