Merge "MediaSession2: Change controller's behavior when connected" (479f0113) · Commits · e / os / android_frameworks_av

packages/MediaComponents/src/com/android/media/IMediaSession2Callback.aidl

+3 −13

Original line number	Diff line number	Diff line
		@@ -33,19 +33,9 @@ oneway interface IMediaSession2Callback {
		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);

packages/MediaComponents/src/com/android/media/MediaController2Impl.java

+37 −18

Original line number	Diff line number	Diff line
		@@ -76,6 +76,8 @@ public class MediaController2Impl implements MediaController2Provider {
		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.
		@@ -504,19 +506,18 @@ public class MediaController2Impl implements MediaController2Provider {
		}
		}

		// 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) {
		@@ -526,9 +527,11 @@ public class MediaController2Impl implements MediaController2Provider {
		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,
		@@ -538,16 +541,19 @@ public class MediaController2Impl implements MediaController2Provider {
		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();
		@@ -651,17 +657,30 @@ public class MediaController2Impl implements MediaController2Provider {
		}

		@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.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.onConnectionChangedNotLocked(
		sessionBinder, CommandGroup.fromBundle(controller.getContext(), commandGroup));
		controller.mInstance.close();
		}

		@Override

packages/MediaComponents/src/com/android/media/MediaSession2Stub.java

+22 −19

Original line number	Diff line number	Diff line
		@@ -18,6 +18,7 @@ package com.android.media;

		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;
		@@ -78,7 +79,7 @@ public class MediaSession2Stub extends IMediaSession2.Stub {
		((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.
		}
		@@ -111,7 +112,11 @@ public class MediaSession2Stub extends IMediaSession2.Stub {
		// 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);
		}
		@@ -119,20 +124,6 @@ public class MediaSession2Stub extends IMediaSession2.Stub {
		// 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
		@@ -141,12 +132,24 @@ public class MediaSession2Stub extends IMediaSession2.Stub {
		// 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) {
		// 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) {
		// TODO(jaewan): Handle this.
		// Controller may be died prematurely.
		// Not an issue because we'll ignore it anyway.
		}
		}
		});

packages/MediaComponents/test/src/android/media/MediaController2Test.java

+9 −16

Original line number	Diff line number	Diff line
		@@ -23,7 +23,6 @@ import android.media.MediaSession2.ControllerInfo;
		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;
		@@ -264,30 +263,24 @@ public class MediaController2Test extends MediaSession2TestBase {
		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
		@@ -400,7 +393,7 @@ public class MediaController2Test extends MediaSession2TestBase {
		});
		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);