Use mapped UUIDs to identify handles and invalidate/delete on disconnect
By specification, image handles can be recycled and reused each time a
device disconnects from BIP (if we/they are not database aware).
Additionally, by specification, we MUST disconnect on successful change
paths when either side is not database aware, or on UIDS_CHANGED when
both sides aredatabase aware.
This means anytime we disconnect we have no guarantees that a handle
we're given will point to the same image we've seen before. It also
means we'll be doing it often.
Because we (and other consumers, likely) are caching images, we need a
layer of indirection between the handles coming in and how we identify
images. Using a specific UUID and a map will ollow us to uniquely
identify an image for a point in time. This will also allow caching
schemes that our consumers may have to actually work since we can use
this UUID in the URI.
This change implements the above described UUID map and makes sure
images are deleted from storage when they are no longer used by the
browse tree or current track.
Tag: #compatibility
Bug: 153461730
Test: atest BluetoothInstrumentationTests
Merged-In: I757c9bae1327b255b83dd1d43624ec4ec8009501
Change-Id: I757c9bae1327b255b83dd1d43624ec4ec8009501
diff --git a/src/com/android/bluetooth/avrcpcontroller/AvrcpControllerService.java b/src/com/android/bluetooth/avrcpcontroller/AvrcpControllerService.java
index a6db63c..b6907ec 100755
--- a/src/com/android/bluetooth/avrcpcontroller/AvrcpControllerService.java
+++ b/src/com/android/bluetooth/avrcpcontroller/AvrcpControllerService.java
@@ -103,13 +103,13 @@
public void onImageDownloadComplete(BluetoothDevice device,
AvrcpCoverArtManager.DownloadEvent event) {
if (DBG) {
- Log.d(TAG, "Image downloaded [device: " + device + ", handle: " + event.getHandle()
+ Log.d(TAG, "Image downloaded [device: " + device + ", uuid: " + event.getUuid()
+ ", uri: " + event.getUri());
}
AvrcpControllerStateMachine stateMachine = getStateMachine(device);
if (stateMachine == null) {
Log.e(TAG, "No state machine found for device " + device);
- mCoverArtManager.removeImage(device, event.getHandle());
+ mCoverArtManager.removeImage(device, event.getUuid());
return;
}
stateMachine.sendMessage(AvrcpControllerStateMachine.MESSAGE_PROCESS_IMAGE_DOWNLOADED,
@@ -423,6 +423,12 @@
aib.setItemType(AvrcpItem.TYPE_MEDIA);
aib.setUuid(UUID.randomUUID().toString());
AvrcpItem item = aib.build();
+ if (mCoverArtManager != null) {
+ String handle = item.getCoverArtHandle();
+ if (handle != null) {
+ item.setCoverArtUuid(mCoverArtManager.getUuidForHandle(device, handle));
+ }
+ }
stateMachine.sendMessage(AvrcpControllerStateMachine.MESSAGE_PROCESS_TRACK_CHANGED,
item);
}
@@ -529,13 +535,19 @@
+ items.length + " items.");
}
+ BluetoothDevice device = mAdapter.getRemoteDevice(address);
List<AvrcpItem> itemsList = new ArrayList<>();
for (AvrcpItem item : items) {
if (VDBG) Log.d(TAG, item.toString());
+ if (mCoverArtManager != null) {
+ String handle = item.getCoverArtHandle();
+ if (handle != null) {
+ item.setCoverArtUuid(mCoverArtManager.getUuidForHandle(device, handle));
+ }
+ }
itemsList.add(item);
}
- BluetoothDevice device = mAdapter.getRemoteDevice(address);
AvrcpControllerStateMachine stateMachine = getStateMachine(device);
if (stateMachine != null) {
stateMachine.sendMessage(AvrcpControllerStateMachine.MESSAGE_PROCESS_GET_FOLDER_ITEMS,
diff --git a/src/com/android/bluetooth/avrcpcontroller/AvrcpControllerStateMachine.java b/src/com/android/bluetooth/avrcpcontroller/AvrcpControllerStateMachine.java
index 440d366..00fed72 100755
--- a/src/com/android/bluetooth/avrcpcontroller/AvrcpControllerStateMachine.java
+++ b/src/com/android/bluetooth/avrcpcontroller/AvrcpControllerStateMachine.java
@@ -560,13 +560,13 @@
case MESSAGE_PROCESS_IMAGE_DOWNLOADED:
AvrcpCoverArtManager.DownloadEvent event =
(AvrcpCoverArtManager.DownloadEvent) msg.obj;
- String handle = event.getHandle();
+ String uuid = event.getUuid();
Uri uri = event.getUri();
- logD("Received image for " + handle + " at " + uri.toString());
+ logD("Received image for " + uuid + " at " + uri.toString());
// Let the addressed player know we got an image so it can see if the current
// track now has cover artwork
- boolean addedArtwork = mAddressedPlayer.notifyImageDownload(handle, uri);
+ boolean addedArtwork = mAddressedPlayer.notifyImageDownload(uuid, uri);
if (addedArtwork && isActive()) {
BluetoothMediaBrowserService.trackChanged(
mAddressedPlayer.getCurrentTrack());
@@ -574,7 +574,7 @@
// Let the browse tree know of the newly downloaded image so it can attach it to
// all the items that need it. Notify of changed nodes accordingly
- Set<BrowseTree.BrowseNode> nodes = mBrowseTree.notifyImageDownload(handle, uri);
+ Set<BrowseTree.BrowseNode> nodes = mBrowseTree.notifyImageDownload(uuid, uri);
for (BrowseTree.BrowseNode node : nodes) {
notifyChanged(node);
}
@@ -705,9 +705,11 @@
// Queue up image download if the item has an image and we don't have it yet
// Only do this if the feature is enabled.
- if (shouldDownloadBrowsedImages()) {
- for (AvrcpItem track : folderList) {
+ for (AvrcpItem track : folderList) {
+ if (shouldDownloadBrowsedImages()) {
downloadImageIfNeeded(track);
+ } else {
+ track.setCoverArtUuid(null);
}
}
@@ -984,14 +986,14 @@
private void downloadImageIfNeeded(AvrcpItem track) {
if (mCoverArtManager == null) return;
- String handle = track.getCoverArtHandle();
+ String uuid = track.getCoverArtUuid();
Uri imageUri = null;
- if (handle != null) {
- imageUri = mCoverArtManager.getImageUri(mDevice, handle);
+ if (uuid != null) {
+ imageUri = mCoverArtManager.getImageUri(mDevice, uuid);
if (imageUri != null) {
track.setCoverArtLocation(imageUri);
} else {
- mCoverArtManager.downloadImage(mDevice, handle);
+ mCoverArtManager.downloadImage(mDevice, uuid);
}
}
}
diff --git a/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtManager.java b/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtManager.java
index 784d464..8315a62 100644
--- a/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtManager.java
+++ b/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtManager.java
@@ -23,6 +23,8 @@
import android.util.Log;
import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import javax.obex.ResponseCodes;
@@ -46,6 +48,7 @@
private final AvrcpControllerService mService;
protected final Map<BluetoothDevice, AvrcpBipClient> mClients = new ConcurrentHashMap<>(1);
+ private Map<BluetoothDevice, AvrcpBipSession> mBipSessions = new ConcurrentHashMap<>(1);
private final AvrcpCoverArtStorage mCoverArtStorage;
private final Callback mCallback;
private final String mDownloadScheme;
@@ -55,14 +58,14 @@
* retrieve the image from storage.
*/
public class DownloadEvent {
- final String mImageHandle;
+ final String mImageUuid;
final Uri mUri;
- public DownloadEvent(String handle, Uri uri) {
- mImageHandle = handle;
+ public DownloadEvent(String uuid, Uri uri) {
+ mImageUuid = uuid;
mUri = uri;
}
- public String getHandle() {
- return mImageHandle;
+ public String getUuid() {
+ return mImageUuid;
}
public Uri getUri() {
return mUri;
@@ -80,6 +83,44 @@
void onImageDownloadComplete(BluetoothDevice device, DownloadEvent event);
}
+ /**
+ * A thread-safe collection of BIP connection specific imformation meant to be cleared each
+ * time a client disconnects from the Target's BIP OBEX server.
+ *
+ * Currently contains the mapping of image handles seen to assigned UUIDs.
+ */
+ private class AvrcpBipSession {
+ private final BluetoothDevice mDevice;
+ private Map<String, String> mUuids = new ConcurrentHashMap<>(1); /* handle -> UUID */
+ private Map<String, String> mHandles = new ConcurrentHashMap<>(1); /* UUID -> handle */
+
+ AvrcpBipSession(BluetoothDevice device) {
+ mDevice = device;
+ }
+
+ public String getHandleUuid(String handle) {
+ if (handle == null) return null;
+ String newUuid = UUID.randomUUID().toString();
+ String existingUuid = mUuids.putIfAbsent(handle, newUuid);
+ if (existingUuid != null) return existingUuid;
+ mHandles.put(newUuid, handle);
+ return newUuid;
+ }
+
+ public String getUuidHandle(String uuid) {
+ return mHandles.get(uuid);
+ }
+
+ public void clearHandleUuids() {
+ mUuids.clear();
+ mHandles.clear();
+ }
+
+ public Set<String> getSessionHandles() {
+ return mUuids.keySet();
+ }
+ }
+
public AvrcpCoverArtManager(AvrcpControllerService service, Callback callback) {
mService = service;
mCoverArtStorage = new AvrcpCoverArtStorage(mService);
@@ -101,6 +142,7 @@
if (mClients.containsKey(device)) return false;
AvrcpBipClient client = new AvrcpBipClient(device, psm, new BipClientCallback(device));
mClients.put(device, client);
+ mBipSessions.put(device, new AvrcpBipSession(device));
return true;
}
@@ -136,6 +178,7 @@
}
client.shutdown();
mClients.remove(device);
+ mBipSessions.remove(device);
mCoverArtStorage.removeImagesForDevice(device);
return true;
}
@@ -165,16 +208,59 @@
return client.getState();
}
+ /**
+ * Get the UUID for an image handle coming from a particular device.
+ *
+ * This UUID is used to request and track downloads.
+ *
+ * Image handles are only good for the life of the BIP client. Since this connection is torn
+ * down frequently by specification, we have a layer of indirection to the images in the form
+ * of an UUID. This UUID will allow images to be identified outside the connection lifecycle.
+ * It also allows handles to be reused by the target in ways that won't impact image consumer's
+ * cache schemes.
+ *
+ * @param device The Bluetooth device you want a handle from
+ * @param handle The image handle you want a UUID for
+ * @return A string UUID by which the handle can be identified during the life of the BIP
+ * connection.
+ */
+ public String getUuidForHandle(BluetoothDevice device, String handle) {
+ AvrcpBipSession session = getSession(device);
+ if (session == null || handle == null) return null;
+ return session.getHandleUuid(handle);
+ }
+
+ /**
+ * Get the handle thats associated with a particular UUID.
+ *
+ * The handle must have been seen during this connection.
+ *
+ * @param device The Bluetooth device you want a handle from
+ * @param uuid The UUID you want the associated handle for
+ * @return The image handle associated with this UUID if it exists, null otherwise.
+ */
+ public String getHandleForUuid(BluetoothDevice device, String uuid) {
+ AvrcpBipSession session = getSession(device);
+ if (session == null || uuid == null) return null;
+ return session.getUuidHandle(uuid);
+ }
+
+ private void clearHandleUuids(BluetoothDevice device) {
+ AvrcpBipSession session = getSession(device);
+ if (session == null) return;
+ session.clearHandleUuids();
+ }
+
/**
* Get the Uri of an image if it has already been downloaded.
*
* @param device The remote Bluetooth device you wish to get an image for
- * @param imageHandle The handle associated with the image you want
+ * @param imageUuid The UUID associated with the image you want
* @return A Uri the image can be found at, null if it does not exist
*/
- public Uri getImageUri(BluetoothDevice device, String imageHandle) {
- if (mCoverArtStorage.doesImageExist(device, imageHandle)) {
- return AvrcpCoverArtProvider.getImageUri(device, imageHandle);
+ public Uri getImageUri(BluetoothDevice device, String imageUuid) {
+ if (mCoverArtStorage.doesImageExist(device, imageUuid)) {
+ return AvrcpCoverArtProvider.getImageUri(device, imageUuid);
}
return null;
}
@@ -190,11 +276,12 @@
* Getting image properties and the image are both asynchronous in nature.
*
* @param device The remote Bluetooth device you wish to download from
- * @param imageHandle The handle associated with the image you wish to download
+ * @param imageUuid The UUID associated with the image you wish to download. This will be
+ * translated into an image handle.
* @return A Uri that will be assign to the image once the download is complete
*/
- public Uri downloadImage(BluetoothDevice device, String imageHandle) {
- debug("Download Image - device: " + device.getAddress() + ", Handle: " + imageHandle);
+ public Uri downloadImage(BluetoothDevice device, String imageUuid) {
+ debug("Download Image - device: " + device.getAddress() + ", Handle: " + imageUuid);
AvrcpBipClient client = getClient(device);
if (client == null) {
error("Cannot download an image. No client is available.");
@@ -202,19 +289,24 @@
}
// Check to see if we have the image already. No need to download it if we do have it.
- if (mCoverArtStorage.doesImageExist(device, imageHandle)) {
+ if (mCoverArtStorage.doesImageExist(device, imageUuid)) {
debug("Image is already downloaded");
- return AvrcpCoverArtProvider.getImageUri(device, imageHandle);
+ return AvrcpCoverArtProvider.getImageUri(device, imageUuid);
}
// Getting image properties will return via the callback created when connecting, which
// invokes the download image function after we're returned the properties. If we already
// have the image, GetImageProperties returns true but does not start a download.
+ String imageHandle = getHandleForUuid(device, imageUuid);
+ if (imageHandle == null) {
+ warn("No handle for UUID");
+ return null;
+ }
boolean status = client.getImageProperties(imageHandle);
if (!status) return null;
// Return the Uri that the caller should use to retrieve the image
- return AvrcpCoverArtProvider.getImageUri(device, imageHandle);
+ return AvrcpCoverArtProvider.getImageUri(device, imageUuid);
}
/**
@@ -223,8 +315,8 @@
* @param device The remote Bluetooth device associated with the image
* @param imageHandle The handle associated with the image you wish to remove
*/
- public void removeImage(BluetoothDevice device, String imageHandle) {
- mCoverArtStorage.removeImage(device, imageHandle);
+ public void removeImage(BluetoothDevice device, String imageUuid) {
+ mCoverArtStorage.removeImage(device, imageUuid);
}
/**
@@ -238,6 +330,16 @@
}
/**
+ * Get a device's BIP session information, if it exists
+ *
+ * @param device The device you want the client for
+ * @return The AvrcpBipSession object associated with the device, or null if it doesn't exist
+ */
+ private AvrcpBipSession getSession(BluetoothDevice device) {
+ return mBipSessions.get(device);
+ }
+
+ /**
* Determines our preferred download descriptor from the list of available image download
* formats presented in the image properties object.
*
@@ -277,9 +379,8 @@
public void onConnectionStateChanged(int oldState, int newState) {
debug(mDevice.getAddress() + ": " + oldState + " -> " + newState);
if (newState == BluetoothProfile.STATE_CONNECTED) {
- // The spec says handles are only good for the life an an OBEX connection. If we're
- // refreshing it, then we need to clear out our storage since its handle mapped.
- mCoverArtStorage.removeImagesForDevice(mDevice);
+ // Ensure the handle map is cleared since old ones are invalid on a new connection
+ clearHandleUuids(mDevice);
// Once we're connected fetch the current metadata again in case the target has an
// image handle they can now give us. Only do this if we don't already have one.
@@ -322,14 +423,15 @@
+ ", Code: " + status);
return;
}
+ String imageUuid = getUuidForHandle(mDevice, imageHandle);
debug(mDevice.getAddress() + ": Received image data for handle: " + imageHandle
- + ", image: " + image);
- Uri uri = mCoverArtStorage.addImage(mDevice, imageHandle, image.getImage());
+ + ", uuid: " + imageUuid + ", image: " + image);
+ Uri uri = mCoverArtStorage.addImage(mDevice, imageUuid, image.getImage());
if (uri == null) {
error("Could not store downloaded image");
return;
}
- DownloadEvent event = new DownloadEvent(imageHandle, uri);
+ DownloadEvent event = new DownloadEvent(imageUuid, uri);
if (mCallback != null) mCallback.onImageDownloadComplete(mDevice, event);
}
}
@@ -337,10 +439,16 @@
@Override
public String toString() {
String s = "CoverArtManager:\n";
- s += " Download Scheme: " + mDownloadScheme + "\n";
+ s += " Download Scheme: " + mDownloadScheme + "\n";
for (BluetoothDevice device : mClients.keySet()) {
AvrcpBipClient client = getClient(device);
- s += " " + client.toString() + "\n";
+ AvrcpBipSession session = getSession(device);
+ s += " " + device.getAddress() + ":" + "\n";
+ s += " Client: " + client.toString() + "\n";
+ s += " Handles: " + "\n";
+ for (String handle : session.getSessionHandles()) {
+ s += " " + handle + " -> " + session.getHandleUuid(handle) + "\n";
+ }
}
s += " " + mCoverArtStorage.toString();
return s;
diff --git a/src/com/android/bluetooth/avrcpcontroller/AvrcpItem.java b/src/com/android/bluetooth/avrcpcontroller/AvrcpItem.java
index 7e62ff4..ca5d3d6 100644
--- a/src/com/android/bluetooth/avrcpcontroller/AvrcpItem.java
+++ b/src/com/android/bluetooth/avrcpcontroller/AvrcpItem.java
@@ -89,7 +89,10 @@
// Our own book keeping value since database unaware players sometimes send repeat UIDs.
private String mUuid;
- // Our owned internal Uri value that points to downloaded cover art image
+ // A status to indicate if the image at the URI is downloaded and cached
+ private String mImageUuid = null;
+
+ // Our own internal Uri value that points to downloaded cover art image
private Uri mImageUri;
private AvrcpItem() {
@@ -159,6 +162,14 @@
return mCoverArtHandle;
}
+ public String getCoverArtUuid() {
+ return mImageUuid;
+ }
+
+ public void setCoverArtUuid(String uuid) {
+ mImageUuid = uuid;
+ }
+
public synchronized Uri getCoverArtLocation() {
return mImageUri;
}
@@ -226,7 +237,8 @@
return "AvrcpItem{mUuid=" + mUuid + ", mUid=" + mUid + ", mItemType=" + mItemType
+ ", mType=" + mType + ", mDisplayableName=" + mDisplayableName
+ ", mTitle=" + mTitle + ", mPlayable=" + mPlayable + ", mBrowsable="
- + mBrowsable + ", mCoverArtHandle=" + getCoverArtHandle() + "}";
+ + mBrowsable + ", mCoverArtHandle=" + getCoverArtHandle()
+ + ", mImageUuid=" + mImageUuid + ", mImageUri" + mImageUri + "}";
}
@Override
diff --git a/src/com/android/bluetooth/avrcpcontroller/AvrcpPlayer.java b/src/com/android/bluetooth/avrcpcontroller/AvrcpPlayer.java
index 9e853ef..14be687 100644
--- a/src/com/android/bluetooth/avrcpcontroller/AvrcpPlayer.java
+++ b/src/com/android/bluetooth/avrcpcontroller/AvrcpPlayer.java
@@ -186,12 +186,12 @@
mCurrentTrack = update;
}
- public synchronized boolean notifyImageDownload(String handle, Uri imageUri) {
- if (DBG) Log.d(TAG, "Got an image download -- handle=" + handle + ", uri=" + imageUri);
- if (handle == null || imageUri == null || mCurrentTrack == null) return false;
- if (handle.equals(mCurrentTrack.getCoverArtHandle())) {
+ public synchronized boolean notifyImageDownload(String uuid, Uri imageUri) {
+ if (DBG) Log.d(TAG, "Got an image download -- uuid=" + uuid + ", uri=" + imageUri);
+ if (uuid == null || imageUri == null || mCurrentTrack == null) return false;
+ if (uuid.equals(mCurrentTrack.getCoverArtUuid())) {
mCurrentTrack.setCoverArtLocation(imageUri);
- if (DBG) Log.d(TAG, "Handle '" + handle + "' was added to current track.");
+ if (DBG) Log.d(TAG, "Image UUID '" + uuid + "' was added to current track.");
return true;
}
return false;
diff --git a/src/com/android/bluetooth/avrcpcontroller/BrowseTree.java b/src/com/android/bluetooth/avrcpcontroller/BrowseTree.java
index 167f244..6311368 100644
--- a/src/com/android/bluetooth/avrcpcontroller/BrowseTree.java
+++ b/src/com/android/bluetooth/avrcpcontroller/BrowseTree.java
@@ -202,9 +202,9 @@
// Each time we add a node to the tree, check for an image handle so we can add
// the artwork URI once it has been downloaded
- String imageHandle = node.getCoverArtHandle();
- if (imageHandle != null) {
- indicateCoverArtUsed(node.getID(), imageHandle);
+ String imageUuid = node.getCoverArtUuid();
+ if (imageUuid != null) {
+ indicateCoverArtUsed(node.getID(), imageUuid);
}
return true;
}
@@ -214,7 +214,7 @@
synchronized void removeChild(BrowseNode node) {
mChildren.remove(node);
mBrowseMap.remove(node.getID());
- indicateCoverArtUnused(node.getID(), node.getCoverArtHandle());
+ indicateCoverArtUnused(node.getID(), node.getCoverArtUuid());
}
synchronized int getChildrenCount() {
@@ -240,8 +240,8 @@
return mItem.getDevice();
}
- synchronized String getCoverArtHandle() {
- return mItem.getCoverArtHandle();
+ synchronized String getCoverArtUuid() {
+ return mItem.getCoverArtUuid();
}
synchronized void setCoverArtUri(Uri uri) {
@@ -277,7 +277,7 @@
if (!cached) {
for (BrowseNode child : mChildren) {
mBrowseMap.remove(child.getID());
- indicateCoverArtUnused(child.getID(), child.getCoverArtHandle());
+ indicateCoverArtUnused(child.getID(), child.getCoverArtUuid());
}
mChildren.clear();
}
@@ -455,15 +455,15 @@
* Returns the set of parent nodes that have children impacted by the new art so clients can
* be notified of the change.
*/
- synchronized Set<BrowseNode> notifyImageDownload(String handle, Uri uri) {
+ synchronized Set<BrowseNode> notifyImageDownload(String uuid, Uri uri) {
if (DBG) Log.d(TAG, "Received downloaded image handle to cascade to BrowseNodes using it");
- ArrayList<String> nodes = getNodesUsingCoverArt(handle);
+ ArrayList<String> nodes = getNodesUsingCoverArt(uuid);
HashSet<BrowseNode> parents = new HashSet<BrowseNode>();
for (String nodeId : nodes) {
BrowseNode node = findBrowseNodeByID(nodeId);
if (node == null) {
Log.e(TAG, "Node was removed without clearing its cover art status");
- indicateCoverArtUnused(nodeId, handle);
+ indicateCoverArtUnused(nodeId, uuid);
continue;
}
node.setCoverArtUri(uri);