Revert "[Android MR] Postpone route selection until session launch"

This reverts commit 7c453583.

Reason for revert: This introduces some regression in some cases. The old logic works just fine so it is safe to revert.

Original change's description:
> [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/1351893
> Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
> Commit-Queue: Zhiqiang Zhang <zqzhang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#611437}

TBR=zqzhang@chromium.org,tguilbert@chromium.org

Change-Id: I30322daecf8c866fdda9e846d0cb37d9a319f033
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 711860
Reviewed-on: https://chromium-review.googlesource.com/c/1352973Reviewed-by: Zhiqiang Zhang <zqzhang@chromium.org>
Commit-Queue: Zhiqiang Zhang <zqzhang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611572}

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

@@ -13,11 +13,6 @@ 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}.
@@ -39,7 +34,7 @@ public class MediaRouteChooserDialogManager extends BaseMediaRouteDialogManager
         private final Handler mHandler = new Handler();
         private final SystemVisibilitySaver mVisibilitySaver = new SystemVisibilitySaver();
         private BaseMediaRouteDialogManager mManager;
-        private boolean mIsSinkSelected;
+        private boolean mCancelled;
 
         public Fragment() {
             mHandler.post(new Runnable() {
@@ -57,7 +52,8 @@ public class MediaRouteChooserDialogManager extends BaseMediaRouteDialogManager
         @Override
         public MediaRouteChooserDialog onCreateChooserDialog(
                 Context context, Bundle savedInstanceState) {
-            MediaRouteChooserDialog dialog = new DelayedSelectionDialog(context, getTheme());
+            MediaRouteChooserDialog dialog =
+                    super.onCreateChooserDialog(context, savedInstanceState);
             dialog.setCanceledOnTouchOutside(true);
             return dialog;
         }
@@ -75,45 +71,26 @@ public class MediaRouteChooserDialogManager extends BaseMediaRouteDialogManager
         }
 
         @Override
-        public void onDismiss(DialogInterface dialog) {
-            super.onDismiss(dialog);
-
-            if (!mIsSinkSelected) mManager.delegate().onDialogCancelled();
-        }
+        public void onCancel(DialogInterface dialog) {
+            mCancelled = true;
 
-        private class DelayedSelectionDialog extends MediaRouteChooserDialog {
-            public DelayedSelectionDialog(Context context) {
-                super(context);
-            }
+            mManager.delegate().onDialogCancelled();
 
-            public DelayedSelectionDialog(Context context, int theme) {
-                super(context, theme);
+            super.onCancel(dialog);
         }
 
         @Override
-            public void onCreate(Bundle savedInstanceState) {
-                super.onCreate(savedInstanceState);
+        public void onDismiss(DialogInterface dialog) {
+            super.onDismiss(dialog);
+            if (mManager == null) return;
 
-                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.
+            MediaSink newSink =
+                    MediaSink.fromRoute(mManager.androidMediaRouter().getSelectedRoute());
             mManager.delegate().onSinkSelected(mManager.sourceId(), newSink);
-                mIsSinkSelected = true;
-
-                dismiss();
-            }
         }
     }
 

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

@@ -6,6 +6,7 @@ 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;
@@ -30,6 +31,7 @@ public class BaseSessionController {
 
     private CastSession mCastSession;
     private final CafBaseMediaRouteProvider mProvider;
+    private final MediaRouter.Callback mMediaRouterCallbackForSessionLaunch;
     private CreateRouteRequestInfo mRouteCreationInfo;
     @VisibleForTesting
     CafNotificationController mNotificationController;
@@ -37,6 +39,7 @@ public class BaseSessionController {
 
     public BaseSessionController(CafBaseMediaRouteProvider provider) {
         mProvider = provider;
+        mMediaRouterCallbackForSessionLaunch = new MediaRouterCallbackForSessionLaunch();
         mNotificationController = new CafNotificationController(this);
         mRemoteMediaClientCallback = new RemoteMediaClientCallback();
     }
@@ -46,11 +49,22 @@ public class BaseSessionController {
         CastUtils.getCastContext().setReceiverApplicationId(
                 mRouteCreationInfo.source.getApplicationId());
 
-        // 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.
+        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();
         }
+    }
 
     public MediaSource getSource() {
         return (mRouteCreationInfo != null) ? mRouteCreationInfo.source : null;
@@ -159,6 +173,20 @@ 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,32 +173,18 @@ public abstract class BaseMediaRouteProvider
             return;
         }
 
-        MediaRouter.RouteInfo targetRouteInfo = null;
-        for (MediaRouter.RouteInfo routeInfo : mAndroidMediaRouter.getRoutes()) {
-            if (routeInfo.getId().equals(sinkId)) {
-                targetRouteInfo = routeInfo;
-                break;
-            }
-        }
-
-        if (targetRouteInfo == null) {
+        MediaSink sink = MediaSink.fromSinkId(sinkId, mAndroidMediaRouter);
+        if (sink == 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,6 +13,7 @@ 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;
@@ -114,14 +115,51 @@ 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() {
+    public void testRequestSessionLaunch_routeNotSelected() {
         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();