Merge "Fix MediaRoute2ProviderService TODOs" into rvc-dev

This commit is contained in:
Kyunglyul Hyun
2020-06-04 07:05:29 +00:00
committed by Android (Google) Code Review

View File

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