Skip to content

Commit

Permalink
Fix broadcasting notifyChildrenChanged for legacy controllers
Browse files Browse the repository at this point in the history
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
  • Loading branch information
marcbaechinger authored and Copybara-Service committed Dec 14, 2023
1 parent 8447181 commit 4974f96
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 7 deletions.
2 changes: 2 additions & 0 deletions RELEASENOTES.md
Expand Up @@ -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
Expand Down
Expand Up @@ -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(
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Expand Up @@ -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));
Expand Down

0 comments on commit 4974f96

Please sign in to comment.