Merge "Fix MediaRoute2ProviderService TODOs" into rvc-dev am: 0eadc17150

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/11675389

Change-Id: I4f1ad6b01723c8e5c0f5550eb9fa9261edb2c2ed

1 files changed, 101 insertions, 71 deletions

diff --git a/media/java/android/media/MediaRoute2ProviderService.java b/media/java/android/media/MediaRoute2ProviderService.java
index 981bf7af9f25..05c6e3ad9392 100644
--- a/media/java/android/media/MediaRoute2ProviderService.java
+++ b/media/java/android/media/MediaRoute2ProviderService.java
@@ -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,