[Presentation API] (5th) (1-UA) integrate controller and receiver side for 1-UA messaging

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 59e441a9ffb9a75dd0ddd9d2672d7bee415f6a40..ba30d6e543ce48df3655645106d38ba290465f66 100644
--- a/chrome/browser/media/router/presentation_service_delegate_impl.cc
+++ b/chrome/browser/media/router/presentation_service_delegate_impl.cc
@@ -13,6 +13,7 @@
 #include "base/guid.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
+#include "chrome/browser/media/router/browser_presentation_connection_proxy.h"
 #include "chrome/browser/media/router/create_presentation_connection_request.h"
 #include "chrome/browser/media/router/media_route.h"
 #include "chrome/browser/media/router/media_router.h"
@@ -164,11 +165,16 @@ class PresentationFrame {
     delegate_observer_ = observer;
   }
 
+  bool IsOffscreenPresentation(const std::string& presentation_id);

[imcheng, 2017/01/31 01:53:25]

Can this be const?

[zhaobin, 2017/01/31 18:44:16]

Done.

+  void OnBrowserConnectionAvailable(
+      const content::PresentationSessionInfo& session,
+      content::PresentationConnectionPtr source_connection,
+      content::PresentationConnectionRequest target_connection_request);
+
  private:
   MediaSource GetMediaSourceFromListener(
       content::PresentationScreenAvailabilityListener* listener) const;
-  base::SmallMap<std::map<std::string, MediaRoute::Id>>
-      presentation_id_to_route_id_;
+  base::SmallMap<std::map<std::string, MediaRoute>> presentation_id_to_route_;
   base::SmallMap<
       std::map<std::string, std::unique_ptr<PresentationMediaSinksObserver>>>
       url_to_sinks_observer_;
@@ -180,6 +186,9 @@ class PresentationFrame {
       MediaRoute::Id,
       std::unique_ptr<PresentationSessionMessagesObserver>>
       session_messages_observers_;
+  std::unordered_map<MediaRoute::Id,
+                     std::unique_ptr<BrowserPresentationConnectionProxy>>
+      browser_connection_proxies_;
 
   RenderFrameHostId render_frame_host_id_;
 
@@ -207,20 +216,21 @@ PresentationFrame::~PresentationFrame() {
 void PresentationFrame::OnPresentationSessionStarted(
     const content::PresentationSessionInfo& session,
     const MediaRoute& route) {
-  presentation_id_to_route_id_[session.presentation_id] =
-      route.media_route_id();
+  presentation_id_to_route_.insert(
+      std::make_pair(session.presentation_id, route));
 }
 
 const MediaRoute::Id PresentationFrame::GetRouteId(
     const std::string& presentation_id) const {
-  auto it = presentation_id_to_route_id_.find(presentation_id);
-  return it != presentation_id_to_route_id_.end() ? it->second : "";
+  auto it = presentation_id_to_route_.find(presentation_id);
+  return it != presentation_id_to_route_.end() ? it->second.media_route_id()
+                                               : "";
 }
 
 const std::vector<MediaRoute::Id> PresentationFrame::GetRouteIds() const {
   std::vector<MediaRoute::Id> route_ids;
-  for (const auto& e : presentation_id_to_route_id_)
-    route_ids.push_back(e.second);
+  for (const auto& e : presentation_id_to_route_)
+    route_ids.push_back(e.second.media_route_id());
   return route_ids;
 }
 
@@ -262,23 +272,32 @@ bool PresentationFrame::HasScreenAvailabilityListenerForTest(
 }
 
 void PresentationFrame::Reset() {
-  for (const auto& pid_route_id : presentation_id_to_route_id_)
-    router_->DetachRoute(pid_route_id.second);
+  auto offscreen_presentation_manager =
+      OffscreenPresentationManagerFactory::GetOrCreateForWebContents(
+          web_contents_);
+
+  for (const auto& pid_route : presentation_id_to_route_) {
+    offscreen_presentation_manager->UnregisterOffscreenPresentationController(

[imcheng, 2017/01/31 01:53:25]

Should this first check if the route is an offscreen presentation?

[zhaobin, 2017/01/31 18:44:16]

Done.

+        pid_route.first, render_frame_host_id_);
+    router_->DetachRoute(pid_route.second.media_route_id());
+  }
 
-  presentation_id_to_route_id_.clear();
+  presentation_id_to_route_.clear();
   url_to_sinks_observer_.clear();
   connection_state_subscriptions_.clear();
   session_messages_observers_.clear();
+  browser_connection_proxies_.clear();
 }
 
 void PresentationFrame::RemoveConnection(const std::string& presentation_id,
                                          const MediaRoute::Id& route_id) {
   // Remove the presentation id mapping so a later call to Reset is a no-op.
-  presentation_id_to_route_id_.erase(presentation_id);
+  presentation_id_to_route_.erase(presentation_id);
 
   // We no longer need to observe route messages.
   session_messages_observers_.erase(route_id);
 
+  browser_connection_proxies_.erase(route_id);
   // We keep the PresentationConnectionStateChangedCallback registered with MR
   // so the MRP can tell us when terminate() completed.
 }
@@ -287,14 +306,14 @@ void PresentationFrame::ListenForConnectionStateChange(
     const content::PresentationSessionInfo& connection,
     const content::PresentationConnectionStateChangedCallback&
         state_changed_cb) {
-  auto it = presentation_id_to_route_id_.find(connection.presentation_id);
-  if (it == presentation_id_to_route_id_.end()) {
+  auto it = presentation_id_to_route_.find(connection.presentation_id);
+  if (it == presentation_id_to_route_.end()) {
     DLOG(ERROR) << __func__ << "route id not found for presentation: "
                 << connection.presentation_id;
     return;
   }
 
-  const MediaRoute::Id& route_id = it->second;
+  const MediaRoute::Id& route_id = it->second.media_route_id();
   if (connection_state_subscriptions_.find(route_id) !=
       connection_state_subscriptions_.end()) {
     DLOG(ERROR) << __func__
@@ -305,20 +324,26 @@ void PresentationFrame::ListenForConnectionStateChange(
 
   connection_state_subscriptions_.insert(std::make_pair(
       route_id, router_->AddPresentationConnectionStateChangedCallback(
-                    it->second, state_changed_cb)));
+                    route_id, state_changed_cb)));
 }
 
 void PresentationFrame::ListenForSessionMessages(
     const content::PresentationSessionInfo& session,
     const content::PresentationConnectionMessageCallback& message_cb) {
-  auto it = presentation_id_to_route_id_.find(session.presentation_id);
-  if (it == presentation_id_to_route_id_.end()) {
+  auto it = presentation_id_to_route_.find(session.presentation_id);
+  if (it == presentation_id_to_route_.end()) {
     DVLOG(2) << "ListenForSessionMessages: no route for "
              << session.presentation_id;
     return;
   }
 
-  const MediaRoute::Id& route_id = it->second;
+  if (it->second.is_offscreen_presentation()) {
+    DVLOG(2) << "ListenForSessionMessages: do not listen for offscreen "
+             << "presentation [id]: " << session.presentation_id;
+    return;
+  }
+
+  const MediaRoute::Id& route_id = it->second.media_route_id();
   if (session_messages_observers_.find(route_id) !=
       session_messages_observers_.end()) {
     DLOG(ERROR) << __func__
@@ -329,7 +354,7 @@ void PresentationFrame::ListenForSessionMessages(
 
   session_messages_observers_.insert(std::make_pair(
       route_id, base::MakeUnique<PresentationSessionMessagesObserver>(
-          router_, it->second, message_cb)));
+                    router_, route_id, message_cb)));
 }
 
 MediaSource PresentationFrame::GetMediaSourceFromListener(
@@ -340,6 +365,42 @@ MediaSource PresentationFrame::GetMediaSourceFromListener(
              : MediaSourceForPresentationUrl(listener->GetAvailabilityUrl());
 }
 
+bool PresentationFrame::IsOffscreenPresentation(
+    const std::string& presentation_id) {
+  if (!base::ContainsKey(presentation_id_to_route_, presentation_id)) {

[imcheng, 2017/01/31 01:53:25]

Move the find() call here to avoid double lookup.

[zhaobin, 2017/01/31 18:44:16]

Done.

+    DLOG(WARNING) << "No route for [presentation_id]: " << presentation_id;
+    return false;
+  }
+
+  const auto it = presentation_id_to_route_.find(presentation_id);
+  return it->second.is_offscreen_presentation();
+}
+
+void PresentationFrame::OnBrowserConnectionAvailable(
+    const content::PresentationSessionInfo& session,
+    content::PresentationConnectionPtr source_connection,
+    content::PresentationConnectionRequest target_connection_request) {
+  DCHECK(source_connection);
+  if (!base::ContainsKey(presentation_id_to_route_, session.presentation_id)) {
+    DLOG(WARNING) << "No route for [presentation_id]: "
+                  << session.presentation_id;
+    return;
+  }
+
+  const auto route =
+      presentation_id_to_route_.find(session.presentation_id)->second;
+  DVLOG(2)
+      << "Creating BrowserPresentationConnectionProxy for [presentation_id]: "
+      << session.presentation_id;
+  auto* proxy = new BrowserPresentationConnectionProxy(router_, &route);
+
+  proxy->Bind(std::move(target_connection_request));

[imcheng, 2017/01/31 01:53:25]

You can probably combine these two calls to proxy into an Init() method? Or even
into the constructor?

[zhaobin, 2017/01/31 18:44:15]

Done.

+  proxy->BindControllerConnection(std::move(source_connection));
+
+  browser_connection_proxies_.insert(
+      std::make_pair(route.media_route_id(), base::WrapUnique(proxy)));
+}
+
 // Used by PresentationServiceDelegateImpl to manage PresentationFrames.
 class PresentationFrameManager {
  public:
@@ -398,6 +459,14 @@ class PresentationFrameManager {
       const content::PresentationSessionInfo& session,
       const MediaRoute& route);
 
+  bool IsOffscreenPresentation(const RenderFrameHostId& render_frame_host_id,
+                               const std::string& presentation_id);

[imcheng, 2017/01/31 01:53:25]

const

[zhaobin, 2017/01/31 18:44:15]

Done.

+  void OnBrowserConnectionAvailable(
+      const RenderFrameHostId& render_frame_host_id,
+      const content::PresentationSessionInfo& session,
+      content::PresentationConnectionPtr source_connection,
+      content::PresentationConnectionRequest target_connection_request);
+
   const MediaRoute::Id GetRouteId(const RenderFrameHostId& render_frame_host_id,
                                   const std::string& presentation_id) const;
   const std::vector<MediaRoute::Id> GetRouteIds(
@@ -479,6 +548,26 @@ void PresentationFrameManager::OnDefaultPresentationSessionStarted(
   }
 }
 
+bool PresentationFrameManager::IsOffscreenPresentation(
+    const RenderFrameHostId& render_frame_host_id,
+    const std::string& presentation_id) {
+  const auto it = presentation_frames_.find(render_frame_host_id);
+  if (it != presentation_frames_.end())
+    return it->second->IsOffscreenPresentation(presentation_id);
+  return false;

[imcheng, 2017/01/31 01:53:25]

nit: insert empty line above

[zhaobin, 2017/01/31 18:44:15]

Done.

+}
+
+void PresentationFrameManager::OnBrowserConnectionAvailable(
+    const RenderFrameHostId& render_frame_host_id,
+    const content::PresentationSessionInfo& session,
+    content::PresentationConnectionPtr source_connection,
+    content::PresentationConnectionRequest target_connection_request) {
+  auto* presentation_frame = GetOrAddPresentationFrame(render_frame_host_id);
+  presentation_frame->OnBrowserConnectionAvailable(
+      session, std::move(source_connection),
+      std::move(target_connection_request));
+}
+
 const MediaRoute::Id PresentationFrameManager::GetRouteId(
     const RenderFrameHostId& render_frame_host_id,
     const std::string& presentation_id) const {
@@ -911,20 +1000,28 @@ void PresentationServiceDelegateImpl::ListenForConnectionStateChange(
       state_changed_cb);
 }
 
-void PresentationServiceDelegateImpl::ConnectToOffscreenPresentation(
+void PresentationServiceDelegateImpl::ConnectToPresentation(
     int render_process_id,
     int render_frame_id,
     const content::PresentationSessionInfo& session,
     content::PresentationConnectionPtr controller_connection_ptr,
     content::PresentationConnectionRequest receiver_connection_request) {
   RenderFrameHostId render_frame_host_id(render_process_id, render_frame_id);
-  auto* const offscreen_presentation_manager =
-      OffscreenPresentationManagerFactory::GetOrCreateForWebContents(
-          web_contents_);
-  offscreen_presentation_manager->RegisterOffscreenPresentationController(
-      session.presentation_id, session.presentation_url, render_frame_host_id,
-      std::move(controller_connection_ptr),
-      std::move(receiver_connection_request));
+
+  if (frame_manager_->IsOffscreenPresentation(render_frame_host_id,

[imcheng, 2017/01/31 01:53:25]

It seems a bit odd that we are checking for offscreen presentation at the
PSDImpl level, but then doing the same for ListenForSessionMessages at the
PresentationFrame level. Can the logic be moved to PresentationFrame to be
consistent?

With the state transition to "connected" happening in Blink, I wonder if there's
some room for simplifying the setup sequence from PresentationDispatcher to
here. It looks like we can combine ListenForConnectionMessages with
ConnectToPresentation.

======

Chatted offline after writing the message above. Bin tried to do this but it
seems we still have some timing issues with state change vs. message arriving
from extension due to them using different mojo message pipes to talk to
renderer. We can fix this by having messages from extension go through
PresentationConnectionProxy, but this will probably make sense in a separate
patch.

[zhaobin, 2017/01/31 18:44:15]

Moved it into PresentationFrame. Removing ListenForSessionMessages is tracked by
crbug.com/687011.

+                                              session.presentation_id)) {
+    auto* const offscreen_presentation_manager =
+        OffscreenPresentationManagerFactory::GetOrCreateForWebContents(
+            web_contents_);
+    offscreen_presentation_manager->RegisterOffscreenPresentationController(
+        session.presentation_id, session.presentation_url, render_frame_host_id,
+        std::move(controller_connection_ptr),
+        std::move(receiver_connection_request));
+  } else {
+    frame_manager_->OnBrowserConnectionAvailable(
+        render_frame_host_id, session, std::move(controller_connection_ptr),
+        std::move(receiver_connection_request));
+  }
 }
 
 void PresentationServiceDelegateImpl::OnRouteResponse(