Merge "Add bug numbers for TODOs in MediaRouter2 related classes" into rvc-dev am: 5c8f3b20d5

Original change: undetermined

Change-Id: I441f51b100a3b7aa5d17e9199ae0e9c122695d64

6 files changed, 13 insertions, 24 deletions

diff --git a/media/java/android/media/MediaRoute2ProviderService.java b/media/java/android/media/MediaRoute2ProviderService.java
index 72162c44ec29..981bf7af9f25 100644
--- a/media/java/android/media/MediaRoute2ProviderService.java
+++ b/media/java/android/media/MediaRoute2ProviderService.java
@@ -233,7 +233,6 @@ public abstract class MediaRoute2ProviderService extends Service {
         String sessionId = sessionInfo.getId();
         synchronized (mSessionLock) {
             if (mSessionInfo.containsKey(sessionId)) {
-                // TODO: Notify failure to the requester, and throw exception if needed.
                 Log.w(TAG, "Ignoring duplicate session id.");
                 return;
             }
@@ -244,7 +243,7 @@ public abstract class MediaRoute2ProviderService extends Service {
             return;
         }
         try {
-            // TODO: Calling binder calls in multiple thread may cause timing issue.
+            // 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);
@@ -519,7 +518,7 @@ public abstract class MediaRoute2ProviderService extends Service {
                     requestCreateSession));
         }
 
-        //TODO: Ignore requests with unknown session ID.
+        //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()) {
diff --git a/media/java/android/media/MediaRouter2.java b/media/java/android/media/MediaRouter2.java
index 6179b483ca53..6634d4b4190e 100644
--- a/media/java/android/media/MediaRouter2.java
+++ b/media/java/android/media/MediaRouter2.java
@@ -54,7 +54,7 @@ import java.util.stream.Collectors;
  * Media Router 2 allows applications to control the routing of media channels
  * and streams from the current device to remote speakers and devices.
  */
-// TODO: Add method names at the beginning of log messages. (e.g. updateControllerOnHandler)
+// TODO(b/157873330): Add method names at the beginning of log messages. (e.g. selectRoute)
 //       Not only MediaRouter2, but also to service / manager / provider.
 // TODO: ensure thread-safe and document it
 public final class MediaRouter2 {
@@ -399,7 +399,7 @@ public final class MediaRouter2 {
         Objects.requireNonNull(controller, "controller must not be null");
         Objects.requireNonNull(route, "route must not be null");
 
-        // TODO: Check thread-safety
+        // TODO(b/157873496): Check thread-safety, at least check "sRouterLock" for every variable
         if (!mRoutes.containsKey(route.getId())) {
             notifyTransferFailure(route);
             return;
@@ -501,7 +501,7 @@ public final class MediaRouter2 {
     }
 
     void addRoutesOnHandler(List<MediaRoute2Info> routes) {
-        // TODO: When onRoutesAdded is first called,
+        // TODO(b/157874065): When onRoutesAdded is first called,
         //  1) clear mRoutes before adding the routes
         //  2) Call onRouteSelected(system_route, reason_fallback) if previously selected route
         //     does not exist anymore. => We may need 'boolean MediaRoute2Info#isSystemRoute()'.
@@ -1214,7 +1214,7 @@ public final class MediaRouter2 {
          * Any operations on this controller after calling this method will be ignored.
          * The devices that are playing media will stop playing it.
          */
-        // TODO: Add tests using {@link MediaRouter2Manager#getActiveSessions()}.
+        // TODO(b/157872573): Add tests using {@link MediaRouter2Manager#getActiveSessions()}.
         public void release() {
             releaseInternal(/* shouldReleaseSession= */ true, /* shouldNotifyStop= */ true);
         }
diff --git a/media/java/android/media/MediaRouter2Manager.java b/media/java/android/media/MediaRouter2Manager.java
index 4ebfce830a70..a382c2de223e 100644
--- a/media/java/android/media/MediaRouter2Manager.java
+++ b/media/java/android/media/MediaRouter2Manager.java
@@ -151,8 +151,6 @@ public final class MediaRouter2Manager {
         return null;
     }
 
-    //TODO: Use cache not to create array. For now, it's unclear when to purge the cache.
-    //Do this when we finalize how to set control categories.
     /**
      * Gets available routes for an application.
      *
@@ -339,7 +337,7 @@ public final class MediaRouter2Manager {
         Objects.requireNonNull(sessionInfo, "sessionInfo must not be null");
         Objects.requireNonNull(route, "route must not be null");
 
-        //TODO: Ignore unknown route.
+        //TODO(b/157875504): Ignore unknown route.
         if (sessionInfo.getTransferableRoutes().contains(route.getId())) {
             transferToRoute(sessionInfo, route);
             return;
@@ -355,7 +353,7 @@ public final class MediaRouter2Manager {
         if (client != null) {
             try {
                 int requestId = mNextRequestId.getAndIncrement();
-                //TODO: Ensure that every request is eventually removed.
+                //TODO(b/157875723): Ensure that every request is eventually removed. (Memory leak)
                 mTransferRequests.add(new TransferRequest(requestId, sessionInfo, route));
 
                 mMediaRouterService.requestCreateSessionWithManager(
diff --git a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java
index c05c21cf2752..1e49f49b37bc 100644
--- a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java
+++ b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java
@@ -109,7 +109,7 @@ public class MediaRouter2ManagerTest {
         mContext = InstrumentationRegistry.getTargetContext();
         mManager = MediaRouter2Manager.getInstance(mContext);
         mRouter2 = MediaRouter2.getInstance(mContext);
-        //TODO: If we need to support thread pool executors, change this to thread pool executor.
+        // If we need to support thread pool executors, change this to thread pool executor.
         mExecutor = Executors.newSingleThreadExecutor();
         mPackageName = mContext.getPackageName();
     }
@@ -253,7 +253,6 @@ public class MediaRouter2ManagerTest {
         CountDownLatch latch = new CountDownLatch(1);
 
         addManagerCallback(new MediaRouter2Manager.Callback());
-        //TODO: remove this when it's not necessary.
         addRouterCallback(new MediaRouter2.RouteCallback() {});
         addTransferCallback(new MediaRouter2.TransferCallback() {
             @Override
diff --git a/services/core/java/com/android/server/media/MediaRoute2ProviderServiceProxy.java b/services/core/java/com/android/server/media/MediaRoute2ProviderServiceProxy.java
index 53205add0b38..d6b98e2de901 100644
--- a/services/core/java/com/android/server/media/MediaRoute2ProviderServiceProxy.java
+++ b/services/core/java/com/android/server/media/MediaRoute2ProviderServiceProxy.java
@@ -44,7 +44,6 @@ import java.util.Objects;
 /**
  * Maintains a connection to a particular {@link MediaRoute2ProviderService}.
  */
-// TODO: Need to revisit the bind/unbind/connect/disconnect logic in this class.
 final class MediaRoute2ProviderServiceProxy extends MediaRoute2Provider
         implements ServiceConnection {
     private static final String TAG = "MR2ProviderSvcProxy";
@@ -265,8 +264,6 @@ final class MediaRoute2ProviderServiceProxy extends MediaRoute2Provider
         if (DEBUG) {
             Slog.d(TAG, this + ": Service binding died");
         }
-        // TODO: Investigate whether it tries to bind endlessly when the service is
-        //       badly implemented.
         if (shouldBind()) {
             unbind();
             bind();
diff --git a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java
index c65800a17f82..75a89a213052 100644
--- a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java
+++ b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java
@@ -1026,7 +1026,8 @@ class MediaRouter2ServiceImpl {
             mHandler = new UserHandler(MediaRouter2ServiceImpl.this, this);
         }
 
-        // TODO: This assumes that only one router exists in a package. Is it true?
+        // TODO: This assumes that only one router exists in a package.
+        //       Do this in Android S or later.
         RouterRecord findRouterRecordLocked(String packageName) {
             for (RouterRecord routerRecord : mRouterRecords) {
                 if (TextUtils.equals(routerRecord.mPackageName, packageName)) {
@@ -1121,7 +1122,6 @@ class MediaRouter2ServiceImpl {
         private final UserRecord mUserRecord;
         private final MediaRoute2ProviderWatcher mWatcher;
 
-        //TODO: Make this thread-safe.
         private final SystemMediaRoute2Provider mSystemProvider;
         private final ArrayList<MediaRoute2Provider> mRouteProviders =
                 new ArrayList<>();
@@ -1153,7 +1153,6 @@ class MediaRouter2ServiceImpl {
         private void stop() {
             if (mRunning) {
                 mRunning = false;
-                //TODO: may unselect routes
                 mWatcher.stop(); // also stops all providers
             }
         }
@@ -1386,7 +1385,6 @@ class MediaRouter2ServiceImpl {
 
             final String providerId = route.getProviderId();
             final MediaRoute2Provider provider = findProvider(providerId);
-            // TODO: Remove this null check when the mMediaProviders are referenced only in handler.
             if (provider == null) {
                 return;
             }
@@ -1405,7 +1403,6 @@ class MediaRouter2ServiceImpl {
 
             final String providerId = route.getProviderId();
             final MediaRoute2Provider provider = findProvider(providerId);
-            // TODO: Remove this null check when the mMediaProviders are referenced only in handler.
             if (provider == null) {
                 return;
             }
@@ -1425,7 +1422,6 @@ class MediaRouter2ServiceImpl {
 
             final String providerId = route.getProviderId();
             final MediaRoute2Provider provider = findProvider(providerId);
-            // TODO: Remove this null check when the mMediaProviders are referenced only in handler.
             if (provider == null) {
                 return;
             }
@@ -1641,8 +1637,8 @@ class MediaRouter2ServiceImpl {
             // TODO: Notify router too when the related callback is introduced.
         }
 
-        // TODO: Find a way to prevent providers from notifying error on random uniqueRequestId.
-        //       Solutions can be:
+        // 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.