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

Commit e5f93195 authored by Sungsoo Lim's avatar Sungsoo Lim
Browse files

Refactor BrowserPlayerConnector 

And added tests including the potential NPE that happens after cleanup().

Bug: 296461529
Bug: 295590999
Test: atest BluetoothInstrumentationTests:ProfileServiceTest 200 times
Change-Id: I7ab59cb7b5b345a37c5e79134e91f04b6966bee8
parent 27f9032b

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

+74 −78
Original line number Diff line number Diff line
@@ -41,7 +41,7 @@ import java.util.Set; 
 * using the MediaBrowserService. This way we do not have to do the same checks

 * when constructing BrowsedPlayerWrappers by hand.

 */

public class BrowsablePlayerConnector {

public class BrowsablePlayerConnector extends Handler {

    private static final String TAG = "AvrcpBrowsablePlayerConnector";

    private static final boolean DEBUG = true;

    private static final long CONNECT_TIMEOUT_MS = 10000; // Time in ms to wait for a connection
@@ -51,7 +51,6 @@ public class BrowsablePlayerConnector { 
    private static final int MSG_TIMEOUT = 2;



    private static BrowsablePlayerConnector sInjectConnector;

    private Handler mHandler;

    private PlayerListCallback mCallback;



    private List<BrowsedPlayerWrapper> mResults = new ArrayList<BrowsedPlayerWrapper>();
@@ -83,7 +82,7 @@ public class BrowsablePlayerConnector { 
            return null;

        }



        BrowsablePlayerConnector newWrapper = new BrowsablePlayerConnector(looper, cb);

        BrowsablePlayerConnector newConnector = new BrowsablePlayerConnector(looper, cb);



        // Try to start connecting all the browsed player wrappers

        for (ResolveInfo info : players) {
@@ -92,7 +91,7 @@ public class BrowsablePlayerConnector { 
                            looper,

                            info.serviceInfo.packageName,

                            info.serviceInfo.name);

            newWrapper.mPendingPlayers.add(player);

            newConnector.mPendingPlayers.add(player);

            player.connect((int status, BrowsedPlayerWrapper wrapper) -> {

                // Use the handler to avoid concurrency issues

                if (DEBUG) {
@@ -100,21 +99,36 @@ public class BrowsablePlayerConnector { 
                            + info.serviceInfo.packageName

                            + " : status=" + status);

                }

                Message msg = newWrapper.mHandler.obtainMessage(MSG_CONNECT_CB);

                msg.arg1 = status;

                msg.obj = wrapper;

                newWrapper.mHandler.sendMessage(msg);

                newConnector.obtainMessage(MSG_CONNECT_CB, status, 0, wrapper).sendToTarget();

            });

        }



        Message msg = newWrapper.mHandler.obtainMessage(MSG_TIMEOUT);

        newWrapper.mHandler.sendMessageDelayed(msg, CONNECT_TIMEOUT_MS);

        return newWrapper;

        newConnector.sendEmptyMessageDelayed(MSG_TIMEOUT, CONNECT_TIMEOUT_MS);

        return newConnector;

    }



    private BrowsablePlayerConnector(Looper looper, PlayerListCallback cb) {

        super(looper);

        mCallback = cb;

        mHandler = new Handler(looper) {

    }



    private void removePendingPlayers() {

        for (BrowsedPlayerWrapper wrapper : mPendingPlayers) {

            if (DEBUG) Log.d(TAG, "Disconnecting " + wrapper.getPackageName());

            wrapper.disconnect();

        }

        mPendingPlayers.clear();

    }



    void cleanup() {

        if (mPendingPlayers.size() != 0) {

            Log.i(TAG, "Bluetooth turn off with " + mPendingPlayers.size() + " pending player(s)");

            removePendingPlayers();

            removeCallbacksAndMessages(null);

        }

    }



    @Override

    public void handleMessage(Message msg) {

        if (DEBUG) Log.d(TAG, "Received a message: msg.what=" + msg.what);

        switch(msg.what) {
@@ -125,6 +139,7 @@ public class BrowsablePlayerConnector { 


                // If we failed to remove the wrapper from the pending set, that

                // means a timeout occurred and the callback was triggered afterwards

                // or the connector was cleaned up.

                if (!mPendingPlayers.remove(wrapper)) {

                    return;

                }
@@ -134,7 +149,8 @@ public class BrowsablePlayerConnector { 
                            + wrapper.getPackageName());

                    mResults.add(wrapper);

                }

                    } break;

                break;

            }



            case MSG_CONNECT_CB: {

                BrowsedPlayerWrapper wrapper = (BrowsedPlayerWrapper) msg.obj;
@@ -157,18 +173,17 @@ public class BrowsablePlayerConnector { 
                        (int status, String mediaId, List<ListItem> results) -> {

                            // Send the response as a message so that it is properly

                            // synchronized

                                    Message cb = obtainMessage(MSG_GET_FOLDER_ITEMS_CB);

                                    cb.arg1 = status;

                                    cb.arg2 = results.size();

                                    cb.obj = wrapper;

                                    sendMessage(cb);

                            obtainMessage(MSG_GET_FOLDER_ITEMS_CB, status, results.size(), wrapper)

                                    .sendToTarget();

                        });

                    } break;

                break;

            }



            case MSG_TIMEOUT: {

                Log.v(TAG, "Timed out waiting for players");

                removePendingPlayers();

                    } break;

                break;

            }

        }



        if (mPendingPlayers.size() == 0) {
@@ -178,23 +193,4 @@ public class BrowsablePlayerConnector { 
            mCallback.run(mResults);

        }

    }

        };

    }



    private void removePendingPlayers() {

        for (BrowsedPlayerWrapper wrapper : mPendingPlayers) {

            if (DEBUG) Log.d(TAG, "Disconnecting " + wrapper.getPackageName());

            wrapper.disconnect();

        }

        mPendingPlayers.clear();

    }



    void cleanup() {

        if (mPendingPlayers.size() != 0) {

            Log.i(TAG, "Bluetooth turn off with " + mPendingPlayers.size() + " pending player(s)");

            mHandler.removeMessages(MSG_TIMEOUT);

            removePendingPlayers();

            mHandler = null;

        }

    }

}

android/app/tests/unit/src/com/android/bluetooth/audio_util/BrowsablePlayerConnectorTest.java

0 → 100644
+118 −0
Original line number Diff line number Diff line 
/*

 * Copyright 2023 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.bluetooth.audio_util;



import static com.google.common.truth.Truth.assertThat;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.timeout;

import static org.mockito.Mockito.verify;



import android.content.Context;

import android.content.pm.ResolveInfo;

import android.content.pm.ServiceInfo;

import android.os.test.TestLooper;



import androidx.test.InstrumentationRegistry;

import androidx.test.runner.AndroidJUnit4;



import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.Mock;

import org.mockito.MockitoAnnotations;



import java.util.ArrayList;

import java.util.Collections;

import java.util.List;

import java.util.concurrent.CountDownLatch;

import java.util.concurrent.TimeUnit;



@RunWith(AndroidJUnit4.class)

public final class BrowsablePlayerConnectorTest {

    private static final int TIMEOUT_MS = 300;



    Context mContext;

    TestLooper mTestLooper;

    List<ResolveInfo> mPlayerList;

    @Mock MediaBrowser mMediaBrowser;

    MediaBrowser.ConnectionCallback mConnectionCallback;

    @Before

    public void setUp() {

        MockitoAnnotations.initMocks(this);

        mContext = InstrumentationRegistry.getTargetContext();

        mTestLooper = new TestLooper();



        doAnswer(invocation -> {

            mConnectionCallback = invocation.getArgument(2);

            return null;

        }).when(mMediaBrowser).testInit(any(), any(), any(), any());

        doAnswer(invocation -> {

            mConnectionCallback.onConnected();

            return null;

        }).when(mMediaBrowser).connect();

        doAnswer(invocation -> {

            String id = invocation.getArgument(0);

            android.media.browse.MediaBrowser.SubscriptionCallback callback

                    = invocation.getArgument(1);

            callback.onChildrenLoaded(id, Collections.emptyList());

            return null;

        }).when(mMediaBrowser).subscribe(any(), any());

        doReturn("testRoot").when(mMediaBrowser).getRoot();

        MediaBrowserFactory.inject(mMediaBrowser);



        ResolveInfo player = new ResolveInfo();

        player.serviceInfo = new ServiceInfo();

        player.serviceInfo.packageName = "com.android.bluetooth.test";

        player.serviceInfo.name = "TestPlayer";

        mPlayerList = new ArrayList();

        mPlayerList.add(player);

    }



    @Test

    public void browsablePlayerConnectorCallback_calledAfterConnection()

            throws InterruptedException {

        mTestLooper.startAutoDispatch();

        CountDownLatch latch = new CountDownLatch(1);

        BrowsablePlayerConnector connector =

                BrowsablePlayerConnector.connectToPlayers(

                        mContext,

                        mTestLooper.getLooper(),

                        mPlayerList,

                        (List<BrowsedPlayerWrapper> players) -> latch.countDown());

        verify(mMediaBrowser, timeout(TIMEOUT_MS)).connect();

        assertThat(latch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)).isTrue();

        connector.cleanup();

        mTestLooper.stopAutoDispatch();

        mTestLooper.dispatchAll();

    }



    @Test

    public void cleanup_doesNotCrash() {

        BrowsablePlayerConnector connector =

                BrowsablePlayerConnector.connectToPlayers(

                        mContext,

                        mTestLooper.getLooper(),

                        mPlayerList,

                        (List<BrowsedPlayerWrapper> players) -> {});

        verify(mMediaBrowser, timeout(TIMEOUT_MS)).connect();

        connector.cleanup();

        mTestLooper.dispatchAll();

    }

}