Merge "Disconnect MediaBrowserWrapper after every operation" (e1df6671) · Commits · e / os / android_packages_modules_Bluetooth

android/app/src/com/android/bluetooth/newavrcp/BrowsablePlayerConnector.java

+18 −15

Original line number	Diff line number	Diff line
		@@ -72,12 +72,12 @@ public class BrowsablePlayerConnector {

		// Try to start connecting all the browsed player wrappers
		for (ResolveInfo info : players) {
		newWrapper.mPendingPlayers.add(
		BrowsedPlayerWrapper.wrap(
		BrowsedPlayerWrapper player = BrowsedPlayerWrapper.wrap(
		context,
		info.serviceInfo.packageName,
		info.serviceInfo.name,
		(int status, BrowsedPlayerWrapper wrapper) -> {
		info.serviceInfo.name);
		newWrapper.mPendingPlayers.add(player);
		player.connect((int status, BrowsedPlayerWrapper wrapper) -> {
		// Use the handler to avoid concurrency issues
		if (DEBUG) {
		Log.d(TAG, "Browse player callback called: package="
		@@ -88,7 +88,7 @@ public class BrowsablePlayerConnector {
		msg.arg1 = status;
		msg.obj = wrapper;
		newWrapper.mHandler.sendMessage(msg);
		}));
		});
		}

		Message msg = newWrapper.mHandler.obtainMessage(MSG_TIMEOUT);
		@@ -126,6 +126,9 @@ public class BrowsablePlayerConnector {
		}

		// Check to see if the root folder has any items
		if (DEBUG) {
		Log.i(TAG, "Checking root contents for " + wrapper.getPackageName());
		}
		wrapper.getFolderItems(wrapper.getRootId(),
		(int status, String mediaId, List<ListItem> results) -> {
		if (status != BrowsedPlayerWrapper.STATUS_SUCCESS) {

android/app/src/com/android/bluetooth/newavrcp/BrowsedPlayerWrapper.java

+37 −62

Original line number	Diff line number	Diff line
		@@ -61,7 +61,6 @@ class BrowsedPlayerWrapper {
		private Context mContext;
		private String mPackageName;
		private ConnectionCallback mCallback;
		private ConnectionState mConnectionState = ConnectionState.DISCONNECTED;

		// TODO(apanicke): We cache this because normally you can only grab the root
		// while connected. We shouldn't cache this since theres nothing in the framework documentation
		@@ -92,10 +91,8 @@ class BrowsedPlayerWrapper {
		// TODO (apanicke): Investigate if there is a way to create this just by passing in the
		// MediaBrowser. Right now there is no obvious way to create the browser then update the
		// connection callback without being forced to re-create the object every time.
		private BrowsedPlayerWrapper(Context context, String packageName, String className,
		ConnectionCallback cb) {
		private BrowsedPlayerWrapper(Context context, String packageName, String className) {
		mContext = context;
		mCallback = cb;
		mPackageName = packageName;

		mWrappedBrowser = MediaBrowserFactory.make(
		@@ -105,14 +102,10 @@ class BrowsedPlayerWrapper {
		null);
		}

		static BrowsedPlayerWrapper wrap(Context context, String packageName, String className,
		ConnectionCallback cb) {
		static BrowsedPlayerWrapper wrap(Context context, String packageName, String className) {
		Log.i(TAG, "Wrapping Media Browser " + packageName);
		BrowsedPlayerWrapper wrapper =
		new BrowsedPlayerWrapper(context, packageName, className, cb);

		wrapper.mConnectionState = ConnectionState.CONNECTING;
		wrapper.mWrappedBrowser.connect();
		new BrowsedPlayerWrapper(context, packageName, className);
		return wrapper;
		}

		@@ -127,21 +120,17 @@ class BrowsedPlayerWrapper {
		}

		if (DEBUG) Log.d(TAG, "connect: Connecting to browsable player: " + mPackageName);
		mCallback = cb;
		mConnectionState = ConnectionState.CONNECTING;
		mCallback = (int status, BrowsedPlayerWrapper wrapper) -> {
		cb.run(status, wrapper);
		wrapper.disconnect();
		};
		mWrappedBrowser.connect();
		}

		void disconnect() {
		if (DEBUG) Log.d(TAG, "disconnect: Disconnecting from " + mPackageName);
		if (mConnectionState != ConnectionState.DISCONNECTED) {
		// According to the API, as soon as disconnect is sent we shouldn't receive
		// any more callbacks.
		mWrappedBrowser.disconnect();
		}

		mCallback = null;
		mConnectionState = ConnectionState.DISCONNECTED;
		}

		public String getPackageName() {
		@@ -152,13 +141,8 @@ class BrowsedPlayerWrapper {
		return mRoot;
		}

		public ConnectionState getConnectionState() {
		return mConnectionState;
		}

		public void playItem(String mediaId) {
		if (DEBUG) Log.d(TAG, "playItem: Play Item from media ID: " + mediaId);
		if (mConnectionState == ConnectionState.DISCONNECTED) {
		connect((int status, BrowsedPlayerWrapper wrapper) -> {
		if (DEBUG) Log.d(TAG, "playItem: Connected to browsable player " + mPackageName);

		@@ -171,13 +155,6 @@ class BrowsedPlayerWrapper {
		return;
		}

		MediaController controller = MediaControllerFactory.make(mContext,
		mWrappedBrowser.getSessionToken());
		MediaController.TransportControls ctrl = controller.getTransportControls();
		Log.i(TAG, "playItem: Playing " + mediaId);
		ctrl.playFromMediaId(mediaId, null);
		}

		// Returns false if the player is in the connecting state. Wait for it to either be
		// connected or disconnected.
		//
		@@ -194,29 +171,28 @@ class BrowsedPlayerWrapper {
		return true;
		}

		// TODO (apanicke): Queue the command here instead of failing so that we can respond
		// eventually.
		if (mConnectionState == ConnectionState.CONNECTING) {
		Log.w(TAG, "getFolderItems: Already trying to connect");
		if (cb == null) {
		Log.wtfStack(TAG, "connect: Trying to connect to " + mPackageName
		+ "with null callback");
		}
		if (mCallback != null) {
		Log.w(TAG, "connect: Already trying to connect to " + mPackageName);
		return false;
		}

		// If we are disconnected, connect first then do the lookup
		if (mConnectionState == ConnectionState.DISCONNECTED) {
		connect((int status, BrowsedPlayerWrapper wrapper) -> {
		if (DEBUG) Log.d(TAG, "connect: Connecting to browsable player: " + mPackageName);
		mCallback = (int status, BrowsedPlayerWrapper wrapper) -> {
		Log.i(TAG, "getFolderItems: Connected to browsable player: " + mPackageName);
		if (status != STATUS_SUCCESS) {
		cb.run(status, "", new ArrayList<ListItem>());
		}

		// This will disconnect when the callback is called
		getFolderItemsInternal(mediaId, cb);
		});
		return true;
		}
		};
		mWrappedBrowser.connect();

		// If already connected
		Log.i(TAG, "getFolderItems: getting Items for mediaId=" + mediaId);
		return getFolderItemsInternal(mediaId, cb);
		return true;
		}

		// Internal function to call once the Browser is connected
		@@ -228,7 +204,6 @@ class BrowsedPlayerWrapper {
		class MediaConnectionCallback extends MediaBrowser.ConnectionCallback {
		@Override
		public void onConnected() {
		mConnectionState = ConnectionState.CONNECTED;
		Log.i(TAG, "onConnected: " + mPackageName + " is connected");
		// Get the root while connected because we may need to use it when disconnected.
		mRoot = mWrappedBrowser.getRoot();
		@@ -239,7 +214,6 @@ class BrowsedPlayerWrapper {

		@Override
		public void onConnectionFailed() {
		mConnectionState = ConnectionState.DISCONNECTED;
		Log.w(TAG, "onConnectionFailed: Connection Failed with " + mPackageName);
		if (mCallback != null) mCallback.run(STATUS_CONN_ERROR, BrowsedPlayerWrapper.this);
		mCallback = null;
		@@ -249,7 +223,6 @@ class BrowsedPlayerWrapper {
		// after connection.
		@Override
		public void onConnectionSuspended() {
		mConnectionState = ConnectionState.DISCONNECTED;
		mWrappedBrowser.disconnect();
		Log.i(TAG, "onConnectionSuspended: Connection Suspended with " + mPackageName);
		}
		@@ -309,6 +282,7 @@ class BrowsedPlayerWrapper {
		// Clone the list so that the callee can mutate it without affecting the cached data
		mCallback.run(STATUS_SUCCESS, parentId, Util.cloneList(return_list));
		mCallback = null;
		disconnect();
		}

		/* mediaId is invalid */
		@@ -316,6 +290,7 @@ class BrowsedPlayerWrapper {
		public void onError(String id) {
		Log.e(TAG, "BrowserSubscriptionCallback: Could not get folder items");
		mCallback.run(STATUS_LOOKUP_ERROR, id, new ArrayList<ListItem>());
		disconnect();
		}
		}

android/app/tests/unit/src/com/android/bluetooth/newavrcp/BrowserPlayerWrapperTest.java

+22 −54

Original line number	Diff line number	Diff line
		@@ -57,101 +57,66 @@ public class BrowserPlayerWrapperTest {

		@Test
		public void testWrap() {
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test", mConnCb);
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test");
		wrapper.connect(mConnCb);
		verify(mMockBrowser).testInit(any(), any(), mBrowserConnCb.capture(), any());
		verify(mMockBrowser).connect();
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.CONNECTING,
		wrapper.getConnectionState());

		MediaBrowser.ConnectionCallback browserConnCb = mBrowserConnCb.getValue();
		browserConnCb.onConnected();

		verify(mConnCb).run(eq(BrowsedPlayerWrapper.STATUS_SUCCESS), eq(wrapper));
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.CONNECTED,
		wrapper.getConnectionState());
		verify(mMockBrowser).disconnect();
		}

		@Test
		public void testConnect() {
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test", mConnCb);
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test");
		wrapper.connect(mConnCb);
		verify(mMockBrowser).testInit(any(), any(), mBrowserConnCb.capture(), any());
		MediaBrowser.ConnectionCallback browserConnCb = mBrowserConnCb.getValue();
		browserConnCb.onConnectionFailed();

		verify(mConnCb).run(eq(BrowsedPlayerWrapper.STATUS_CONN_ERROR), eq(wrapper));
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.DISCONNECTED,
		wrapper.getConnectionState());

		wrapper.connect(mConnCb);
		verify(mMockBrowser, times(2)).connect();
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.CONNECTING,
		wrapper.getConnectionState());

		browserConnCb.onConnected();
		verify(mConnCb).run(eq(BrowsedPlayerWrapper.STATUS_SUCCESS), eq(wrapper));
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.CONNECTED,
		wrapper.getConnectionState());
		verify(mMockBrowser, times(2)).disconnect();
		}

		@Test
		public void testDisconnect() {
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test", mConnCb);
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test");
		wrapper.connect(mConnCb);
		verify(mMockBrowser).testInit(any(), any(), mBrowserConnCb.capture(), any());
		MediaBrowser.ConnectionCallback browserConnCb = mBrowserConnCb.getValue();
		browserConnCb.onConnected();
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.CONNECTED,
		wrapper.getConnectionState());

		wrapper.disconnect();
		verify(mMockBrowser).disconnect();
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.DISCONNECTED,
		wrapper.getConnectionState());
		}

		@Test
		public void testGetRootId() {
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test", mConnCb);
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test");
		wrapper.connect(mConnCb);
		verify(mMockBrowser).testInit(any(), any(), mBrowserConnCb.capture(), any());
		MediaBrowser.ConnectionCallback browserConnCb = mBrowserConnCb.getValue();
		browserConnCb.onConnected();
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.CONNECTED,
		wrapper.getConnectionState());

		Assert.assertEquals("root_folder", wrapper.getRootId());
		verify(mMockBrowser).disconnect();
		}

		@Test
		public void testPlayItemWhileConnected() {
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test", mConnCb);
		verify(mMockBrowser).testInit(any(), any(), mBrowserConnCb.capture(), any());
		MediaBrowser.ConnectionCallback browserConnCb = mBrowserConnCb.getValue();
		browserConnCb.onConnected();
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.CONNECTED,
		wrapper.getConnectionState());

		MediaController mockController = mock(MediaController.class);
		MediaController.TransportControls mockTransport =
		mock(MediaController.TransportControls.class);
		when(mockController.getTransportControls()).thenReturn(mockTransport);
		MediaControllerFactory.inject(mockController);

		wrapper.playItem("test_item");
		verify(mockTransport).playFromMediaId(eq("test_item"), eq(null));
		}

		@Test
		public void testPlayItemWhileDisconnected() {
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test", mConnCb);
		public void testPlayItem() {
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test");
		verify(mMockBrowser).testInit(any(), any(), mBrowserConnCb.capture(), any());
		MediaBrowser.ConnectionCallback browserConnCb = mBrowserConnCb.getValue();
		browserConnCb.onConnectionFailed();
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.DISCONNECTED,
		wrapper.getConnectionState());

		wrapper.playItem("test_item");
		verify(mMockBrowser, times(2)).connect();
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.CONNECTING,
		wrapper.getConnectionState());
		verify(mMockBrowser, times(1)).connect();

		MediaController mockController = mock(MediaController.class);
		MediaController.TransportControls mockTransport =
		@@ -161,18 +126,19 @@ public class BrowserPlayerWrapperTest {

		browserConnCb.onConnected();
		verify(mockTransport).playFromMediaId(eq("test_item"), eq(null));
		verify(mMockBrowser).disconnect();
		}

		@Test
		public void testGetFolderItemsWhileConnected() {
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test", mConnCb);
		public void testGetFolderItems() {
		BrowsedPlayerWrapper wrapper = BrowsedPlayerWrapper.wrap(null, "test", "test");
		verify(mMockBrowser).testInit(any(), any(), mBrowserConnCb.capture(), any());
		MediaBrowser.ConnectionCallback browserConnCb = mBrowserConnCb.getValue();
		browserConnCb.onConnected();
		Assert.assertEquals(BrowsedPlayerWrapper.ConnectionState.CONNECTED,
		wrapper.getConnectionState());

		wrapper.getFolderItems("test_folder", mBrowseCb);


		browserConnCb.onConnected();
		verify(mMockBrowser).subscribe(any(), mSubscriptionCb.capture());
		MediaBrowser.SubscriptionCallback subscriptionCb = mSubscriptionCb.getValue();

		@@ -195,5 +161,7 @@ public class BrowserPlayerWrapperTest {
		Assert.assertFalse(item_list.get(i).isFolder);
		Assert.assertEquals(item_list.get(i).song, Util.toMetadata(items.get(i)));
		}

		verify(mMockBrowser).disconnect();
		}
		}