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: #644 PiperOrigin-RevId: 590904037 (cherry picked from commit 4974f96)
androidx · Jan 9, 2024 · 09d30a1 · 09d30a1
1 parent c8a403e
commit 09d30a1
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 7 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -52,6 +52,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));