[Media Router] Handle multiple Presentation URLs when creating routes

chrome/browser/media/router/presentation_service_delegate_impl.cc

Index: chrome/browser/media/router/presentation_service_delegate_impl.cc
diff --git a/chrome/browser/media/router/presentation_service_delegate_impl.cc b/chrome/browser/media/router/presentation_service_delegate_impl.cc
index 634c219ee31e4e8449aa5a9826d65686f46c80f2..1f89b76d4ca7dc653a79a80a0094cc1f1041146f 100644
--- a/chrome/browser/media/router/presentation_service_delegate_impl.cc
+++ b/chrome/browser/media/router/presentation_service_delegate_impl.cc
@@ -359,9 +359,9 @@ class PresentationFrameManager {
   // Sets or clears the default presentation request and callback for the given
   // frame. Also sets / clears the default presentation requests for the owning
   // tab WebContents.
-  void SetDefaultPresentationUrl(
+  void SetDefaultPresentationUrls(
       const RenderFrameHostId& render_frame_host_id,
-      const GURL& default_presentation_url,
+      const std::vector<GURL>& default_presentation_urls,
       const content::PresentationSessionStartedCallback& callback);
   void AddDelegateObserver(const RenderFrameHostId& render_frame_host_id,
                            DelegateObserver* observer);
@@ -538,20 +538,19 @@ void PresentationFrameManager::ListenForSessionMessages(
   it->second->ListenForSessionMessages(session, message_cb);
 }
 
-void PresentationFrameManager::SetDefaultPresentationUrl(
+void PresentationFrameManager::SetDefaultPresentationUrls(
     const RenderFrameHostId& render_frame_host_id,
-    const GURL& default_presentation_url,
+    const std::vector<GURL>& default_presentation_urls,
     const content::PresentationSessionStartedCallback& callback) {
   if (!IsMainFrame(render_frame_host_id))
     return;
 
-  if (default_presentation_url.is_empty()) {
+  if (default_presentation_urls.empty()) {
     ClearDefaultPresentationRequest();
   } else {
     DCHECK(!callback.is_null());
     GURL frame_url(GetLastCommittedURLForFrame(render_frame_host_id));
-    PresentationRequest request(render_frame_host_id,
-                                std::vector<GURL>({default_presentation_url}),
+    PresentationRequest request(render_frame_host_id, default_presentation_urls,
                                 frame_url);
     default_presentation_started_callback_ = callback;
     SetDefaultPresentationRequest(request);
@@ -718,14 +717,8 @@ void PresentationServiceDelegateImpl::SetDefaultPresentationUrls(
     const std::vector<GURL>& default_presentation_urls,
     const content::PresentationSessionStartedCallback& callback) {
   RenderFrameHostId render_frame_host_id(render_process_id, render_frame_id);
-  if (default_presentation_urls.empty()) {
-    frame_manager_->SetDefaultPresentationUrl(render_frame_host_id, GURL(),
-                                              callback);
-  } else {
-    // TODO(crbug.com/627655): Handle multiple URLs.
-    frame_manager_->SetDefaultPresentationUrl(
-        render_frame_host_id, default_presentation_urls[0], callback);
-  }
+  frame_manager_->SetDefaultPresentationUrls(
+      render_frame_host_id, default_presentation_urls, callback);
 }
 
 void PresentationServiceDelegateImpl::OnJoinRouteResponse(
@@ -782,10 +775,9 @@ void PresentationServiceDelegateImpl::StartSession(
     return;
   }
 
-  // TODO(crbug.com/627655): Handle multiple URLs.
-  const GURL& presentation_url = presentation_urls[0];
-  if (presentation_url.is_empty() ||
-      !IsValidPresentationUrl(presentation_url)) {
+  if (presentation_urls.empty() ||
+      std::find_if_not(presentation_urls.begin(), presentation_urls.end(),
+                       IsValidPresentationUrl) != presentation_urls.end()) {
     error_cb.Run(content::PresentationError(content::PRESENTATION_ERROR_UNKNOWN,

[mark a. foltz, 2016/12/02 20:59:08]

A couple of things:

- Ideally the validation in the PresentationRequest constructor in Blink should
match up as closely as we can with any re-validation we do on the browser side,
so the developer can't construct a PresentationRequest that is guaranteed to
fail.

- It would be better to define a specific content:: error code for this case so
we can reject the Promise to start() with a NotFoundError; that seems like the
best thing to do.

I think it would be better to make any changes separately.  Filed
crbug.com/670848, can you add a TODO() here?

[zhaobin, 2016/12/02 22:50:57]

Done.

                                             "Invalid presentation arguments."));

[mark a. foltz, 2016/12/02 20:59:08]

Nit: "Invalid presentation URL."

[zhaobin, 2016/12/02 22:50:57]

Done.

     return;
@@ -794,7 +786,7 @@ void PresentationServiceDelegateImpl::StartSession(
   RenderFrameHostId render_frame_host_id(render_process_id, render_frame_id);
   std::unique_ptr<CreatePresentationConnectionRequest> request(
       new CreatePresentationConnectionRequest(
-          render_frame_host_id, presentation_url,
+          render_frame_host_id, presentation_urls,
           GetLastCommittedURLForFrame(render_frame_host_id),
           base::Bind(&PresentationServiceDelegateImpl::OnStartSessionSucceeded,
                      weak_factory_.GetWeakPtr(), render_process_id,
@@ -817,22 +809,34 @@ void PresentationServiceDelegateImpl::JoinSession(
     const std::string& presentation_id,
     const content::PresentationSessionStartedCallback& success_cb,
     const content::PresentationSessionErrorCallback& error_cb) {
+  DVLOG(2) << "PresentationServiceDelegateImpl::JoinSession";
   if (presentation_urls.empty()) {
     error_cb.Run(content::PresentationError(
         content::PRESENTATION_ERROR_NO_PRESENTATION_FOUND,
         "Invalid presentation arguments."));
+    return;
+  }
+
+  // TODO(zhaobin): This may break if we have multiple cast specific urls.

[mark a. foltz, 2016/12/02 20:59:09]

Can you elaborate on this?  I would say:

To fix this, we need the Media Router to keep track of which presentation URL
was used to start the presentation named by |presentation_id|.

[zhaobin, 2016/12/02 22:50:57]

Done.

+  auto presentation_url = std::find_if(
+      presentation_urls.begin(), presentation_urls.end(), [](const GURL& url) {
+        return CanConnectToMediaSource(MediaSource(url));
+      });
+  if (presentation_url == presentation_urls.end()) {
+    error_cb.Run(
+        content::PresentationError(content::PRESENTATION_ERROR_UNKNOWN,
+                                   "Presentation urls do support reconnect."));

[mark a. foltz, 2016/12/02 20:59:09]

This should probably be content::PRESENTATION_ERROR_NO_PRESENTATION_FOUND

[zhaobin, 2016/12/02 22:50:57]

Done.

+    return;
   }
 
-  // TODO(crbug.com/627655): Handle multiple URLs.
-  const GURL& presentation_url = presentation_urls[0];
   bool incognito = web_contents_->GetBrowserContext()->IsOffTheRecord();
   std::vector<MediaRouteResponseCallback> route_response_callbacks;
   route_response_callbacks.push_back(
       base::Bind(&PresentationServiceDelegateImpl::OnJoinRouteResponse,
                  weak_factory_.GetWeakPtr(), render_process_id, render_frame_id,
-                 presentation_url, presentation_id, success_cb, error_cb));
+                 *presentation_url, presentation_id, success_cb, error_cb));
   router_->JoinRoute(
-      MediaSourceForPresentationUrl(presentation_url).id(), presentation_id,
+      MediaSourceForPresentationUrl(*presentation_url).id(), presentation_id,
       GetLastCommittedURLForFrame(
           RenderFrameHostId(render_process_id, render_frame_id))
           .GetOrigin(),
@@ -920,11 +924,14 @@ void PresentationServiceDelegateImpl::ListenForConnectionStateChange(
 void PresentationServiceDelegateImpl::OnRouteResponse(
     const PresentationRequest& presentation_request,
     const RouteRequestResult& result) {
-  if (!result.route())
+  if (!result.route() ||
+      base::STLCount(presentation_request.presentation_urls(),
+                     result.presentation_url()) == 0) {
     return;
+  }
 
-  content::PresentationSessionInfo session_info(
-      presentation_request.presentation_url(), result.presentation_id());
+  content::PresentationSessionInfo session_info(result.presentation_url(),
+                                                result.presentation_id());
   frame_manager_->OnDefaultPresentationSessionStarted(
       presentation_request, session_info, *result.route());
 }