From c943f212a5179cdc435909bf4e06ba1b81367e73 Mon Sep 17 00:00:00 2001 From: Kyunglyul Hyun Date: Fri, 14 Feb 2020 17:58:23 +0900 Subject: [PATCH] MediaRouter2: Remove sendControlRequest Unused sendControlRequest is removed. According to that, MediaRouter2ManagerTest is a little bit modified. Bug: 145487522 Test: atest cts realted to media router 2 Change-Id: I3c2c5d1c2c67a3398a45bb2fab4e90dcf90f627e --- .../android/media/IMediaRoute2Provider.aidl | 1 - .../android/media/IMediaRouterService.aidl | 2 - .../media/MediaRoute2ProviderService.java | 19 ------ media/java/android/media/MediaRouter2.java | 27 -------- .../MediaRouter2ManagerTest.java | 68 ++++++++++--------- .../SampleMediaRoute2ProviderService.java | 67 +++++++++++++----- .../server/media/MediaRoute2Provider.java | 2 - .../media/MediaRoute2ProviderProxy.java | 16 ----- .../server/media/MediaRouter2ServiceImpl.java | 36 ---------- .../server/media/MediaRouterService.java | 7 -- .../media/SystemMediaRoute2Provider.java | 5 -- 11 files changed, 87 insertions(+), 163 deletions(-) diff --git a/media/java/android/media/IMediaRoute2Provider.aidl b/media/java/android/media/IMediaRoute2Provider.aidl index 0c645641ee376..709716612dae7 100644 --- a/media/java/android/media/IMediaRoute2Provider.aidl +++ b/media/java/android/media/IMediaRoute2Provider.aidl @@ -35,7 +35,6 @@ oneway interface IMediaRoute2Provider { void deselectRoute(String sessionId, String routeId); void transferToRoute(String sessionId, String routeId); - void notifyControlRequestSent(String id, in Intent request); void setRouteVolume(String routeId, int volume); void setSessionVolume(String sessionId, int volume); } diff --git a/media/java/android/media/IMediaRouterService.aidl b/media/java/android/media/IMediaRouterService.aidl index f919dce17b2f9..8be288417bfde 100644 --- a/media/java/android/media/IMediaRouterService.aidl +++ b/media/java/android/media/IMediaRouterService.aidl @@ -49,8 +49,6 @@ interface IMediaRouterService { RoutingSessionInfo getSystemSessionInfo(); void registerClient2(IMediaRouter2Client client, String packageName); void unregisterClient2(IMediaRouter2Client client); - void sendControlRequest(IMediaRouter2Client client, in MediaRoute2Info route, - in Intent request); void setRouteVolume2(IMediaRouter2Client client, in MediaRoute2Info route, int volume); void setSessionVolume2(IMediaRouter2Client client, String sessionId, int volume); diff --git a/media/java/android/media/MediaRoute2ProviderService.java b/media/java/android/media/MediaRoute2ProviderService.java index aac195dc76d5d..7d72b1c6bd4f2 100644 --- a/media/java/android/media/MediaRoute2ProviderService.java +++ b/media/java/android/media/MediaRoute2ProviderService.java @@ -114,16 +114,6 @@ public abstract class MediaRoute2ProviderService extends Service { return null; } - /** - * Called when sendControlRequest is called on a route of the provider - * - * @param routeId the id of the target route - * @param request the media control request intent - * @hide - */ - //TODO: Discuss what to use for request (e.g., Intent? Request class?) - public void onControlRequest(@NonNull String routeId, @NonNull Intent request) {} - /** * Called when a volume setting is requested on a route of the provider * @@ -512,15 +502,6 @@ public abstract class MediaRoute2ProviderService extends Service { MediaRoute2ProviderService.this, sessionId, routeId)); } - @Override - public void notifyControlRequestSent(String routeId, Intent request) { - if (!checkCallerisSystem()) { - return; - } - mHandler.sendMessage(obtainMessage(MediaRoute2ProviderService::onControlRequest, - MediaRoute2ProviderService.this, routeId, request)); - } - @Override public void setRouteVolume(String routeId, int volume) { if (!checkCallerisSystem()) { diff --git a/media/java/android/media/MediaRouter2.java b/media/java/android/media/MediaRouter2.java index 2178393cdd5ac..274169c997efe 100644 --- a/media/java/android/media/MediaRouter2.java +++ b/media/java/android/media/MediaRouter2.java @@ -22,7 +22,6 @@ import android.annotation.CallbackExecutor; import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; -import android.content.Intent; import android.os.Bundle; import android.os.Handler; import android.os.Looper; @@ -411,32 +410,6 @@ public class MediaRouter2 { return result; } - /** - * Sends a media control request to be performed asynchronously by the route's destination. - * - * @param route the route that will receive the control request - * @param request the media control request - * @hide - */ - //TODO: Discuss what to use for request (e.g., Intent? Request class?) - //TODO: Provide a way to obtain the result - public void sendControlRequest(@NonNull MediaRoute2Info route, @NonNull Intent request) { - Objects.requireNonNull(route, "route must not be null"); - Objects.requireNonNull(request, "request must not be null"); - - Client2 client; - synchronized (sRouterLock) { - client = mClient; - } - if (client != null) { - try { - mMediaRouterService.sendControlRequest(client, route, request); - } catch (RemoteException ex) { - Log.e(TAG, "Unable to send control request.", ex); - } - } - } - /** * Requests a volume change for the route asynchronously. *

diff --git a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java index 615dc48b969c8..cbaf527e7cd83 100644 --- a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java +++ b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java @@ -20,7 +20,6 @@ import static android.media.MediaRoute2Info.FEATURE_LIVE_AUDIO; import static android.media.MediaRoute2Info.PLAYBACK_VOLUME_FIXED; import static android.media.MediaRoute2Info.PLAYBACK_VOLUME_VARIABLE; -import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.ACTION_REMOVE_ROUTE; import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.FEATURE_SAMPLE; import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.FEATURE_SPECIAL; import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.ROUTE_ID1; @@ -29,7 +28,6 @@ import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.ROUTE import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.ROUTE_ID_FIXED_VOLUME; import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.ROUTE_ID_SPECIAL_FEATURE; import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.ROUTE_ID_VARIABLE_VOLUME; -import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.ROUTE_NAME1; import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.ROUTE_NAME2; import static com.android.mediaroutertest.SampleMediaRoute2ProviderService.VOLUME_MAX; @@ -38,7 +36,6 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import android.content.Context; -import android.content.Intent; import android.media.MediaRoute2Info; import android.media.MediaRouter2; import android.media.MediaRouter2.RouteCallback; @@ -116,34 +113,18 @@ public class MediaRouter2ManagerTest { clearCallbacks(); } - /** - * Tests if routes are added correctly when a new callback is registered. - */ @Test - public void testOnRoutesAdded() throws Exception { - CountDownLatch latch = new CountDownLatch(1); - addManagerCallback(new MediaRouter2Manager.Callback() { - @Override - public void onRoutesAdded(List routes) { - assertTrue(routes.size() > 0); - for (MediaRoute2Info route : routes) { - if (route.getOriginalId().equals(ROUTE_ID1) - && route.getName().equals(ROUTE_NAME1)) { - latch.countDown(); - } - } - } - }); + public void testOnRoutesRemovedAndAdded() throws Exception { + RouteCallback routeCallback = new RouteCallback(); + mRouteCallbacks.add(routeCallback); + mRouter2.registerRouteCallback(mExecutor, routeCallback, + new RouteDiscoveryPreference.Builder(FEATURES_ALL, true).build()); - assertTrue(latch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); - } - - @Test - public void testOnRoutesRemoved() throws Exception { - CountDownLatch latch = new CountDownLatch(1); Map routes = waitAndGetRoutesWithManager(FEATURES_ALL); - addRouterCallback(new RouteCallback()); + CountDownLatch removedLatch = new CountDownLatch(1); + CountDownLatch addedLatch = new CountDownLatch(1); + addManagerCallback(new MediaRouter2Manager.Callback() { @Override public void onRoutesRemoved(List routes) { @@ -151,16 +132,39 @@ public class MediaRouter2ManagerTest { for (MediaRoute2Info route : routes) { if (route.getOriginalId().equals(ROUTE_ID2) && route.getName().equals(ROUTE_NAME2)) { - latch.countDown(); + removedLatch.countDown(); + } + } + } + @Override + public void onRoutesAdded(List routes) { + assertTrue(routes.size() > 0); + if (removedLatch.getCount() > 0) { + return; + } + for (MediaRoute2Info route : routes) { + if (route.getOriginalId().equals(ROUTE_ID2) + && route.getName().equals(ROUTE_NAME2)) { + addedLatch.countDown(); } } } }); - //TODO: Figure out a more proper way to test. - // (Control requests shouldn't be used in this way.) - mRouter2.sendControlRequest(routes.get(ROUTE_ID2), new Intent(ACTION_REMOVE_ROUTE)); - assertTrue(latch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + MediaRoute2Info routeToRemove = routes.get(ROUTE_ID2); + + try { + SampleMediaRoute2ProviderService sInstance = + SampleMediaRoute2ProviderService.getInstance(); + assertNotNull(sInstance); + sInstance.removeRoute(ROUTE_ID2); + assertTrue(removedLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + + sInstance.addRoute(routeToRemove); + assertTrue(addedLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + } finally { + mRouter2.unregisterRouteCallback(routeCallback); + } } /** diff --git a/media/tests/MediaRouter/src/com/android/mediaroutertest/SampleMediaRoute2ProviderService.java b/media/tests/MediaRouter/src/com/android/mediaroutertest/SampleMediaRoute2ProviderService.java index f1dcf3dfd3076..3faefdb148e15 100644 --- a/media/tests/MediaRouter/src/com/android/mediaroutertest/SampleMediaRoute2ProviderService.java +++ b/media/tests/MediaRouter/src/com/android/mediaroutertest/SampleMediaRoute2ProviderService.java @@ -20,6 +20,7 @@ import static android.media.MediaRoute2Info.DEVICE_TYPE_REMOTE_SPEAKER; import static android.media.MediaRoute2Info.DEVICE_TYPE_REMOTE_TV; import static android.media.MediaRoute2Info.PLAYBACK_VOLUME_VARIABLE; +import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Intent; import android.media.MediaRoute2Info; @@ -31,9 +32,13 @@ import android.text.TextUtils; import java.util.HashMap; import java.util.Map; +import java.util.Objects; + +import javax.annotation.concurrent.GuardedBy; public class SampleMediaRoute2ProviderService extends MediaRoute2ProviderService { private static final String TAG = "SampleMR2ProviderSvc"; + private static final Object sLock = new Object(); public static final String ROUTE_ID1 = "route_id1"; public static final String ROUTE_NAME1 = "Sample Route 1"; @@ -59,9 +64,6 @@ public class SampleMediaRoute2ProviderService extends MediaRoute2ProviderService public static final String ROUTE_ID_VARIABLE_VOLUME = "route_variable_volume"; public static final String ROUTE_NAME_VARIABLE_VOLUME = "Variable Volume Route"; - public static final String ACTION_REMOVE_ROUTE = - "com.android.mediarouteprovider.action_remove_route"; - public static final String FEATURE_SAMPLE = "com.android.mediarouteprovider.FEATURE_SAMPLE"; public static final String FEATURE_SPECIAL = @@ -71,6 +73,9 @@ public class SampleMediaRoute2ProviderService extends MediaRoute2ProviderService Map mRouteIdToSessionId = new HashMap<>(); private int mNextSessionId = 1000; + @GuardedBy("sLock") + private static SampleMediaRoute2ProviderService sInstance; + private void initializeRoutes() { MediaRoute2Info route1 = new MediaRoute2Info.Builder(ROUTE_ID1, ROUTE_NAME1) .addFeature(FEATURE_SAMPLE) @@ -92,6 +97,7 @@ public class SampleMediaRoute2ProviderService extends MediaRoute2ProviderService ROUTE_ID5_TO_TRANSFER_TO, ROUTE_NAME5) .addFeature(FEATURE_SAMPLE) .build(); + MediaRoute2Info routeSpecial = new MediaRoute2Info.Builder(ROUTE_ID_SPECIAL_FEATURE, ROUTE_NAME_SPECIAL_FEATURE) .addFeature(FEATURE_SAMPLE) @@ -114,35 +120,64 @@ public class SampleMediaRoute2ProviderService extends MediaRoute2ProviderService mRoutes.put(route3.getId(), route3); mRoutes.put(route4.getId(), route4); mRoutes.put(route5.getId(), route5); + mRoutes.put(routeSpecial.getId(), routeSpecial); mRoutes.put(fixedVolumeRoute.getId(), fixedVolumeRoute); mRoutes.put(variableVolumeRoute.getId(), variableVolumeRoute); } + public static SampleMediaRoute2ProviderService getInstance() { + synchronized (sLock) { + return sInstance; + } + } + + /** + * Adds a route and publishes it. It could replace a route in the provider if + * they have the same route id. + */ + public void addRoute(@NonNull MediaRoute2Info route) { + Objects.requireNonNull(route, "route must not be null"); + mRoutes.put(route.getOriginalId(), route); + publishRoutes(); + } + + /** + * Removes a route and publishes it. + */ + public void removeRoute(@NonNull String routeId) { + Objects.requireNonNull(routeId, "routeId must not be null"); + MediaRoute2Info route = mRoutes.get(routeId); + if (route != null) { + mRoutes.remove(routeId); + publishRoutes(); + } + } + @Override public void onCreate() { + synchronized (sLock) { + sInstance = this; + } initializeRoutes(); } + @Override + public void onDestroy() { + super.onDestroy(); + synchronized (sLock) { + if (sInstance == this) { + sInstance = null; + } + } + } + @Override public IBinder onBind(Intent intent) { publishRoutes(); return super.onBind(intent); } - @Override - public void onControlRequest(String routeId, Intent request) { - String action = request.getAction(); - if (ACTION_REMOVE_ROUTE.equals(action)) { - MediaRoute2Info route = mRoutes.get(routeId); - if (route != null) { - mRoutes.remove(routeId); - publishRoutes(); - mRoutes.put(routeId, route); - } - } - } - @Override public void onSetRouteVolume(String routeId, int volume) { MediaRoute2Info route = mRoutes.get(routeId); diff --git a/services/core/java/com/android/server/media/MediaRoute2Provider.java b/services/core/java/com/android/server/media/MediaRoute2Provider.java index 3de5cf1ceaff9..477122cfa9c7c 100644 --- a/services/core/java/com/android/server/media/MediaRoute2Provider.java +++ b/services/core/java/com/android/server/media/MediaRoute2Provider.java @@ -19,7 +19,6 @@ package com.android.server.media; import android.annotation.NonNull; import android.annotation.Nullable; import android.content.ComponentName; -import android.content.Intent; import android.media.MediaRoute2ProviderInfo; import android.media.RouteDiscoveryPreference; import android.media.RoutingSessionInfo; @@ -61,7 +60,6 @@ abstract class MediaRoute2Provider { public abstract void deselectRoute(String sessionId, String routeId); public abstract void transferToRoute(String sessionId, String routeId); - public abstract void sendControlRequest(String routeId, Intent request); public abstract void setRouteVolume(String routeId, int volume); public abstract void setSessionVolume(String sessionId, int volume); diff --git a/services/core/java/com/android/server/media/MediaRoute2ProviderProxy.java b/services/core/java/com/android/server/media/MediaRoute2ProviderProxy.java index c1ea697d8a2d8..252855eaab107 100644 --- a/services/core/java/com/android/server/media/MediaRoute2ProviderProxy.java +++ b/services/core/java/com/android/server/media/MediaRoute2ProviderProxy.java @@ -122,14 +122,6 @@ final class MediaRoute2ProviderProxy extends MediaRoute2Provider implements Serv } } - @Override - public void sendControlRequest(String routeId, Intent request) { - if (mConnectionReady) { - mActiveConnection.sendControlRequest(routeId, request); - updateBinding(); - } - } - @Override public void setRouteVolume(String routeId, int volume) { if (mConnectionReady) { @@ -508,14 +500,6 @@ final class MediaRoute2ProviderProxy extends MediaRoute2Provider implements Serv } } - public void sendControlRequest(String routeId, Intent request) { - try { - mProvider.notifyControlRequestSent(routeId, request); - } catch (RemoteException ex) { - Slog.e(TAG, "sendControlRequest: Failed to deliver request.", ex); - } - } - public void setRouteVolume(String routeId, int volume) { try { mProvider.setRouteVolume(routeId, volume); diff --git a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java index 2096531e76115..c5320b620633b 100644 --- a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java +++ b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java @@ -25,7 +25,6 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.app.ActivityManager; import android.content.Context; -import android.content.Intent; import android.content.pm.PackageManager; import android.media.IMediaRouter2Client; import android.media.IMediaRouter2Manager; @@ -288,22 +287,6 @@ class MediaRouter2ServiceImpl { } } - public void sendControlRequest(@NonNull IMediaRouter2Client client, - @NonNull MediaRoute2Info route, @NonNull Intent request) { - Objects.requireNonNull(client, "client must not be null"); - Objects.requireNonNull(route, "route must not be null"); - Objects.requireNonNull(request, "request must not be null"); - - final long token = Binder.clearCallingIdentity(); - try { - synchronized (mLock) { - sendControlRequestLocked(client, route, request); - } - } finally { - Binder.restoreCallingIdentity(token); - } - } - public void setDiscoveryRequest2(@NonNull IMediaRouter2Client client, @NonNull RouteDiscoveryPreference preference) { Objects.requireNonNull(client, "client must not be null"); @@ -605,18 +588,6 @@ class MediaRouter2ServiceImpl { } } - private void sendControlRequestLocked(IMediaRouter2Client client, MediaRoute2Info route, - Intent request) { - final IBinder binder = client.asBinder(); - Client2Record clientRecord = mAllClientRecords.get(binder); - - if (clientRecord != null) { - clientRecord.mUserRecord.mHandler.sendMessage( - obtainMessage(UserHandler::sendControlRequest, - clientRecord.mUserRecord.mHandler, route, request)); - } - } - private void setRouteVolumeLocked(IMediaRouter2Client client, MediaRoute2Info route, int volume) { final IBinder binder = client.asBinder(); @@ -1446,13 +1417,6 @@ class MediaRouter2ServiceImpl { } } - private void sendControlRequest(MediaRoute2Info route, Intent request) { - final MediaRoute2Provider provider = findProvider(route.getProviderId()); - if (provider != null) { - provider.sendControlRequest(route.getOriginalId(), request); - } - } - private void setRouteVolume(MediaRoute2Info route, int volume) { final MediaRoute2Provider provider = findProvider(route.getProviderId()); if (provider != null) { diff --git a/services/core/java/com/android/server/media/MediaRouterService.java b/services/core/java/com/android/server/media/MediaRouterService.java index b38e47a1c25ec..83cc89469c627 100644 --- a/services/core/java/com/android/server/media/MediaRouterService.java +++ b/services/core/java/com/android/server/media/MediaRouterService.java @@ -496,13 +496,6 @@ public final class MediaRouterService extends IMediaRouterService.Stub mService2.releaseSession(client, sessionId); } - // Binder call - @Override - public void sendControlRequest(IMediaRouter2Client client, MediaRoute2Info route, - Intent request) { - mService2.sendControlRequest(client, route, request); - } - // Binder call @Override public void registerManager(IMediaRouter2Manager manager, String packageName) { diff --git a/services/core/java/com/android/server/media/SystemMediaRoute2Provider.java b/services/core/java/com/android/server/media/SystemMediaRoute2Provider.java index 7f7a663276677..777a8fec34643 100644 --- a/services/core/java/com/android/server/media/SystemMediaRoute2Provider.java +++ b/services/core/java/com/android/server/media/SystemMediaRoute2Provider.java @@ -151,11 +151,6 @@ class SystemMediaRoute2Provider extends MediaRoute2Provider { } } - //TODO: implement method - @Override - public void sendControlRequest(@NonNull String routeId, @NonNull Intent request) { - } - @Override public void setRouteVolume(String routeId, int volume) { if (!TextUtils.equals(routeId, mSelectedRouteId)) {