From 4974f960e7b8a34a37c2206b87973339c0e81cef Mon Sep 17 00:00:00 2001 From: bachinger Date: Thu, 14 Dec 2023 05:18:18 -0800 Subject: [PATCH] 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 --- RELEASENOTES.md | 2 + .../session/MediaLibrarySessionImpl.java | 10 ++- ...wserCompatWithMediaLibraryServiceTest.java | 66 +++++++++++++++++++ .../session/MockMediaLibraryService.java | 3 +- 4 files changed, 74 insertions(+), 7 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 57573c24fe..f34b078c85 100644 --- 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 index 27dfad70c4..c6c8aa7e32 100644 --- 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 @@ import java.util.concurrent.Future; public void notifyChildrenChanged( String parentId, int itemCount, @Nullable LibraryParams params) { - dispatchRemoteControllerTaskWithoutReturn( - (callback, seq) -> { - if (isSubscribed(callback, parentId)) { - callback.onChildrenChanged(seq, parentId, itemCount, params); - } - }); + List connectedControllers = instance.getConnectedControllers(); + for (int i = 0; i < connectedControllers.size(); i++) { + notifyChildrenChanged(connectedControllers.get(i), parentId, itemCount, params); + } } public void notifyChildrenChanged( diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserCompatWithMediaLibraryServiceTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserCompatWithMediaLibraryServiceTest.java index cd5bf3c205..effbead386 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserCompatWithMediaLibraryServiceTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserCompatWithMediaLibraryServiceTest.java @@ -19,6 +19,7 @@ import static androidx.media.utils.MediaConstants.BROWSER_SERVICE_EXTRAS_KEY_SEA 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_Q 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 class MediaBrowserCompatWithMediaLibraryServiceTest 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 parentIds = new ArrayList<>(); + List> 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 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 parentIds = new ArrayList<>(); + List> 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 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/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java b/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java index 7aa60a333e..1f90c68609 100644 --- a/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java +++ b/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java @@ -337,7 +337,8 @@ public class MockMediaLibraryService extends MediaLibraryService { 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));