Only use result.sendError where supported by legacy media library
`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: androidx/media#78 Issue: androidx/media#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
This commit is contained in:
parent
f766936140
commit
17ee5277d5
@ -111,6 +111,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)).
|
||||
* UI:
|
||||
* Add a `Player.Listener` implementation for Wear OS devices that handles
|
||||
playback suppression due to
|
||||
|
@ -197,12 +197,12 @@ import java.util.concurrent.atomic.AtomicReference;
|
||||
@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();
|
||||
@ -212,7 +212,7 @@ import java.util.concurrent.atomic.AtomicReference;
|
||||
if (!getConnectedControllersManager()
|
||||
.isSessionCommandAvailable(
|
||||
controller, SessionCommand.COMMAND_CODE_LIBRARY_GET_CHILDREN)) {
|
||||
result.sendError(/* extras= */ null);
|
||||
result.sendResult(/* result= */ null);
|
||||
return;
|
||||
}
|
||||
if (options != null) {
|
||||
@ -260,12 +260,12 @@ import java.util.concurrent.atomic.AtomicReference;
|
||||
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();
|
||||
@ -275,7 +275,7 @@ import java.util.concurrent.atomic.AtomicReference;
|
||||
if (!getConnectedControllersManager()
|
||||
.isSessionCommandAvailable(
|
||||
controller, SessionCommand.COMMAND_CODE_LIBRARY_GET_ITEM)) {
|
||||
result.sendError(/* extras= */ null);
|
||||
result.sendResult(/* result= */ null);
|
||||
return;
|
||||
}
|
||||
ListenableFuture<LibraryResult<MediaItem>> future =
|
||||
@ -291,12 +291,12 @@ import java.util.concurrent.atomic.AtomicReference;
|
||||
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)) {
|
||||
@ -308,7 +308,7 @@ import java.util.concurrent.atomic.AtomicReference;
|
||||
() -> {
|
||||
if (!getConnectedControllersManager()
|
||||
.isSessionCommandAvailable(controller, SessionCommand.COMMAND_CODE_LIBRARY_SEARCH)) {
|
||||
result.sendError(/* extras= */ null);
|
||||
result.sendResult(/* result= */ null);
|
||||
return;
|
||||
}
|
||||
BrowserLegacyCb cb = (BrowserLegacyCb) checkStateNotNull(controller.getControllerCb());
|
||||
@ -389,7 +389,7 @@ import java.util.concurrent.atomic.AtomicReference;
|
||||
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());
|
||||
@ -408,7 +408,7 @@ import java.util.concurrent.atomic.AtomicReference;
|
||||
: 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());
|
||||
|
@ -54,6 +54,7 @@ import static androidx.media3.test.session.common.MediaBrowserConstants.SEARCH_R
|
||||
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;
|
||||
@ -210,6 +211,29 @@ public class MediaBrowserCompatWithMediaLibraryServiceTest
|
||||
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;
|
||||
@ -415,6 +439,34 @@ public class MediaBrowserCompatWithMediaLibraryServiceTest
|
||||
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;
|
||||
@ -511,6 +563,35 @@ public class MediaBrowserCompatWithMediaLibraryServiceTest
|
||||
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;
|
||||
@ -607,8 +688,9 @@ public class MediaBrowserCompatWithMediaLibraryServiceTest
|
||||
@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 =
|
||||
|
@ -89,12 +89,12 @@ public class MockMediaLibraryService extends MediaLibraryService {
|
||||
"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";
|
||||
|
||||
@ -198,11 +198,11 @@ public class MockMediaLibraryService extends MediaLibraryService {
|
||||
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(),
|
||||
|
Loading…
x
Reference in New Issue
Block a user