Skip to content

Commit

Permalink
Only use result.sendError where supported by legacy media library
Browse files Browse the repository at this point in the history 
`MediaLibraryServiceLegacyStub` handles various edge cases by calling
`result.sendError(null)` with the intention to send back an error to
the legacy browser [1].

`MediaBrowserServiceCompat` of the legacy media1 Compat library has an
inner base class `Result` that has a default implementation of
`onErrorSent` that throws an `UnsupportedOperationException` [2].
However, most anonymous inner classes for `Result` created in
`MediaBrowserServiceCompat` do not override `onErrorSent` [3].

Hence Media3 must not call `sendError` in these cases. Instead we call
`sendResult(null)` according to what the default implementation of
the callbacks in `MediaBrowserServiceCompat` do ([4] as an example).

Issue: #78
Issue: #334

[1] https://github.com/androidx/media/blob/release/libraries/session/src/main/java/androidx/media3/session/MediaLibraryServiceLegacyStub.java#L200
[2] https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:media/media/src/main/java/androidx/media/MediaBrowserServiceCompat.java;l=872
[3] https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:media/media/src/main/java/androidx/media/MediaBrowserServiceCompat.java;l=578-604?q=MediaBrowserServiceCompat&ss=androidx
[4] https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:media/media/src/main/java/androidx/media/MediaBrowserServiceCompat.java;l=1395

PiperOrigin-RevId: 551210137
(cherry picked from commit 17ee527)
  • Loading branch information
@marcbaechinger @tianyif
marcbaechinger authored and tianyif committed Jul 26, 2023
1 parent ac175a9 commit ab2505d
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 22 deletions.
4 changes: 4 additions & 0 deletions RELEASENOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
do this in `MediaSession.Callback.onConnect` by using an
`AcceptedResultBuilder` to make sure the custom layout is available to
the controller when connection completes.
* Fix cases where `MediaLibraryServiceLegacyStub` sent an error to a
`Result` that didn't support this which produced an
`UnsuportedOperationException`
([#78](https://github.com/androidx/media/issues/78)).
* Test Utilities:
* Add a `nanoTime()` method to `Clock` to provide override support of
`System.nanoTime()`
Expand Down
22 changes: 11 additions & 11 deletions libraries/session/src/main/java/androidx/media3/session/MediaLibraryServiceLegacyStub.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,12 @@ public void onLoadChildren(
@Nullable Bundle options) {
@Nullable ControllerInfo controller = getCurrentController();
if (controller == null) {
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
return;
}
if (TextUtils.isEmpty(parentId)) {
Log.w(TAG, "onLoadChildren(): Ignoring empty parentId from " + controller);
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
return;
}
result.detach();
Expand All @@ -212,7 +212,7 @@ public void onLoadChildren(
if (!getConnectedControllersManager()
.isSessionCommandAvailable(
controller, SessionCommand.COMMAND_CODE_LIBRARY_GET_CHILDREN)) {
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
return;
}
if (options != null) {
Expand Down Expand Up @@ -260,12 +260,12 @@ public void onLoadChildren(
public void onLoadItem(String itemId, Result<MediaBrowserCompat.MediaItem> result) {
@Nullable ControllerInfo controller = getCurrentController();
if (controller == null) {
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
return;
}
if (TextUtils.isEmpty(itemId)) {
Log.w(TAG, "Ignoring empty itemId from " + controller);
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
return;
}
result.detach();
Expand All @@ -275,7 +275,7 @@ public void onLoadItem(String itemId, Result<MediaBrowserCompat.MediaItem> resul
if (!getConnectedControllersManager()
.isSessionCommandAvailable(
controller, SessionCommand.COMMAND_CODE_LIBRARY_GET_ITEM)) {
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
return;
}
ListenableFuture<LibraryResult<MediaItem>> future =
Expand All @@ -291,12 +291,12 @@ public void onSearch(
String query, @Nullable Bundle extras, Result<List<MediaBrowserCompat.MediaItem>> result) {
@Nullable ControllerInfo controller = getCurrentController();
if (controller == null) {
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
return;
}
if (TextUtils.isEmpty(query)) {
Log.w(TAG, "Ignoring empty query from " + controller);
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
return;
}
if (!(controller.getControllerCb() instanceof BrowserLegacyCb)) {
Expand All @@ -308,7 +308,7 @@ public void onSearch(
() -> {
if (!getConnectedControllersManager()
.isSessionCommandAvailable(controller, SessionCommand.COMMAND_CODE_LIBRARY_SEARCH)) {
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
return;
}
BrowserLegacyCb cb = (BrowserLegacyCb) checkStateNotNull(controller.getControllerCb());
Expand Down Expand Up @@ -389,7 +389,7 @@ private static void sendLibraryResultWithMediaItemWhenReady(
result.sendResult(mediaItem);
} catch (CancellationException | ExecutionException | InterruptedException e) {
Log.w(TAG, "Library operation failed", e);
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
}
},
MoreExecutors.directExecutor());
Expand All @@ -408,7 +408,7 @@ private static void sendLibraryResultWithMediaItemsWhenReady(
: MediaUtils.truncateListBySize(mediaItems, TRANSACTION_SIZE_LIMIT_IN_BYTES));
} catch (CancellationException | ExecutionException | InterruptedException e) {
Log.w(TAG, "Library operation failed", e);
result.sendError(/* extras= */ null);
result.sendResult(/* result= */ null);
}
},
MoreExecutors.directExecutor());
Expand Down
86 changes: 84 additions & 2 deletions ...droidTest/java/androidx/media3/session/MediaBrowserCompatWithMediaLibraryServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import static androidx.media3.test.session.common.MediaBrowserConstants.SEARCH_RESULT_COUNT;
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;
import static androidx.media3.test.session.common.TestUtils.TIMEOUT_MS;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
Expand Down Expand Up @@ -210,6 +211,29 @@ public void onError(String itemId) {
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
}

@Test
public void getItem_commandGetItemNotAvailable_reportsNull() throws Exception {
Bundle rootHints = new Bundle();
rootHints.putInt(
MockMediaLibraryService.CONNECTION_HINTS_KEY_REMOVE_COMMAND_CODE,
SessionCommand.COMMAND_CODE_LIBRARY_GET_ITEM);
connectAndWait(rootHints);
CountDownLatch latch = new CountDownLatch(1);
List<MediaItem> capturedMediaItems = new ArrayList<>();
browserCompat.getItem(
MEDIA_ID_GET_BROWSABLE_ITEM,
new ItemCallback() {
@Override
public void onItemLoaded(MediaItem item) {
capturedMediaItems.add(item);
latch.countDown();
}
});

assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
assertThat(capturedMediaItems).containsExactly((Object) null);
}

@Test
public void getChildren() throws Exception {
String testParentId = PARENT_ID;
Expand Down Expand Up @@ -415,6 +439,34 @@ public void onChildrenLoaded(String parentId, List<MediaItem> children, Bundle o
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
}

@Test
public void getChildren_commandGetChildrenNotAvailable_reportsError() throws Exception {
Bundle rootHints = new Bundle();
rootHints.putInt(
MockMediaLibraryService.CONNECTION_HINTS_KEY_REMOVE_COMMAND_CODE,
SessionCommand.COMMAND_CODE_LIBRARY_GET_CHILDREN);
handler.postAndSync(
() -> {
browserCompat =
new MediaBrowserCompat(context, getServiceComponent(), connectionCallback, rootHints);
});
browserCompat.connect();
assertThat(connectionCallback.connectedLatch.await(SERVICE_CONNECTION_TIMEOUT_MS, MILLISECONDS))
.isTrue();
CountDownLatch errorLatch = new CountDownLatch(1);

browserCompat.subscribe(
PARENT_ID,
new MediaBrowserCompat.SubscriptionCallback() {
@Override
public void onError(String parentId) {
errorLatch.countDown();
}
});

assertThat(errorLatch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
}

@Test
public void search() throws Exception {
String testQuery = SEARCH_QUERY;
Expand Down Expand Up @@ -511,6 +563,35 @@ public void onSearchResult(String query, Bundle extras, List<MediaItem> items) {
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
}

@Test
public void search_commandSearchNotAvailable_reportsError() throws Exception {
String testQuery = SEARCH_QUERY;
int page = 4;
int pageSize = 10;
Bundle testExtras = new Bundle();
testExtras.putString(testQuery, testQuery);
testExtras.putInt(MediaBrowserCompat.EXTRA_PAGE, page);
testExtras.putInt(MediaBrowserCompat.EXTRA_PAGE_SIZE, pageSize);
Bundle rootHints = new Bundle();
rootHints.putInt(
MockMediaLibraryService.CONNECTION_HINTS_KEY_REMOVE_COMMAND_CODE,
SessionCommand.COMMAND_CODE_LIBRARY_SEARCH);
connectAndWait(rootHints);
CountDownLatch latch = new CountDownLatch(1);

browserCompat.search(
testQuery,
testExtras,
new SearchCallback() {
@Override
public void onError(String query, Bundle extras) {
latch.countDown();
}
});

assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
}

@Test
public void search_error() throws Exception {
String testQuery = SEARCH_QUERY_ERROR;
Expand Down Expand Up @@ -607,8 +688,9 @@ public void rootBrowserHints_searchSupported_reportsSearchSupported() throws Exc
@Test
public void rootBrowserHints_searchNotSupported_reportsSearchNotSupported() throws Exception {
Bundle connectionHints = new Bundle();
connectionHints.putBoolean(
MockMediaLibraryService.CONNECTION_HINTS_KEY_REMOVE_COMMAND_CODE_LIBRARY_SEARCH, true);
connectionHints.putInt(
MockMediaLibraryService.CONNECTION_HINTS_KEY_REMOVE_COMMAND_CODE,
SessionCommand.COMMAND_CODE_LIBRARY_SEARCH);
connectAndWait(connectionHints);

boolean isSearchSupported =
Expand Down
18 changes: 9 additions & 9 deletions ...s/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ public class MockMediaLibraryService extends MediaLibraryService {
public static final String CONNECTION_HINTS_CUSTOM_LIBRARY_ROOT =
"CONNECTION_HINTS_CUSTOM_LIBRARY_ROOT";
/**
* Key used in connection hints to instruct the mock service to remove {@link
* SessionCommand#COMMAND_CODE_LIBRARY_SEARCH} from the available commands in {@link
* Key used in connection hints to instruct the mock service to remove a {@link SessionCommand}
* identified by its command code from the available commands in {@link
* MediaSession.Callback#onConnect(MediaSession, ControllerInfo)}.
*/
public static final String CONNECTION_HINTS_KEY_REMOVE_COMMAND_CODE_LIBRARY_SEARCH =
"CONNECTION_HINTS_KEY_REMOVE_SEARCH_SESSION_COMMAND";
public static final String CONNECTION_HINTS_KEY_REMOVE_COMMAND_CODE =
"CONNECTION_HINTS_KEY_REMOVE_COMMAND_CODE";

private static final String TEST_IMAGE_PATH = "media/png/non-motion-photo-shortened.png";

Expand Down Expand Up @@ -196,11 +196,11 @@ public MediaSession.ConnectionResult onConnect(
SessionCommands.Builder builder = connectionResult.availableSessionCommands.buildUpon();
builder.add(new SessionCommand(CUSTOM_ACTION, /* extras= */ Bundle.EMPTY));
builder.add(new SessionCommand(CUSTOM_ACTION_ASSERT_PARAMS, /* extras= */ Bundle.EMPTY));
if (controller
.getConnectionHints()
.getBoolean(
CONNECTION_HINTS_KEY_REMOVE_COMMAND_CODE_LIBRARY_SEARCH, /* defaultValue= */ false)) {
builder.remove(SessionCommand.COMMAND_CODE_LIBRARY_SEARCH);
Bundle connectionHints = controller.getConnectionHints();
int commandCodeToRemove =
connectionHints.getInt(CONNECTION_HINTS_KEY_REMOVE_COMMAND_CODE, /* defaultValue= */ -1);
if (commandCodeToRemove != -1) {
builder.remove(commandCodeToRemove);
}
return MediaSession.ConnectionResult.accept(
/* availableSessionCommands= */ builder.build(),
Expand Down

0 comments on commit ab2505d

Please sign in to comment.