Merge "Notify a failure on transfer timeout" into rvc-dev

media/java/android/media/MediaRouter2Manager.java

@@ -25,12 +25,14 @@ import android.content.Context;
 import android.media.session.MediaController;
 import android.media.session.MediaSessionManager;
 import android.os.Handler;
+import android.os.Message;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.text.TextUtils;
 import android.util.Log;
 
 import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -52,6 +54,9 @@ import java.util.stream.Collectors;
 public final class MediaRouter2Manager {
     private static final String TAG = "MR2Manager";
     private static final Object sLock = new Object();
+    /** @hide */
+    @VisibleForTesting
+    public static final int TRANSFER_TIMEOUT_MS = 30_000;
 
     @GuardedBy("sLock")
     private static MediaRouter2Manager sInstance;
@@ -340,27 +345,8 @@ public final class MediaRouter2Manager {
         //TODO(b/157875504): Ignore unknown route.
         if (sessionInfo.getTransferableRoutes().contains(route.getId())) {
             transferToRoute(sessionInfo, route);
-            return;
+        } else {
-        }
+            requestCreateSession(sessionInfo, route);
-
-        if (TextUtils.isEmpty(sessionInfo.getClientPackageName())) {
-            Log.w(TAG, "transfer: Ignoring transfer without package name.");
-            notifyTransferFailed(sessionInfo, route);
-            return;
-        }
-
-        Client client = getOrCreateClient();
-        if (client != null) {
-            try {
-                int requestId = mNextRequestId.getAndIncrement();
-                //TODO(b/157875723): Ensure that every request is eventually removed. (Memory leak)
-                mTransferRequests.add(new TransferRequest(requestId, sessionInfo, route));
-
-                mMediaRouterService.requestCreateSessionWithManager(
-                        client, requestId, sessionInfo.getClientPackageName(), route);
-            } catch (RemoteException ex) {
-                Log.e(TAG, "Unable to select media route", ex);
-            }
         }
     }
 
@@ -523,8 +509,8 @@ public final class MediaRouter2Manager {
                 continue;
             }
             if (sessionInfo.getSelectedRoutes().contains(request.mTargetRoute.getId())) {
-                notifyTransferred(request.mOldSessionInfo, sessionInfo);
                 mTransferRequests.remove(request);
+                notifyTransferred(request.mOldSessionInfo, sessionInfo);
                 break;
             }
         }
@@ -724,41 +710,6 @@ public final class MediaRouter2Manager {
         }
     }
 
-    /**
-     * Transfers to a given route for the remote session.
-     *
-     * @hide
-     */
-    void transferToRoute(@NonNull RoutingSessionInfo sessionInfo,
-            @NonNull MediaRoute2Info route) {
-        Objects.requireNonNull(sessionInfo, "sessionInfo must not be null");
-        Objects.requireNonNull(route, "route must not be null");
-
-        if (sessionInfo.getSelectedRoutes().contains(route.getId())) {
-            Log.w(TAG, "Ignoring transferring to a route that is already added. route="
-                    + route);
-            return;
-        }
-
-        if (!sessionInfo.getTransferableRoutes().contains(route.getId())) {
-            Log.w(TAG, "Ignoring transferring to a non-transferable route=" + route);
-            return;
-        }
-
-        int requestId = mNextRequestId.getAndIncrement();
-        mTransferRequests.add(new TransferRequest(requestId, sessionInfo, route));
-
-        Client client = getOrCreateClient();
-        if (client != null) {
-            try {
-                mMediaRouterService.transferToRouteWithManager(
-                        client, requestId, sessionInfo.getId(), route);
-            } catch (RemoteException ex) {
-                Log.e(TAG, "transferToRoute: Failed to send a request.", ex);
-            }
-        }
-    }
-
     /**
      * Requests releasing a session.
      * <p>
@@ -784,6 +735,65 @@ public final class MediaRouter2Manager {
         }
     }
 
+    /**
+     * Transfers the remote session to the given route.
+     *
+     * @hide
+     */
+    private void transferToRoute(@NonNull RoutingSessionInfo session,
+            @NonNull MediaRoute2Info route) {
+        int requestId = createTransferRequest(session, route);
+
+        Client client = getOrCreateClient();
+        if (client != null) {
+            try {
+                mMediaRouterService.transferToRouteWithManager(
+                        client, requestId, session.getId(), route);
+            } catch (RemoteException ex) {
+                Log.e(TAG, "transferToRoute: Failed to send a request.", ex);
+            }
+        }
+    }
+
+    private void requestCreateSession(RoutingSessionInfo oldSession, MediaRoute2Info route) {
+        if (TextUtils.isEmpty(oldSession.getClientPackageName())) {
+            Log.w(TAG, "requestCreateSession: Can't create a session without package name.");
+            notifyTransferFailed(oldSession, route);
+            return;
+        }
+
+        int requestId = createTransferRequest(oldSession, route);
+
+        Client client = getOrCreateClient();
+        if (client != null) {
+            try {
+                mMediaRouterService.requestCreateSessionWithManager(
+                        client, requestId, oldSession.getClientPackageName(), route);
+            } catch (RemoteException ex) {
+                Log.e(TAG, "requestCreateSession: Failed to send a request", ex);
+            }
+        }
+    }
+
+    private int createTransferRequest(RoutingSessionInfo session, MediaRoute2Info route) {
+        int requestId = mNextRequestId.getAndIncrement();
+        TransferRequest transferRequest = new TransferRequest(requestId, session, route);
+        mTransferRequests.add(transferRequest);
+
+        Message timeoutMessage =
+                obtainMessage(MediaRouter2Manager::handleTransferTimeout, this, transferRequest);
+        mHandler.sendMessageDelayed(timeoutMessage, TRANSFER_TIMEOUT_MS);
+        return requestId;
+    }
+
+    private void handleTransferTimeout(TransferRequest request) {
+        boolean removed = mTransferRequests.remove(request);
+        if (removed) {
+            notifyTransferFailed(request.mOldSessionInfo, request.mTargetRoute);
+        }
+    }
+
+
     private boolean areSessionsMatched(MediaController mediaController,
             RoutingSessionInfo sessionInfo) {
         MediaController.PlaybackInfo playbackInfo = mediaController.getPlaybackInfo();
@@ -905,7 +915,7 @@ public final class MediaRouter2Manager {
             if (!(obj instanceof CallbackRecord)) {
                 return false;
             }
-            return mCallback ==  ((CallbackRecord) obj).mCallback;
+            return mCallback == ((CallbackRecord) obj).mCallback;
         }
 
         @Override

media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java

@@ -28,6 +28,7 @@ import static com.android.mediaroutertest.StubMediaRoute2ProviderService.ROUTE_I
 import static com.android.mediaroutertest.StubMediaRoute2ProviderService.ROUTE_ID2;
 import static com.android.mediaroutertest.StubMediaRoute2ProviderService.ROUTE_ID4_TO_SELECT_AND_DESELECT;
 import static com.android.mediaroutertest.StubMediaRoute2ProviderService.ROUTE_ID5_TO_TRANSFER_TO;
+import static com.android.mediaroutertest.StubMediaRoute2ProviderService.ROUTE_ID6_TO_BE_IGNORED;
 import static com.android.mediaroutertest.StubMediaRoute2ProviderService.ROUTE_ID_FIXED_VOLUME;
 import static com.android.mediaroutertest.StubMediaRoute2ProviderService.ROUTE_ID_SPECIAL_FEATURE;
 import static com.android.mediaroutertest.StubMediaRoute2ProviderService.ROUTE_ID_VARIABLE_VOLUME;
@@ -51,6 +52,7 @@ import android.media.RouteDiscoveryPreference;
 import android.media.RoutingSessionInfo;
 import android.os.Bundle;
 import android.support.test.InstrumentationRegistry;
+import android.support.test.filters.LargeTest;
 import android.support.test.filters.SmallTest;
 import android.support.test.runner.AndroidJUnit4;
 import android.text.TextUtils;
@@ -349,6 +351,35 @@ public class MediaRouter2ManagerTest {
                 route -> TextUtils.equals(route.getClientPackageName(), null));
     }
 
+    @Test
+    @LargeTest
+    public void testTransfer_ignored_fails() throws Exception {
+        Map<String, MediaRoute2Info> routes = waitAndGetRoutesWithManager(FEATURES_ALL);
+        addRouterCallback(new RouteCallback() {});
+
+        CountDownLatch onSessionCreatedLatch = new CountDownLatch(1);
+        CountDownLatch onFailedLatch = new CountDownLatch(1);
+
+        addManagerCallback(new MediaRouter2Manager.Callback() {
+            @Override
+            public void onTransferred(RoutingSessionInfo oldSessionInfo,
+                    RoutingSessionInfo newSessionInfo) {
+                onSessionCreatedLatch.countDown();
+            }
+            @Override
+            public void onTransferFailed(RoutingSessionInfo session, MediaRoute2Info route) {
+                onFailedLatch.countDown();
+            }
+        });
+
+        List<RoutingSessionInfo> sessions = mManager.getRoutingSessions(mPackageName);
+        RoutingSessionInfo targetSession = sessions.get(sessions.size() - 1);
+        mManager.transfer(targetSession, routes.get(ROUTE_ID6_TO_BE_IGNORED));
+
+        assertFalse(onSessionCreatedLatch.await(WAIT_TIME_MS, TimeUnit.MILLISECONDS));
+        assertTrue(onFailedLatch.await(MediaRouter2Manager.TRANSFER_TIMEOUT_MS,
+                TimeUnit.MILLISECONDS));
+    }
     @Test
     public void testSetSystemRouteVolume() throws Exception {
         // ensure client

media/tests/MediaRouter/src/com/android/mediaroutertest/StubMediaRoute2ProviderService.java

@@ -53,6 +53,9 @@ public class StubMediaRoute2ProviderService extends MediaRoute2ProviderService {
     public static final String ROUTE_ID5_TO_TRANSFER_TO = "route_id5_to_transfer_to";
     public static final String ROUTE_NAME5 = "Sample Route 5 - Route to transfer to";
 
+    public static final String ROUTE_ID6_TO_BE_IGNORED = "route_id6_to_be_ignored";
+    public static final String ROUTE_NAME6 = "Sample Route 6 - Route to be ignored";
+
     public static final String ROUTE_ID_SPECIAL_FEATURE = "route_special_feature";
     public static final String ROUTE_NAME_SPECIAL_FEATURE = "Special Feature Route";
 
@@ -98,7 +101,10 @@ public class StubMediaRoute2ProviderService extends MediaRoute2ProviderService {
                 ROUTE_ID5_TO_TRANSFER_TO, ROUTE_NAME5)
                 .addFeature(FEATURE_SAMPLE)
                 .build();
-
+        MediaRoute2Info route6 = new MediaRoute2Info.Builder(
+                ROUTE_ID6_TO_BE_IGNORED, ROUTE_NAME6)
+                .addFeature(FEATURE_SAMPLE)
+                .build();
         MediaRoute2Info routeSpecial =
                 new MediaRoute2Info.Builder(ROUTE_ID_SPECIAL_FEATURE, ROUTE_NAME_SPECIAL_FEATURE)
                         .addFeature(FEATURE_SAMPLE)
@@ -121,6 +127,7 @@ public class StubMediaRoute2ProviderService extends MediaRoute2ProviderService {
         mRoutes.put(route3.getId(), route3);
         mRoutes.put(route4.getId(), route4);
         mRoutes.put(route5.getId(), route5);
+        mRoutes.put(route6.getId(), route6);
 
         mRoutes.put(routeSpecial.getId(), routeSpecial);
         mRoutes.put(fixedVolumeRoute.getId(), fixedVolumeRoute);
@@ -219,6 +226,10 @@ public class StubMediaRoute2ProviderService extends MediaRoute2ProviderService {
             notifyRequestFailed(requestId, REASON_UNKNOWN_ERROR);
             return;
         }
+        // Ignores the request intentionally for testing
+        if (TextUtils.equals(ROUTE_ID6_TO_BE_IGNORED, routeId)) {
+            return;
+        }
         maybeDeselectRoute(routeId);
 
         final String sessionId = String.valueOf(mNextSessionId);