Resolve TODOs in MediaRouter2

including trivial cleanup

Bug: 145490612
Test: cts related to MR2
Change-Id: I1d5230dbfe21315a6f1b59f272088049396b1147

media/java/android/media/MediaRoute2Info.java

@@ -411,11 +411,10 @@ public final class MediaRoute2Info implements Parcelable {
 
     /**
      * Returns true if the route info has all of the required field.
-     * A route info only obtained from {@link com.android.server.media.MediaRouterService}
-     * is valid.
+     * A route is valid if and only if it is obtained from
+     * {@link com.android.server.media.MediaRouterService}.
      * @hide
      */
-    //TODO: Reconsider the validity of a route info when fields are added.
     public boolean isValid() {
         if (TextUtils.isEmpty(getId()) || TextUtils.isEmpty(getName())
                 || TextUtils.isEmpty(getProviderId())) {

media/java/android/media/MediaRoute2ProviderService.java

@@ -104,7 +104,6 @@ public abstract class MediaRoute2ProviderService extends Service {
     @Override
     @Nullable
     public IBinder onBind(@NonNull Intent intent) {
-        //TODO: Allow binding from media router service only?
         if (SERVICE_INTERFACE.equals(intent.getAction())) {
             if (mStub == null) {
                 mStub = new MediaRoute2ProviderServiceStub();

media/java/android/media/MediaRouter2.java

@@ -363,7 +363,7 @@ public class MediaRouter2 {
     /**
      * Transfers the current media to the given route.
      * If it's necessary a new {@link RoutingController} is created or it is handled within
-     * the current controller.
+     * the current routing controller.
      *
      * @param route the route you want to transfer the current media to. Pass {@code null} to
      *              stop routing of the current media.
@@ -393,8 +393,11 @@ public class MediaRouter2 {
             return;
         }
 
-        // TODO: Check the given route exists
         // TODO: Check thread-safety
+        if (!mRoutes.containsKey(route.getId())) {
+            notifyTransferFailed(route);
+            return;
+        }
         if (controller.getRoutingSessionInfo().getTransferableRoutes().contains(route.getId())) {
             controller.transferToRoute(route);
             return;
@@ -877,9 +880,11 @@ public class MediaRouter2 {
          */
         @NonNull
         public List<MediaRoute2Info> getSelectedRoutes() {
+            List<String> selectedRouteIds;
             synchronized (mControllerLock) {
-                return getRoutesWithIdsLocked(mSessionInfo.getSelectedRoutes());
+                selectedRouteIds = mSessionInfo.getSelectedRoutes();
             }
+            return getRoutesWithIds(selectedRouteIds);
         }
 
         /**
@@ -887,9 +892,11 @@ public class MediaRouter2 {
          */
         @NonNull
         public List<MediaRoute2Info> getSelectableRoutes() {
+            List<String> selectableRouteIds;
             synchronized (mControllerLock) {
-                return getRoutesWithIdsLocked(mSessionInfo.getSelectableRoutes());
+                selectableRouteIds = mSessionInfo.getSelectableRoutes();
             }
+            return getRoutesWithIds(selectableRouteIds);
         }
 
         /**
@@ -897,9 +904,11 @@ public class MediaRouter2 {
          */
         @NonNull
         public List<MediaRoute2Info> getDeselectableRoutes() {
+            List<String> deselectableRouteIds;
             synchronized (mControllerLock) {
-                return getRoutesWithIdsLocked(mSessionInfo.getDeselectableRoutes());
+                deselectableRouteIds = mSessionInfo.getDeselectableRoutes();
             }
+            return getRoutesWithIds(deselectableRouteIds);
         }
 
         /**
@@ -1203,20 +1212,12 @@ public class MediaRouter2 {
             }
         }
 
-        // TODO: This method uses two locks (mLock outside, sLock inside).
-        //       Check if there is any possiblity of deadlock.
-        private List<MediaRoute2Info> getRoutesWithIdsLocked(List<String> routeIds) {
-            List<MediaRoute2Info> routes = new ArrayList<>();
+        private List<MediaRoute2Info> getRoutesWithIds(List<String> routeIds) {
             synchronized (sRouterLock) {
-                // TODO: Maybe able to change using Collection.stream()?
-                for (String routeId : routeIds) {
-                    MediaRoute2Info route = mRoutes.get(routeId);
-                    if (route != null) {
-                        routes.add(route);
-                    }
-                }
+                return routeIds.stream().map(mRoutes::get)
+                        .filter(Objects::nonNull)
+                        .collect(Collectors.toList());
             }
-            return Collections.unmodifiableList(routes);
         }
     }
 

media/java/android/media/MediaRouter2Manager.java

@@ -453,6 +453,10 @@ public class MediaRouter2Manager {
             record.mExecutor.execute(() -> record.mCallback
                     .onControlCategoriesChanged(packageName, preferredFeatures));
         }
+        for (CallbackRecord record : mCallbackRecords) {
+            record.mExecutor.execute(() -> record.mCallback
+                    .onPreferredFeaturesChanged(packageName, preferredFeatures));
+        }
     }
 
     /**
@@ -760,6 +764,7 @@ public class MediaRouter2Manager {
          */
         public void onSessionsUpdated() {}
 
+        //TODO: remove this
         /**
          * Called when the preferred route features of an app is changed.
          *
@@ -768,6 +773,16 @@ public class MediaRouter2Manager {
          */
         public void onControlCategoriesChanged(@NonNull String packageName,
                 @NonNull List<String> preferredFeatures) {}
+
+        /**
+         * Called when the preferred route features of an app is changed.
+         *
+         * @param packageName the package name of the application
+         * @param preferredFeatures the list of preferred route features set by an application.
+         */
+        public void onPreferredFeaturesChanged(@NonNull String packageName,
+                @NonNull List<String> preferredFeatures) {}
+
     }
 
     final class CallbackRecord {