From ca9db78aab969d614698728b521e3effd303715b Mon Sep 17 00:00:00 2001 From: Hyundo Moon Date: Wed, 1 Jan 2020 18:31:15 +0900 Subject: [PATCH] MediaRouter2: Implement RouteSessionController#release This CL implements followings: - RouteSessionController#release - Router side logic of calling SessionCallback#onSessionReleased() The test should be added when MediaRouterService actually notifies the clients of session release. (i.e. No new test added.) Bug: 146400872 Test: atest mediaroutertest Change-Id: I2bad73e00986903b8c925704b0144d9e75f3bbf7 --- .../android/media/IMediaRouter2Client.aidl | 1 + .../android/media/IMediaRouterService.aidl | 1 + media/java/android/media/MediaRouter2.java | 201 +++++++++++++----- .../java/android/media/RouteSessionInfo.java | 42 +++- .../mediaroutertest/MediaRouter2Test.java | 95 ++++++++- .../server/media/MediaRoute2Provider.java | 3 + .../server/media/MediaRouter2ServiceImpl.java | 128 +++++++++-- .../server/media/MediaRouterService.java | 6 + 8 files changed, 390 insertions(+), 87 deletions(-) diff --git a/media/java/android/media/IMediaRouter2Client.aidl b/media/java/android/media/IMediaRouter2Client.aidl index 18a6428f570e7..f90c7c59cbebd 100644 --- a/media/java/android/media/IMediaRouter2Client.aidl +++ b/media/java/android/media/IMediaRouter2Client.aidl @@ -30,4 +30,5 @@ oneway interface IMediaRouter2Client { void notifyRoutesChanged(in List routes); void notifySessionCreated(in @nullable RouteSessionInfo sessionInfo, int requestId); void notifySessionInfoChanged(in RouteSessionInfo sessionInfo); + void notifySessionReleased(in RouteSessionInfo sessionInfo); } diff --git a/media/java/android/media/IMediaRouterService.aidl b/media/java/android/media/IMediaRouterService.aidl index 4b7d802dbe9e7..e5b62ff10aa65 100644 --- a/media/java/android/media/IMediaRouterService.aidl +++ b/media/java/android/media/IMediaRouterService.aidl @@ -56,6 +56,7 @@ interface IMediaRouterService { void selectRoute(IMediaRouter2Client client, String sessionId, in MediaRoute2Info route); void deselectRoute(IMediaRouter2Client client, String sessionId, in MediaRoute2Info route); void transferToRoute(IMediaRouter2Client client, String sessionId, in MediaRoute2Info route); + void releaseSession(IMediaRouter2Client client, String sessionId); void registerManager(IMediaRouter2Manager manager, String packageName); void unregisterManager(IMediaRouter2Manager manager); diff --git a/media/java/android/media/MediaRouter2.java b/media/java/android/media/MediaRouter2.java index 9100032708457..f5cfde4b968bd 100644 --- a/media/java/android/media/MediaRouter2.java +++ b/media/java/android/media/MediaRouter2.java @@ -96,7 +96,7 @@ public class MediaRouter2 { private static final String TAG = "MR2"; private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); - private static final Object sLock = new Object(); + private static final Object sRouterLock = new Object(); @GuardedBy("sLock") private static MediaRouter2 sInstance; @@ -122,8 +122,9 @@ public class MediaRouter2 { // TODO: Make MediaRouter2 is always connected to the MediaRouterService. @GuardedBy("sLock") - private Client2 mClient; + Client2 mClient; + @GuardedBy("sLock") private Map mSessionControllers = new ArrayMap<>(); private AtomicInteger mSessionCreationRequestCnt = new AtomicInteger(1); @@ -138,7 +139,7 @@ public class MediaRouter2 { */ public static MediaRouter2 getInstance(@NonNull Context context) { Objects.requireNonNull(context, "context must not be null"); - synchronized (sLock) { + synchronized (sRouterLock) { if (sInstance == null) { sInstance = new MediaRouter2(context.getApplicationContext()); } @@ -210,7 +211,7 @@ public class MediaRouter2 { return; } - synchronized (sLock) { + synchronized (sRouterLock) { if (mClient == null) { Client2 client = new Client2(); try { @@ -242,7 +243,7 @@ public class MediaRouter2 { return; } - synchronized (sLock) { + synchronized (sRouterLock) { if (mRouteCallbackRecords.size() == 0 && mClient != null) { try { mMediaRouterService.unregisterClient2(mClient); @@ -266,7 +267,7 @@ public class MediaRouter2 { List newControlCategories = new ArrayList<>(controlCategories); - synchronized (sLock) { + synchronized (sRouterLock) { mShouldUpdateRoutes = true; // invoke callbacks due to control categories change @@ -291,7 +292,7 @@ public class MediaRouter2 { */ @NonNull public List getRoutes() { - synchronized (sLock) { + synchronized (sRouterLock) { if (mShouldUpdateRoutes) { mShouldUpdateRoutes = false; @@ -372,7 +373,7 @@ public class MediaRouter2 { mSessionCreationRequests.add(request); Client2 client; - synchronized (sLock) { + synchronized (sRouterLock) { client = mClient; } if (client != null) { @@ -400,7 +401,7 @@ public class MediaRouter2 { Objects.requireNonNull(request, "request must not be null"); Client2 client; - synchronized (sLock) { + synchronized (sRouterLock) { client = mClient; } if (client != null) { @@ -424,7 +425,7 @@ public class MediaRouter2 { Objects.requireNonNull(route, "route must not be null"); Client2 client; - synchronized (sLock) { + synchronized (sRouterLock) { client = mClient; } if (client != null) { @@ -448,7 +449,7 @@ public class MediaRouter2 { Objects.requireNonNull(route, "route must not be null"); Client2 client; - synchronized (sLock) { + synchronized (sRouterLock) { client = mClient; } if (client != null) { @@ -496,7 +497,7 @@ public class MediaRouter2 { // 2) Call onRouteSelected(system_route, reason_fallback) if previously selected route // does not exist anymore. => We may need 'boolean MediaRoute2Info#isSystemRoute()'. List addedRoutes = new ArrayList<>(); - synchronized (sLock) { + synchronized (sRouterLock) { for (MediaRoute2Info route : routes) { mRoutes.put(route.getUniqueId(), route); if (route.supportsControlCategories(mControlCategories)) { @@ -512,7 +513,7 @@ public class MediaRouter2 { void removeRoutesOnHandler(List routes) { List removedRoutes = new ArrayList<>(); - synchronized (sLock) { + synchronized (sRouterLock) { for (MediaRoute2Info route : routes) { mRoutes.remove(route.getUniqueId()); if (route.supportsControlCategories(mControlCategories)) { @@ -528,7 +529,7 @@ public class MediaRouter2 { void changeRoutesOnHandler(List routes) { List changedRoutes = new ArrayList<>(); - synchronized (sLock) { + synchronized (sRouterLock) { for (MediaRoute2Info route : routes) { mRoutes.put(route.getUniqueId(), route); if (route.supportsControlCategories(mControlCategories)) { @@ -596,7 +597,9 @@ public class MediaRouter2 { if (sessionInfo != null) { RouteSessionController controller = new RouteSessionController(sessionInfo); - mSessionControllers.put(controller.getUniqueSessionId(), controller); + synchronized (sRouterLock) { + mSessionControllers.put(controller.getUniqueSessionId(), controller); + } notifySessionCreated(controller); } } @@ -607,8 +610,10 @@ public class MediaRouter2 { return; } - RouteSessionController matchingController = mSessionControllers.get( - sessionInfo.getUniqueSessionId()); + RouteSessionController matchingController; + synchronized (sRouterLock) { + matchingController = mSessionControllers.get(sessionInfo.getUniqueSessionId()); + } if (matchingController == null) { Log.w(TAG, "changeSessionInfoOnHandler: Matching controller not found. uniqueSessionId=" @@ -627,6 +632,40 @@ public class MediaRouter2 { notifySessionInfoChanged(matchingController, oldInfo, sessionInfo); } + void releaseControllerOnHandler(RouteSessionInfo sessionInfo) { + if (sessionInfo == null) { + Log.w(TAG, "releaseControllerOnHandler: Ignoring null sessionInfo."); + return; + } + + final String uniqueSessionId = sessionInfo.getUniqueSessionId(); + RouteSessionController matchingController; + synchronized (sRouterLock) { + matchingController = mSessionControllers.get(uniqueSessionId); + } + + if (matchingController == null) { + if (DEBUG) { + Log.d(TAG, "releaseControllerOnHandler: Matching controller not found. " + + "uniqueSessionId=" + sessionInfo.getUniqueSessionId()); + } + return; + } + + RouteSessionInfo oldInfo = matchingController.getRouteSessionInfo(); + if (!TextUtils.equals(oldInfo.getProviderId(), sessionInfo.getProviderId())) { + Log.w(TAG, "releaseControllerOnHandler: Provider IDs are not matched. old=" + + oldInfo.getProviderId() + ", new=" + sessionInfo.getProviderId()); + return; + } + + synchronized (sRouterLock) { + mSessionControllers.remove(uniqueSessionId, matchingController); + } + matchingController.release(); + notifyControllerReleased(matchingController); + } + private void notifyRoutesAdded(List routes) { for (RouteCallbackRecord record: mRouteCallbackRecords) { record.mExecutor.execute( @@ -671,6 +710,13 @@ public class MediaRouter2 { } } + private void notifyControllerReleased(RouteSessionController controller) { + for (SessionCallbackRecord record: mSessionCallbackRecords) { + record.mExecutor.execute( + () -> record.mSessionCallback.onSessionReleased(controller)); + } + } + /** * Callback for receiving events about media route discovery. */ @@ -737,14 +783,20 @@ public class MediaRouter2 { @NonNull RouteSessionInfo newInfo) {} /** - * Called when the session is released. Session can be released by the controller using - * {@link RouteSessionController#release(boolean)}, or by the - * {@link MediaRoute2ProviderService} itself. One can do clean-ups here. + * Called when the session is released by {@link MediaRoute2ProviderService}. + * Before this method is called, the controller would be released by the system, + * which means the {@link RouteSessionController#isReleased()} will always return true + * for the {@code controller} here. + *

+ * Note: Calling {@link RouteSessionController#release()} will NOT trigger + * this method to be called. * - * TODO: When Provider#notifySessionDestroyed is introduced, add @see for the method. + * TODO: Add tests for checking whether this method is called. + * TODO: When service process dies, this should be called. + * + * @see RouteSessionController#isReleased() */ - public void onSessionReleased(@NonNull RouteSessionController controller, int reason, - boolean shouldStop) {} + public void onSessionReleased(@NonNull RouteSessionController controller) {} } /** @@ -755,7 +807,7 @@ public class MediaRouter2 { * TODO: Need to add toString() */ public final class RouteSessionController { - private final Object mLock = new Object(); + private final Object mControllerLock = new Object(); @GuardedBy("mLock") private RouteSessionInfo mSessionInfo; @@ -771,7 +823,7 @@ public class MediaRouter2 { * @return the ID of the session */ public int getSessionId() { - synchronized (mLock) { + synchronized (mControllerLock) { return mSessionInfo.getSessionId(); } } @@ -782,7 +834,7 @@ public class MediaRouter2 { */ @NonNull public String getUniqueSessionId() { - synchronized (mLock) { + synchronized (mControllerLock) { return mSessionInfo.getUniqueSessionId(); } } @@ -792,7 +844,7 @@ public class MediaRouter2 { */ @NonNull public String getControlCategory() { - synchronized (mLock) { + synchronized (mControllerLock) { return mSessionInfo.getControlCategory(); } } @@ -802,7 +854,7 @@ public class MediaRouter2 { */ @Nullable public Bundle getControlHints() { - synchronized (mLock) { + synchronized (mControllerLock) { return mSessionInfo.getControlHints(); } } @@ -812,7 +864,7 @@ public class MediaRouter2 { */ @NonNull public List getSelectedRoutes() { - synchronized (mLock) { + synchronized (mControllerLock) { return getRoutesWithIdsLocked(mSessionInfo.getSelectedRoutes()); } } @@ -822,7 +874,7 @@ public class MediaRouter2 { */ @NonNull public List getSelectableRoutes() { - synchronized (mLock) { + synchronized (mControllerLock) { return getRoutesWithIdsLocked(mSessionInfo.getSelectableRoutes()); } } @@ -832,7 +884,7 @@ public class MediaRouter2 { */ @NonNull public List getDeselectableRoutes() { - synchronized (mLock) { + synchronized (mControllerLock) { return getRoutesWithIdsLocked(mSessionInfo.getDeselectableRoutes()); } } @@ -842,7 +894,7 @@ public class MediaRouter2 { */ @NonNull public List getTransferrableRoutes() { - synchronized (mLock) { + synchronized (mControllerLock) { return getRoutesWithIdsLocked(mSessionInfo.getTransferrableRoutes()); } } @@ -853,10 +905,9 @@ public class MediaRouter2 { * Also, any operations to this instance will be ignored once released. * * @see #release - * @see SessionCallback#onSessionReleased */ public boolean isReleased() { - synchronized (mLock) { + synchronized (mControllerLock) { return mIsReleased; } } @@ -876,6 +927,12 @@ public class MediaRouter2 { */ public void selectRoute(@NonNull MediaRoute2Info route) { Objects.requireNonNull(route, "route must not be null"); + synchronized (mControllerLock) { + if (mIsReleased) { + Log.w(TAG, "selectRoute() called on released controller. Ignoring."); + return; + } + } List selectedRoutes = getSelectedRoutes(); if (checkRouteListContainsRouteId(selectedRoutes, route.getUniqueId())) { @@ -890,12 +947,12 @@ public class MediaRouter2 { } Client2 client; - synchronized (sLock) { + synchronized (sRouterLock) { client = mClient; } if (client != null) { try { - mMediaRouterService.selectRoute(mClient, getUniqueSessionId(), route); + mMediaRouterService.selectRoute(client, getUniqueSessionId(), route); } catch (RemoteException ex) { Log.e(TAG, "Unable to select route for session.", ex); } @@ -917,6 +974,12 @@ public class MediaRouter2 { */ public void deselectRoute(@NonNull MediaRoute2Info route) { Objects.requireNonNull(route, "route must not be null"); + synchronized (mControllerLock) { + if (mIsReleased) { + Log.w(TAG, "deselectRoute() called on released controller. Ignoring."); + return; + } + } List selectedRoutes = getSelectedRoutes(); if (!checkRouteListContainsRouteId(selectedRoutes, route.getUniqueId())) { @@ -931,12 +994,12 @@ public class MediaRouter2 { } Client2 client; - synchronized (sLock) { + synchronized (sRouterLock) { client = mClient; } if (client != null) { try { - mMediaRouterService.deselectRoute(mClient, getUniqueSessionId(), route); + mMediaRouterService.deselectRoute(client, getUniqueSessionId(), route); } catch (RemoteException ex) { Log.e(TAG, "Unable to remove route from session.", ex); } @@ -958,6 +1021,12 @@ public class MediaRouter2 { */ public void transferToRoute(@NonNull MediaRoute2Info route) { Objects.requireNonNull(route, "route must not be null"); + synchronized (mControllerLock) { + if (mIsReleased) { + Log.w(TAG, "transferToRoute() called on released controller. Ignoring."); + return; + } + } List selectedRoutes = getSelectedRoutes(); if (checkRouteListContainsRouteId(selectedRoutes, route.getUniqueId())) { @@ -973,12 +1042,12 @@ public class MediaRouter2 { } Client2 client; - synchronized (sLock) { + synchronized (sRouterLock) { client = mClient; } if (client != null) { try { - mMediaRouterService.transferToRoute(mClient, getUniqueSessionId(), route); + mMediaRouterService.transferToRoute(client, getUniqueSessionId(), route); } catch (RemoteException ex) { Log.e(TAG, "Unable to transfer to route for session.", ex); } @@ -986,45 +1055,53 @@ public class MediaRouter2 { } /** - * Release this session. - * Any operation on this session after calling this method will be ignored. + * Release this controller and corresponding session. + * Any operations on this controller after calling this method will be ignored. + * The devices that are playing media will stop playing it. * - * @param stopMedia Should the media that is playing on the device be stopped after this - * session is released. - * @see SessionCallback#onSessionReleased + * TODO: Add tests using {@link MediaRouter2Manager#getActiveSessions()}. */ - public void release(boolean stopMedia) { - synchronized (mLock) { + public void release() { + synchronized (mControllerLock) { if (mIsReleased) { + Log.w(TAG, "release() called on released controller. Ignoring."); return; } mIsReleased = true; } - // TODO: Use stopMedia variable when the actual connection logic is implemented. + + Client2 client; + synchronized (sRouterLock) { + mSessionControllers.remove(getUniqueSessionId(), this); + client = mClient; + } + if (client != null) { + try { + mMediaRouterService.releaseSession(client, getUniqueSessionId()); + } catch (RemoteException ex) { + Log.e(TAG, "Unable to notify of controller release", ex); + } + } } - /** - * @hide - */ @NonNull - public RouteSessionInfo getRouteSessionInfo() { - synchronized (mLock) { + RouteSessionInfo getRouteSessionInfo() { + synchronized (mControllerLock) { return mSessionInfo; } } - /** - * @hide - */ - public void setRouteSessionInfo(@NonNull RouteSessionInfo info) { - synchronized (mLock) { + void setRouteSessionInfo(@NonNull RouteSessionInfo info) { + synchronized (mControllerLock) { mSessionInfo = info; } } + // TODO: This method uses two locks (mLock outside, sLock inside). + // Check if there is any possiblity of deadlock. private List getRoutesWithIdsLocked(List routeIds) { List routes = new ArrayList<>(); - synchronized (mLock) { + synchronized (sRouterLock) { for (String routeId : routeIds) { MediaRoute2Info route = mRoutes.get( MediaRoute2Info.toUniqueId(mSessionInfo.mProviderId, routeId)); @@ -1139,5 +1216,11 @@ public class MediaRouter2 { mHandler.sendMessage(obtainMessage(MediaRouter2::changeSessionInfoOnHandler, MediaRouter2.this, sessionInfo)); } + + @Override + public void notifySessionReleased(RouteSessionInfo sessionInfo) { + mHandler.sendMessage(obtainMessage(MediaRouter2::releaseControllerOnHandler, + MediaRouter2.this, sessionInfo)); + } } } diff --git a/media/java/android/media/RouteSessionInfo.java b/media/java/android/media/RouteSessionInfo.java index b9cf15edb1018..2d7bc24ae7a21 100644 --- a/media/java/android/media/RouteSessionInfo.java +++ b/media/java/android/media/RouteSessionInfo.java @@ -106,9 +106,47 @@ public class RouteSessionInfo implements Parcelable { /** * Gets non-unique session id (int) from unique session id (string). + * If the corresponding session id could not be generated, it will return null. + * @hide */ - public static int getSessionId(@NonNull String uniqueSessionId, @NonNull String providerId) { - return Integer.parseInt(uniqueSessionId.substring(providerId.length() + 1)); + @Nullable + public static Integer getSessionId(@NonNull String uniqueSessionId) { + int lastIndexOfSeparator = uniqueSessionId.lastIndexOf("/"); + if (lastIndexOfSeparator == -1 || lastIndexOfSeparator + 1 >= uniqueSessionId.length()) { + return null; + } + + String integerString = uniqueSessionId.substring(lastIndexOfSeparator + 1); + if (TextUtils.isEmpty(integerString)) { + return null; + } + + try { + return Integer.parseInt(integerString); + } catch (NumberFormatException ex) { + return null; + } + } + + /** + * Gets provider ID (string) from unique session id (string). + * If the corresponding provider ID could not be generated, it will return null. + * @hide + * + * TODO: This logic seems error-prone. Consider to use long uniqueId. + */ + @Nullable + public static String getProviderId(@NonNull String uniqueSessionId) { + int lastIndexOfSeparator = uniqueSessionId.lastIndexOf("/"); + if (lastIndexOfSeparator == -1) { + return null; + } + + String result = uniqueSessionId.substring(0, lastIndexOfSeparator); + if (TextUtils.isEmpty(result)) { + return null; + } + return result; } /** diff --git a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2Test.java b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2Test.java index 10c17dc2c77b4..6fe847bf5f3a8 100644 --- a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2Test.java +++ b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2Test.java @@ -258,6 +258,7 @@ public class MediaRouter2Test { final CountDownLatch successLatch = new CountDownLatch(1); final CountDownLatch failureLatch = new CountDownLatch(1); + final List controllers = new ArrayList<>(); // Create session with this route SessionCallback sessionCallback = new SessionCallback() { @@ -266,6 +267,7 @@ public class MediaRouter2Test { assertNotNull(controller); assertTrue(createRouteMap(controller.getSelectedRoutes()).containsKey(ROUTE_ID1)); assertTrue(TextUtils.equals(CATEGORY_SAMPLE, controller.getControlCategory())); + controllers.add(controller); successLatch.countDown(); } @@ -288,7 +290,7 @@ public class MediaRouter2Test { // onSessionCreationFailed should not be called. assertFalse(failureLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); } finally { - // TODO: Release controllers + releaseControllers(controllers); mRouter2.unregisterRouteCallback(routeCallback); mRouter2.unregisterSessionCallback(sessionCallback); } @@ -305,11 +307,13 @@ public class MediaRouter2Test { final CountDownLatch successLatch = new CountDownLatch(1); final CountDownLatch failureLatch = new CountDownLatch(1); + final List controllers = new ArrayList<>(); // Create session with this route SessionCallback sessionCallback = new SessionCallback() { @Override public void onSessionCreated(RouteSessionController controller) { + controllers.add(controller); successLatch.countDown(); } @@ -334,7 +338,7 @@ public class MediaRouter2Test { // onSessionCreated should not be called. assertFalse(successLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); } finally { - // TODO: Release controllers + releaseControllers(controllers); mRouter2.unregisterRouteCallback(routeCallback); mRouter2.unregisterSessionCallback(sessionCallback); } @@ -347,7 +351,6 @@ public class MediaRouter2Test { final CountDownLatch successLatch = new CountDownLatch(2); final CountDownLatch failureLatch = new CountDownLatch(1); - final List createdControllers = new ArrayList<>(); // Create session with this route @@ -395,7 +398,7 @@ public class MediaRouter2Test { assertTrue(TextUtils.equals(CATEGORY_SAMPLE, controller1.getControlCategory())); assertTrue(TextUtils.equals(CATEGORY_SAMPLE, controller2.getControlCategory())); } finally { - // TODO: Release controllers + releaseControllers(createdControllers); mRouter2.unregisterRouteCallback(routeCallback); mRouter2.unregisterSessionCallback(sessionCallback); } @@ -412,11 +415,13 @@ public class MediaRouter2Test { final CountDownLatch successLatch = new CountDownLatch(1); final CountDownLatch failureLatch = new CountDownLatch(1); + final List controllers = new ArrayList<>(); // Create session with this route SessionCallback sessionCallback = new SessionCallback() { @Override public void onSessionCreated(RouteSessionController controller) { + controllers.add(controller); successLatch.countDown(); } @@ -442,7 +447,7 @@ public class MediaRouter2Test { assertFalse(successLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); assertFalse(failureLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); } finally { - // TODO: Release controllers + releaseControllers(controllers); mRouter2.unregisterRouteCallback(routeCallback); mRouter2.unregisterSessionCallback(sessionCallback); } @@ -541,8 +546,7 @@ public class MediaRouter2Test { assertTrue(onSessionInfoChangedLatchForDeselect.await( TIMEOUT_MS, TimeUnit.MILLISECONDS)); } finally { - // TODO: Release controllers - controllers.clear(); + releaseControllers(controllers); mRouter2.unregisterRouteCallback(routeCallback); mRouter2.unregisterSessionCallback(sessionCallback); } @@ -618,12 +622,79 @@ public class MediaRouter2Test { controller.transferToRoute(routeToTransferTo); assertTrue(onSessionInfoChangedLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); } finally { - // TODO: Release controllers - controllers.clear(); + releaseControllers(controllers); mRouter2.unregisterRouteCallback(routeCallback); mRouter2.unregisterSessionCallback(sessionCallback); } + } + // TODO: Add tests for onSessionReleased() call. + + @Test + public void testRouteSessionControllerReleaseShouldIgnoreTransferTo() throws Exception { + final List sampleControlCategory = new ArrayList<>(); + sampleControlCategory.add(CATEGORY_SAMPLE); + + Map routes = waitAndGetRoutes(sampleControlCategory); + MediaRoute2Info routeToCreateSessionWith = routes.get(ROUTE_ID1); + assertNotNull(routeToCreateSessionWith); + + final CountDownLatch onSessionCreatedLatch = new CountDownLatch(1); + final CountDownLatch onSessionInfoChangedLatch = new CountDownLatch(1); + final List controllers = new ArrayList<>(); + + // Create session with ROUTE_ID1 + SessionCallback sessionCallback = new SessionCallback() { + @Override + public void onSessionCreated(RouteSessionController controller) { + assertNotNull(controller); + assertTrue(getRouteIds(controller.getSelectedRoutes()).contains(ROUTE_ID1)); + assertTrue(TextUtils.equals(CATEGORY_SAMPLE, controller.getControlCategory())); + controllers.add(controller); + onSessionCreatedLatch.countDown(); + } + + @Override + public void onSessionInfoChanged(RouteSessionController controller, + RouteSessionInfo oldInfo, RouteSessionInfo newInfo) { + if (onSessionCreatedLatch.getCount() != 0 + || controllers.get(0).getSessionId() != controller.getSessionId()) { + return; + } + onSessionInfoChangedLatch.countDown(); + } + }; + + // TODO: Remove this once the MediaRouter2 becomes always connected to the service. + RouteCallback routeCallback = new RouteCallback(); + mRouter2.registerRouteCallback(mExecutor, routeCallback); + + try { + mRouter2.registerSessionCallback(mExecutor, sessionCallback); + mRouter2.requestCreateSession(routeToCreateSessionWith, CATEGORY_SAMPLE); + assertTrue(onSessionCreatedLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + + assertEquals(1, controllers.size()); + RouteSessionController controller = controllers.get(0); + assertTrue(getRouteIds(controller.getTransferrableRoutes()) + .contains(ROUTE_ID5_TO_TRANSFER_TO)); + + // Release controller. Future calls should be ignored. + controller.release(); + + // Transfer to ROUTE_ID5_TO_TRANSFER_TO + MediaRoute2Info routeToTransferTo = routes.get(ROUTE_ID5_TO_TRANSFER_TO); + assertNotNull(routeToTransferTo); + + // This call should be ignored. + // The onSessionInfoChanged() shouldn't be called. + controller.transferToRoute(routeToTransferTo); + assertFalse(onSessionInfoChangedLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + } finally { + releaseControllers(controllers); + mRouter2.unregisterRouteCallback(routeCallback); + mRouter2.unregisterSessionCallback(sessionCallback); + } } // Helper for getting routes easily @@ -663,6 +734,12 @@ public class MediaRouter2Test { } } + static void releaseControllers(@NonNull List controllers) { + for (RouteSessionController controller : controllers) { + controller.release(); + } + } + /** * Returns a list of IDs (not uniqueId) of the given route list. */ diff --git a/services/core/java/com/android/server/media/MediaRoute2Provider.java b/services/core/java/com/android/server/media/MediaRoute2Provider.java index f11b70ebbf71a..115155ce24b5d 100644 --- a/services/core/java/com/android/server/media/MediaRoute2Provider.java +++ b/services/core/java/com/android/server/media/MediaRoute2Provider.java @@ -108,5 +108,8 @@ abstract class MediaRoute2Provider { // TODO: Remove this when MediaRouter2ServiceImpl notifies clients of session changes. void onSessionInfoChanged(@NonNull MediaRoute2Provider provider, @NonNull RouteSessionInfo sessionInfo); + // TODO: Call this when service actually notifies of session release. + void onSessionReleased(@NonNull MediaRoute2Provider provider, + @NonNull RouteSessionInfo sessionInfo); } } diff --git a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java index 82d22505b303c..22ba1bf1a8515 100644 --- a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java +++ b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java @@ -233,6 +233,19 @@ class MediaRouter2ServiceImpl { } } + public void releaseSession(IMediaRouter2Client client, String uniqueSessionId) { + Objects.requireNonNull(client, "client must not be null"); + + final long token = Binder.clearCallingIdentity(); + try { + synchronized (mLock) { + releaseSessionLocked(client, uniqueSessionId); + } + } finally { + Binder.restoreCallingIdentity(token); + } + } + public void sendControlRequest(@NonNull IMediaRouter2Client client, @NonNull MediaRoute2Info route, @NonNull Intent request) { Objects.requireNonNull(client, "client must not be null"); @@ -477,6 +490,18 @@ class MediaRouter2ServiceImpl { } } + private void releaseSessionLocked(@NonNull IMediaRouter2Client client, String uniqueSessionId) { + final IBinder binder = client.asBinder(); + final Client2Record clientRecord = mAllClientRecords.get(binder); + + if (clientRecord != null) { + clientRecord.mUserRecord.mHandler.sendMessage( + obtainMessage(UserHandler::releaseSessionOnHandler, + clientRecord.mUserRecord.mHandler, + clientRecord, uniqueSessionId)); + } + } + private void setControlCategoriesLocked(Client2Record clientRecord, List categories) { if (clientRecord != null) { if (clientRecord.mControlCategories.equals(categories)) { @@ -835,25 +860,32 @@ class MediaRouter2ServiceImpl { @Override public void onProviderStateChanged(@NonNull MediaRoute2Provider provider) { - sendMessage(PooledLambda.obtainMessage(UserHandler::updateProvider, this, provider)); + sendMessage(PooledLambda.obtainMessage(UserHandler::onProviderStateChangedOnHandler, + this, provider)); } @Override public void onSessionCreated(@NonNull MediaRoute2Provider provider, @Nullable RouteSessionInfo sessionInfo, long requestId) { - sendMessage(PooledLambda.obtainMessage(UserHandler::handleCreateSessionResultOnHandler, + sendMessage(PooledLambda.obtainMessage(UserHandler::onSessionCreatedOnHandler, this, provider, sessionInfo, requestId)); } @Override public void onSessionInfoChanged(@NonNull MediaRoute2Provider provider, @NonNull RouteSessionInfo sessionInfo) { - sendMessage(PooledLambda.obtainMessage(UserHandler::updateSession, + sendMessage(PooledLambda.obtainMessage(UserHandler::onSessionInfoChangedOnHandler, + this, provider, sessionInfo)); + } + + @Override + public void onSessionReleased(MediaRoute2Provider provider, RouteSessionInfo sessionInfo) { + sendMessage(PooledLambda.obtainMessage(UserHandler::onSessionReleasedOnHandler, this, provider, sessionInfo)); } //TODO: notify session info updates - private void updateProvider(MediaRoute2Provider provider) { + private void onProviderStateChangedOnHandler(MediaRoute2Provider provider) { int providerIndex = getProviderInfoIndex(provider.getUniqueId()); MediaRoute2ProviderInfo providerInfo = provider.getProviderInfo(); MediaRoute2ProviderInfo prevInfo = @@ -975,7 +1007,7 @@ class MediaRouter2ServiceImpl { if (provider == null) { return; } - provider.selectRoute(RouteSessionInfo.getSessionId(uniqueSessionId, providerId), route); + provider.selectRoute(RouteSessionInfo.getSessionId(uniqueSessionId), route); } private void deselectRouteOnHandler(@NonNull Client2Record clientRecord, @@ -991,8 +1023,7 @@ class MediaRouter2ServiceImpl { if (provider == null) { return; } - provider.deselectRoute( - RouteSessionInfo.getSessionId(uniqueSessionId, providerId), route); + provider.deselectRoute(RouteSessionInfo.getSessionId(uniqueSessionId), route); } private void transferToRouteOnHandler(@NonNull Client2Record clientRecord, @@ -1008,8 +1039,7 @@ class MediaRouter2ServiceImpl { if (provider == null) { return; } - provider.transferToRoute( - RouteSessionInfo.getSessionId(uniqueSessionId, providerId), route); + provider.transferToRoute(RouteSessionInfo.getSessionId(uniqueSessionId), route); } private boolean checkArgumentsForSessionControl(@NonNull Client2Record clientRecord, @@ -1040,20 +1070,57 @@ class MediaRouter2ServiceImpl { return false; } - try { - RouteSessionInfo.getSessionId(uniqueSessionId, providerId); - } catch (Exception ex) { + final Integer sessionId = RouteSessionInfo.getSessionId(uniqueSessionId); + if (sessionId == null) { Slog.w(TAG, "Failed to get int session id from unique session id. " - + "uniqueSessionId=" + uniqueSessionId + " providerId=" + providerId); + + "uniqueSessionId=" + uniqueSessionId); return false; } return true; } - private void handleCreateSessionResultOnHandler( - @NonNull MediaRoute2Provider provider, @Nullable RouteSessionInfo sessionInfo, - long requestId) { + private void releaseSessionOnHandler(@NonNull Client2Record clientRecord, + String uniqueSessionId) { + if (TextUtils.isEmpty(uniqueSessionId)) { + Slog.w(TAG, "Ignoring releasing session with empty unique session ID."); + return; + } + + final Client2Record matchingRecord = mSessionToClientMap.get(uniqueSessionId); + if (matchingRecord != clientRecord) { + Slog.w(TAG, "Ignoring releasing session from non-matching client." + + " packageName=" + clientRecord.mPackageName + + " uniqueSessionId=" + uniqueSessionId); + return; + } + + final String providerId = RouteSessionInfo.getProviderId(uniqueSessionId); + if (providerId == null) { + Slog.w(TAG, "Ignoring releasing session with invalid unique session ID. " + + "uniqueSessionId=" + uniqueSessionId); + return; + } + + final Integer sessionId = RouteSessionInfo.getSessionId(uniqueSessionId); + if (sessionId == null) { + Slog.w(TAG, "Ignoring releasing session with invalid unique session ID. " + + "uniqueSessionId=" + uniqueSessionId + " providerId=" + providerId); + return; + } + + final MediaRoute2Provider provider = findProvider(providerId); + if (provider == null) { + Slog.w(TAG, "Ignoring releasing session since no provider found for given " + + "providerId=" + providerId); + return; + } + + provider.releaseSession(sessionId); + } + + private void onSessionCreatedOnHandler(@NonNull MediaRoute2Provider provider, + @Nullable RouteSessionInfo sessionInfo, long requestId) { SessionCreationRequest matchingRequest = null; for (SessionCreationRequest request : mSessionCreationRequests) { @@ -1103,7 +1170,7 @@ class MediaRouter2ServiceImpl { // TODO: Tell managers for the session creation } - private void updateSession(@NonNull MediaRoute2Provider provider, + private void onSessionInfoChangedOnHandler(@NonNull MediaRoute2Provider provider, @NonNull RouteSessionInfo sessionInfo) { RouteSessionInfo sessionInfoWithProviderId = new RouteSessionInfo.Builder(sessionInfo) .setProviderId(provider.getUniqueId()) @@ -1120,6 +1187,23 @@ class MediaRouter2ServiceImpl { // TODO: Tell managers for the session update } + private void onSessionReleasedOnHandler(@NonNull MediaRoute2Provider provider, + @NonNull RouteSessionInfo sessionInfo) { + RouteSessionInfo sessionInfoWithProviderId = new RouteSessionInfo.Builder(sessionInfo) + .setProviderId(provider.getUniqueId()) + .build(); + + Client2Record client2Record = mSessionToClientMap.get( + sessionInfoWithProviderId.getUniqueSessionId()); + if (client2Record == null) { + Slog.w(TAG, "No matching client found for session=" + sessionInfoWithProviderId); + // TODO: Tell managers for the session release + return; + } + notifySessionReleased(client2Record, sessionInfoWithProviderId); + // TODO: Tell managers for the session release + } + private void notifySessionCreated(Client2Record clientRecord, RouteSessionInfo sessionInfo, int requestId) { try { @@ -1149,6 +1233,16 @@ class MediaRouter2ServiceImpl { } } + private void notifySessionReleased(Client2Record clientRecord, + RouteSessionInfo sessionInfo) { + try { + clientRecord.mClient.notifySessionReleased(sessionInfo); + } catch (RemoteException ex) { + Slog.w(TAG, "Failed to notify client of the session release." + + " Client probably died.", ex); + } + } + private void sendControlRequest(MediaRoute2Info route, Intent request) { 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 3e2bf4e66aaab..d77f43b4435d0 100644 --- a/services/core/java/com/android/server/media/MediaRouterService.java +++ b/services/core/java/com/android/server/media/MediaRouterService.java @@ -482,6 +482,12 @@ public final class MediaRouterService extends IMediaRouterService.Stub mService2.transferToRoute(client, sessionId, route); } + // Binder call + @Override + public void releaseSession(IMediaRouter2Client client, String sessionId) { + mService2.releaseSession(client, sessionId); + } + // Binder call @Override public void sendControlRequest(IMediaRouter2Client client, MediaRoute2Info route,