Use a binder instead of a bundle in removeSubscriptionWithOptions
Bug: 27845319
Change-Id: I0264b39940481f6571187a42ec859ecf4967ae62
diff --git a/media/java/android/media/browse/MediaBrowser.java b/media/java/android/media/browse/MediaBrowser.java
index 7c6adad..2cd9872 100644
--- a/media/java/android/media/browse/MediaBrowser.java
+++ b/media/java/android/media/browse/MediaBrowser.java
@@ -27,6 +27,7 @@
import android.media.MediaDescription;
import android.media.session.MediaController;
import android.media.session.MediaSession;
+import android.os.Binder;
import android.os.Bundle;
import android.os.Handler;
import android.os.IBinder;
@@ -475,14 +476,8 @@
// the service will be told when we connect.
if (mState == CONNECT_STATE_CONNECTED) {
try {
- // NOTE: Do not call addSubscriptionWithOptions when options are null. Otherwise,
- // it will break the action of support library which expects addSubscription will
- // be called when options are null.
- if (options == null) {
- mServiceBinder.addSubscription(parentId, mServiceCallbacks);
- } else {
- mServiceBinder.addSubscriptionWithOptions(parentId, options, mServiceCallbacks);
- }
+ mServiceBinder.addSubscription(parentId, callback.mToken, options,
+ mServiceCallbacks);
} catch (RemoteException ex) {
// Process is crashing. We will disconnect, and upon reconnect we will
// automatically reregister. So nothing to do here.
@@ -497,34 +492,37 @@
throw new IllegalArgumentException("parentId is empty.");
}
- // Remove from our list.
Subscription sub = mSubscriptions.get(parentId);
-
+ if (sub == null) {
+ return;
+ }
// Tell the service if necessary.
- if (mState == CONNECT_STATE_CONNECTED && sub != null) {
- try {
- if (callback == null) {
- mServiceBinder.removeSubscription(parentId, mServiceCallbacks);
- } else {
- final List<SubscriptionCallback> callbacks = sub.getCallbacks();
- final List<Bundle> optionsList = sub.getOptionsList();
- for (int i = callbacks.size() - 1; i >= 0; --i) {
- if (callbacks.get(i) == callback) {
- mServiceBinder.removeSubscriptionWithOptions(
- parentId, optionsList.get(i), mServiceCallbacks);
- callbacks.remove(i);
- optionsList.remove(i);
+ try {
+ if (callback == null) {
+ if (mState == CONNECT_STATE_CONNECTED) {
+ mServiceBinder.removeSubscription(parentId, null, mServiceCallbacks);
+ }
+ } else {
+ final List<SubscriptionCallback> callbacks = sub.getCallbacks();
+ final List<Bundle> optionsList = sub.getOptionsList();
+ for (int i = callbacks.size() - 1; i >= 0; --i) {
+ if (callbacks.get(i) == callback) {
+ if (mState == CONNECT_STATE_CONNECTED) {
+ mServiceBinder.removeSubscription(
+ parentId, callback.mToken, mServiceCallbacks);
}
+ callbacks.remove(i);
+ optionsList.remove(i);
}
}
- } catch (RemoteException ex) {
- // Process is crashing. We will disconnect, and upon reconnect we will
- // automatically reregister. So nothing to do here.
- Log.d(TAG, "removeSubscription failed with RemoteException parentId=" + parentId);
}
+ } catch (RemoteException ex) {
+ // Process is crashing. We will disconnect, and upon reconnect we will
+ // automatically reregister. So nothing to do here.
+ Log.d(TAG, "removeSubscription failed with RemoteException parentId=" + parentId);
}
- if (sub != null && (sub.isEmpty() || callback == null)) {
+ if (sub.isEmpty() || callback == null) {
mSubscriptions.remove(parentId);
}
}
@@ -579,17 +577,12 @@
for (Entry<String, Subscription> subscriptionEntry : mSubscriptions.entrySet()) {
String id = subscriptionEntry.getKey();
Subscription sub = subscriptionEntry.getValue();
- for (Bundle options : sub.getOptionsList()) {
+ List<SubscriptionCallback> callbackList = sub.getCallbacks();
+ List<Bundle> optionsList = sub.getOptionsList();
+ for (int i = 0; i < callbackList.size(); ++i) {
try {
- // NOTE: Do not call addSubscriptionWithOptions when options are null.
- // Otherwise, it will break the action of support library which expects
- // addSubscription will be called when options are null.
- if (options == null) {
- mServiceBinder.addSubscription(id, mServiceCallbacks);
- } else {
- mServiceBinder.addSubscriptionWithOptions(
- id, options, mServiceCallbacks);
- }
+ mServiceBinder.addSubscription(id, callbackList.get(i).mToken,
+ optionsList.get(i), mServiceCallbacks);
} catch (RemoteException ex) {
// Process is crashing. We will disconnect, and upon reconnect we will
// automatically reregister. So nothing to do here.
@@ -859,6 +852,12 @@
* Callbacks for subscription related events.
*/
public static abstract class SubscriptionCallback {
+ Binder mToken;
+
+ public SubscriptionCallback() {
+ mToken = new Binder();
+ }
+
/**
* Called when the list of children is loaded or updated.
*
@@ -1071,12 +1070,7 @@
}
@Override
- public void onLoadChildren(String parentId, ParceledListSlice list) {
- onLoadChildrenWithOptions(parentId, list, null);
- }
-
- @Override
- public void onLoadChildrenWithOptions(String parentId, ParceledListSlice list,
+ public void onLoadChildren(String parentId, ParceledListSlice list,
final Bundle options) {
MediaBrowser mediaBrowser = mMediaBrowser.get();
if (mediaBrowser != null) {
diff --git a/media/java/android/service/media/IMediaBrowserService.aidl b/media/java/android/service/media/IMediaBrowserService.aidl
index 6ca5ac5..eef5a7c 100644
--- a/media/java/android/service/media/IMediaBrowserService.aidl
+++ b/media/java/android/service/media/IMediaBrowserService.aidl
@@ -14,19 +14,11 @@
* @hide
*/
oneway interface IMediaBrowserService {
-
- // Warning: DO NOT CHANGE the methods signature and order of methods.
- // A change of the order or the method signatures could break the support library.
-
void connect(String pkg, in Bundle rootHints, IMediaBrowserServiceCallbacks callbacks);
void disconnect(IMediaBrowserServiceCallbacks callbacks);
- void addSubscription(String uri, IMediaBrowserServiceCallbacks callbacks);
- void removeSubscription(String uri, IMediaBrowserServiceCallbacks callbacks);
+ void addSubscription(String uri, in IBinder token, in Bundle options,
+ IMediaBrowserServiceCallbacks callbacks);
+ void removeSubscription(String uri, in IBinder token, IMediaBrowserServiceCallbacks callbacks);
void getMediaItem(String uri, in ResultReceiver cb);
-
- void addSubscriptionWithOptions(String uri, in Bundle options,
- IMediaBrowserServiceCallbacks callbacks);
- void removeSubscriptionWithOptions(String uri, in Bundle options,
- IMediaBrowserServiceCallbacks callbacks);
}
diff --git a/media/java/android/service/media/IMediaBrowserServiceCallbacks.aidl b/media/java/android/service/media/IMediaBrowserServiceCallbacks.aidl
index e6b0e8c..dadb025 100644
--- a/media/java/android/service/media/IMediaBrowserServiceCallbacks.aidl
+++ b/media/java/android/service/media/IMediaBrowserServiceCallbacks.aidl
@@ -13,10 +13,6 @@
* @hide
*/
oneway interface IMediaBrowserServiceCallbacks {
-
- // Warning: DO NOT CHANGE the methods signature and order of methods.
- // A change of the order or the method signatures could break the support library.
-
/**
* Invoked when the connected has been established.
* @param root The root media id for browsing.
@@ -26,6 +22,5 @@
*/
void onConnect(String root, in MediaSession.Token session, in Bundle extras);
void onConnectFailed();
- void onLoadChildren(String mediaId, in ParceledListSlice list);
- void onLoadChildrenWithOptions(String mediaId, in ParceledListSlice list, in Bundle options);
+ void onLoadChildren(String mediaId, in ParceledListSlice list, in Bundle options);
}
diff --git a/media/java/android/service/media/MediaBrowserService.java b/media/java/android/service/media/MediaBrowserService.java
index 6954045..ddc0e88 100644
--- a/media/java/android/service/media/MediaBrowserService.java
+++ b/media/java/android/service/media/MediaBrowserService.java
@@ -39,6 +39,7 @@
import android.text.TextUtils;
import android.util.ArrayMap;
import android.util.Log;
+import android.util.Pair;
import java.io.FileDescriptor;
import java.io.PrintWriter;
@@ -108,7 +109,7 @@
Bundle rootHints;
IMediaBrowserServiceCallbacks callbacks;
BrowserRoot root;
- HashMap<String, List<Bundle>> subscriptions = new HashMap<>();
+ HashMap<String, List<Pair<IBinder, Bundle>>> subscriptions = new HashMap<>();
}
/**
@@ -247,13 +248,7 @@
}
@Override
- public void addSubscription(final String id,
- final IMediaBrowserServiceCallbacks callbacks) {
- addSubscriptionWithOptions(id, null, callbacks);
- }
-
- @Override
- public void addSubscriptionWithOptions(final String id, final Bundle options,
+ public void addSubscription(final String id, final IBinder token, final Bundle options,
final IMediaBrowserServiceCallbacks callbacks) {
mHandler.post(new Runnable() {
@Override
@@ -268,19 +263,13 @@
return;
}
- MediaBrowserService.this.addSubscription(id, connection, options);
+ MediaBrowserService.this.addSubscription(id, connection, token, options);
}
});
}
@Override
- public void removeSubscription(final String id,
- final IMediaBrowserServiceCallbacks callbacks) {
- removeSubscriptionWithOptions(id, null, callbacks);
- }
-
- @Override
- public void removeSubscriptionWithOptions(final String id, final Bundle options,
+ public void removeSubscription(final String id, final IBinder token,
final IMediaBrowserServiceCallbacks callbacks) {
mHandler.post(new Runnable() {
@Override
@@ -293,7 +282,7 @@
+ id);
return;
}
- if (!MediaBrowserService.this.removeSubscription(id, connection, options)) {
+ if (!MediaBrowserService.this.removeSubscription(id, connection, token)) {
Log.w(TAG, "removeSubscription called for " + id
+ " which is not subscribed");
}
@@ -519,11 +508,12 @@
public void run() {
for (IBinder binder : mConnections.keySet()) {
ConnectionRecord connection = mConnections.get(binder);
- List<Bundle> optionsList = connection.subscriptions.get(parentId);
- if (optionsList != null) {
- for (Bundle bundle : optionsList) {
- if (MediaBrowserUtils.hasDuplicatedItems(options, bundle)) {
- performLoadChildren(parentId, connection, bundle);
+ List<Pair<IBinder, Bundle>> callbackList =
+ connection.subscriptions.get(parentId);
+ if (callbackList != null) {
+ for (Pair<IBinder, Bundle> callback : callbackList) {
+ if (MediaBrowserUtils.hasDuplicatedItems(options, callback.second)) {
+ performLoadChildren(parentId, connection, callback.second);
}
}
}
@@ -553,19 +543,21 @@
/**
* Save the subscription and if it is a new subscription send the results.
*/
- private void addSubscription(String id, ConnectionRecord connection, Bundle options) {
+ private void addSubscription(String id, ConnectionRecord connection, IBinder token,
+ Bundle options) {
// Save the subscription
- List<Bundle> optionsList = connection.subscriptions.get(id);
- if (optionsList == null) {
- optionsList = new ArrayList<>();
+ List<Pair<IBinder, Bundle>> callbackList = connection.subscriptions.get(id);
+ if (callbackList == null) {
+ callbackList = new ArrayList<>();
}
- for (Bundle bundle : optionsList) {
- if (MediaBrowserUtils.areSameOptions(options, bundle)) {
+ for (Pair<IBinder, Bundle> callback : callbackList) {
+ if (token == callback.first
+ && MediaBrowserUtils.areSameOptions(options, callback.second)) {
return;
}
}
- optionsList.add(options);
- connection.subscriptions.put(id, optionsList);
+ callbackList.add(new Pair<>(token, options));
+ connection.subscriptions.put(id, callbackList);
// send the results
performLoadChildren(id, connection, options);
}
@@ -573,21 +565,20 @@
/**
* Remove the subscription.
*/
- private boolean removeSubscription(String id, ConnectionRecord connection, Bundle options) {
- if (options == null) {
+ private boolean removeSubscription(String id, ConnectionRecord connection, IBinder token) {
+ if (token == null) {
return connection.subscriptions.remove(id) != null;
}
boolean removed = false;
- List<Bundle> optionsList = connection.subscriptions.get(id);
- if (optionsList != null) {
- for (Bundle bundle : optionsList) {
- if (MediaBrowserUtils.areSameOptions(options, bundle)) {
+ List<Pair<IBinder, Bundle>> callbackList = connection.subscriptions.get(id);
+ if (callbackList != null) {
+ for (Pair<IBinder, Bundle> callback : callbackList) {
+ if (token == callback.first) {
removed = true;
- optionsList.remove(bundle);
- break;
+ callbackList.remove(callback);
}
}
- if (optionsList.size() == 0) {
+ if (callbackList.size() == 0) {
connection.subscriptions.remove(id);
}
}
@@ -619,14 +610,7 @@
final ParceledListSlice<MediaBrowser.MediaItem> pls =
filteredList == null ? null : new ParceledListSlice<>(filteredList);
try {
- // NOTE: Do not call onLoadChildrenWithOptions when options are null. Otherwise,
- // it will break the action of support library which expects onLoadChildren will
- // be called when options are null.
- if (options == null) {
- connection.callbacks.onLoadChildren(parentId, pls);
- } else {
- connection.callbacks.onLoadChildrenWithOptions(parentId, pls, options);
- }
+ connection.callbacks.onLoadChildren(parentId, pls, options);
} catch (RemoteException ex) {
// The other side is in the process of crashing.
Log.w(TAG, "Calling onLoadChildren() failed for id=" + parentId