Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 046d89ad authored by Ajay Panicker's avatar Ajay Panicker
Browse files

Disconnect MediaBrowserWrapper after every operation 

Instead of trying to keep Browsable Players in a conected state whenever
possible, disconnect them immediatly after every opperation in order to
prevent them from keeping a high priority.

Bug: 78787396
Test: "adb shell dumpsys activity" and see that browsable players don't
have thier priorities bumped.
Change-Id: I66eede99fe256676ef64596f73aed00d7142f8f7
parent ba0051a5

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();

    }

}