From 6a2f1d67b777bc7e4446401202d8c1281b2aad71 Mon Sep 17 00:00:00 2001 From: Kyunglyul Hyun Date: Tue, 19 May 2020 23:00:17 +0900 Subject: [PATCH] MediaRouter: Defer releasing routing controller when transfer In order to allow media router to communicate with the old routing controller for synchronizing media status during transfer, this CL postpone releasing the old routing controller. Basically media router should release the old routing controller by itself when it's done with the old routing controller. If it didn't, the old routing controller is automatically released after timeout (30s for now) This CL also clarifies "old routing controller" in onTransfer callback. Now it is consistent regardless of who requested transfer. A request from MediaRouter2Manager is handled by MediaRouter2 to simplify session creation logic. Limitation of this approach is that a routing session unknown to MediaRouter2 can't be transferred to a different provider but it is out of R scope so it's okay. Bug: 158713035 Test: CTS test && atest mediaroutertest && manually transfer using support v7 demos Change-Id: I168af69e2a25240227aea0fd9e892eaa91e78ee3 --- media/java/android/media/IMediaRouter2.aidl | 5 +- .../android/media/IMediaRouterService.aidl | 11 +- media/java/android/media/MediaRouter2.java | 353 ++++++++++-------- .../android/media/MediaRouter2Manager.java | 9 +- .../android/media/RoutingSessionInfo.java | 2 +- .../MediaRouter2ManagerTest.java | 74 ++++ .../StubMediaRoute2ProviderService.java | 20 + .../server/media/MediaRoute2Provider.java | 1 + .../MediaRoute2ProviderServiceProxy.java | 107 ++++-- .../server/media/MediaRouter2ServiceImpl.java | 314 ++++++++-------- .../server/media/MediaRouterService.java | 16 +- .../media/SystemMediaRoute2Provider.java | 5 + 12 files changed, 544 insertions(+), 373 deletions(-) diff --git a/media/java/android/media/IMediaRouter2.aidl b/media/java/android/media/IMediaRouter2.aidl index ca14052c964fa..fe15f0e67b1dc 100644 --- a/media/java/android/media/IMediaRouter2.aidl +++ b/media/java/android/media/IMediaRouter2.aidl @@ -34,7 +34,8 @@ oneway interface IMediaRouter2 { void notifySessionReleased(in RoutingSessionInfo sessionInfo); /** * Gets hints of the new session for the given route. - * Call MediaRouterService#notifySessionHintsForCreatingSession to pass the result. + * Call MediaRouterService#requestCreateSessionWithRouter2 to pass the result. */ - void getSessionHintsForCreatingSession(long uniqueRequestId, in MediaRoute2Info route); + void requestCreateSessionByManager(long uniqueRequestId, in RoutingSessionInfo oldSession, + in MediaRoute2Info route); } diff --git a/media/java/android/media/IMediaRouterService.aidl b/media/java/android/media/IMediaRouterService.aidl index 52bac671cc6f7..068f9689d06f9 100644 --- a/media/java/android/media/IMediaRouterService.aidl +++ b/media/java/android/media/IMediaRouterService.aidl @@ -44,7 +44,7 @@ interface IMediaRouterService { void requestSetVolume(IMediaRouterClient client, String routeId, int volume); void requestUpdateVolume(IMediaRouterClient client, String routeId, int direction); - // Note: When changing this file, match the order of methods below with + // Note: When changing this file, match the order of methods below with // MediaRouterService.java for readability. // Methods for MediaRouter2 @@ -57,10 +57,9 @@ interface IMediaRouterService { in RouteDiscoveryPreference preference); void setRouteVolumeWithRouter2(IMediaRouter2 router, in MediaRoute2Info route, int volume); - void requestCreateSessionWithRouter2(IMediaRouter2 router, int requestId, - in MediaRoute2Info route, in @nullable Bundle sessionHints); - void notifySessionHintsForCreatingSession(IMediaRouter2 router, long uniqueRequestId, - in MediaRoute2Info route, in @nullable Bundle sessionHints); + void requestCreateSessionWithRouter2(IMediaRouter2 router, int requestId, long managerRequestId, + in RoutingSessionInfo oldSession, in MediaRoute2Info route, + in @nullable Bundle sessionHints); void selectRouteWithRouter2(IMediaRouter2 router, String sessionId, in MediaRoute2Info route); void deselectRouteWithRouter2(IMediaRouter2 router, String sessionId, in MediaRoute2Info route); void transferToRouteWithRouter2(IMediaRouter2 router, String sessionId, @@ -76,7 +75,7 @@ interface IMediaRouterService { in MediaRoute2Info route, int volume); void requestCreateSessionWithManager(IMediaRouter2Manager manager, int requestId, - String packageName, in @nullable MediaRoute2Info route); + in RoutingSessionInfo oldSession, in @nullable MediaRoute2Info route); void selectRouteWithManager(IMediaRouter2Manager manager, int requestId, String sessionId, in MediaRoute2Info route); void deselectRouteWithManager(IMediaRouter2Manager manager, int requestId, diff --git a/media/java/android/media/MediaRouter2.java b/media/java/android/media/MediaRouter2.java index 8e95239a73f89..f22222d10ad80 100644 --- a/media/java/android/media/MediaRouter2.java +++ b/media/java/android/media/MediaRouter2.java @@ -36,7 +36,6 @@ import com.android.internal.annotations.GuardedBy; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -62,6 +61,11 @@ public final class MediaRouter2 { private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); private static final Object sRouterLock = new Object(); + // The maximum time for the old routing controller available after transfer. + private static final int TRANSFER_TIMEOUT_MS = 30_000; + // The manager request ID representing that no manager is involved. + private static final long MANAGER_REQUEST_ID_NONE = MediaRoute2ProviderService.REQUEST_ID_NONE; + @GuardedBy("sRouterLock") private static MediaRouter2 sInstance; @@ -80,7 +84,7 @@ public final class MediaRouter2 { private final String mPackageName; @GuardedBy("sRouterLock") - final Map mRoutes = new HashMap<>(); + final Map mRoutes = new ArrayMap<>(); final RoutingController mSystemController; @@ -94,7 +98,7 @@ public final class MediaRouter2 { @GuardedBy("sRouterLock") private final Map mNonSystemRoutingControllers = new ArrayMap<>(); - private final AtomicInteger mControllerCreationRequestCnt = new AtomicInteger(1); + private final AtomicInteger mNextRequestId = new AtomicInteger(1); final Handler mHandler; @GuardedBy("sRouterLock") @@ -412,9 +416,16 @@ public final class MediaRouter2 { return; } - final int requestId = mControllerCreationRequestCnt.getAndIncrement(); + requestCreateController(controller, route, MANAGER_REQUEST_ID_NONE); + } - ControllerCreationRequest request = new ControllerCreationRequest(requestId, route); + void requestCreateController(@NonNull RoutingController controller, + @NonNull MediaRoute2Info route, long managerRequestId) { + + final int requestId = mNextRequestId.getAndIncrement(); + + ControllerCreationRequest request = new ControllerCreationRequest(requestId, + managerRequestId, route, controller); mControllerCreationRequests.add(request); OnGetControllerHintsListener listener = mOnGetControllerHintsListener; @@ -433,11 +444,15 @@ public final class MediaRouter2 { if (stub != null) { try { mMediaRouterService.requestCreateSessionWithRouter2( - stub, requestId, route, controllerHints); + stub, requestId, managerRequestId, + controller.getRoutingSessionInfo(), route, controllerHints); } catch (RemoteException ex) { - Log.e(TAG, "transfer: Unable to request to create controller.", ex); - mHandler.sendMessage(obtainMessage(MediaRouter2::createControllerOnHandler, - MediaRouter2.this, requestId, null)); + Log.e(TAG, "createControllerForTransfer: " + + "Failed to request for creating a controller.", ex); + mControllerCreationRequests.remove(request); + if (managerRequestId == MANAGER_REQUEST_ID_NONE) { + notifyTransferFailure(route); + } } } } @@ -463,7 +478,8 @@ public final class MediaRouter2 { } /** - * Gets the list of currently non-released {@link RoutingController routing controllers}. + * Gets the list of currently active {@link RoutingController routing controllers} on which + * media can be played. *

* Note: The list returned here will never be empty. The first element in the list is * always the {@link #getSystemController() system controller}. @@ -554,13 +570,13 @@ public final class MediaRouter2 { mShouldUpdateRoutes = true; } - if (addedRoutes.size() > 0) { + if (!addedRoutes.isEmpty()) { notifyRoutesAdded(addedRoutes); } - if (removedRoutes.size() > 0) { + if (!removedRoutes.isEmpty()) { notifyRoutesRemoved(removedRoutes); } - if (changedRoutes.size() > 0) { + if (!changedRoutes.isEmpty()) { notifyRoutesChanged(changedRoutes); } @@ -582,7 +598,7 @@ public final class MediaRouter2 { } mShouldUpdateRoutes = true; } - if (addedRoutes.size() > 0) { + if (!addedRoutes.isEmpty()) { notifyRoutesAdded(addedRoutes); } } @@ -598,7 +614,7 @@ public final class MediaRouter2 { } mShouldUpdateRoutes = true; } - if (removedRoutes.size() > 0) { + if (!removedRoutes.isEmpty()) { notifyRoutesRemoved(removedRoutes); } } @@ -614,7 +630,7 @@ public final class MediaRouter2 { } mShouldUpdateRoutes = true; } - if (changedRoutes.size() > 0) { + if (!changedRoutes.isEmpty()) { notifyRoutesChanged(changedRoutes); } } @@ -635,44 +651,47 @@ public final class MediaRouter2 { } } - if (matchingRequest != null) { - mControllerCreationRequests.remove(matchingRequest); - - MediaRoute2Info requestedRoute = matchingRequest.mRoute; - - if (sessionInfo == null) { - // TODO: We may need to distinguish between failure and rejection. - // One way can be introducing 'reason'. - notifyTransferFailure(requestedRoute); - return; - } else if (!sessionInfo.getSelectedRoutes().contains(requestedRoute.getId())) { - Log.w(TAG, "The session does not contain the requested route. " - + "(requestedRouteId=" + requestedRoute.getId() - + ", actualRoutes=" + sessionInfo.getSelectedRoutes() - + ")"); - notifyTransferFailure(requestedRoute); - return; - } else if (!TextUtils.equals(requestedRoute.getProviderId(), - sessionInfo.getProviderId())) { - Log.w(TAG, "The session's provider ID does not match the requested route's. " - + "(requested route's providerId=" + requestedRoute.getProviderId() - + ", actual providerId=" + sessionInfo.getProviderId() - + ")"); - notifyTransferFailure(requestedRoute); - return; - } - } - - if (sessionInfo == null) { + if (matchingRequest == null) { + Log.w(TAG, "createControllerOnHandler: Ignoring an unknown request."); return; } - RoutingController oldController = getCurrentController(); - if (!oldController.releaseInternal( - /* shouldReleaseSession= */ matchingRequest != null, - /* shouldNotifyStop= */ false)) { - // Could not release the controller since it was just released by other thread. - oldController = getSystemController(); + mControllerCreationRequests.remove(matchingRequest); + MediaRoute2Info requestedRoute = matchingRequest.mRoute; + + // TODO: Notify the reason for failure. + if (sessionInfo == null) { + notifyTransferFailure(requestedRoute); + return; + } else if (!sessionInfo.getSelectedRoutes().contains(requestedRoute.getId())) { + Log.w(TAG, "The session does not contain the requested route. " + + "(requestedRouteId=" + requestedRoute.getId() + + ", actualRoutes=" + sessionInfo.getSelectedRoutes() + + ")"); + notifyTransferFailure(requestedRoute); + return; + } else if (!TextUtils.equals(requestedRoute.getProviderId(), + sessionInfo.getProviderId())) { + Log.w(TAG, "The session's provider ID does not match the requested route's. " + + "(requested route's providerId=" + requestedRoute.getProviderId() + + ", actual providerId=" + sessionInfo.getProviderId() + + ")"); + notifyTransferFailure(requestedRoute); + return; + } + + RoutingController oldController = matchingRequest.mOldController; + // When the old controller is released before transferred, treat it as a failure. + // This could also happen when transfer is requested twice or more. + if (!oldController.scheduleRelease()) { + Log.w(TAG, "createControllerOnHandler: " + + "Ignoring controller creation for released old controller. " + + "oldController=" + oldController); + if (!sessionInfo.isSystemSession()) { + new RoutingController(sessionInfo).release(); + } + notifyTransferFailure(requestedRoute); + return; } RoutingController newController; @@ -686,12 +705,7 @@ public final class MediaRouter2 { } } - // Two controller can be same if stop() is called before the result of Cast -> Phone comes. - if (oldController != newController) { - notifyTransfer(oldController, newController); - } else if (matchingRequest != null) { - notifyTransferFailure(matchingRequest.mRoute); - } + notifyTransfer(oldController, newController); } void updateControllerOnHandler(RoutingSessionInfo sessionInfo) { @@ -736,10 +750,9 @@ public final class MediaRouter2 { return; } - final String uniqueSessionId = sessionInfo.getId(); RoutingController matchingController; synchronized (sRouterLock) { - matchingController = mNonSystemRoutingControllers.get(uniqueSessionId); + matchingController = mNonSystemRoutingControllers.get(sessionInfo.getId()); } if (matchingController == null) { @@ -757,34 +770,23 @@ public final class MediaRouter2 { return; } - matchingController.releaseInternal( - /* shouldReleaseSession= */ false, /* shouldNotifyStop= */ true); + matchingController.releaseInternal(/* shouldReleaseSession= */ false); } - void onGetControllerHintsForCreatingSessionOnHandler(long uniqueRequestId, - MediaRoute2Info route) { - OnGetControllerHintsListener listener = mOnGetControllerHintsListener; - Bundle controllerHints = null; - if (listener != null) { - controllerHints = listener.onGetControllerHints(route); - if (controllerHints != null) { - controllerHints = new Bundle(controllerHints); + void onRequestCreateControllerByManagerOnHandler(RoutingSessionInfo oldSession, + MediaRoute2Info route, long managerRequestId) { + RoutingController controller; + if (oldSession.isSystemSession()) { + controller = getSystemController(); + } else { + synchronized (sRouterLock) { + controller = mNonSystemRoutingControllers.get(oldSession.getId()); } } - - MediaRouter2Stub stub; - synchronized (sRouterLock) { - stub = mStub; - } - if (stub != null) { - try { - mMediaRouterService.notifySessionHintsForCreatingSession( - stub, uniqueRequestId, route, controllerHints); - } catch (RemoteException ex) { - Log.e(TAG, "onGetControllerHintsForCreatingSessionOnHandler: Unable to notify " - + " session hints for creating session.", ex); - } + if (controller == null) { + return; } + requestCreateController(controller, route, managerRequestId); } private List filterRoutes(List routes, @@ -886,8 +888,13 @@ public final class MediaRouter2 { /** * Called when a media is transferred between two different routing controllers. * This can happen by calling {@link #transferTo(MediaRoute2Info)}. - * The {@code oldController} is released before this method is called, except for the - * {@link #getSystemController() system controller}. + *

Override this to start playback with {@code newController}. You may want to get + * the status of the media that is being played with {@code oldController} and resume it + * continuously with {@code newController}. + * After this is called, any callbacks with {@code oldController} will not be invoked + * unless {@code oldController} is the {@link #getSystemController() system controller}. + * You need to {@link RoutingController#release() release} {@code oldController} before + * playing the media with {@code newController}. * * @param oldController the previous controller that controlled routing * @param newController the new controller to control routing @@ -906,16 +913,15 @@ public final class MediaRouter2 { /** * Called when a media routing stops. It can be stopped by a user or a provider. * App should not continue playing media locally when this method is called. - * The {@code oldController} is released before this method is called, except for the - * {@link #getSystemController() system controller}. + * The {@code controller} is released before this method is called. * - * @param controller the controller that controlled the stopped media routing. + * @param controller the controller that controlled the stopped media routing */ public void onStop(@NonNull RoutingController controller) { } } /** - * A listener interface to send an optional app-specific hints when creating the + * A listener interface to send optional app-specific hints when creating a * {@link RoutingController}. */ public interface OnGetControllerHintsListener { @@ -929,9 +935,9 @@ public final class MediaRouter2 { * The method will be called on the same thread that calls * {@link #transferTo(MediaRoute2Info)} or the main thread if it is requested by the system. * - * @param route The route to create controller with + * @param route the route to create a controller with * @return An optional bundle of app-specific arguments to send to the provider, - * or null if none. The contents of this bundle may affect the result of + * or {@code null} if none. The contents of this bundle may affect the result of * controller creation. * @see MediaRoute2ProviderService#onCreateSession(long, String, String, Bundle) */ @@ -944,10 +950,11 @@ public final class MediaRouter2 { */ public abstract static class ControllerCallback { /** - * Called when a controller is updated. (e.g., the selected routes of the - * controller is changed or the volume of the controller is changed.) + * Called when a controller is updated. (e.g., when the selected routes of the + * controller is changed or when the volume of the controller is changed.) * - * @param controller the updated controller. Can be the system controller. + * @param controller the updated controller. It may be the + * {@link #getSystemController() system controller}. * @see #getSystemController() */ public void onControllerUpdated(@NonNull RoutingController controller) { } @@ -955,20 +962,28 @@ public final class MediaRouter2 { /** * A class to control media routing session in media route provider. - * For example, selecting/deselecting/transferring routes to a session can be done through this - * class. Instances are created by {@link #transferTo(MediaRoute2Info)}. + * For example, selecting/deselecting/transferring to routes of a session can be done through + * this. Instances are created when + * {@link TransferCallback#onTransfer(RoutingController, RoutingController)} is called, + * which is invoked after {@link #transferTo(MediaRoute2Info)} is called. */ public class RoutingController { private final Object mControllerLock = new Object(); + private static final int CONTROLLER_STATE_UNKNOWN = 0; + private static final int CONTROLLER_STATE_ACTIVE = 1; + private static final int CONTROLLER_STATE_RELEASING = 2; + private static final int CONTROLLER_STATE_RELEASED = 3; + @GuardedBy("mControllerLock") private RoutingSessionInfo mSessionInfo; @GuardedBy("mControllerLock") - private volatile boolean mIsReleased; + private int mState; RoutingController(@NonNull RoutingSessionInfo sessionInfo) { mSessionInfo = sessionInfo; + mState = CONTROLLER_STATE_ACTIVE; } /** @@ -982,7 +997,7 @@ public final class MediaRouter2 { } /** - * Gets the original session id set by + * Gets the original session ID set by * {@link RoutingSessionInfo.Builder#Builder(String, String)}. * * @hide @@ -996,7 +1011,8 @@ public final class MediaRouter2 { } /** - * @return the control hints used to control routing session if available. + * Gets the control hints used to control routing session if available. + * It is set by the media route provider. */ @Nullable public Bundle getControlHints() { @@ -1042,7 +1058,9 @@ public final class MediaRouter2 { } /** - * Gets information about how volume is handled on the session. + * Gets the information about how volume is handled on the session. + *

Please note that you may not control the volume of the session even when + * you can control the volume of each selected route in the session. * * @return {@link MediaRoute2Info#PLAYBACK_VOLUME_FIXED} or * {@link MediaRoute2Info#PLAYBACK_VOLUME_VARIABLE} @@ -1067,8 +1085,8 @@ public final class MediaRouter2 { * Gets the current volume of the session. *

* When it's available, it represents the volume of routing session, which is a group - * of selected routes. To get the volume of a route, - * use {@link MediaRoute2Info#getVolume()}. + * of selected routes. Use {@link MediaRoute2Info#getVolume()} + * to get the volume of a route, *

* @see MediaRoute2Info#getVolume() */ @@ -1087,7 +1105,7 @@ public final class MediaRouter2 { */ public boolean isReleased() { synchronized (mControllerLock) { - return mIsReleased; + return mState == CONTROLLER_STATE_RELEASED; } } @@ -1099,8 +1117,8 @@ public final class MediaRouter2 { *

* The given route must satisfy all of the following conditions: *

    - *
  • ID should not be included in {@link #getSelectedRoutes()}
    • - *
  • ID should be included in {@link #getSelectableRoutes()}
    • + *
  • It should not be included in {@link #getSelectedRoutes()}
    • + *
  • It should be included in {@link #getSelectableRoutes()}
    • *
* If the route doesn't meet any of above conditions, it will be ignored. * @@ -1111,11 +1129,9 @@ public final 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; - } + if (isReleased()) { + Log.w(TAG, "selectRoute: Called on released controller. Ignoring."); + return; } List selectedRoutes = getSelectedRoutes(); @@ -1145,12 +1161,12 @@ public final class MediaRouter2 { /** * Deselects a route from the remote session. After a route is deselected, the media is - * expected to be stopped on the deselected routes. + * expected to be stopped on the deselected route. *

* The given route must satisfy all of the following conditions: *

    - *
  • ID should be included in {@link #getSelectedRoutes()}
    • - *
  • ID should be included in {@link #getDeselectableRoutes()}
    • + *
  • It should be included in {@link #getSelectedRoutes()}
    • + *
  • It should be included in {@link #getDeselectableRoutes()}
    • *
* If the route doesn't meet any of above conditions, it will be ignored. * @@ -1160,11 +1176,9 @@ public final 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; - } + if (isReleased()) { + Log.w(TAG, "deselectRoute: called on released controller. Ignoring."); + return; } List selectedRoutes = getSelectedRoutes(); @@ -1193,13 +1207,8 @@ public final class MediaRouter2 { } /** - * Transfers to a given route for the remote session. The given route must satisfy - * all of the following conditions: - *
    - *
  • ID should not be included in {@link RoutingSessionInfo#getSelectedRoutes()}
    • - *
  • ID should be included in {@link RoutingSessionInfo#getTransferableRoutes()}
    • - *
- * If the route doesn't meet any of above conditions, it will be ignored. + * Transfers to a given route for the remote session. The given route must be included + * in {@link RoutingSessionInfo#getTransferableRoutes()}. * * @see RoutingSessionInfo#getSelectedRoutes() * @see RoutingSessionInfo#getTransferableRoutes() @@ -1208,19 +1217,13 @@ public final class MediaRouter2 { void transferToRoute(@NonNull MediaRoute2Info route) { Objects.requireNonNull(route, "route must not be null"); synchronized (mControllerLock) { - if (mIsReleased) { + if (isReleased()) { Log.w(TAG, "transferToRoute: Called on released controller. Ignoring."); return; } - if (mSessionInfo.getSelectedRoutes().contains(route.getId())) { - Log.w(TAG, "Ignoring transferring to a route that is already added. " - + "route=" + route); - return; - } - if (!mSessionInfo.getTransferableRoutes().contains(route.getId())) { - Log.w(TAG, "Ignoring transferring to a non-transferrable route=" + route); + Log.w(TAG, "Ignoring transferring to a non-transferable route=" + route); return; } } @@ -1255,11 +1258,9 @@ public final class MediaRouter2 { return; } - synchronized (mControllerLock) { - if (mIsReleased) { - Log.w(TAG, "setVolume: Called on released controller. Ignoring."); - return; - } + if (isReleased()) { + Log.w(TAG, "setVolume: Called on released controller. Ignoring."); + return; } MediaRouter2Stub stub; synchronized (sRouterLock) { @@ -1275,33 +1276,58 @@ public final class MediaRouter2 { } /** - * Release this controller and corresponding session. + * Releases this controller and the corresponding session. * Any operations on this controller after calling this method will be ignored. * The devices that are playing media will stop playing it. */ - // TODO(b/157872573): Add tests using {@link MediaRouter2Manager#getActiveSessions()}. public void release() { - releaseInternal(/* shouldReleaseSession= */ true, /* shouldNotifyStop= */ true); + releaseInternal(/* shouldReleaseSession= */ true); } /** - * Returns {@code true} when succeeded to release, {@code false} if the controller is - * already released. + * Schedules release of the controller. + * @return {@code true} if it's successfully scheduled, {@code false} if it's already + * scheduled to be released or released. */ - boolean releaseInternal(boolean shouldReleaseSession, boolean shouldNotifyStop) { + boolean scheduleRelease() { synchronized (mControllerLock) { - if (mIsReleased) { - Log.w(TAG, "releaseInternal: Called on released controller. Ignoring."); + if (mState != CONTROLLER_STATE_ACTIVE) { return false; } - mIsReleased = true; + mState = CONTROLLER_STATE_RELEASING; } synchronized (sRouterLock) { + // It could happen if the controller is released by the another thread + // in between two locks if (!mNonSystemRoutingControllers.remove(getId(), this)) { - Log.w(TAG, "releaseInternal: Ignoring unknown controller."); - return false; + // In that case, onStop isn't called so we return true to call onTransfer. + // It's also consistent with that the another thread acquires the lock later. + return true; } + } + + mHandler.postDelayed(this::release, TRANSFER_TIMEOUT_MS); + + return true; + } + + void releaseInternal(boolean shouldReleaseSession) { + boolean shouldNotifyStop; + + synchronized (mControllerLock) { + if (mState == CONTROLLER_STATE_RELEASED) { + if (DEBUG) { + Log.d(TAG, "releaseInternal: Called on released controller. Ignoring."); + } + return; + } + shouldNotifyStop = (mState == CONTROLLER_STATE_ACTIVE); + mState = CONTROLLER_STATE_RELEASED; + } + + synchronized (sRouterLock) { + mNonSystemRoutingControllers.remove(getId(), this); if (shouldReleaseSession && mStub != null) { try { @@ -1326,7 +1352,6 @@ public final class MediaRouter2 { mStub = null; } } - return true; } @Override @@ -1389,9 +1414,14 @@ public final class MediaRouter2 { } @Override - boolean releaseInternal(boolean shouldReleaseSession, boolean shouldNotifyStop) { + boolean scheduleRelease() { + // SystemRoutingController can be always transferred + return true; + } + + @Override + void releaseInternal(boolean shouldReleaseSession) { // Do nothing. SystemRoutingController will never be released - return false; } } @@ -1442,8 +1472,7 @@ public final class MediaRouter2 { if (!(obj instanceof TransferCallbackRecord)) { return false; } - return mTransferCallback - == ((TransferCallbackRecord) obj).mTransferCallback; + return mTransferCallback == ((TransferCallbackRecord) obj).mTransferCallback; } @Override @@ -1481,11 +1510,17 @@ public final class MediaRouter2 { static final class ControllerCreationRequest { public final int mRequestId; + public final long mManagerRequestId; public final MediaRoute2Info mRoute; + public final RoutingController mOldController; - ControllerCreationRequest(int requestId, @NonNull MediaRoute2Info route) { + ControllerCreationRequest(int requestId, long managerRequestId, + @NonNull MediaRoute2Info route, @NonNull RoutingController oldController) { mRequestId = requestId; - mRoute = route; + mManagerRequestId = managerRequestId; + mRoute = Objects.requireNonNull(route, "route must not be null"); + mOldController = Objects.requireNonNull(oldController, + "oldController must not be null"); } } @@ -1534,11 +1569,11 @@ public final class MediaRouter2 { } @Override - public void getSessionHintsForCreatingSession(long uniqueRequestId, - @NonNull MediaRoute2Info route) { + public void requestCreateSessionByManager(long managerRequestId, + RoutingSessionInfo oldSession, MediaRoute2Info route) { mHandler.sendMessage(obtainMessage( - MediaRouter2::onGetControllerHintsForCreatingSessionOnHandler, - MediaRouter2.this, uniqueRequestId, route)); + MediaRouter2::onRequestCreateControllerByManagerOnHandler, + MediaRouter2.this, oldSession, route, managerRequestId)); } } } diff --git a/media/java/android/media/MediaRouter2Manager.java b/media/java/android/media/MediaRouter2Manager.java index dad7859db622e..4b09a5f19fb03 100644 --- a/media/java/android/media/MediaRouter2Manager.java +++ b/media/java/android/media/MediaRouter2Manager.java @@ -54,6 +54,12 @@ import java.util.stream.Collectors; public final class MediaRouter2Manager { private static final String TAG = "MR2Manager"; private static final Object sLock = new Object(); + /** + * The request ID for requests not asked by this instance. + * Shouldn't be used for a valid request. + * @hide + */ + public static final int REQUEST_ID_NONE = 0; /** @hide */ @VisibleForTesting public static final int TRANSFER_TIMEOUT_MS = 30_000; @@ -480,7 +486,6 @@ public final class MediaRouter2Manager { notifyTransferFailed(matchingRequest.mOldSessionInfo, requestedRoute); return; } - releaseSession(matchingRequest.mOldSessionInfo); notifyTransferred(matchingRequest.mOldSessionInfo, sessionInfo); } @@ -777,7 +782,7 @@ public final class MediaRouter2Manager { if (client != null) { try { mMediaRouterService.requestCreateSessionWithManager( - client, requestId, oldSession.getClientPackageName(), route); + client, requestId, oldSession, route); } catch (RemoteException ex) { Log.e(TAG, "requestCreateSession: Failed to send a request", ex); } diff --git a/media/java/android/media/RoutingSessionInfo.java b/media/java/android/media/RoutingSessionInfo.java index edf1fc58ecf56..a5d25e0771fd9 100644 --- a/media/java/android/media/RoutingSessionInfo.java +++ b/media/java/android/media/RoutingSessionInfo.java @@ -220,7 +220,7 @@ public final class RoutingSessionInfo implements Parcelable { } /** - * Gets information about how volume is handled on the session. + * Gets the information about how volume is handled on the session. * * @return {@link MediaRoute2Info#PLAYBACK_VOLUME_FIXED} or * {@link MediaRoute2Info#PLAYBACK_VOLUME_VARIABLE}. diff --git a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java index 0979627e5e8d7..ddefe266d897b 100644 --- a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java +++ b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java @@ -126,6 +126,7 @@ public class MediaRouter2ManagerTest { StubMediaRoute2ProviderService instance = StubMediaRoute2ProviderService.getInstance(); if (instance != null) { instance.setProxy(null); + instance.setSpy(null); } } @@ -423,6 +424,79 @@ public class MediaRouter2ManagerTest { route -> TextUtils.equals(route.getClientPackageName(), null)); } + @Test + @LargeTest + public void testTransferTwice() throws Exception { + Map routes = waitAndGetRoutesWithManager(FEATURES_ALL); + addRouterCallback(new RouteCallback() { }); + + CountDownLatch successLatch1 = new CountDownLatch(1); + CountDownLatch successLatch2 = new CountDownLatch(1); + CountDownLatch failureLatch = new CountDownLatch(1); + CountDownLatch managerOnSessionReleasedLatch = new CountDownLatch(1); + CountDownLatch serviceOnReleaseSessionLatch = new CountDownLatch(1); + List sessions = new ArrayList<>(); + + StubMediaRoute2ProviderService instance = StubMediaRoute2ProviderService.getInstance(); + assertNotNull(instance); + instance.setSpy(new StubMediaRoute2ProviderService.Spy() { + @Override + public void onReleaseSession(long requestId, String sessionId) { + serviceOnReleaseSessionLatch.countDown(); + } + }); + + addManagerCallback(new MediaRouter2Manager.Callback() { + @Override + public void onTransferred(RoutingSessionInfo oldSession, + RoutingSessionInfo newSession) { + sessions.add(newSession); + if (successLatch1.getCount() > 0) { + successLatch1.countDown(); + } else { + successLatch2.countDown(); + } + } + + @Override + public void onTransferFailed(RoutingSessionInfo session, MediaRoute2Info route) { + failureLatch.countDown(); + } + + @Override + public void onSessionReleased(RoutingSessionInfo session) { + managerOnSessionReleasedLatch.countDown(); + } + }); + + MediaRoute2Info route1 = routes.get(ROUTE_ID1); + MediaRoute2Info route2 = routes.get(ROUTE_ID2); + assertNotNull(route1); + assertNotNull(route2); + + mManager.selectRoute(mPackageName, route1); + assertTrue(successLatch1.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + mManager.selectRoute(mPackageName, route2); + assertTrue(successLatch2.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + + // onTransferFailed/onSessionReleased should not be called. + assertFalse(failureLatch.await(WAIT_TIME_MS, TimeUnit.MILLISECONDS)); + assertFalse(managerOnSessionReleasedLatch.await(WAIT_TIME_MS, TimeUnit.MILLISECONDS)); + + assertEquals(2, sessions.size()); + List activeSessionIds = mManager.getActiveSessions().stream() + .map(RoutingSessionInfo::getId) + .collect(Collectors.toList()); + // The old session shouldn't appear on the active session list. + assertFalse(activeSessionIds.contains(sessions.get(0).getId())); + assertTrue(activeSessionIds.contains(sessions.get(1).getId())); + + assertFalse(serviceOnReleaseSessionLatch.await(WAIT_TIME_MS, TimeUnit.MILLISECONDS)); + mManager.releaseSession(sessions.get(0)); + assertTrue(serviceOnReleaseSessionLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + assertFalse(managerOnSessionReleasedLatch.await(WAIT_TIME_MS, TimeUnit.MILLISECONDS)); + } + @Test @LargeTest public void testTransfer_ignored_fails() throws Exception { diff --git a/media/tests/MediaRouter/src/com/android/mediaroutertest/StubMediaRoute2ProviderService.java b/media/tests/MediaRouter/src/com/android/mediaroutertest/StubMediaRoute2ProviderService.java index 4551876d774a6..a51e3714b6f7e 100644 --- a/media/tests/MediaRouter/src/com/android/mediaroutertest/StubMediaRoute2ProviderService.java +++ b/media/tests/MediaRouter/src/com/android/mediaroutertest/StubMediaRoute2ProviderService.java @@ -79,6 +79,7 @@ public class StubMediaRoute2ProviderService extends MediaRoute2ProviderService { @GuardedBy("sLock") private static StubMediaRoute2ProviderService sInstance; private Proxy mProxy; + private Spy mSpy; private void initializeRoutes() { MediaRoute2Info route1 = new MediaRoute2Info.Builder(ROUTE_ID1, ROUTE_NAME1) @@ -256,6 +257,11 @@ public class StubMediaRoute2ProviderService extends MediaRoute2ProviderService { @Override public void onReleaseSession(long requestId, String sessionId) { + Spy spy = mSpy; + if (spy != null) { + spy.onReleaseSession(requestId, sessionId); + } + RoutingSessionInfo sessionInfo = getSessionInfo(sessionId); if (sessionInfo == null) { return; @@ -375,7 +381,21 @@ public class StubMediaRoute2ProviderService extends MediaRoute2ProviderService { mProxy = proxy; } + public void setSpy(@Nullable Spy spy) { + mSpy = spy; + } + + /** + * It overrides the original service + */ public static class Proxy { public void onSetRouteVolume(String routeId, int volume, long requestId) {} } + + /** + * It gets notified but doesn't prevent the original methods to be called. + */ + public static class Spy { + public void onReleaseSession(long requestId, String sessionId) {} + } } diff --git a/services/core/java/com/android/server/media/MediaRoute2Provider.java b/services/core/java/com/android/server/media/MediaRoute2Provider.java index 27216783d0d28..f882c57e49ba4 100644 --- a/services/core/java/com/android/server/media/MediaRoute2Provider.java +++ b/services/core/java/com/android/server/media/MediaRoute2Provider.java @@ -62,6 +62,7 @@ abstract class MediaRoute2Provider { public abstract void setRouteVolume(long requestId, String routeId, int volume); public abstract void setSessionVolume(long requestId, String sessionId, int volume); + public abstract void prepareReleaseSession(@NonNull String sessionId); @NonNull public String getUniqueId() { diff --git a/services/core/java/com/android/server/media/MediaRoute2ProviderServiceProxy.java b/services/core/java/com/android/server/media/MediaRoute2ProviderServiceProxy.java index d6b98e2de9018..85af346aa88a7 100644 --- a/services/core/java/com/android/server/media/MediaRoute2ProviderServiceProxy.java +++ b/services/core/java/com/android/server/media/MediaRoute2ProviderServiceProxy.java @@ -34,11 +34,16 @@ import android.os.IBinder.DeathRecipient; import android.os.Looper; import android.os.RemoteException; import android.os.UserHandle; +import android.text.TextUtils; import android.util.Log; import android.util.Slog; +import com.android.internal.annotations.GuardedBy; + import java.io.PrintWriter; import java.lang.ref.WeakReference; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; /** @@ -61,6 +66,9 @@ final class MediaRoute2ProviderServiceProxy extends MediaRoute2Provider private RouteDiscoveryPreference mLastDiscoveryPreference = null; + @GuardedBy("mLock") + final List mReleasingSessions = new ArrayList<>(); + MediaRoute2ProviderServiceProxy(@NonNull Context context, @NonNull ComponentName componentName, int userId) { super(componentName); @@ -141,6 +149,19 @@ final class MediaRoute2ProviderServiceProxy extends MediaRoute2Provider } } + @Override + public void prepareReleaseSession(@NonNull String sessionId) { + synchronized (mLock) { + for (RoutingSessionInfo session : mSessionInfos) { + if (TextUtils.equals(session.getId(), sessionId)) { + mSessionInfos.remove(session); + mReleasingSessions.add(session); + break; + } + } + } + } + public boolean hasComponentName(String packageName, String className) { return mComponentName.getPackageName().equals(packageName) && mComponentName.getClassName().equals(className); @@ -300,88 +321,97 @@ final class MediaRoute2ProviderServiceProxy extends MediaRoute2Provider } private void onSessionCreated(Connection connection, long requestId, - RoutingSessionInfo sessionInfo) { + RoutingSessionInfo newSession) { if (mActiveConnection != connection) { return; } - if (sessionInfo == null) { - Slog.w(TAG, "onSessionCreated: Ignoring null sessionInfo sent from " + mComponentName); + if (newSession == null) { + Slog.w(TAG, "onSessionCreated: Ignoring null session sent from " + mComponentName); return; } - sessionInfo = updateSessionInfo(sessionInfo); + newSession = assignProviderIdForSession(newSession); + String newSessionId = newSession.getId(); - boolean duplicateSessionAlreadyExists = false; synchronized (mLock) { - for (int i = 0; i < mSessionInfos.size(); i++) { - if (mSessionInfos.get(i).getId().equals(sessionInfo.getId())) { - duplicateSessionAlreadyExists = true; - break; - } + if (mSessionInfos.stream() + .anyMatch(session -> TextUtils.equals(session.getId(), newSessionId)) + || mReleasingSessions.stream() + .anyMatch(session -> TextUtils.equals(session.getId(), newSessionId))) { + Slog.w(TAG, "onSessionCreated: Duplicate session already exists. Ignoring."); + return; } - mSessionInfos.add(sessionInfo); + mSessionInfos.add(newSession); } - if (duplicateSessionAlreadyExists) { - Slog.w(TAG, "onSessionCreated: Duplicate session already exists. Ignoring."); - return; - } - - mCallback.onSessionCreated(this, requestId, sessionInfo); + mCallback.onSessionCreated(this, requestId, newSession); } - private void onSessionUpdated(Connection connection, RoutingSessionInfo sessionInfo) { + private void onSessionUpdated(Connection connection, RoutingSessionInfo updatedSession) { if (mActiveConnection != connection) { return; } - if (sessionInfo == null) { - Slog.w(TAG, "onSessionUpdated: Ignoring null sessionInfo sent from " + if (updatedSession == null) { + Slog.w(TAG, "onSessionUpdated: Ignoring null session sent from " + mComponentName); return; } - sessionInfo = updateSessionInfo(sessionInfo); + updatedSession = assignProviderIdForSession(updatedSession); boolean found = false; synchronized (mLock) { for (int i = 0; i < mSessionInfos.size(); i++) { - if (mSessionInfos.get(i).getId().equals(sessionInfo.getId())) { - mSessionInfos.set(i, sessionInfo); + if (mSessionInfos.get(i).getId().equals(updatedSession.getId())) { + mSessionInfos.set(i, updatedSession); found = true; break; } } + + if (!found) { + for (RoutingSessionInfo releasingSession : mReleasingSessions) { + if (TextUtils.equals(releasingSession.getId(), updatedSession.getId())) { + return; + } + } + Slog.w(TAG, "onSessionUpdated: Matching session info not found"); + return; + } } - if (!found) { - Slog.w(TAG, "onSessionUpdated: Matching session info not found"); - return; - } - - mCallback.onSessionUpdated(this, sessionInfo); + mCallback.onSessionUpdated(this, updatedSession); } - private void onSessionReleased(Connection connection, RoutingSessionInfo sessionInfo) { + private void onSessionReleased(Connection connection, RoutingSessionInfo releaedSession) { if (mActiveConnection != connection) { return; } - if (sessionInfo == null) { - Slog.w(TAG, "onSessionReleased: Ignoring null sessionInfo sent from " + mComponentName); + if (releaedSession == null) { + Slog.w(TAG, "onSessionReleased: Ignoring null session sent from " + mComponentName); return; } - sessionInfo = updateSessionInfo(sessionInfo); + releaedSession = assignProviderIdForSession(releaedSession); boolean found = false; synchronized (mLock) { - for (int i = 0; i < mSessionInfos.size(); i++) { - if (mSessionInfos.get(i).getId().equals(sessionInfo.getId())) { - mSessionInfos.remove(i); + for (RoutingSessionInfo session : mSessionInfos) { + if (TextUtils.equals(session.getId(), releaedSession.getId())) { + mSessionInfos.remove(session); found = true; break; } } + if (!found) { + for (RoutingSessionInfo session : mReleasingSessions) { + if (TextUtils.equals(session.getId(), releaedSession.getId())) { + mReleasingSessions.remove(session); + return; + } + } + } } if (!found) { @@ -389,10 +419,10 @@ final class MediaRoute2ProviderServiceProxy extends MediaRoute2Provider return; } - mCallback.onSessionReleased(this, sessionInfo); + mCallback.onSessionReleased(this, releaedSession); } - private RoutingSessionInfo updateSessionInfo(RoutingSessionInfo sessionInfo) { + private RoutingSessionInfo assignProviderIdForSession(RoutingSessionInfo sessionInfo) { return new RoutingSessionInfo.Builder(sessionInfo) .setOwnerPackageName(mComponentName.getPackageName()) .setProviderId(getUniqueId()) @@ -423,6 +453,7 @@ final class MediaRoute2ProviderServiceProxy extends MediaRoute2Provider mCallback.onSessionReleased(this, sessionInfo); } mSessionInfos.clear(); + mReleasingSessions.clear(); } } } diff --git a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java index 72d296fc5f6b1..cc9503995ad97 100644 --- a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java +++ b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java @@ -16,9 +16,7 @@ package com.android.server.media; -import static android.media.MediaRoute2ProviderService.REASON_ROUTE_NOT_AVAILABLE; import static android.media.MediaRoute2ProviderService.REASON_UNKNOWN_ERROR; -import static android.media.MediaRoute2ProviderService.REQUEST_ID_NONE; import static android.media.MediaRouter2Utils.getOriginalId; import static android.media.MediaRouter2Utils.getProviderId; @@ -33,6 +31,8 @@ import android.media.IMediaRouter2; import android.media.IMediaRouter2Manager; import android.media.MediaRoute2Info; import android.media.MediaRoute2ProviderInfo; +import android.media.MediaRoute2ProviderService; +import android.media.MediaRouter2Manager; import android.media.RouteDiscoveryPreference; import android.media.RoutingSessionInfo; import android.os.Binder; @@ -235,30 +235,17 @@ class MediaRouter2ServiceImpl { } public void requestCreateSessionWithRouter2(IMediaRouter2 router, int requestId, + long managerRequestId, RoutingSessionInfo oldSession, MediaRoute2Info route, Bundle sessionHints) { Objects.requireNonNull(router, "router must not be null"); + Objects.requireNonNull(oldSession, "oldSession must not be null"); Objects.requireNonNull(route, "route must not be null"); final long token = Binder.clearCallingIdentity(); try { synchronized (mLock) { - requestCreateSessionWithRouter2Locked(requestId, router, route, sessionHints); - } - } finally { - Binder.restoreCallingIdentity(token); - } - } - - public void notifySessionHintsForCreatingSession(IMediaRouter2 router, - long uniqueRequestId, MediaRoute2Info route, Bundle sessionHints) { - Objects.requireNonNull(router, "router must not be null"); - Objects.requireNonNull(route, "route must not be null"); - - final long token = Binder.clearCallingIdentity(); - try { - synchronized (mLock) { - notifySessionHintsForCreatingSessionLocked(uniqueRequestId, - router, route, sessionHints); + requestCreateSessionWithRouter2Locked(requestId, managerRequestId, + router, oldSession, route, sessionHints); } } finally { Binder.restoreCallingIdentity(token); @@ -417,16 +404,14 @@ class MediaRouter2ServiceImpl { } public void requestCreateSessionWithManager(IMediaRouter2Manager manager, int requestId, - String packageName, MediaRoute2Info route) { + RoutingSessionInfo oldSession, MediaRoute2Info route) { Objects.requireNonNull(manager, "manager must not be null"); - if (TextUtils.isEmpty(packageName)) { - throw new IllegalArgumentException("packageName must not be empty"); - } + Objects.requireNonNull(oldSession, "oldSession must not be null"); final long token = Binder.clearCallingIdentity(); try { synchronized (mLock) { - requestCreateSessionWithManagerLocked(requestId, manager, packageName, route); + requestCreateSessionWithManagerLocked(requestId, manager, oldSession, route); } } finally { Binder.restoreCallingIdentity(token); @@ -638,7 +623,8 @@ class MediaRouter2ServiceImpl { } } - private void requestCreateSessionWithRouter2Locked(int requestId, @NonNull IMediaRouter2 router, + private void requestCreateSessionWithRouter2Locked(int requestId, long managerRequestId, + @NonNull IMediaRouter2 router, @NonNull RoutingSessionInfo oldSession, @NonNull MediaRoute2Info route, @Nullable Bundle sessionHints) { final IBinder binder = router.asBinder(); final RouterRecord routerRecord = mAllRouterRecords.get(binder); @@ -647,41 +633,60 @@ class MediaRouter2ServiceImpl { return; } - if (route.isSystemRoute() && !routerRecord.mHasModifyAudioRoutingPermission - && !TextUtils.equals(route.getId(), - routerRecord.mUserRecord.mHandler.mSystemProvider.getDefaultRoute().getId())) { - Slog.w(TAG, "MODIFY_AUDIO_ROUTING permission is required to transfer to" - + route); - routerRecord.mUserRecord.mHandler.notifySessionCreationFailedToRouter( - routerRecord, requestId); - return; + if (managerRequestId != MediaRoute2ProviderService.REQUEST_ID_NONE) { + ManagerRecord manager = routerRecord.mUserRecord.mHandler.findManagerWithId( + toRequesterId(managerRequestId)); + if (manager == null || manager.mLastSessionCreationRequest == null) { + Slog.w(TAG, "requestCreateSessionWithRouter2Locked: " + + "Ignoring unknown request."); + routerRecord.mUserRecord.mHandler.notifySessionCreationFailedToRouter( + routerRecord, requestId); + return; + } + if (!TextUtils.equals(manager.mLastSessionCreationRequest.mOldSession.getId(), + oldSession.getId())) { + Slog.w(TAG, "requestCreateSessionWithRouter2Locked: " + + "Ignoring unmatched routing session."); + routerRecord.mUserRecord.mHandler.notifySessionCreationFailedToRouter( + routerRecord, requestId); + return; + } + if (!TextUtils.equals(manager.mLastSessionCreationRequest.mRoute.getId(), + route.getId())) { + // When media router has no permission + if (!routerRecord.mHasModifyAudioRoutingPermission + && manager.mLastSessionCreationRequest.mRoute.isSystemRoute() + && route.isSystemRoute()) { + route = manager.mLastSessionCreationRequest.mRoute; + } else { + Slog.w(TAG, "requestCreateSessionWithRouter2Locked: " + + "Ignoring unmatched route."); + routerRecord.mUserRecord.mHandler.notifySessionCreationFailedToRouter( + routerRecord, requestId); + return; + } + } + manager.mLastSessionCreationRequest = null; + } else { + if (route.isSystemRoute() && !routerRecord.mHasModifyAudioRoutingPermission + && !TextUtils.equals(route.getId(), + routerRecord.mUserRecord.mHandler.mSystemProvider.getDefaultRoute().getId())) { + Slog.w(TAG, "MODIFY_AUDIO_ROUTING permission is required to transfer to" + + route); + routerRecord.mUserRecord.mHandler.notifySessionCreationFailedToRouter( + routerRecord, requestId); + return; + } } long uniqueRequestId = toUniqueRequestId(routerRecord.mRouterId, requestId); routerRecord.mUserRecord.mHandler.sendMessage( obtainMessage(UserHandler::requestCreateSessionWithRouter2OnHandler, routerRecord.mUserRecord.mHandler, - uniqueRequestId, routerRecord, route, + uniqueRequestId, managerRequestId, routerRecord, oldSession, route, sessionHints)); } - private void notifySessionHintsForCreatingSessionLocked(long uniqueRequestId, - @NonNull IMediaRouter2 router, - @NonNull MediaRoute2Info route, @Nullable Bundle sessionHints) { - final IBinder binder = router.asBinder(); - final RouterRecord routerRecord = mAllRouterRecords.get(binder); - - if (routerRecord == null) { - Slog.w(TAG, "notifySessionHintsForCreatingSessionLocked: Ignoring unknown router."); - return; - } - - routerRecord.mUserRecord.mHandler.sendMessage( - obtainMessage(UserHandler::requestCreateSessionWithManagerOnHandler, - routerRecord.mUserRecord.mHandler, - uniqueRequestId, routerRecord, route, sessionHints)); - } - private void selectRouteWithRouter2Locked(@NonNull IMediaRouter2 router, @NonNull String uniqueSessionId, @NonNull MediaRoute2Info route) { final IBinder binder = router.asBinder(); @@ -853,27 +858,46 @@ class MediaRouter2ServiceImpl { private void requestCreateSessionWithManagerLocked(int requestId, @NonNull IMediaRouter2Manager manager, - @NonNull String packageName, @NonNull MediaRoute2Info route) { + @NonNull RoutingSessionInfo oldSession, @NonNull MediaRoute2Info route) { ManagerRecord managerRecord = mAllManagerRecords.get(manager.asBinder()); if (managerRecord == null) { return; } + String packageName = oldSession.getClientPackageName(); + RouterRecord routerRecord = managerRecord.mUserRecord.findRouterRecordLocked(packageName); if (routerRecord == null) { Slog.w(TAG, "requestCreateSessionWithManagerLocked: Ignoring session creation for " + "unknown router."); + try { + managerRecord.mManager.notifyRequestFailed(requestId, REASON_UNKNOWN_ERROR); + } catch (RemoteException ex) { + Slog.w(TAG, "requestCreateSessionWithManagerLocked: Failed to notify failure. " + + "Manager probably died."); + } return; } long uniqueRequestId = toUniqueRequestId(managerRecord.mManagerId, requestId); + if (managerRecord.mLastSessionCreationRequest != null) { + managerRecord.mUserRecord.mHandler.notifyRequestFailedToManager( + managerRecord.mManager, + toOriginalRequestId(managerRecord.mLastSessionCreationRequest + .mManagerRequestId), + REASON_UNKNOWN_ERROR); + managerRecord.mLastSessionCreationRequest = null; + } + managerRecord.mLastSessionCreationRequest = new SessionCreationRequest(routerRecord, + MediaRoute2ProviderService.REQUEST_ID_NONE, uniqueRequestId, + oldSession, route); // Before requesting to the provider, get session hints from the media router. // As a return, media router will request to create a session. routerRecord.mUserRecord.mHandler.sendMessage( - obtainMessage(UserHandler::getSessionHintsForCreatingSessionOnHandler, + obtainMessage(UserHandler::requestRouterCreateSessionOnHandler, routerRecord.mUserRecord.mHandler, - uniqueRequestId, routerRecord, managerRecord, route)); + uniqueRequestId, routerRecord, managerRecord, oldSession, route)); } private void selectRouteWithManagerLocked(int requestId, @NonNull IMediaRouter2Manager manager, @@ -887,7 +911,7 @@ class MediaRouter2ServiceImpl { // Can be null if the session is system's or RCN. RouterRecord routerRecord = managerRecord.mUserRecord.mHandler - .findRouterforSessionLocked(uniqueSessionId); + .findRouterWithSessionLocked(uniqueSessionId); long uniqueRequestId = toUniqueRequestId(managerRecord.mManagerId, requestId); managerRecord.mUserRecord.mHandler.sendMessage( @@ -908,7 +932,7 @@ class MediaRouter2ServiceImpl { // Can be null if the session is system's or RCN. RouterRecord routerRecord = managerRecord.mUserRecord.mHandler - .findRouterforSessionLocked(uniqueSessionId); + .findRouterWithSessionLocked(uniqueSessionId); long uniqueRequestId = toUniqueRequestId(managerRecord.mManagerId, requestId); managerRecord.mUserRecord.mHandler.sendMessage( @@ -929,7 +953,7 @@ class MediaRouter2ServiceImpl { // Can be null if the session is system's or RCN. RouterRecord routerRecord = managerRecord.mUserRecord.mHandler - .findRouterforSessionLocked(uniqueSessionId); + .findRouterWithSessionLocked(uniqueSessionId); long uniqueRequestId = toUniqueRequestId(managerRecord.mManagerId, requestId); managerRecord.mUserRecord.mHandler.sendMessage( @@ -966,7 +990,7 @@ class MediaRouter2ServiceImpl { } RouterRecord routerRecord = managerRecord.mUserRecord.mHandler - .findRouterforSessionLocked(uniqueSessionId); + .findRouterWithSessionLocked(uniqueSessionId); long uniqueRequestId = toUniqueRequestId(managerRecord.mManagerId, requestId); managerRecord.mUserRecord.mHandler.sendMessage( @@ -1097,6 +1121,7 @@ class MediaRouter2ServiceImpl { public final int mPid; public final String mPackageName; public final int mManagerId; + public SessionCreationRequest mLastSessionCreationRequest; ManagerRecord(UserRecord userRecord, IMediaRouter2Manager manager, int uid, int pid, String packageName) { @@ -1222,10 +1247,20 @@ class MediaRouter2ServiceImpl { } @Nullable - public RouterRecord findRouterforSessionLocked(@NonNull String uniqueSessionId) { + public RouterRecord findRouterWithSessionLocked(@NonNull String uniqueSessionId) { return mSessionToRouterMap.get(uniqueSessionId); } + @Nullable + public ManagerRecord findManagerWithId(int managerId) { + for (ManagerRecord manager : getManagerRecords()) { + if (manager.mManagerId == managerId) { + return manager; + } + } + return null; + } + private void onProviderStateChangedOnHandler(@NonNull MediaRoute2Provider provider) { int providerInfoIndex = getLastProviderInfoIndex(provider.getUniqueId()); MediaRoute2ProviderInfo currentInfo = provider.getProviderInfo(); @@ -1318,26 +1353,28 @@ class MediaRouter2ServiceImpl { return -1; } - private void getSessionHintsForCreatingSessionOnHandler(long uniqueRequestId, + private void requestRouterCreateSessionOnHandler(long uniqueRequestId, @NonNull RouterRecord routerRecord, @NonNull ManagerRecord managerRecord, - @NonNull MediaRoute2Info route) { - SessionCreationRequest request = - new SessionCreationRequest(routerRecord, uniqueRequestId, route, managerRecord); - mSessionCreationRequests.add(request); - + @NonNull RoutingSessionInfo oldSession, @NonNull MediaRoute2Info route) { try { - routerRecord.mRouter.getSessionHintsForCreatingSession(uniqueRequestId, route); + if (route.isSystemRoute() && !routerRecord.mHasModifyAudioRoutingPermission) { + routerRecord.mRouter.requestCreateSessionByManager(uniqueRequestId, + oldSession, mSystemProvider.getDefaultRoute()); + } else { + routerRecord.mRouter.requestCreateSessionByManager(uniqueRequestId, + oldSession, route); + } } catch (RemoteException ex) { Slog.w(TAG, "getSessionHintsForCreatingSessionOnHandler: " + "Failed to request. Router probably died.", ex); - mSessionCreationRequests.remove(request); notifyRequestFailedToManager(managerRecord.mManager, toOriginalRequestId(uniqueRequestId), REASON_UNKNOWN_ERROR); } } private void requestCreateSessionWithRouter2OnHandler(long uniqueRequestId, - @NonNull RouterRecord routerRecord, + long managerRequestId, @NonNull RouterRecord routerRecord, + @NonNull RoutingSessionInfo oldSession, @NonNull MediaRoute2Info route, @Nullable Bundle sessionHints) { final MediaRoute2Provider provider = findProvider(route.getProviderId()); @@ -1350,49 +1387,14 @@ class MediaRouter2ServiceImpl { } SessionCreationRequest request = - new SessionCreationRequest(routerRecord, uniqueRequestId, route, null); + new SessionCreationRequest(routerRecord, uniqueRequestId, + managerRequestId, oldSession, route); mSessionCreationRequests.add(request); provider.requestCreateSession(uniqueRequestId, routerRecord.mPackageName, route.getOriginalId(), sessionHints); } - private void requestCreateSessionWithManagerOnHandler(long uniqueRequestId, - @NonNull RouterRecord routerRecord, - @NonNull MediaRoute2Info route, @Nullable Bundle sessionHints) { - SessionCreationRequest matchingRequest = null; - for (SessionCreationRequest request : mSessionCreationRequests) { - if (request.mUniqueRequestId == uniqueRequestId) { - matchingRequest = request; - break; - } - } - if (matchingRequest == null) { - Slog.w(TAG, "requestCreateSessionWithManagerOnHandler: " - + "Ignoring an unknown session creation request."); - return; - } - - if (!TextUtils.equals(matchingRequest.mRoute.getId(), route.getId())) { - Slog.w(TAG, "requestCreateSessionWithManagerOnHandler: " - + "The given route is different from the requested route."); - return; - } - - final MediaRoute2Provider provider = findProvider(route.getProviderId()); - if (provider == null) { - Slog.w(TAG, "requestCreateSessionWithManagerOnHandler: Ignoring session " - + "creation request since no provider found for given route=" + route); - mSessionCreationRequests.remove(matchingRequest); - notifyRequestFailedToManager(matchingRequest.mRequestedManagerRecord.mManager, - toOriginalRequestId(uniqueRequestId), REASON_ROUTE_NOT_AVAILABLE); - return; - } - - provider.requestCreateSession(uniqueRequestId, routerRecord.mPackageName, - route.getOriginalId(), sessionHints); - } - // routerRecord can be null if the session is system's or RCN. private void selectRouteOnHandler(long uniqueRequestId, @Nullable RouterRecord routerRecord, @NonNull String uniqueSessionId, @NonNull MediaRoute2Info route) { @@ -1539,25 +1541,23 @@ class MediaRouter2ServiceImpl { private void onSessionCreatedOnHandler(@NonNull MediaRoute2Provider provider, long uniqueRequestId, @NonNull RoutingSessionInfo sessionInfo) { - notifySessionCreatedToManagers(getManagers(), - toOriginalRequestId(uniqueRequestId), sessionInfo); - - if (uniqueRequestId == REQUEST_ID_NONE) { - // The session is created without any matching request. - return; - } - SessionCreationRequest matchingRequest = null; for (SessionCreationRequest request : mSessionCreationRequests) { if (request.mUniqueRequestId == uniqueRequestId && TextUtils.equals( - request.mRoute.getProviderId(), provider.getUniqueId())) { + request.mRoute.getProviderId(), provider.getUniqueId())) { matchingRequest = request; break; } } + long managerRequestId = (matchingRequest == null) + ? MediaRoute2ProviderService.REQUEST_ID_NONE + : matchingRequest.mManagerRequestId; + // Managers should know created session even if it's not requested. + notifySessionCreatedToManagers(managerRequestId, sessionInfo); + if (matchingRequest == null) { Slog.w(TAG, "Ignoring session creation result for unknown request. " + "uniqueRequestId=" + uniqueRequestId + ", sessionInfo=" + sessionInfo); @@ -1565,12 +1565,14 @@ class MediaRouter2ServiceImpl { } mSessionCreationRequests.remove(matchingRequest); - - if (sessionInfo == null) { - // Failed - notifySessionCreationFailedToRouter(matchingRequest.mRouterRecord, - toOriginalRequestId(uniqueRequestId)); - return; + // Not to show old session + MediaRoute2Provider oldProvider = + findProvider(matchingRequest.mOldSession.getProviderId()); + if (oldProvider != null) { + oldProvider.prepareReleaseSession(matchingRequest.mOldSession.getId()); + } else { + Slog.w(TAG, "onSessionCreatedOnHandler: Can't find provider for an old session. " + + "session=" + matchingRequest.mOldSession); } String originalRouteId = matchingRequest.mRoute.getId(); @@ -1645,23 +1647,17 @@ class MediaRouter2ServiceImpl { } final int requesterId = toRequesterId(uniqueRequestId); - for (ManagerRecord manager : getManagerRecords()) { - if (manager.mManagerId == requesterId) { - notifyRequestFailedToManager( - manager.mManager, toOriginalRequestId(uniqueRequestId), reason); - return; - } + ManagerRecord manager = findManagerWithId(requesterId); + if (manager != null) { + notifyRequestFailedToManager( + manager.mManager, toOriginalRequestId(uniqueRequestId), reason); + return; } // Currently, only the manager can get notified of failures. // TODO: Notify router too when the related callback is introduced. } - // TODO(b/157873556): Find a way to prevent providers from notifying error on random reqID. - // Possible solutions can be: - // 1) Record the other type of requests too (not only session creation request) - // 2) Throw exception on providers when they try to notify error on - // random uniqueRequestId. private boolean handleSessionCreationRequestFailed(@NonNull MediaRoute2Provider provider, long uniqueRequestId, int reason) { // Check whether the failure is about creating a session @@ -1683,12 +1679,16 @@ class MediaRouter2ServiceImpl { // Notify the requester about the failure. // The call should be made by either MediaRouter2 or MediaRouter2Manager. - if (matchingRequest.mRequestedManagerRecord == null) { + if (matchingRequest.mManagerRequestId == MediaRouter2Manager.REQUEST_ID_NONE) { notifySessionCreationFailedToRouter( matchingRequest.mRouterRecord, toOriginalRequestId(uniqueRequestId)); } else { - notifyRequestFailedToManager(matchingRequest.mRequestedManagerRecord.mManager, - toOriginalRequestId(uniqueRequestId), reason); + final int requesterId = toRequesterId(matchingRequest.mManagerRequestId); + ManagerRecord manager = findManagerWithId(requesterId); + if (manager != null) { + notifyRequestFailedToManager(manager.mManager, + toOriginalRequestId(matchingRequest.mManagerRequestId), reason); + } } return true; } @@ -1921,14 +1921,19 @@ class MediaRouter2ServiceImpl { } } - private void notifySessionCreatedToManagers(@NonNull List managers, - int requestId, @NonNull RoutingSessionInfo sessionInfo) { - for (IMediaRouter2Manager manager : managers) { + private void notifySessionCreatedToManagers(long managerRequestId, + @NonNull RoutingSessionInfo session) { + int requesterId = toRequesterId(managerRequestId); + int originalRequestId = toOriginalRequestId(managerRequestId); + + for (ManagerRecord manager : getManagerRecords()) { try { - manager.notifySessionCreated(requestId, sessionInfo); + manager.mManager.notifySessionCreated( + ((manager.mManagerId == requesterId) ? originalRequestId : + MediaRouter2Manager.REQUEST_ID_NONE), session); } catch (RemoteException ex) { Slog.w(TAG, "notifySessionCreatedToManagers: " - + "failed to notify. Manager probably died.", ex); + + "Failed to notify. Manager probably died.", ex); } } } @@ -2014,7 +2019,7 @@ class MediaRouter2ServiceImpl { } mUserRecord.mCompositeDiscoveryPreference = new RouteDiscoveryPreference.Builder(discoveryPreferences) - .build(); + .build(); } for (MediaRoute2Provider provider : mRouteProviders) { provider.updateDiscoveryPreference(mUserRecord.mCompositeDiscoveryPreference); @@ -2030,21 +2035,22 @@ class MediaRouter2ServiceImpl { return null; } - final class SessionCreationRequest { - public final RouterRecord mRouterRecord; - public final long mUniqueRequestId; - public final MediaRoute2Info mRoute; - public final ManagerRecord mRequestedManagerRecord; + } + static final class SessionCreationRequest { + public final RouterRecord mRouterRecord; + public final long mUniqueRequestId; + public final long mManagerRequestId; + public final RoutingSessionInfo mOldSession; + public final MediaRoute2Info mRoute; - // requestedManagerRecord is not null only when the request is made by manager. - SessionCreationRequest(@NonNull RouterRecord routerRecord, long uniqueRequestId, - @NonNull MediaRoute2Info route, - @Nullable ManagerRecord requestedManagerRecord) { - mRouterRecord = routerRecord; - mUniqueRequestId = uniqueRequestId; - mRoute = route; - mRequestedManagerRecord = requestedManagerRecord; - } + SessionCreationRequest(@NonNull RouterRecord routerRecord, long uniqueRequestId, + long managerRequestId, @NonNull RoutingSessionInfo oldSession, + @NonNull MediaRoute2Info route) { + mRouterRecord = routerRecord; + mUniqueRequestId = uniqueRequestId; + mManagerRequestId = managerRequestId; + mOldSession = oldSession; + mRoute = route; } } } diff --git a/services/core/java/com/android/server/media/MediaRouterService.java b/services/core/java/com/android/server/media/MediaRouterService.java index 3337b480d6a8f..0e52a67c8d39c 100644 --- a/services/core/java/com/android/server/media/MediaRouterService.java +++ b/services/core/java/com/android/server/media/MediaRouterService.java @@ -481,16 +481,10 @@ public final class MediaRouterService extends IMediaRouterService.Stub // Binder call @Override public void requestCreateSessionWithRouter2(IMediaRouter2 router, int requestId, + long managerRequestId, RoutingSessionInfo oldSession, MediaRoute2Info route, Bundle sessionHints) { - mService2.requestCreateSessionWithRouter2(router, requestId, route, sessionHints); - } - - // Binder call - @Override - public void notifySessionHintsForCreatingSession(IMediaRouter2 router, - long uniqueRequestId, MediaRoute2Info route, Bundle sessionHints) { - mService2.notifySessionHintsForCreatingSession(router, - uniqueRequestId, route, sessionHints); + mService2.requestCreateSessionWithRouter2(router, requestId, managerRequestId, + oldSession, route, sessionHints); } // Binder call @@ -558,8 +552,8 @@ public final class MediaRouterService extends IMediaRouterService.Stub // Binder call @Override public void requestCreateSessionWithManager(IMediaRouter2Manager manager, - int requestId, String packageName, MediaRoute2Info route) { - mService2.requestCreateSessionWithManager(manager, requestId, packageName, route); + int requestId, RoutingSessionInfo oldSession, MediaRoute2Info route) { + mService2.requestCreateSessionWithManager(manager, requestId, oldSession, route); } // Binder call diff --git a/services/core/java/com/android/server/media/SystemMediaRoute2Provider.java b/services/core/java/com/android/server/media/SystemMediaRoute2Provider.java index 42d4c88959bd3..2c089ca8300ea 100644 --- a/services/core/java/com/android/server/media/SystemMediaRoute2Provider.java +++ b/services/core/java/com/android/server/media/SystemMediaRoute2Provider.java @@ -222,6 +222,11 @@ class SystemMediaRoute2Provider extends MediaRoute2Provider { // Do nothing since we don't support grouping volume yet. } + @Override + public void prepareReleaseSession(String sessionId) { + // Do nothing since the system session persists. + } + public MediaRoute2Info getDefaultRoute() { return mDefaultRoute; }