MediaSession2: Change controller's behavior when connected
This stops calling
MediaController2.ControllerCallback.onPlaybackStateChanged() when the
controller is connected.
This is the preliminary step towards sending session's current values
to a controller when it's connected.
Bug: 72547163
Test: Run all tests once
Change-Id: I73b45268dba4ac1fe3fce99a575748db15f19168
diff --git a/packages/MediaComponents/src/com/android/media/IMediaSession2Callback.aidl b/packages/MediaComponents/src/com/android/media/IMediaSession2Callback.aidl
index eaf404c..b3c83e5 100644
--- a/packages/MediaComponents/src/com/android/media/IMediaSession2Callback.aidl
+++ b/packages/MediaComponents/src/com/android/media/IMediaSession2Callback.aidl
@@ -33,19 +33,9 @@
void onPlaylistParamsChanged(in Bundle params);
void onPlaybackInfoChanged(in Bundle playbackInfo);
- /**
- * Called only when the controller is created with service's token.
- *
- * @param sessionBinder {@code null} if the connect is rejected or is disconnected. a session
- * binder if the connect is accepted.
- * @param commands initially allowed commands.
- */
- // TODO(jaewan): Also need to pass flags for allowed actions for permission check.
- // For example, a media can allow setRating only for whitelisted apps
- // it's better for controller to know such information in advance.
- // Follow-up TODO: Add similar functions to the session.
- // TODO(jaewan): Is term 'accepted/rejected' correct? For permission, 'grant' is used.
- void onConnectionChanged(IMediaSession2 sessionBinder, in Bundle commandGroup);
+
+ void onConnected(IMediaSession2 sessionBinder, in Bundle commandGroup, in Bundle playbackState);
+ void onDisconnected();
void onCustomLayoutChanged(in List<Bundle> commandButtonlist);
diff --git a/packages/MediaComponents/src/com/android/media/MediaController2Impl.java b/packages/MediaComponents/src/com/android/media/MediaController2Impl.java
index 6ee79a0..06883f2 100644
--- a/packages/MediaComponents/src/com/android/media/MediaController2Impl.java
+++ b/packages/MediaComponents/src/com/android/media/MediaController2Impl.java
@@ -76,6 +76,8 @@
private PlaylistParams mPlaylistParams;
@GuardedBy("mLock")
private PlaybackInfo mPlaybackInfo;
+ @GuardedBy("mLock")
+ private CommandGroup mCommandGroup;
// Assignment should be used with the lock hold, but should be used without a lock to prevent
// potential deadlock.
@@ -484,19 +486,18 @@
}
}
- // Called when the result for connecting to the session was delivered.
// Should be used without a lock to prevent potential deadlock.
- private void onConnectionChangedNotLocked(IMediaSession2 sessionBinder,
- CommandGroup commandGroup) {
+ private void onConnectedNotLocked(IMediaSession2 sessionBinder,
+ final CommandGroup commandGroup, final PlaybackState2 state) {
if (DEBUG) {
- Log.d(TAG, "onConnectionChangedNotLocked sessionBinder=" + sessionBinder
+ Log.d(TAG, "onConnectedNotLocked sessionBinder=" + sessionBinder
+ ", commands=" + commandGroup);
}
- boolean release = false;
+ boolean close = false;
try {
if (sessionBinder == null || commandGroup == null) {
// Connection rejected.
- release = true;
+ close = true;
return;
}
synchronized (mLock) {
@@ -506,9 +507,11 @@
if (mSessionBinder != null) {
Log.e(TAG, "Cannot be notified about the connection result many times."
+ " Probably a bug or malicious app.");
- release = true;
+ close = true;
return;
}
+ mCommandGroup = commandGroup;
+ mPlaybackState = state;
mSessionBinder = sessionBinder;
try {
// Implementation for the local binder is no-op,
@@ -518,16 +521,19 @@
if (DEBUG) {
Log.d(TAG, "Session died too early.", e);
}
- release = true;
+ close = true;
return;
}
}
// TODO(jaewan): Keep commands to prevents illegal API calls.
mCallbackExecutor.execute(() -> {
+ // Note: We may trigger ControllerCallbacks with the initial values
+ // But it's hard to define the order of the controller callbacks
+ // Only notify about the
mCallback.onConnected(commandGroup);
});
} finally {
- if (release) {
+ if (close) {
// Trick to call release() without holding the lock, to prevent potential deadlock
// with the developer's custom lock within the ControllerCallback.onDisconnected().
mInstance.close();
@@ -631,17 +637,30 @@
}
@Override
- public void onConnectionChanged(IMediaSession2 sessionBinder, Bundle commandGroup)
- throws RuntimeException {
- final MediaController2Impl controller;
- try {
- controller = getController();
- } catch (IllegalStateException e) {
- Log.w(TAG, "Don't fail silently here. Highly likely a bug");
+ public void onConnected(IMediaSession2 sessionBinder, Bundle commandGroup,
+ Bundle playbackState) {
+ final MediaController2Impl controller = mController.get();
+ if (controller == null) {
+ if (DEBUG) {
+ Log.d(TAG, "onConnected after MediaController2.close()");
+ }
return;
}
- controller.onConnectionChangedNotLocked(
- sessionBinder, CommandGroup.fromBundle(controller.getContext(), commandGroup));
+ controller.onConnectedNotLocked(sessionBinder,
+ CommandGroup.fromBundle(controller.getContext(), commandGroup),
+ PlaybackState2.fromBundle(controller.getContext(), playbackState));
+ }
+
+ @Override
+ public void onDisconnected() {
+ final MediaController2Impl controller = mController.get();
+ if (controller == null) {
+ if (DEBUG) {
+ Log.d(TAG, "onDisconnected after MediaController2.close()");
+ }
+ return;
+ }
+ controller.mInstance.close();
}
@Override
diff --git a/packages/MediaComponents/src/com/android/media/MediaSession2Stub.java b/packages/MediaComponents/src/com/android/media/MediaSession2Stub.java
index 5d3a06e..3909fe2 100644
--- a/packages/MediaComponents/src/com/android/media/MediaSession2Stub.java
+++ b/packages/MediaComponents/src/com/android/media/MediaSession2Stub.java
@@ -18,6 +18,7 @@
import android.content.Context;
import android.media.MediaController2;
+import android.media.MediaController2.PlaybackInfo;
import android.media.MediaItem2;
import android.media.MediaLibraryService2.LibraryRoot;
import android.media.MediaLibraryService2.MediaLibrarySessionCallback;
@@ -77,7 +78,7 @@
((ControllerInfoImpl) list.get(i).getProvider()).getControllerBinder();
try {
// Should be used without a lock hold to prevent potential deadlock.
- callbackBinder.onConnectionChanged(null, null);
+ callbackBinder.onDisconnected();
} catch (RemoteException e) {
// Controller is gone. Should be fine because we're destroying.
}
@@ -110,7 +111,11 @@
// media keys to.
boolean accept = allowedCommands != null || request.isTrusted();
ControllerInfoImpl impl = ControllerInfoImpl.from(request);
- if (accept) {
+ if (accept && allowedCommands != null) {
+ if (DEBUG) {
+ Log.d(TAG, "Accepting connection, request=" + request
+ + " allowedCommands=" + allowedCommands);
+ }
synchronized (mLock) {
mControllers.put(impl.getId(), request);
}
@@ -118,20 +123,6 @@
// For trusted apps, send non-null allowed commands to keep connection.
allowedCommands = new CommandGroup(context);
}
- }
- if (DEBUG) {
- Log.d(TAG, "onConnectResult, request=" + request
- + " accept=" + accept);
- }
- try {
- callback.onConnectionChanged(
- accept ? MediaSession2Stub.this : null,
- allowedCommands == null ? null : allowedCommands.toBundle());
- } catch (RemoteException e) {
- // Controller may be died prematurely.
- }
- if (accept) {
- // TODO(jaewan): We need to send current PlaybackInfo.
// If connection is accepted, notify the current state to the controller.
// It's needed because we cannot call synchronous calls between session/controller.
// Note: We're doing this after the onConnectionChanged(), but there's no guarantee
@@ -140,12 +131,24 @@
// use thread poll for incoming calls.
// TODO(jaewan): Should we protect getting playback state?
final PlaybackState2 state = session.getInstance().getPlaybackState();
- final Bundle bundle = state != null ? state.toBundle() : null;
+ final Bundle playbackStateBundle = (state != null) ? state.toBundle() : null;
+
try {
- callback.onPlaybackStateChanged(bundle);
+ callback.onConnected(MediaSession2Stub.this,
+ allowedCommands.toBundle(), playbackStateBundle);
} catch (RemoteException e) {
- // TODO(jaewan): Handle this.
// Controller may be died prematurely.
+ // TODO(jaewan): Handle here.
+ }
+ } else {
+ if (DEBUG) {
+ Log.d(TAG, "Rejecting connection, request=" + request);
+ }
+ try {
+ callback.onDisconnected();
+ } catch (RemoteException e) {
+ // Controller may be died prematurely.
+ // Not an issue because we'll ignore it anyway.
}
}
});
diff --git a/packages/MediaComponents/test/src/android/media/MediaController2Test.java b/packages/MediaComponents/test/src/android/media/MediaController2Test.java
index e8eaa20..358beb7 100644
--- a/packages/MediaComponents/test/src/android/media/MediaController2Test.java
+++ b/packages/MediaComponents/test/src/android/media/MediaController2Test.java
@@ -23,7 +23,6 @@
import android.media.MediaSession2.PlaylistParams;
import android.media.MediaSession2.SessionCallback;
import android.media.TestUtils.SyncHandler;
-import android.media.session.PlaybackState;
import android.os.Bundle;
import android.os.Handler;
import android.os.HandlerThread;
@@ -228,30 +227,24 @@
assertEquals(mContext.getPackageName(), mController.getSessionToken().getPackageName());
}
- // This also tests testGetPlaybackState().
+ // This also tests getPlaybackState().
@Test
public void testControllerCallback_onPlaybackStateChanged() throws InterruptedException {
- final CountDownLatch latch = new CountDownLatch(2);
+ final CountDownLatch latch = new CountDownLatch(1);
final TestControllerCallbackInterface callback = new TestControllerCallbackInterface() {
@Override
public void onPlaybackStateChanged(PlaybackState2 state) {
- switch ((int) latch.getCount()) {
- case 2:
- assertEquals(PlaybackState.STATE_PLAYING, state.getState());
- break;
- case 1:
- assertEquals(PlaybackState.STATE_PAUSED, state.getState());
- break;
- }
+ // Called only once when the player's playback state is changed after this.
+ assertEquals(PlaybackState2.STATE_PAUSED, state.getState());
latch.countDown();
}
};
-
- mPlayer.notifyPlaybackState(createPlaybackState(PlaybackState.STATE_PLAYING));
+ mPlayer.notifyPlaybackState(createPlaybackState(PlaybackState2.STATE_PLAYING));
mController = createController(mSession.getToken(), true, callback);
- mPlayer.notifyPlaybackState(createPlaybackState(PlaybackState.STATE_PAUSED));
+ assertEquals(PlaybackState2.STATE_PLAYING, mController.getPlaybackState().getState());
+ mPlayer.notifyPlaybackState(createPlaybackState(PlaybackState2.STATE_PAUSED));
assertTrue(latch.await(WAIT_TIME_MS, TimeUnit.MILLISECONDS));
- assertEquals(PlaybackState.STATE_PAUSED, mController.getPlaybackState().getState());
+ assertEquals(PlaybackState2.STATE_PAUSED, mController.getPlaybackState().getState());
}
@Test
@@ -364,7 +357,7 @@
});
final MediaController2 controller = createController(mSession.getToken());
testHandler.post(() -> {
- final PlaybackState2 state = createPlaybackState(PlaybackState.STATE_ERROR);
+ final PlaybackState2 state = createPlaybackState(PlaybackState2.STATE_ERROR);
for (int i = 0; i < 100; i++) {
// triggers call from session to controller.
player.notifyPlaybackState(state);