[Android MR] Postpone route selection until session launch

Previously, we unselect and re-select the route due to CAF limitation.
The reason is that we need to set the receiver app ID after the user
choose a device. However CAF won't be able to know a device was selected
after the receiver app ID gets changed.

In this CL, to solve the issue, we intercept the click event in
MediaRouteChooserDialog (which would select the route automatically).
The route selection is postponed right before session launch. Therefore
we are able to set the receiver app ID and select the route to get rid
of CAF limitation.

Bug: 711860
Change-Id: I5b1534d36d540b62199a6fb739c56ad5131c2711
Reviewed-on: https://chromium-review.googlesource.com/c/1351893Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Zhiqiang Zhang <zqzhang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611437}

chrome/android/java/src/org/chromium/chrome/browser/media/router/MediaRouteChooserDialogManager.java

@@ -13,6 +13,11 @@ import android.support.v4.app.FragmentManager;
 import android.support.v7.app.MediaRouteChooserDialog;
 import android.support.v7.app.MediaRouteChooserDialogFragment;
 import android.support.v7.media.MediaRouteSelector;
+import android.support.v7.media.MediaRouter;
+import android.support.v7.mediarouter.R;
+import android.view.View;
+import android.widget.AdapterView;
+import android.widget.ListView;
 
 /**
  * Manages the dialog responsible for selecting a {@link MediaSink}.
@@ -34,7 +39,7 @@ public class MediaRouteChooserDialogManager extends BaseMediaRouteDialogManager
         private final Handler mHandler = new Handler();
         private final SystemVisibilitySaver mVisibilitySaver = new SystemVisibilitySaver();
         private BaseMediaRouteDialogManager mManager;
-        private boolean mCancelled;
+        private boolean mIsSinkSelected;
 
         public Fragment() {
             mHandler.post(new Runnable() {
@@ -52,8 +57,7 @@ public class MediaRouteChooserDialogManager extends BaseMediaRouteDialogManager
         @Override
         public MediaRouteChooserDialog onCreateChooserDialog(
                 Context context, Bundle savedInstanceState) {
-            MediaRouteChooserDialog dialog =
-                    super.onCreateChooserDialog(context, savedInstanceState);
+            MediaRouteChooserDialog dialog = new DelayedSelectionDialog(context, getTheme());
             dialog.setCanceledOnTouchOutside(true);
             return dialog;
         }
@@ -71,26 +75,45 @@ public class MediaRouteChooserDialogManager extends BaseMediaRouteDialogManager
         }
 
         @Override
-        public void onCancel(DialogInterface dialog) {
-            mCancelled = true;
-
-            mManager.delegate().onDialogCancelled();
+        public void onDismiss(DialogInterface dialog) {
+            super.onDismiss(dialog);
 
-            super.onCancel(dialog);
+            if (!mIsSinkSelected) mManager.delegate().onDialogCancelled();
         }
 
-        @Override
-        public void onDismiss(DialogInterface dialog) {
-            super.onDismiss(dialog);
-            if (mManager == null) return;
+        private class DelayedSelectionDialog extends MediaRouteChooserDialog {
+            public DelayedSelectionDialog(Context context) {
+                super(context);
+            }
+
+            public DelayedSelectionDialog(Context context, int theme) {
+                super(context, theme);
+            }
+
+            @Override
+            public void onCreate(Bundle savedInstanceState) {
+                super.onCreate(savedInstanceState);
+
+                ListView listView = (ListView) findViewById(R.id.mr_chooser_list);
+                if (listView != null) {
+                    listView.setOnItemClickListener(Fragment.this::onItemClick);
+                }
+            }
+        }
 
-            mManager.mDialogFragment = null;
+        private void onItemClick(AdapterView<?> parent, View view, int position, long id) {
+            MediaRouter.RouteInfo routeInfo =
+                    (MediaRouter.RouteInfo) parent.getItemAtPosition(position);
+            if (routeInfo != null && routeInfo.isEnabled()) {
+                MediaSink newSink = MediaSink.fromRoute(routeInfo);
 
-            if (mCancelled) return;
+                // When a item is clicked, the route is not selected right away. Instead, the route
+                // selection is postponed to the actual session launch.
+                mManager.delegate().onSinkSelected(mManager.sourceId(), newSink);
+                mIsSinkSelected = true;
 
-            MediaSink newSink =
-                    MediaSink.fromRoute(mManager.androidMediaRouter().getSelectedRoute());
-            mManager.delegate().onSinkSelected(mManager.sourceId(), newSink);
+                dismiss();
+            }
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/media/router/caf/BaseSessionController.java

@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.media.router.caf;
 
 import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
-import android.support.v7.media.MediaRouter;
 
 import com.google.android.gms.cast.CastDevice;
 import com.google.android.gms.cast.framework.CastSession;
@@ -31,7 +30,6 @@ public class BaseSessionController {
 
     private CastSession mCastSession;
     private final CafBaseMediaRouteProvider mProvider;
-    private final MediaRouter.Callback mMediaRouterCallbackForSessionLaunch;
     private CreateRouteRequestInfo mRouteCreationInfo;
     @VisibleForTesting
     CafNotificationController mNotificationController;
@@ -39,7 +37,6 @@ public class BaseSessionController {
 
     public BaseSessionController(CafBaseMediaRouteProvider provider) {
         mProvider = provider;
-        mMediaRouterCallbackForSessionLaunch = new MediaRouterCallbackForSessionLaunch();
         mNotificationController = new CafNotificationController(this);
         mRemoteMediaClientCallback = new RemoteMediaClientCallback();
     }
@@ -49,21 +46,10 @@ public class BaseSessionController {
         CastUtils.getCastContext().setReceiverApplicationId(
                 mRouteCreationInfo.source.getApplicationId());
 
-        if (mRouteCreationInfo.routeInfo.isSelected()) {
-            // If a route has just been selected, CAF might not be ready yet before setting the app
-            // ID. So unselect and select the route will let CAF be aware that the route has been
-            // selected thus it can start the session.
-            //
-            // An issue of this workaround is that if a route is unselected and selected in a very
-            // short time, the selection might be ignored by MediaRouter, so put the reselection in
-            // a callback.
-            mProvider.getAndroidMediaRouter().addCallback(
-                    mRouteCreationInfo.source.buildRouteSelector(),
-                    mMediaRouterCallbackForSessionLaunch);
-            mProvider.getAndroidMediaRouter().unselect(MediaRouter.UNSELECT_REASON_UNKNOWN);
-        } else {
-            mRouteCreationInfo.routeInfo.select();
-        }
+        // When the user clicks a route on the MediaRouteChooserDialog, we intercept the click event
+        // and do not select the route. Instead the route selection is postponed to here. This will
+        // trigger CAF to launch the session.
+        mRouteCreationInfo.routeInfo.select();
     }
 
     public MediaSource getSource() {
@@ -173,20 +159,6 @@ public class BaseSessionController {
         }
     }
 
-    private class MediaRouterCallbackForSessionLaunch extends MediaRouter.Callback {
-        @Override
-        public void onRouteUnselected(MediaRouter mediaRouter, MediaRouter.RouteInfo routeInfo) {
-            if (mProvider.getPendingCreateRouteRequestInfo() == null) return;
-
-            if (routeInfo.getId().equals(
-                        mProvider.getPendingCreateRouteRequestInfo().routeInfo.getId())) {
-                routeInfo.select();
-                mProvider.getAndroidMediaRouter().removeCallback(
-                        mMediaRouterCallbackForSessionLaunch);
-            }
-        }
-    }
-
     private class RemoteMediaClientCallback extends RemoteMediaClient.Callback {
         @Override
         public void onStatusUpdated() {

chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/BaseMediaRouteProvider.java

@@ -173,18 +173,32 @@ public abstract class BaseMediaRouteProvider
             return;
         }
 
-        MediaSink sink = MediaSink.fromSinkId(sinkId, mAndroidMediaRouter);
-        if (sink == null) {
+        MediaRouter.RouteInfo targetRouteInfo = null;
+        for (MediaRouter.RouteInfo routeInfo : mAndroidMediaRouter.getRoutes()) {
+            if (routeInfo.getId().equals(sinkId)) {
+                targetRouteInfo = routeInfo;
+                break;
+            }
+        }
+
+        if (targetRouteInfo == null) {
             mManager.onRouteRequestError("No sink", nativeRequestId);
             return;
         }
 
+        MediaSink sink = MediaSink.fromRoute(targetRouteInfo);
+
         MediaSource source = getSourceFromId(sourceId);
         if (source == null) {
             mManager.onRouteRequestError("Unsupported source URL", nativeRequestId);
             return;
         }
 
+        // When the user clicks a route on the MediaRouteChooserDialog, we intercept the click event
+        // and do not select the route. Instead the route selection is postponed to here. This will
+        // make sure the MediaRouteControllerDialog show up properly.
+        targetRouteInfo.select();
+
         ChromeCastSessionManager.CastSessionLaunchRequest request = createSessionLaunchRequest(
                 source, sink, presentationId, origin, tabId, isIncognito, nativeRequestId);
 

chrome/android/junit/src/org/chromium/chrome/browser/media/router/caf/BaseSessionControllerTest.java

@@ -13,7 +13,6 @@ import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyInt;
 import static org.mockito.Mockito.anyString;
 import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -115,51 +114,14 @@ public class BaseSessionControllerTest {
         assertNull(mController.getRouteCreationInfo());
     }
 
-    /**
-     * Test the case of requesting a session launch while the requested route is not selected. The
-     * controller should set the receiver app ID and then select the route so CAF will start the
-     * session once observed the route selection.
-     */
     @Test
-    public void testRequestSessionLaunch_routeNotSelected() {
+    public void testRequestSessionLaunch() {
         mController.requestSessionLaunch();
         verify(mCastContext).setReceiverApplicationId(APP_ID);
         verify(mMediaRouterHelper.getCastRoute()).select();
         assertSame(mRequestInfo, mController.getRouteCreationInfo());
     }
 
-    /**
-     * Test the case of requesting a session launch while the requested route is already selected.
-     * The controller should first unselect the route, set the application ID and then select the
-     * route again to notify CAF to start the session.
-     */
-    @Test
-    public void testRequestSessionLaunch_routeSelected() {
-        mMediaRouterHelper.selectRoute(mMediaRouterHelper.getCastRoute());
-
-        ArgumentCaptor<MediaRouter.Callback> callbackCaptor =
-                ArgumentCaptor.forClass(MediaRouter.Callback.class);
-
-        mController.requestSessionLaunch();
-        verify(mCastContext).setReceiverApplicationId(APP_ID);
-        verify(mMediaRouterHelper.getShadowImpl())
-                .addCallback(eq(mMediaRouteSelector), callbackCaptor.capture());
-        assertSame(mRequestInfo, mController.getRouteCreationInfo());
-        verify(mMediaRouterHelper.getShadowImpl()).unselect(MediaRouter.UNSELECT_REASON_UNKNOWN);
-
-        // Simulate another route has been unselected.
-        callbackCaptor.getValue().onRouteUnselected(
-                MediaRouter.getInstance(mContext), mMediaRouterHelper.getOtherCastRoute());
-
-        assertFalse(mMediaRouterHelper.getCastRoute().isSelected());
-
-        // Simulate the cast route has been unselected.
-        callbackCaptor.getValue().onRouteUnselected(
-                MediaRouter.getInstance(mContext), mMediaRouterHelper.getCastRoute());
-
-        assertTrue(mMediaRouterHelper.getCastRoute().isSelected());
-    }
-
     @Test
     public void testEndSession() {
         mController.endSession();