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

Commit 2365c3fe authored by Sal Savage's avatar Sal Savage
Browse files

Don't notify of changes to non-VFS state for inactive devices

There were three left over places where we could notifyChanged for
inactive devices:
(1) On Connected, we requestFocus() but ignored the status and updated
state anyway
(2) When browsing anything from the now playing scope we would always
notifyChanged for the node
(3) Cover art downloads for items in the now playing list would result
in notifyChanged calls even when inactive

Case (1) has been addressed by centralizing the state changes in
setActive() in a way thats safer and providers better testability in the
future. Case (2) was solved by checking the scope of a node before
updating anyone. Case (3) moved the notifications from the BrowseTree to
the state machine and piggy-backed off case (2)'s solution.

Tag: #compatibility
Bug: 154959439
Test: atest BluetoothInstrumentationTests, build/flash and test with
various target devices.

Change-Id: Id6967daaa057c4269186df3765ae760a7bb27ace
parent 42278a2b
Loading
Loading
Loading
Loading
+44 −28
Original line number Original line Diff line number Diff line
@@ -42,6 +42,7 @@ import com.android.internal.annotations.VisibleForTesting;


import java.util.ArrayList;
import java.util.ArrayList;
import java.util.List;
import java.util.List;
import java.util.Set;


/**
/**
 * Provides Bluetooth AVRCP Controller State Machine responsible for all remote control connections
 * Provides Bluetooth AVRCP Controller State Machine responsible for all remote control connections
@@ -231,25 +232,39 @@ class AvrcpControllerStateMachine extends StateMachine {
        if (sActiveDevice == null
        if (sActiveDevice == null
                || BluetoothMediaBrowserService.getPlaybackState()
                || BluetoothMediaBrowserService.getPlaybackState()
                != PlaybackStateCompat.STATE_PLAYING) {
                != PlaybackStateCompat.STATE_PLAYING) {
            return setActive();
            return setActive(true);
        }
        }
        return false;
        return false;
    }
    }


    /*
    /**
     * setActive
     * Attempt to set the active status for this device
     *
     * Set this state machine as the active device and update media browse service
     */
     */
    private boolean setActive() {
    boolean setActive(boolean becomeActive) {
        if (DBG) Log.d(TAG, "setActive" + mDevice);
        logD("setActive(" + becomeActive + ")");
        if (A2dpSinkService.getA2dpSinkService().setActiveDeviceNative(mDeviceAddress)) {
        if (becomeActive) {
            if (isActive()) {
                return true;
            }

            A2dpSinkService a2dpSinkService = A2dpSinkService.getA2dpSinkService();
            if (a2dpSinkService == null) {
                return false;
            }

            if (a2dpSinkService.setActiveDeviceNative(mDeviceAddress)) {
                sActiveDevice = mDevice;
                sActiveDevice = mDevice;
                BluetoothMediaBrowserService.addressedPlayerChanged(mSessionCallbacks);
                BluetoothMediaBrowserService.addressedPlayerChanged(mSessionCallbacks);
                BluetoothMediaBrowserService.notifyChanged(mAddressedPlayer.getPlaybackState());
                BluetoothMediaBrowserService.notifyChanged(mAddressedPlayer.getPlaybackState());
                BluetoothMediaBrowserService.notifyChanged(mBrowseTree.mNowPlayingNode);
                BluetoothMediaBrowserService.notifyChanged(mBrowseTree.mNowPlayingNode);
            }
            }
            return mDevice == sActiveDevice;
            return mDevice == sActiveDevice;
        } else if (isActive()) {
            sActiveDevice = null;
            BluetoothMediaBrowserService.trackChanged(null);
            BluetoothMediaBrowserService.addressedPlayerChanged(null);
        }
        return true;
    }
    }


    @Override
    @Override
@@ -303,8 +318,15 @@ class AvrcpControllerStateMachine extends StateMachine {
    }
    }


    private void notifyChanged(BrowseTree.BrowseNode node) {
    private void notifyChanged(BrowseTree.BrowseNode node) {
        // We should only notify now playing content updates if we're the active device. VFS
        // updates are fine at any time
        int scope = node.getScope();
        if (scope != AvrcpControllerService.BROWSE_SCOPE_NOW_PLAYING
                || (scope == AvrcpControllerService.BROWSE_SCOPE_NOW_PLAYING
                && isActive())) {
            BluetoothMediaBrowserService.notifyChanged(node);
            BluetoothMediaBrowserService.notifyChanged(node);
        }
        }
    }


    private void notifyChanged(PlaybackStateCompat state) {
    private void notifyChanged(PlaybackStateCompat state) {
        if (isActive()) {
        if (isActive()) {
@@ -372,8 +394,6 @@ class AvrcpControllerStateMachine extends StateMachine {
        public void enter() {
        public void enter() {
            if (mMostRecentState == BluetoothProfile.STATE_CONNECTING) {
            if (mMostRecentState == BluetoothProfile.STATE_CONNECTING) {
                requestActive();
                requestActive();
                BluetoothMediaBrowserService.addressedPlayerChanged(mSessionCallbacks);
                BluetoothMediaBrowserService.notifyChanged(mAddressedPlayer.getPlaybackState());
                broadcastConnectionStateChanged(BluetoothProfile.STATE_CONNECTED);
                broadcastConnectionStateChanged(BluetoothProfile.STATE_CONNECTED);
                connectCoverArt(); // only works if we have a valid PSM
                connectCoverArt(); // only works if we have a valid PSM
            } else {
            } else {
@@ -468,10 +488,7 @@ class AvrcpControllerStateMachine extends StateMachine {
                case MESSAGE_PROCESS_PLAY_POS_CHANGED:
                case MESSAGE_PROCESS_PLAY_POS_CHANGED:
                    if (msg.arg2 != -1) {
                    if (msg.arg2 != -1) {
                        mAddressedPlayer.setPlayTime(msg.arg2);
                        mAddressedPlayer.setPlayTime(msg.arg2);
                        if (isActive()) {
                        notifyChanged(mAddressedPlayer.getPlaybackState());
                            BluetoothMediaBrowserService.notifyChanged(
                                    mAddressedPlayer.getPlaybackState());
                        }
                    }
                    }
                    return true;
                    return true;


@@ -540,8 +557,11 @@ class AvrcpControllerStateMachine extends StateMachine {
                    }
                    }


                    // Let the browse tree know of the newly downloaded image so it can attach it to
                    // Let the browse tree know of the newly downloaded image so it can attach it to
                    // all the items that need it
                    // all the items that need it. Notify of changed nodes accordingly
                    mBrowseTree.notifyImageDownload(handle, uri);
                    Set<BrowseTree.BrowseNode> nodes = mBrowseTree.notifyImageDownload(handle, uri);
                    for (BrowseTree.BrowseNode node : nodes) {
                        notifyChanged(node);
                    }
                    return true;
                    return true;


                case DISCONNECT:
                case DISCONNECT:
@@ -555,7 +575,7 @@ class AvrcpControllerStateMachine extends StateMachine {
        }
        }


        private void processPlayItem(BrowseTree.BrowseNode node) {
        private void processPlayItem(BrowseTree.BrowseNode node) {
            setActive();
            setActive(true);
            if (node == null) {
            if (node == null) {
                Log.w(TAG, "Invalid item to play");
                Log.w(TAG, "Invalid item to play");
            } else {
            } else {
@@ -872,11 +892,7 @@ class AvrcpControllerStateMachine extends StateMachine {
        public void enter() {
        public void enter() {
            disconnectCoverArt();
            disconnectCoverArt();
            onBrowsingDisconnected();
            onBrowsingDisconnected();
            if (isActive()) {
            setActive(false);
                sActiveDevice = null;
                BluetoothMediaBrowserService.trackChanged(null);
                BluetoothMediaBrowserService.addressedPlayerChanged(null);
            }
            broadcastConnectionStateChanged(BluetoothProfile.STATE_DISCONNECTING);
            broadcastConnectionStateChanged(BluetoothProfile.STATE_DISCONNECTING);
            transitionTo(mDisconnected);
            transitionTo(mDisconnected);
        }
        }
+6 −0
Original line number Original line Diff line number Diff line
@@ -163,6 +163,11 @@ public class BluetoothMediaBrowserService extends MediaBrowserServiceCompat {
        mSession.setQueue(mMediaQueue);
        mSession.setQueue(mMediaQueue);
    }
    }


    private void clearNowPlayingQueue() {
        mMediaQueue.clear();
        mSession.setQueue(mMediaQueue);
    }

    static synchronized void notifyChanged(BrowseTree.BrowseNode node) {
    static synchronized void notifyChanged(BrowseTree.BrowseNode node) {
        if (sBluetoothMediaBrowserService != null) {
        if (sBluetoothMediaBrowserService != null) {
            if (node.getScope() == AvrcpControllerService.BROWSE_SCOPE_NOW_PLAYING) {
            if (node.getScope() == AvrcpControllerService.BROWSE_SCOPE_NOW_PLAYING) {
@@ -177,6 +182,7 @@ public class BluetoothMediaBrowserService extends MediaBrowserServiceCompat {
        if (sBluetoothMediaBrowserService != null) {
        if (sBluetoothMediaBrowserService != null) {
            if (callback == null) {
            if (callback == null) {
                sBluetoothMediaBrowserService.setErrorPlaybackState();
                sBluetoothMediaBrowserService.setErrorPlaybackState();
                sBluetoothMediaBrowserService.clearNowPlayingQueue();
            }
            }
            sBluetoothMediaBrowserService.mSession.setCallback(callback);
            sBluetoothMediaBrowserService.mSession.setCallback(callback);
        } else {
        } else {
+5 −7
Original line number Original line Diff line number Diff line
@@ -25,6 +25,7 @@ import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashMap;
import java.util.HashSet;
import java.util.HashSet;
import java.util.List;
import java.util.List;
import java.util.Set;
import java.util.UUID;
import java.util.UUID;


/**
/**
@@ -451,8 +452,10 @@ public class BrowseTree {


    /**
    /**
     * Adds the Uri of a newly downloaded image to all tree nodes using that specific handle.
     * Adds the Uri of a newly downloaded image to all tree nodes using that specific handle.
     * Returns the set of parent nodes that have children impacted by the new art so clients can
     * be notified of the change.
     */
     */
    synchronized void notifyImageDownload(String handle, Uri uri) {
    synchronized Set<BrowseNode> notifyImageDownload(String handle, Uri uri) {
        if (DBG) Log.d(TAG, "Received downloaded image handle to cascade to BrowseNodes using it");
        if (DBG) Log.d(TAG, "Received downloaded image handle to cascade to BrowseNodes using it");
        ArrayList<String> nodes = getNodesUsingCoverArt(handle);
        ArrayList<String> nodes = getNodesUsingCoverArt(handle);
        HashSet<BrowseNode> parents = new HashSet<BrowseNode>();
        HashSet<BrowseNode> parents = new HashSet<BrowseNode>();
@@ -468,12 +471,7 @@ public class BrowseTree {
                parents.add(node.mParent);
                parents.add(node.mParent);
            }
            }
        }
        }

        return parents;
        // Update *parents* of changed nodes so applications will re-grab this node, now with art
        for (BrowseNode node : parents) {
            if (DBG) Log.d(TAG, "Notify node '" + node.getID() + "' that child has cover art now");
            BluetoothMediaBrowserService.notifyChanged(node);
        }
    }
    }