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

Commit 9d065175 authored by Sal Savage's avatar Sal Savage
Browse files

Uncache the current browsed player when we change browsed players 

This change fixes a bug where we don't uncache a player object when we
switch to a new player. This keeps us from ever being able to get fresh
player contents when things get updated and we haven't received an
available players changed event. This also doesn't fit in with our
current caching scheme.

Bug: 337259487
Bug: 337257397
Test: atest AvrcpControllerStateMachineTest.java
Change-Id: I36b0120dfe418b9a220ffaed8406e8c3d0fbd0e4
parent 9545810f

android/app/src/com/android/bluetooth/avrcpcontroller/AvrcpControllerStateMachine.java

+26 −1
Original line number Diff line number Diff line
@@ -41,6 +41,7 @@ import com.android.bluetooth.a2dpsink.A2dpSinkService; 
import com.android.bluetooth.btservice.AdapterService;

import com.android.bluetooth.btservice.MetricsLogger;

import com.android.bluetooth.btservice.ProfileService;

import com.android.bluetooth.flags.Flags;

import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.util.State;

import com.android.internal.util.StateMachine;
@@ -262,6 +263,7 @@ class AvrcpControllerStateMachine extends StateMachine { 
                        + (mCoverArtManager.getState(mDevice) == BluetoothProfile.STATE_CONNECTED));



        ProfileService.println(sb, "Addressed Player ID: " + mAddressedPlayerId);

        ProfileService.println(sb, "Browsed Player ID: " + mBrowseTree.getCurrentBrowsedPlayer());

        ProfileService.println(sb, "Available Players (" + mAvailablePlayerList.size() + "): ");

        for (int i = 0; i < mAvailablePlayerList.size(); i++) {

            AvrcpPlayer player = mAvailablePlayerList.valueAt(i);
@@ -1119,11 +1121,34 @@ class AvrcpControllerStateMachine extends StateMachine { 
                fetchContents(mNextStep);

            } else if (mNextStep.isPlayer()) {

                debug("GetFolderList: NAVIGATING Player " + mNextStep.toString());

                BrowseTree.BrowseNode currentBrowsedPlayer = mBrowseTree.getCurrentBrowsedPlayer();

                if (Flags.uncachePlayerWhenBrowsedPlayerChanges()) {

                    if (currentBrowsedPlayer != null) {

                        debug(

                                "GetFolderList: Uncache current browsed player, player="

                                        + currentBrowsedPlayer);

                        mBrowseTree.getCurrentBrowsedPlayer().setCached(false);

                    } else {

                        debug(

                                "GetFolderList: Browsed player unset, no need to uncache the"

                                        + " previous player");

                    }

                } else {

                    debug(

                            "GetFolderList: Feature not available: uncache current browsed player"

                                    + " on switch");

                }



                if (mNextStep.isBrowsable()) {

                    debug(

                            "GetFolderList: Set browsed player, old="

                                    + currentBrowsedPlayer

                                    + ", new="

                                    + mNextStep);

                    mNativeInterface.setBrowsedPlayer(

                            mDeviceAddress, (int) mNextStep.getBluetoothID());

                } else {

                    debug("GetFolderList: Player doesn't support browsing");

                    debug("GetFolderList: Target player doesn't support browsing");

                    mNextStep.setCached(true);

                    transitionTo(mConnected);

                }

android/app/src/com/android/bluetooth/avrcpcontroller/BrowseTree.java

+9 −4
Original line number Diff line number Diff line
@@ -305,10 +305,13 @@ public class BrowseTree { 
        }



        synchronized void setCached(boolean cached) {

            Log.d(TAG, "Set Cache" + cached + "Node" + toString());

            Log.d(TAG, "Set cached=" + cached + ", node=" + toString());

            mCached = cached;

            if (!cached) {

                for (BrowseNode child : mChildren) {

                    if (Flags.uncachePlayerWhenBrowsedPlayerChanges()) {

                        child.setCached(false);

                    }

                    mBrowseMap.remove(child.getID());

                    indicateCoverArtUnused(child.getID(), child.getCoverArtUuid());

                }
@@ -374,11 +377,13 @@ public class BrowseTree { 


        @Override

        public synchronized String toString() {

            return "[Id: "

            return "[id="

                    + getID()

                    + " Name: "

                    + ", name="

                    + getMediaItem().getDescription().getTitle()

                    + " Size: "

                    + ", cached="

                    + isCached()

                    + ", size="

                    + mChildren.size()

                    + "]";

        }

android/app/tests/unit/src/com/android/bluetooth/avrcpcontroller/AvrcpControllerStateMachineTest.java

+99 −0
Original line number Diff line number Diff line
@@ -2077,4 +2077,103 @@ public class AvrcpControllerStateMachineTest { 
        TestUtils.waitForLooperToBeIdle(mAvrcpStateMachine.getHandler().getLooper());

        verify(mNativeInterface, times(1)).getPlayerList(eq(mTestAddress), eq(0), eq(19));

    }



    @Test

    @EnableFlags(Flags.FLAG_UNCACHE_PLAYER_WHEN_BROWSED_PLAYER_CHANGES)

    public void testBrowsingContentsOfOtherBrowsablePlayer_browsedPlayerUncached() {

        setUpConnectedState(true, true);

        sendAudioFocusUpdate(AudioManager.AUDIOFOCUS_GAIN);



        BrowseTree.BrowseNode results = mAvrcpStateMachine.mBrowseTree.mRootNode;



        // Request fetch the list of players

        BrowseTree.BrowseNode playerNodes = mAvrcpStateMachine.findNode(results.getID());

        mAvrcpStateMachine.requestContents(playerNodes);

        verify(mNativeInterface, timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(1))

                .getPlayerList(eq(mTestAddress), eq(0), eq(19));



        // Provide back two player objects

        byte[] playerFeatures =

                new byte[] {0, 0, 0, 0, 0, (byte) 0xb7, 0x01, 0x0c, 0x0a, 0, 0, 0, 0, 0, 0, 0};

        AvrcpPlayer playerOne = makePlayer(mTestDevice, 1, "player 1", playerFeatures, 1);

        AvrcpPlayer playerTwo = makePlayer(mTestDevice, 2, "player 2", playerFeatures, 1);

        List<AvrcpPlayer> testPlayers = new ArrayList<>();

        testPlayers.add(playerOne);

        testPlayers.add(playerTwo);

        mAvrcpStateMachine.sendMessage(

                AvrcpControllerStateMachine.MESSAGE_PROCESS_GET_PLAYER_ITEMS, testPlayers);

        TestUtils.waitForLooperToFinishScheduledTask(mAvrcpStateMachine.getHandler().getLooper());



        // Verify that the player objects are both available and properly formatted

        playerNodes = mAvrcpStateMachine.findNode(results.getID());

        assertThat(playerNodes.isCached()).isTrue();

        assertThat(playerNodes.getChildren()).isNotNull();

        assertThat(playerNodes.getChildren().size()).isEqualTo(2);

        assertThat(playerNodes.getChildren().get(0).getMediaItem().toString())

                .isEqualTo("MediaItem{mFlags=1, mDescription=player 1, null, null}");

        assertThat(playerNodes.getChildren().get(1).getMediaItem().toString())

                .isEqualTo("MediaItem{mFlags=1, mDescription=player 2, null, null}");



        // Fetch contents of the first player object

        BrowseTree.BrowseNode playerOneNode =

                mAvrcpStateMachine.findNode(results.getChildren().get(0).getID());

        mAvrcpStateMachine.requestContents(playerOneNode);

        verify(mNativeInterface, timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(1))

                .setBrowsedPlayer(eq(mTestAddress), eq(1));

        mAvrcpStateMachine.sendMessage(

                AvrcpControllerStateMachine.MESSAGE_PROCESS_SET_BROWSED_PLAYER,

                /* items= */ 5,

                /* depth= */ 0);

        verify(mNativeInterface, timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(1))

                .getFolderList(eq(mTestAddress), eq(0), eq(4));



        // Return some results for Player One

        List<AvrcpItem> testFolderContents = new ArrayList<AvrcpItem>();

        for (int i = 0; i < 5; i++) {

            String title = "Song " + Integer.toString(i);

            testFolderContents.add(makeNowPlayingItem(i, title));

        }

        mAvrcpStateMachine.sendMessage(

                AvrcpControllerStateMachine.MESSAGE_PROCESS_GET_FOLDER_ITEMS, testFolderContents);

        mAvrcpStateMachine.sendMessage(

                AvrcpControllerStateMachine.MESSAGE_PROCESS_GET_FOLDER_ITEMS_OUT_OF_RANGE);

        TestUtils.waitForLooperToFinishScheduledTask(mAvrcpStateMachine.getHandler().getLooper());



        // Make sure the player/folder is cached

        playerOneNode = mAvrcpStateMachine.findNode(results.getChildren().get(0).getID());

        assertThat(playerOneNode.isCached()).isTrue();



        // Browse to the Player Two

        BrowseTree.BrowseNode playerTwoNode =

                mAvrcpStateMachine.findNode(results.getChildren().get(1).getID());

        mAvrcpStateMachine.requestContents(playerTwoNode);

        verify(mNativeInterface, timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(1))

                .setBrowsedPlayer(eq(mTestAddress), eq(2));

        mAvrcpStateMachine.sendMessage(

                AvrcpControllerStateMachine.MESSAGE_PROCESS_SET_BROWSED_PLAYER,

                /* items= */ 5,

                /* depth= */ 0);

        verify(mNativeInterface, timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(2))

                .getFolderList(eq(mTestAddress), eq(0), eq(4));



        // Make sure the first player is uncached

        playerOneNode = mAvrcpStateMachine.findNode(results.getChildren().get(0).getID());

        assertThat(playerOneNode.isCached()).isFalse();



        // Send items for Player Two

        testFolderContents = new ArrayList<AvrcpItem>();

        for (int i = 5; i < 10; i++) {

            String title = "Song " + Integer.toString(i);

            testFolderContents.add(makeNowPlayingItem(i, title));

        }

        mAvrcpStateMachine.sendMessage(

                AvrcpControllerStateMachine.MESSAGE_PROCESS_GET_FOLDER_ITEMS, testFolderContents);

        mAvrcpStateMachine.sendMessage(

                AvrcpControllerStateMachine.MESSAGE_PROCESS_GET_FOLDER_ITEMS_OUT_OF_RANGE);

        TestUtils.waitForLooperToFinishScheduledTask(mAvrcpStateMachine.getHandler().getLooper());



        // make sure the second player is cached now

        playerTwoNode = mAvrcpStateMachine.findNode(results.getChildren().get(1).getID());

        assertThat(playerTwoNode.isCached()).isTrue();

    }

}

android/app/tests/unit/src/com/android/bluetooth/avrcpcontroller/BrowseNodeTest.java

+48 −5
Original line number Diff line number Diff line
@@ -184,6 +184,49 @@ public class BrowseNodeTest { 
        assertThat(mRootNode.getChildrenCount()).isEqualTo(0);

    }



    @Test

    @EnableFlags(Flags.FLAG_UNCACHE_PLAYER_WHEN_BROWSED_PLAYER_CHANGES)

    public void setUncached_whenNodeHasChildrenNodes() {

        BrowseNode deviceNode = mBrowseTree.new BrowseNode(mTestDevice);

        mRootNode.addChild(deviceNode);

        mRootNode.setCached(true);



        BrowseNode browseNodeChild1 =

                mBrowseTree.new BrowseNode(new AvrcpItem.Builder().setUuid("child1").build());

        BrowseNode browseNodeChild2 =

                mBrowseTree.new BrowseNode(new AvrcpItem.Builder().setUuid("child2").build());

        BrowseNode browseNodeChild3 =

                mBrowseTree.new BrowseNode(new AvrcpItem.Builder().setUuid("child3").build());

        deviceNode.addChild(browseNodeChild1);

        deviceNode.addChild(browseNodeChild2);

        deviceNode.addChild(browseNodeChild3);

        deviceNode.setCached(true);



        BrowseNode browseNodeChild1_1 =

                mBrowseTree.new BrowseNode(new AvrcpItem.Builder().setUuid("child1_1").build());

        browseNodeChild1.addChild(browseNodeChild1_1);

        browseNodeChild1.setCached(true);



        assertThat(mRootNode.isCached()).isTrue();

        assertThat(deviceNode.isCached()).isTrue();

        assertThat(browseNodeChild1.isCached()).isTrue();

        assertThat(mRootNode.getChildrenCount()).isEqualTo(1);

        assertThat(deviceNode.getChildrenCount()).isEqualTo(3);

        assertThat(browseNodeChild1.getChildrenCount()).isEqualTo(1);



        deviceNode.setCached(false);



        assertThat(mRootNode.isCached()).isTrue();

        assertThat(deviceNode.isCached()).isFalse();

        assertThat(browseNodeChild1.isCached()).isFalse();

        assertThat(browseNodeChild2.isCached()).isFalse();

        assertThat(browseNodeChild3.isCached()).isFalse();

        assertThat(browseNodeChild1_1.isCached()).isFalse();

        assertThat(mRootNode.getChildrenCount()).isEqualTo(1);

        assertThat(deviceNode.getChildrenCount()).isEqualTo(0);

        assertThat(browseNodeChild1.getChildrenCount()).isEqualTo(0);

    }



    @Test

    public void getters() {

        BrowseNode browseNode =
@@ -224,10 +267,10 @@ public class BrowseNodeTest { 
    @Test

    public void toTreeString_returnFormattedString() {

        final String expected =

                "  [Id: 1111 Name: item Size: 2]\n"

                        + "    [Id: child1 Name: child1 Size: 1]\n"

                        + "      [Id: child3 Name: child3 Size: 0]\n"

                        + "    [Id: child2 Name: child2 Size: 0]\n";

                "  [id=1111, name=item, cached=false, size=2]\n"

                        + "    [id=child1, name=child1, cached=false, size=1]\n"

                        + "      [id=child3, name=child3, cached=false, size=0]\n"

                        + "    [id=child2, name=child2, cached=false, size=0]\n";



        BrowseNode browseNode =

                mBrowseTree
@@ -277,6 +320,6 @@ public class BrowseNodeTest { 
                                .build());



        assertThat(browseNode.toString())

                .isEqualTo("[Id: " + TEST_UUID + " Name: " + TEST_NAME + " Size: 0]");

                .isEqualTo("[id=" + TEST_UUID + ", name=" + TEST_NAME + ", cached=false, size=0]");

    }

}