Merge "Use a binder instead of a bundle in removeSubscriptionWithOptions" into nyc-dev
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