Unit test for CafMediaRouteProvider

This CL adds unit test for CafMediaRouteProvider. Also to clean
up, the provider reads the sink directly from the session
controller when onSessionStarted().

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

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

@@ -205,4 +205,12 @@ public class BaseSessionController {
     public FlingingController getFlingingController() {
         return null;
     }
+
+    /**
+     *  Helper message to get the session ID of the attached session. For stubbing in tests as
+     * {@link CastSession#getSessionId()} is final.
+     */
+    public String getSessionId() {
+        return isConnected() ? getSession().getSessionId() : null;
+    }
 }

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

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.media.router.caf;
 import android.os.Handler;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
+import android.support.annotation.VisibleForTesting;
 import android.support.v7.media.MediaRouteSelector;
 import android.support.v7.media.MediaRouter;
 import android.support.v7.media.MediaRouter.RouteInfo;
@@ -43,7 +44,8 @@ public abstract class CafBaseMediaRouteProvider
     protected final MediaRouteManager mManager;
     protected final Map<String, DiscoveryCallback> mDiscoveryCallbacks =
             new HashMap<String, DiscoveryCallback>();
-    protected final Map<String, MediaRoute> mRoutes = new HashMap<String, MediaRoute>();
+    @VisibleForTesting
+    final Map<String, MediaRoute> mRoutes = new HashMap<String, MediaRoute>();
     protected Handler mHandler = new Handler();
 
     private CreateRouteRequestInfo mPendingCreateRouteRequestInfo;
@@ -199,6 +201,8 @@ public abstract class CafBaseMediaRouteProvider
             return;
         }
 
+        // Don't remove the route while the session is still active. All the routes will be removed
+        // upon session end.
         sessionController().endSession();
     }
 

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

@@ -8,6 +8,7 @@ import static org.chromium.chrome.browser.media.router.caf.CastUtils.isSameOrigi
 
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
+import android.support.annotation.VisibleForTesting;
 import android.support.v7.media.MediaRouter;
 
 import com.google.android.gms.cast.framework.CastSession;
@@ -36,12 +37,14 @@ public class CafMediaRouteProvider extends CafBaseMediaRouteProvider {
 
     private CreateRouteRequestInfo mPendingCreateRouteRequestInfo;
 
-    private ClientRecord mLastRemovedRouteRecord;
+    @VisibleForTesting
+    ClientRecord mLastRemovedRouteRecord;
     // The records for clients, which must match mRoutes. This is used for the saving last record
     // for autojoin.
     private final Map<String, ClientRecord> mClientIdToRecords =
             new HashMap<String, ClientRecord>();
-    private CafMessageHandler mMessageHandler;
+    @VisibleForTesting
+    CafMessageHandler mMessageHandler;
     // The session controller which is always attached to the current CastSession.
     private final CastSessionController mSessionController;
 
@@ -75,7 +78,6 @@ public class CafMediaRouteProvider extends CafBaseMediaRouteProvider {
         MediaRoute route =
                 new MediaRoute(sessionController().getSink().getId(), sourceId, presentationId);
         addRoute(route, origin, tabId, nativeRequestId, /* wasLaunched= */ false);
-        mManager.onRouteCreated(route.id, route.sinkId, nativeRequestId, this, false);
     }
 
     // TODO(zqzhang): the clientRecord/route management is not clean and the logic seems to be
@@ -90,8 +92,7 @@ public class CafMediaRouteProvider extends CafBaseMediaRouteProvider {
 
         ClientRecord client = getClientRecordByRouteId(routeId);
         if (client != null) {
-            MediaSink sink = MediaSink.fromSinkId(
-                    sessionController().getSink().getId(), getAndroidMediaRouter());
+            MediaSink sink = sessionController().getSink();
             if (sink != null) {
                 mMessageHandler.sendReceiverActionToClient(routeId, sink, client.clientId, "stop");
             }
@@ -153,10 +154,8 @@ public class CafMediaRouteProvider extends CafBaseMediaRouteProvider {
 
         for (ClientRecord clientRecord : mClientIdToRecords.values()) {
             // Should be exactly one instance of MediaRoute/ClientRecord at this moment.
-            MediaRoute route = mRoutes.get(clientRecord.routeId);
-            MediaSink sink = MediaSink.fromSinkId(route.sinkId, getAndroidMediaRouter());
-            mMessageHandler.sendReceiverActionToClient(
-                    clientRecord.routeId, sink, clientRecord.clientId, "cast");
+            mMessageHandler.sendReceiverActionToClient(clientRecord.routeId,
+                    sessionController().getSink(), clientRecord.clientId, "cast");
         }
 
         mMessageHandler.onSessionStarted();
@@ -200,14 +199,15 @@ public class CafMediaRouteProvider extends CafBaseMediaRouteProvider {
         mMessageHandler = new CafMessageHandler(this, mSessionController);
     }
 
-    private boolean canJoinExistingSession(
+    @VisibleForTesting
+    boolean canJoinExistingSession(
             String presentationId, String origin, int tabId, CastMediaSource source) {
         if (AUTO_JOIN_PRESENTATION_ID.equals(presentationId)) {
             return canAutoJoin(source, origin, tabId);
         }
         if (presentationId.startsWith(PRESENTATION_ID_SESSION_ID_PREFIX)) {
             String sessionId = presentationId.substring(PRESENTATION_ID_SESSION_ID_PREFIX.length());
-            return sessionController().getSession().getSessionId().equals(sessionId);
+            return sessionId != null && sessionId.equals(sessionController().getSessionId());
         }
         for (MediaRoute route : mRoutes.values()) {
             if (route.presentationId.equals(presentationId)) return true;

chrome/android/java_sources.gni

@@ -2378,9 +2378,11 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/media/router/caf/MediaRouterTestHelper.java",
   "junit/src/org/chromium/chrome/browser/media/router/caf/ShadowMediaRouter.java",
   "junit/src/org/chromium/chrome/browser/media/router/caf/ShadowCastContext.java",
+  "junit/src/org/chromium/chrome/browser/media/router/caf/ShadowCastMediaSource.java",
   "junit/src/org/chromium/chrome/browser/media/router/caf/BaseSessionControllerTest.java",
   "junit/src/org/chromium/chrome/browser/media/router/caf/CastSessionControllerTest.java",
   "junit/src/org/chromium/chrome/browser/media/router/caf/CafBaseMediaRouteProviderTest.java",
+  "junit/src/org/chromium/chrome/browser/media/router/caf/CafMediaRouteProviderTest.java",
   "junit/src/org/chromium/chrome/browser/media/ui/MediaImageManagerTest.java",
   "junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationActionsUpdatedTest.java",
   "junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationButtonComputationTest.java",

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

@@ -50,7 +50,6 @@ import org.chromium.chrome.browser.media.router.MediaSource;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 
 /**
  * Robolectric tests for CafBaseMediaRouteProvider.
@@ -394,9 +393,9 @@ public class CafBaseMediaRouteProviderTest {
 
         inOrder.verify(mSessionController).attachToCastSession(mCastSession);
         inOrder.verify(mSessionController).onSessionStarted();
-        assertEquals(mProvider.getRoutes().size(), 1);
+        assertEquals(mProvider.mRoutes.size(), 1);
 
-        MediaRoute route = (MediaRoute) (mProvider.getRoutes().values().toArray()[0]);
+        MediaRoute route = (MediaRoute) (mProvider.mRoutes.values().toArray()[0]);
         assertEquals(route.sinkId, "cast-route");
         assertEquals(route.sourceId, "source-id");
         assertEquals(route.presentationId, "presentation-id");
@@ -586,10 +585,10 @@ public class CafBaseMediaRouteProviderTest {
     }
 
     private void assertContainsRoutes(MediaRoute... routes) {
-        assertEquals(mProvider.getRoutes().size(), routes.length);
+        assertEquals(mProvider.mRoutes.size(), routes.length);
 
         for (MediaRoute route : routes) {
-            assertEquals(mProvider.getRoutes().get(route.id), route);
+            assertEquals(mProvider.mRoutes.get(route.id), route);
         }
     }
 
@@ -614,9 +613,5 @@ public class CafBaseMediaRouteProviderTest {
         @Override
         public void joinRoute(String routeId, String presentationId, String origin, int tabId,
                 int nativeRequestId) {}
-
-        public Map<String, MediaRoute> getRoutes() {
-            return mRoutes;
-        }
     }
 }

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

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.media.router.caf;
+
+import org.robolectric.annotation.Implementation;
+import org.robolectric.annotation.Implements;
+
+import org.chromium.chrome.browser.media.router.cast.CastMediaSource;
+
+/** Shadow implementation for {@link MediaRouter}. */
+@Implements(CastMediaSource.class)
+public class ShadowCastMediaSource {
+    private static ShadowImplementation sImpl;
+
+    @Implementation
+    public static CastMediaSource from(String sourceId) {
+        return sImpl.from(sourceId);
+    }
+
+    public static void setImplementation(ShadowImplementation impl) {
+        sImpl = impl;
+    }
+
+    /** The implementation skeleton for the implementation backing the shadow. */
+    public static class ShadowImplementation {
+        public CastMediaSource from(String sourceId) {
+            return null;
+        }
+    }
+}