Convert MediaRouter mojom apis to intake url::Origin objects instead of strings

chrome/browser/media/router/presentation_service_delegate_impl.cc

 // Copyright 2015 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.
 
 #include "chrome/browser/media/router/presentation_service_delegate_impl.h"
 
 #include <string>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ skipping 39 matching lines @@
 
 // Returns the unique identifier for the supplied RenderFrameHost.
 RenderFrameHostId GetRenderFrameHostId(RenderFrameHost* render_frame_host) {
   int render_process_id = render_frame_host->GetProcess()->GetID();
   int render_frame_id = render_frame_host->GetRoutingID();
   return RenderFrameHostId(render_process_id, render_frame_id);
 }
 
 // Gets the last committed URL for the render frame specified by
 // |render_frame_host_id|.
-GURL GetLastCommittedURLForFrame(RenderFrameHostId render_frame_host_id) {
+url::Origin GetLastCommittedURLForFrame(
+    RenderFrameHostId render_frame_host_id) {
   RenderFrameHost* render_frame_host = RenderFrameHost::FromID(
       render_frame_host_id.first, render_frame_host_id.second);
-  if (!render_frame_host)
-    return GURL();
-
-  // TODO(crbug.com/632623): Use url::Origin in place of GURL for origins
-  return render_frame_host->GetLastCommittedOrigin().GetURL();
+  return render_frame_host ? render_frame_host->GetLastCommittedOrigin()

[dcheng, 2017/01/26 04:16:32]

I was trying to figure out why |render_frame_host| can be null, but I'm a bit
confused. I guess this is needed for PresentationFrame, but I don't understand
why we'd ever need to handle a case where the RenderFrameHost is gone.

Looking at this in more detail, it seem like |presentation_frames_| is never
cleaned up:
https://cs.chromium.org/chromium/src/chrome/browser/media/router/presentation...

So maybe there's a bug here -- we need to be removing entries from the map when
RenderFrameHosts are destroyed...

[steimel, 2017/01/27 00:28:59]

Just had some offline discussion with mfoltz@ and imcheng@ regarding this. There
used to be a DCHECK here, but it was removed a while back for some tests that
were using dummy ids. Those tests are no longer around, so I've re-added the
DCHECK and now assume that render_frame_host exists. Also, regarding your
comment on removing entries, the entries are removed in RenderFrameHostImpl's
dtor:

https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_...

[imcheng, 2017/01/27 00:55:06]

I think what dcheng@ is saying is the corresponding PresentationFrame object is
never removed from the PresentationFrameManager.

The answer is that we used to erase the PresentationFrame entry during
RemoveDelegateObserver (which gets called when the RFH is about to be
destroyed). A recent refactoring (you will need to rebase) inadvertently removed
that statement. So this is a subtle memory leak.

We can fix it by removing the entry in PresentationFrameManager::Reset() (which
gets called if the frame is being destroyed or is navgiating). I will file a bug
for this.

[imcheng, 2017/01/27 01:04:36]

Filed crbug.com/685879

[dcheng, 2017/01/27 09:11:11]

So it sounds like for now, we still need the NULL check. But we can annotate
this with TODO(crbug.com/685879) to clean it up once we're correctly removing
entries from the map.

[steimel, 2017/01/27 18:55:19]

Hmm so what's getting cleaned up? Do you mean removing the DCHECK? I'm not sure
that it'd be a good idea to remove it, even after removing entries from the map

[imcheng, 2017/01/27 19:02:50]

I do not think the nullptr check is necessary. To clarify, the bug is that we
are not cleaning up PresentationFrame objects for dead RFHs. But they are never
referenced again after the RFH is dead. Also, we never invoke
PresentationServiceDelegateImpl / PresentationFrameManager / PresentationFrame
methods with a dead RFH ID. So this method should never get invoked with a dead
RFH ID.

+                           : url::Origin();
 }
 
 // Observes messages originating from the MediaSink connected to a MediaRoute
 // that represents a presentation. Converts the messages into
 // content::PresentationSessionMessages and dispatches them via the provided
 // PresentationSessionMessageCallback.
 class PresentationSessionMessagesObserver : public RouteMessageObserver {
  public:
   // |message_cb|: The callback to invoke whenever messages are received.
   // |route_id|: ID of MediaRoute to listen for messages.
@@ skipping 151 matching lines @@
 
 bool PresentationFrame::SetScreenAvailabilityListener(
     content::PresentationScreenAvailabilityListener* listener) {
   MediaSource source(GetMediaSourceFromListener(listener));
   auto& sinks_observer = url_to_sinks_observer_[source.id()];
   if (sinks_observer && sinks_observer->listener() == listener)
     return false;
 
   sinks_observer.reset(new PresentationMediaSinksObserver(
       router_, listener, source,
-      GetLastCommittedURLForFrame(render_frame_host_id_).GetOrigin()));
+      GetLastCommittedURLForFrame(render_frame_host_id_)));
 
   if (!sinks_observer->Init()) {
     url_to_sinks_observer_.erase(source.id());
     listener->OnScreenAvailabilityNotSupported();
     return false;
   }
 
   return true;
 }
 
@@ skipping 303 matching lines @@
     const RenderFrameHostId& render_frame_host_id,
     const std::vector<GURL>& default_presentation_urls,
     const content::PresentationSessionStartedCallback& callback) {
   if (!IsMainFrame(render_frame_host_id))
     return;
 
   if (default_presentation_urls.empty()) {
     ClearDefaultPresentationRequest();
   } else {
     DCHECK(!callback.is_null());
-    GURL frame_url(GetLastCommittedURLForFrame(render_frame_host_id));
+    const auto& frame_origin =
+        GetLastCommittedURLForFrame(render_frame_host_id);
     PresentationRequest request(render_frame_host_id, default_presentation_urls,
-                                frame_url);
+                                frame_origin);
     default_presentation_started_callback_ = callback;
     SetDefaultPresentationRequest(request);
   }
 }
 
 void PresentationFrameManager::AddDelegateObserver(
     const RenderFrameHostId& render_frame_host_id,
     DelegateObserver* observer) {
   auto* presentation_frame = GetOrAddPresentationFrame(render_frame_host_id);
   presentation_frame->set_delegate_observer(observer);
@@ skipping 248 matching lines @@
     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;
   }
 
-  const url::Origin& origin = url::Origin(GetLastCommittedURLForFrame(
-      RenderFrameHostId(render_process_id, render_frame_id)));
+  const url::Origin& origin = GetLastCommittedURLForFrame(
+      RenderFrameHostId(render_process_id, render_frame_id));
 
 #if !defined(OS_ANDROID)
   if (IsAutoJoinPresentationId(presentation_id) &&
       ShouldCancelAutoJoinForOrigin(origin)) {
     error_cb.Run(content::PresentationError(
         content::PRESENTATION_ERROR_SESSION_REQUEST_CANCELLED,
         "Auto-join request cancelled by user preferences."));
     return;
   }
 #endif  // !defined(OS_ANDROID)
 
   // 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));
   router_->JoinRoute(MediaSourceForPresentationUrl(presentation_url).id(),
-                     presentation_id, origin.GetURL(), web_contents_,
+                     presentation_id, origin, web_contents_,
                      route_response_callbacks, base::TimeDelta(), incognito);
 }
 
 void PresentationServiceDelegateImpl::CloseConnection(
     int render_process_id,
     int render_frame_id,
     const std::string& presentation_id) {
   const RenderFrameHostId rfh_id(render_process_id, render_frame_id);
   const MediaRoute::Id& route_id =
       frame_manager_->GetRouteId(rfh_id, presentation_id);
@@ skipping 129 matching lines @@
   const base::ListValue* origins =
       Profile::FromBrowserContext(web_contents_->GetBrowserContext())
           ->GetPrefs()
           ->GetList(prefs::kMediaRouterTabMirroringSources);
   return origins &&
          origins->Find(base::StringValue(origin.Serialize())) != origins->end();
 }
 #endif  // !defined(OS_ANDROID)
 
 }  // namespace media_router