Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix MediaRoute2ProviderService TODOs

Browse Source 
This CL ignores requests that contain unknown session IDs and
make sure sequence of binder calls are aligned with notifySession
Created/Updated/Released.

Bug: 157873546
Bug: 157873487
Test: Run CTS and atest mediarouter test
Change-Id: Ie31e2fca8160093cf54886180e67d40865a35d6d
This commit is contained in:
Kyunglyul Hyun
2020-06-01 21:25:15 +09:00
parent 5c8f3b20d5
commit d344655969
1 changed files with 101 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

172
media/java/android/media/MediaRoute2ProviderService.java
View File

@@ -137,7 +137,7 @@ public abstract class MediaRoute2ProviderService extends Service {
private final AtomicBoolean mStatePublishScheduled = new AtomicBoolean(false);
private MediaRoute2ProviderServiceStub mStub;
private IMediaRoute2ProviderServiceCallback mRemoteCallback;
private MediaRoute2ProviderInfo mProviderInfo;
private volatile MediaRoute2ProviderInfo mProviderInfo;
@GuardedBy("mSessionLock")
private ArrayMap<String, RoutingSessionInfo> mSessionInfo = new ArrayMap<>();
@@ -167,8 +167,8 @@ public abstract class MediaRoute2ProviderService extends Service {
/**
* Called when a volume setting is requested on a route of the provider
*
* @param requestId the id of this request
* @param routeId the id of the route
* @param requestId the ID of this request
* @param routeId the ID of the route
* @param volume the target volume
* @see MediaRoute2Info.Builder#setVolume(int)
*/
@@ -178,8 +178,8 @@ public abstract class MediaRoute2ProviderService extends Service {
* Called when {@link MediaRouter2.RoutingController#setVolume(int)} is called on
* a routing session of the provider
*
* @param requestId the id of this request
* @param sessionId the id of the routing session
* @param requestId the ID of this request
* @param sessionId the ID of the routing session
* @param volume the target volume
* @see RoutingSessionInfo.Builder#setVolume(int)
*/
@@ -188,7 +188,7 @@ public abstract class MediaRoute2ProviderService extends Service {
/**
* Gets information of the session with the given id.
*
* @param sessionId id of the session
* @param sessionId the ID of the session
* @return information of the session with the given id.
* null if the session is released or ID is not valid.
*/
@@ -218,7 +218,7 @@ public abstract class MediaRoute2ProviderService extends Service {
* If this session is created without any creation request, use {@link #REQUEST_ID_NONE}
* as the request ID.
*
* @param requestId id of the previous request to create this session provided in
* @param requestId the ID of the previous request to create this session provided in
* {@link #onCreateSession(long, String, String, Bundle)}. Can be
* {@link #REQUEST_ID_NONE} if this session is created without any request.
* @param sessionInfo information of the new session.
@@ -237,18 +237,15 @@ public abstract class MediaRoute2ProviderService extends Service {
return;
}
mSessionInfo.put(sessionInfo.getId(), sessionInfo);
}
if (mRemoteCallback == null) {
return;
}
try {
// TODO(b/157873487): Calling binder calls in multiple thread may cause timing issue.
// Consider to change implementations to avoid the problems.
// For example, post binder calls, always send all sessions at once, etc.
mRemoteCallback.notifySessionCreated(requestId, sessionInfo);
} catch (RemoteException ex) {
Log.w(TAG, "Failed to notify session created.");
if (mRemoteCallback == null) {
return;
}
try {
mRemoteCallback.notifySessionCreated(requestId, sessionInfo);
} catch (RemoteException ex) {
Log.w(TAG, "Failed to notify session created.");
}
}
}
@@ -267,22 +264,22 @@ public abstract class MediaRoute2ProviderService extends Service {
Log.w(TAG, "Ignoring unknown session info.");
return;
}
}
if (mRemoteCallback == null) {
return;
}
try {
mRemoteCallback.notifySessionUpdated(sessionInfo);
} catch (RemoteException ex) {
Log.w(TAG, "Failed to notify session info changed.");
if (mRemoteCallback == null) {
return;
}
try {
mRemoteCallback.notifySessionUpdated(sessionInfo);
} catch (RemoteException ex) {
Log.w(TAG, "Failed to notify session info changed.");
}
}
}
/**
* Notifies that the session is released.
*
* @param sessionId id of the released session.
* @param sessionId the ID of the released session.
* @see #onReleaseSession(long, String)
*/
public final void notifySessionReleased(@NonNull String sessionId) {
@@ -292,20 +289,20 @@ public abstract class MediaRoute2ProviderService extends Service {
RoutingSessionInfo sessionInfo;
synchronized (mSessionLock) {
sessionInfo = mSessionInfo.remove(sessionId);
}
if (sessionInfo == null) {
Log.w(TAG, "Ignoring unknown session info.");
return;
}
if (sessionInfo == null) {
Log.w(TAG, "Ignoring unknown session info.");
return;
}
if (mRemoteCallback == null) {
return;
}
try {
mRemoteCallback.notifySessionReleased(sessionInfo);
} catch (RemoteException ex) {
Log.w(TAG, "Failed to notify session info changed.");
if (mRemoteCallback == null) {
return;
}
try {
mRemoteCallback.notifySessionReleased(sessionInfo);
} catch (RemoteException ex) {
Log.w(TAG, "Failed to notify session info changed.");
}
}
}
@@ -348,9 +345,9 @@ public abstract class MediaRoute2ProviderService extends Service {
* If you can't create the session or want to reject the request, call
* {@link #notifyRequestFailed(long, int)} with the given {@code requestId}.
*
* @param requestId the id of this request
* @param requestId the ID of this request
* @param packageName the package name of the application that selected the route
* @param routeId the id of the route initially being connected
* @param routeId the ID of the route initially being connected
* @param sessionHints an optional bundle of app-specific arguments sent by
* {@link MediaRouter2}, or null if none. The contents of this bundle
* may affect the result of session creation.
@@ -372,8 +369,8 @@ public abstract class MediaRoute2ProviderService extends Service {
* Note: Calling {@link #notifySessionReleased(String)} will <em>NOT</em> trigger
* this method to be called.
*
* @param requestId the id of this request
* @param sessionId id of the session being released.
* @param requestId the ID of this request
* @param sessionId the ID of the session being released.
* @see #notifySessionReleased(String)
* @see #getSessionInfo(String)
*/
@@ -384,9 +381,9 @@ public abstract class MediaRoute2ProviderService extends Service {
* After the route is selected, call {@link #notifySessionUpdated(RoutingSessionInfo)}
* to update session info.
*
* @param requestId the id of this request
* @param sessionId id of the session
* @param routeId id of the route
* @param requestId the ID of this request
* @param sessionId the ID of the session
* @param routeId the ID of the route
*/
public abstract void onSelectRoute(long requestId, @NonNull String sessionId,
@NonNull String routeId);
@@ -396,9 +393,9 @@ public abstract class MediaRoute2ProviderService extends Service {
* After the route is deselected, call {@link #notifySessionUpdated(RoutingSessionInfo)}
* to update session info.
*
* @param requestId the id of this request
* @param sessionId id of the session
* @param routeId id of the route
* @param requestId the ID of this request
* @param sessionId the ID of the session
* @param routeId the ID of the route
*/
public abstract void onDeselectRoute(long requestId, @NonNull String sessionId,
@NonNull String routeId);
@@ -408,9 +405,9 @@ public abstract class MediaRoute2ProviderService extends Service {
* After the transfer is finished, call {@link #notifySessionUpdated(RoutingSessionInfo)}
* to update session info.
*
* @param requestId the id of this request
* @param sessionId id of the session
* @param routeId id of the route
* @param requestId the ID of this request
* @param sessionId the ID of the session
* @param routeId the ID of the route
*/
public abstract void onTransferToRoute(long requestId, @NonNull String sessionId,
@NonNull String routeId);
@@ -475,13 +472,39 @@ public abstract class MediaRoute2ProviderService extends Service {
final class MediaRoute2ProviderServiceStub extends IMediaRoute2ProviderService.Stub {
MediaRoute2ProviderServiceStub() { }
boolean checkCallerisSystem() {
private boolean checkCallerIsSystem() {
return Binder.getCallingUid() == Process.SYSTEM_UID;
}
private boolean checkSessionIdIsValid(String sessionId, String description) {
if (TextUtils.isEmpty(sessionId)) {
Log.w(TAG, description + ": Ignoring empty sessionId from system service.");
return false;
}
if (getSessionInfo(sessionId) == null) {
Log.w(TAG, description + ": Ignoring unknown session from system service. "
+ "sessionId=" + sessionId);
return false;
}
return true;
}
private boolean checkRouteIdIsValid(String routeId, String description) {
if (TextUtils.isEmpty(routeId)) {
Log.w(TAG, description + ": Ignoring empty routeId from system service.");
return false;
}
if (mProviderInfo == null || mProviderInfo.getRoute(routeId) == null) {
Log.w(TAG, description + ": Ignoring unknown route from system service. "
+ "routeId=" + routeId);
return false;
}
return true;
}
@Override
public void setCallback(IMediaRoute2ProviderServiceCallback callback) {
if (!checkCallerisSystem()) {
if (!checkCallerIsSystem()) {
return;
}
mHandler.sendMessage(obtainMessage(MediaRoute2ProviderService::setCallback,
@@ -490,7 +513,7 @@ public abstract class MediaRoute2ProviderService extends Service {
@Override
public void updateDiscoveryPreference(RouteDiscoveryPreference discoveryPreference) {
if (!checkCallerisSystem()) {
if (!checkCallerIsSystem()) {
return;
}
mHandler.sendMessage(obtainMessage(
@@ -500,7 +523,10 @@ public abstract class MediaRoute2ProviderService extends Service {
@Override
public void setRouteVolume(long requestId, String routeId, int volume) {
if (!checkCallerisSystem()) {
if (!checkCallerIsSystem()) {
return;
}
if (!checkRouteIdIsValid(routeId, "setRouteVolume")) {
return;
}
mHandler.sendMessage(obtainMessage(MediaRoute2ProviderService::onSetRouteVolume,
@@ -510,7 +536,10 @@ public abstract class MediaRoute2ProviderService extends Service {
@Override
public void requestCreateSession(long requestId, String packageName, String routeId,
@Nullable Bundle requestCreateSession) {
if (!checkCallerisSystem()) {
if (!checkCallerIsSystem()) {
return;
}
if (!checkRouteIdIsValid(routeId, "requestCreateSession")) {
return;
}
mHandler.sendMessage(obtainMessage(MediaRoute2ProviderService::onCreateSession,
@@ -518,14 +547,13 @@ public abstract class MediaRoute2ProviderService extends Service {
requestCreateSession));
}
//TODO(b/157873546): Ignore requests with unknown session ID. -> For all similar commands.
@Override
public void selectRoute(long requestId, String sessionId, String routeId) {
if (!checkCallerisSystem()) {
if (!checkCallerIsSystem()) {
return;
}
if (TextUtils.isEmpty(sessionId)) {
Log.w(TAG, "selectRoute: Ignoring empty sessionId from system service.");
if (!checkSessionIdIsValid(sessionId, "selectRoute")
|| !checkRouteIdIsValid(routeId, "selectRoute")) {
return;
}
mHandler.sendMessage(obtainMessage(MediaRoute2ProviderService::onSelectRoute,
@@ -534,11 +562,11 @@ public abstract class MediaRoute2ProviderService extends Service {
@Override
public void deselectRoute(long requestId, String sessionId, String routeId) {
if (!checkCallerisSystem()) {
if (!checkCallerIsSystem()) {
return;
}
if (TextUtils.isEmpty(sessionId)) {
Log.w(TAG, "deselectRoute: Ignoring empty sessionId from system service.");
if (!checkSessionIdIsValid(sessionId, "deselectRoute")
|| !checkRouteIdIsValid(routeId, "deselectRoute")) {
return;
}
mHandler.sendMessage(obtainMessage(MediaRoute2ProviderService::onDeselectRoute,
@@ -547,11 +575,11 @@ public abstract class MediaRoute2ProviderService extends Service {
@Override
public void transferToRoute(long requestId, String sessionId, String routeId) {
if (!checkCallerisSystem()) {
if (!checkCallerIsSystem()) {
return;
}
if (TextUtils.isEmpty(sessionId)) {
Log.w(TAG, "transferToRoute: Ignoring empty sessionId from system service.");
if (!checkSessionIdIsValid(sessionId, "transferToRoute")
|| !checkRouteIdIsValid(routeId, "transferToRoute")) {
return;
}
mHandler.sendMessage(obtainMessage(MediaRoute2ProviderService::onTransferToRoute,
@@ -560,7 +588,10 @@ public abstract class MediaRoute2ProviderService extends Service {
@Override
public void setSessionVolume(long requestId, String sessionId, int volume) {
if (!checkCallerisSystem()) {
if (!checkCallerIsSystem()) {
return;
}
if (!checkSessionIdIsValid(sessionId, "setSessionVolume")) {
return;
}
mHandler.sendMessage(obtainMessage(MediaRoute2ProviderService::onSetSessionVolume,
@@ -569,11 +600,10 @@ public abstract class MediaRoute2ProviderService extends Service {
@Override
public void releaseSession(long requestId, String sessionId) {
if (!checkCallerisSystem()) {
if (!checkCallerIsSystem()) {
return;
}
if (TextUtils.isEmpty(sessionId)) {
Log.w(TAG, "releaseSession: Ignoring empty sessionId from system service.");
if (!checkSessionIdIsValid(sessionId, "releaseSession")) {
return;
}
mHandler.sendMessage(obtainMessage(MediaRoute2ProviderService::onReleaseSession,