Resolve TODOs in MediaRouter2
including trivial cleanup
Bug: 145490612
Test: cts related to MR2
Change-Id: I1d5230dbfe21315a6f1b59f272088049396b1147
diff --git a/media/java/android/media/MediaRoute2Info.java b/media/java/android/media/MediaRoute2Info.java
index 0e88c75..bf04fe8 100644
--- a/media/java/android/media/MediaRoute2Info.java
+++ b/media/java/android/media/MediaRoute2Info.java
@@ -411,11 +411,10 @@
/**
* 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())) {
diff --git a/media/java/android/media/MediaRoute2ProviderService.java b/media/java/android/media/MediaRoute2ProviderService.java
index e205bbb..38233fd 100644
--- a/media/java/android/media/MediaRoute2ProviderService.java
+++ b/media/java/android/media/MediaRoute2ProviderService.java
@@ -104,7 +104,6 @@
@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();
diff --git a/media/java/android/media/MediaRouter2.java b/media/java/android/media/MediaRouter2.java
index 6281ccd..28bb4c1 100644
--- a/media/java/android/media/MediaRouter2.java
+++ b/media/java/android/media/MediaRouter2.java
@@ -363,7 +363,7 @@
/**
* 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 @@
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 @@
*/
@NonNull
public List<MediaRoute2Info> getSelectedRoutes() {
+ List<String> selectedRouteIds;
synchronized (mControllerLock) {
- return getRoutesWithIdsLocked(mSessionInfo.getSelectedRoutes());
+ selectedRouteIds = mSessionInfo.getSelectedRoutes();
}
+ return getRoutesWithIds(selectedRouteIds);
}
/**
@@ -887,9 +892,11 @@
*/
@NonNull
public List<MediaRoute2Info> getSelectableRoutes() {
+ List<String> selectableRouteIds;
synchronized (mControllerLock) {
- return getRoutesWithIdsLocked(mSessionInfo.getSelectableRoutes());
+ selectableRouteIds = mSessionInfo.getSelectableRoutes();
}
+ return getRoutesWithIds(selectableRouteIds);
}
/**
@@ -897,9 +904,11 @@
*/
@NonNull
public List<MediaRoute2Info> getDeselectableRoutes() {
+ List<String> deselectableRouteIds;
synchronized (mControllerLock) {
- return getRoutesWithIdsLocked(mSessionInfo.getDeselectableRoutes());
+ deselectableRouteIds = mSessionInfo.getDeselectableRoutes();
}
+ return getRoutesWithIds(deselectableRouteIds);
}
/**
@@ -1203,20 +1212,12 @@
}
}
- // 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);
}
}
diff --git a/media/java/android/media/MediaRouter2Manager.java b/media/java/android/media/MediaRouter2Manager.java
index 8a08d14..636ee92 100644
--- a/media/java/android/media/MediaRouter2Manager.java
+++ b/media/java/android/media/MediaRouter2Manager.java
@@ -453,6 +453,10 @@
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 void onSessionsUpdated() {}
+ //TODO: remove this
/**
* Called when the preferred route features of an app is changed.
*
@@ -768,6 +773,16 @@
*/
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 {