From 6bda0da6bed70c6ba639ca190fb96951634ce67b Mon Sep 17 00:00:00 2001 From: bachinger Date: Thu, 19 Sep 2024 12:32:40 -0700 Subject: [PATCH] Fix sending custom commands with a media browser When sending a custom command with `browser.sendCustomCommand` when connected to a legacy browser service, the custom command was delivered to `MediaSessionCompat.Callback.onCustomAction` instead of the service method `onCustomAction`. The difference is that the service version can return an async response with a bundle, while the session callback version doesn't have a return value. Hence, the service method was never called and it wasn't possible to send a reponse or signal an error back to the browser. The resulting `ListanableFuture` simply always immediately resolved to a success. This change overrides `ListenableFuture sendCustomCommand(SessionCommand command, Bundle args)` in `MediaBrowserImplLegacy` to use the `MediaBrowserCompat` method to send instead of the `MediaControlleCompat` method that was used by the subclass `MediaControllerImplLegacy`. This involves the service callback instead of the session callback and enables `MediaBrowser` to get the actual return value from the legacy service. Issue: androidx/media#1474 #cherrypick PiperOrigin-RevId: 676519314 --- RELEASENOTES.md | 12 +++ .../session/MediaBrowserImplLegacy.java | 29 +++++++ .../session/common/MediaBrowserConstants.java | 2 + .../MediaBrowserServiceCompatConstants.java | 1 + ...enerWithMediaBrowserServiceCompatTest.java | 83 +++++++++++++++++++ .../MockMediaBrowserServiceCompat.java | 46 ++++++++++ 6 files changed, 173 insertions(+) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index f531233636..ffa1d47520 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -206,6 +206,18 @@ This release includes the following changes since the playback can't be suppressed without the system crashing the service with a `ForegroundServiceDidNotStartInTimeException` ([#1528](https://github.com/google/ExoPlayer/issues/1528)). + * Fix bug that caused custom commands sent from a `MediaBrowser` being + dispatched to the `MediaSessionCompat.Callback` instead of the + `MediaBrowserServiceCompat` variant of the method when connected to a + legacy service. This prevented the `MediaBrowser` to receive the actual + return value sent back by the legacy service + ([#1474](https://github.com/androidx/media/issues/1474)). +* UI: +* Downloads: +* OkHttp Extension: +* Cronet Extension: +* RTMP Extension: +* HLS Extension: * DASH Extension: * Add support for periods starting in the middle of a segment ([#1440](https://github.com/androidx/media/issues/1440)). diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaBrowserImplLegacy.java b/libraries/session/src/main/java/androidx/media3/session/MediaBrowserImplLegacy.java index 28a2bcf981..15996ef839 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaBrowserImplLegacy.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaBrowserImplLegacy.java @@ -291,6 +291,35 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; return future; } + @Override + public ListenableFuture sendCustomCommand(SessionCommand command, Bundle args) { + MediaBrowserCompat browserCompat = getBrowserCompat(); + if (browserCompat != null && instance.isSessionCommandAvailable(command)) { + SettableFuture settable = SettableFuture.create(); + browserCompat.sendCustomAction( + command.customAction, + args, + new MediaBrowserCompat.CustomActionCallback() { + @Override + public void onResult( + String action, @Nullable Bundle extras, @Nullable Bundle resultData) { + Bundle mergedBundles = new Bundle(extras); + mergedBundles.putAll(resultData); + settable.set(new SessionResult(SessionResult.RESULT_SUCCESS, mergedBundles)); + } + + @Override + public void onError(String action, @Nullable Bundle extras, @Nullable Bundle data) { + Bundle mergedBundles = new Bundle(extras); + mergedBundles.putAll(data); + settable.set(new SessionResult(SessionResult.RESULT_ERROR_UNKNOWN, mergedBundles)); + } + }); + return settable; + } + return super.sendCustomCommand(command, args); + } + private MediaBrowserCompat getBrowserCompat(LibraryParams extras) { return browserCompats.get(extras); } diff --git a/libraries/test_session_common/src/main/java/androidx/media3/test/session/common/MediaBrowserConstants.java b/libraries/test_session_common/src/main/java/androidx/media3/test/session/common/MediaBrowserConstants.java index 409aa5238a..00e91f0d9a 100644 --- a/libraries/test_session_common/src/main/java/androidx/media3/test/session/common/MediaBrowserConstants.java +++ b/libraries/test_session_common/src/main/java/androidx/media3/test/session/common/MediaBrowserConstants.java @@ -29,6 +29,8 @@ public class MediaBrowserConstants { public static final String ROOT_EXTRAS_KEY = "root_extras_key"; public static final int ROOT_EXTRAS_VALUE = 4321; + public static final String COMMAND_ACTION_PLAYLIST_ADD = "androidx.media3.actions.playlist_add"; + public static final String MEDIA_ID_GET_BROWSABLE_ITEM = "media_id_get_browsable_item"; public static final String MEDIA_ID_GET_PLAYABLE_ITEM = "media_id_get_playable_item"; public static final String MEDIA_ID_GET_ITEM_WITH_METADATA = "media_id_get_item_with_metadata"; diff --git a/libraries/test_session_common/src/main/java/androidx/media3/test/session/common/MediaBrowserServiceCompatConstants.java b/libraries/test_session_common/src/main/java/androidx/media3/test/session/common/MediaBrowserServiceCompatConstants.java index acb63444c1..367abe3ba9 100644 --- a/libraries/test_session_common/src/main/java/androidx/media3/test/session/common/MediaBrowserServiceCompatConstants.java +++ b/libraries/test_session_common/src/main/java/androidx/media3/test/session/common/MediaBrowserServiceCompatConstants.java @@ -29,6 +29,7 @@ public class MediaBrowserServiceCompatConstants { "getLibraryRoot_fatalAuthenticationError_receivesPlaybackException"; public static final String TEST_GET_CHILDREN_NON_FATAL_AUTHENTICATION_ERROR = "getLibraryRoot_nonFatalAuthenticationError_receivesPlaybackException"; + public static final String TEST_SEND_CUSTOM_COMMAND = "sendCustomCommand"; private MediaBrowserServiceCompatConstants() {} } diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserListenerWithMediaBrowserServiceCompatTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserListenerWithMediaBrowserServiceCompatTest.java index 830a7122ec..907598f698 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserListenerWithMediaBrowserServiceCompatTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserListenerWithMediaBrowserServiceCompatTest.java @@ -32,8 +32,10 @@ import static androidx.media3.test.session.common.MediaBrowserServiceCompatConst import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN_WITH_NULL_LIST; import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_LIBRARY_ROOT; import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_ON_CHILDREN_CHANGED_SUBSCRIBE_AND_UNSUBSCRIBE; +import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_SEND_CUSTOM_COMMAND; import static androidx.media3.test.session.common.TestUtils.TIMEOUT_MS; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.util.concurrent.MoreExecutors.directExecutor; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.junit.Assert.assertThrows; @@ -46,14 +48,19 @@ import androidx.media3.common.PlaybackException; import androidx.media3.common.Player; import androidx.media3.session.MediaLibraryService.LibraryParams; import androidx.media3.test.session.common.HandlerThreadTestRule; +import androidx.media3.test.session.common.MediaBrowserConstants; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.LargeTest; import com.google.common.collect.ImmutableList; +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicReference; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -336,4 +343,80 @@ public class MediaBrowserListenerWithMediaBrowserServiceCompatTest { assertThat(sessionErrors.get(0).code) .isEqualTo(PlaybackException.ERROR_CODE_AUTHENTICATION_EXPIRED); } + + @Test + public void sendCustomCommand_success_correctAsyncResult() throws Exception { + remoteService.setProxyForTest(TEST_SEND_CUSTOM_COMMAND); + MediaBrowser browser = createBrowser(/* listener= */ null); + CountDownLatch latch = new CountDownLatch(/* count= */ 1); + AtomicReference sessionResultRef = new AtomicReference<>(); + + ListenableFuture resultFuture = + threadTestRule + .getHandler() + .postAndSync( + () -> + browser.sendCustomCommand( + new SessionCommand( + MediaBrowserConstants.COMMAND_ACTION_PLAYLIST_ADD, + /* extras= */ Bundle.EMPTY), + /* args= */ Bundle.EMPTY)); + Futures.addCallback( + resultFuture, + new FutureCallback() { + @Override + public void onSuccess(SessionResult result) { + sessionResultRef.set(result); + latch.countDown(); + } + + @Override + public void onFailure(Throwable t) {} + }, + directExecutor()); + + assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); + assertThat(sessionResultRef.get()).isNotNull(); + assertThat(sessionResultRef.get().resultCode).isEqualTo(SessionResult.RESULT_SUCCESS); + assertThat(sessionResultRef.get().extras.getString("key-1")).isEqualTo("success-from-service"); + } + + @Test + public void sendCustomCommand_failure_correctAsyncResult() throws Exception { + remoteService.setProxyForTest(TEST_SEND_CUSTOM_COMMAND); + MediaBrowser browser = createBrowser(/* listener= */ null); + CountDownLatch latch = new CountDownLatch(/* count= */ 1); + AtomicReference sessionResultRef = new AtomicReference<>(); + Bundle args = new Bundle(); + args.putBoolean("request_error", true); + + ListenableFuture resultFuture = + threadTestRule + .getHandler() + .postAndSync( + () -> + browser.sendCustomCommand( + new SessionCommand( + MediaBrowserConstants.COMMAND_ACTION_PLAYLIST_ADD, + /* extras= */ Bundle.EMPTY), + args)); + Futures.addCallback( + resultFuture, + new FutureCallback() { + @Override + public void onSuccess(SessionResult result) { + sessionResultRef.set(result); + latch.countDown(); + } + + @Override + public void onFailure(Throwable t) {} + }, + directExecutor()); + + assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); + assertThat(sessionResultRef.get()).isNotNull(); + assertThat(sessionResultRef.get().resultCode).isEqualTo(SessionResult.RESULT_ERROR_UNKNOWN); + assertThat(sessionResultRef.get().extras.getString("key-1")).isEqualTo("error-from-service"); + } } diff --git a/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaBrowserServiceCompat.java b/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaBrowserServiceCompat.java index 922511f0b0..58f0a24f69 100644 --- a/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaBrowserServiceCompat.java +++ b/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaBrowserServiceCompat.java @@ -30,6 +30,7 @@ import static androidx.media3.test.session.common.MediaBrowserServiceCompatConst import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_CHILDREN_WITH_NULL_LIST; import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_GET_LIBRARY_ROOT; import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_ON_CHILDREN_CHANGED_SUBSCRIBE_AND_UNSUBSCRIBE; +import static androidx.media3.test.session.common.MediaBrowserServiceCompatConstants.TEST_SEND_CUSTOM_COMMAND; import android.content.Intent; import android.net.Uri; @@ -46,6 +47,7 @@ import androidx.annotation.GuardedBy; import androidx.annotation.Nullable; import androidx.media.MediaBrowserServiceCompat; import androidx.media3.test.session.common.IRemoteMediaBrowserServiceCompat; +import androidx.media3.test.session.common.MediaBrowserConstants; import androidx.media3.test.session.common.MediaBrowserServiceCompatConstants; import com.google.common.collect.ImmutableList; import java.lang.reflect.Method; @@ -271,6 +273,9 @@ public class MockMediaBrowserServiceCompat extends MediaBrowserServiceCompat { case TEST_GET_CHILDREN_NON_FATAL_AUTHENTICATION_ERROR: getChildren_authenticationError_receivesPlaybackException(session, /* isFatal= */ false); break; + case TEST_SEND_CUSTOM_COMMAND: + setProxyForTestSendCustomCommand(); + break; default: throw new IllegalArgumentException("Unknown testName: " + testName); } @@ -374,6 +379,47 @@ public class MockMediaBrowserServiceCompat extends MediaBrowserServiceCompat { }); } + private void setProxyForTestSendCustomCommand() { + setMediaBrowserServiceProxy( + new MockMediaBrowserServiceCompat.Proxy() { + @Override + public BrowserRoot onGetRoot( + String clientPackageName, int clientUid, Bundle rootHints) { + session.setPlaybackState( + new PlaybackStateCompat.Builder() + .setState( + PlaybackStateCompat.STATE_PLAYING, + /* position= */ 123L, + /* playbackSpeed= */ 1.0f) + .addCustomAction( + new PlaybackStateCompat.CustomAction.Builder( + MediaBrowserConstants.COMMAND_ACTION_PLAYLIST_ADD, + "Add to playlist", + CommandButton.ICON_PLAYLIST_ADD) + .build()) + .build()); + + return new BrowserRoot(ROOT_ID, Bundle.EMPTY); + } + + @Override + public void onCustomAction(String action, Bundle extras, Result result) { + Bundle resultBundle = new Bundle(); + if (action.equals(MediaBrowserConstants.COMMAND_ACTION_PLAYLIST_ADD)) { + if (extras.getBoolean("request_error", /* defaultValue= */ false)) { + resultBundle.putString("key-1", "error-from-service"); + result.sendError(resultBundle); + } else { + resultBundle.putString("key-1", "success-from-service"); + result.sendResult(resultBundle); + } + } else { + result.sendError(resultBundle); + } + } + }); + } + private void setProxyForTestGetLibraryRoot_correctExtraKeyAndValue() { setMediaBrowserServiceProxy( new MockMediaBrowserServiceCompat.Proxy() {