[Presentation API / Media Router] Clean up default pres URL logic.

chrome/browser/ui/webui/media_router/media_router_ui.cc

Index: chrome/browser/ui/webui/media_router/media_router_ui.cc
diff --git a/chrome/browser/ui/webui/media_router/media_router_ui.cc b/chrome/browser/ui/webui/media_router/media_router_ui.cc
index 30b76029fc9654d52c1987a02cea80fc4e7c5bc4..47c9f58a62a0d2953e2ce542dd8f9b77a69e38da 100644
--- a/chrome/browser/ui/webui/media_router/media_router_ui.cc
+++ b/chrome/browser/ui/webui/media_router/media_router_ui.cc
@@ -163,11 +163,12 @@ MediaRouterUI::~MediaRouterUI() {
   if (query_result_manager_.get())
     query_result_manager_->RemoveObserver(this);
   if (presentation_service_delegate_.get())
-    presentation_service_delegate_->RemoveDefaultMediaSourceObserver(this);
-  // If |presentation_request_| still exists, then it means presentation route
+    presentation_service_delegate_->RemoveDefaultPresentationRequestObserver(
+        this);
+  // If |create_session_request_| still exists, then it means presentation route
   // request was never attempted.
-  if (presentation_request_) {
-    presentation_request_->InvokeErrorCallback(content::PresentationError(
+  if (create_session_request_) {
+    create_session_request_->InvokeErrorCallback(content::PresentationError(
         content::PRESENTATION_ERROR_SESSION_REQUEST_CANCELLED,
         "Dialog closed."));
   }
@@ -180,30 +181,29 @@ void MediaRouterUI::InitWithDefaultMediaSource(
   DCHECK(!query_result_manager_.get());
 
   presentation_service_delegate_ = delegate;
-  presentation_service_delegate_->AddDefaultMediaSourceObserver(this);
-  InitCommon(presentation_service_delegate_->web_contents(),
-             presentation_service_delegate_->default_source(),
-             presentation_service_delegate_->default_frame_url());
+  presentation_service_delegate_->AddDefaultPresentationRequestObserver(this);
+  InitCommon(presentation_service_delegate_->web_contents());
+  OnDefaultPresentationChanged(
+      presentation_service_delegate_->GetDefaultPresentationRequest());
 }
 
 void MediaRouterUI::InitWithPresentationSessionRequest(
     content::WebContents* initiator,
     const base::WeakPtr<PresentationServiceDelegateImpl>& delegate,
-    scoped_ptr<CreatePresentationSessionRequest> presentation_request) {
+    scoped_ptr<CreatePresentationSessionRequest> create_session_request) {
   DCHECK(initiator);
-  DCHECK(presentation_request);
-  DCHECK(!presentation_request_);
+  DCHECK(create_session_request);
+  DCHECK(!create_session_request_);
   DCHECK(!query_result_manager_);
 
-  presentation_request_ = presentation_request.Pass();
+  create_session_request_ = create_session_request.Pass();
   presentation_service_delegate_ = delegate;
-  InitCommon(initiator, presentation_request_->media_source(),
-             presentation_request_->frame_url());
+  InitCommon(initiator);
+  OnDefaultPresentationChanged(
+      &create_session_request_->presentation_request());
 }
 
-void MediaRouterUI::InitCommon(content::WebContents* initiator,
-                               const MediaSource& default_source,
-                               const GURL& default_frame_url) {
+void MediaRouterUI::InitCommon(content::WebContents* initiator) {
   DCHECK(initiator);
   DCHECK(router_);
 
@@ -223,26 +223,28 @@ void MediaRouterUI::InitCommon(content::WebContents* initiator,
       MediaSourceForTab(SessionTabHelper::IdForTab(initiator)));
   query_result_manager_->StartSinksQuery(
       MediaCastMode::TAB_MIRROR, mirroring_source);
-
-  OnDefaultMediaSourceChanged(default_source, default_frame_url);
 }
 
-void MediaRouterUI::OnDefaultMediaSourceChanged(const MediaSource& source,
-                                                const GURL& frame_url) {
-  if (source.Empty()) {
+void MediaRouterUI::OnDefaultPresentationChanged(
+    const PresentationRequest* presentation_request) {
+  if (!presentation_request) {
+    presentation_request_.reset();
     query_result_manager_->StopSinksQuery(MediaCastMode::DEFAULT);
   } else {
+    presentation_request_.reset(new PresentationRequest(*presentation_request));
+    MediaSource source(MediaSourceForPresentationUrl(
+        presentation_request_->presentation_url()));
     query_result_manager_->StartSinksQuery(MediaCastMode::DEFAULT, source);
   }
-  UpdateSourceHostAndCastModes(frame_url);
-}
 
-void MediaRouterUI::UpdateSourceHostAndCastModes(const GURL& frame_url) {
-  DCHECK(query_result_manager_);
-  frame_url_ = frame_url;
+  // Gets updated cast modes from |query_result_manager_| and forwards it to UI.
   query_result_manager_->GetSupportedCastModes(&cast_modes_);
-  if (ui_initialized_)
-    handler_->UpdateCastModes(cast_modes_, GetHostFromURL(frame_url_));
+  if (ui_initialized_) {
+    handler_->UpdateCastModes(
+        cast_modes_, presentation_request_
+                         ? GetHostFromURL(presentation_request_->frame_url())

[mark a. foltz, 2015/10/20 20:15:25]

Is frame_url a GURL?  Can you say presentation_request_->frame_url()->host()?

[imcheng, 2015/10/24 00:41:19]

There's some additional truncating logic in GetHostFromURL().

+                         : std::string());
+  }
 }
 
 void MediaRouterUI::Close() {
@@ -268,11 +270,11 @@ bool MediaRouterUI::CreateRoute(const MediaSink::Id& sink_id,
   DCHECK(initiator_);
 
   // Note that there is a rarely-encountered bug, where the MediaCastMode to
-  // MediaSource mapping could have been updated, between when the user
-  // clicked on the UI to start a create route request,
-  // and when this function is called.
-  // However, since the user does not have visibility into the MediaSource, and
-  // that it occurs very rarely in practice, we leave it as-is for now.
+  // MediaSource mapping could have been updated, between when the user clicked
+  // on the UI to start a create route request, and when this function is
+  // called. However, since the user does not have visibility into the
+  // MediaSource, and that it occurs very rarely in practice, we leave it as-is
+  // for now.
   MediaSource source = query_result_manager_->GetSourceForCastMode(cast_mode);
   if (source.Empty()) {
     LOG(ERROR) << "No corresponding MediaSource for cast mode " << cast_mode;
@@ -280,11 +282,17 @@ bool MediaRouterUI::CreateRoute(const MediaSink::Id& sink_id,
   }
 
   requesting_route_for_default_source_ = cast_mode == MediaCastMode::DEFAULT;
+  DCHECK_IMPLIES(requesting_route_for_default_source_, presentation_request_);

[mark a. foltz, 2015/10/20 20:15:25]

Remove DCHECK since you're handling this below.

[imcheng, 2015/10/24 00:41:19]

Below is how I would handle it in release builds. For debug build it would be
better to fail early IMO.

+  if (requesting_route_for_default_source_ && !presentation_request_) {
+    DLOG(ERROR) << "Requested to create a route for presentation, but "
+                << "presentation request is missing.";
+    return false;
+  }
+
   current_route_request_id_ = ++route_request_counter_;
   GURL origin;
-  // TODO(imcheng): What is the origin if not creating route in DEFAULT mode?
   if (requesting_route_for_default_source_) {
-    origin = frame_url_.GetOrigin();
+    origin = presentation_request_->frame_url().GetOrigin();
   } else {
     // Requesting route for mirroring. Use a placeholder URL as origin.
     origin = GURL(chrome::kChromeUIMediaRouterURL);
@@ -310,16 +318,16 @@ bool MediaRouterUI::CreateRoute(const MediaSink::Id& sink_id,
                  weak_factory_.GetWeakPtr(), current_route_request_id_,
                  sink_id));
   if (requesting_route_for_default_source_) {
-    if (presentation_request_) {
-      // |presentation_request_| will be nullptr after this call, as the
+    if (create_session_request_) {
+      // |create_session_request_| will be nullptr after this call, as the
       // object will be transferred to the callback.
       route_response_callbacks.push_back(
           base::Bind(&CreatePresentationSessionRequest::HandleRouteResponse,
-                     base::Passed(&presentation_request_)));
+                     base::Passed(&create_session_request_)));
     } else if (presentation_service_delegate_) {
       route_response_callbacks.push_back(
           base::Bind(&PresentationServiceDelegateImpl::OnRouteResponse,
-                     presentation_service_delegate_));
+                     presentation_service_delegate_, *presentation_request_));
     }
   }