[MediaRouter] Renames CloseRoute() to Terminate() and creates DetachRoute()

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 "base/containers/scoped_ptr_hash_map.h"
 #include "base/containers/small_map.h"
@@ skipping 65 matching lines @@
   std::string GetDefaultPresentationId() const;
   void ListenForConnectionStateChange(
       const content::PresentationSessionInfo& connection,
       const content::PresentationConnectionStateChangedCallback&
           state_changed_cb);
   void ListenForSessionMessages(
       const content::PresentationSessionInfo& session,
       const content::PresentationSessionMessageCallback& message_cb);
 
   void Reset();
+  void RemoveConnection(const std::string& presentation_id,
+                        const MediaRoute::Id& route_id);
 
   const MediaRoute::Id GetRouteId(const std::string& presentation_id) const;
   const std::vector<MediaRoute::Id> GetRouteIds() const;
 
   void OnPresentationSessionStarted(
       const content::PresentationSessionInfo& session,
       const MediaRoute::Id& route_id);
   void OnPresentationServiceDelegateDestroyed() const;
 
   void set_delegate_observer(DelegateObserver* observer) {
@@ skipping 81 matching lines @@
   return false;
 }
 
 bool PresentationFrame::HasScreenAvailabilityListenerForTest(
     const MediaSource::Id& source_id) const {
   return sinks_observer_ && sinks_observer_->source().id() == source_id;
 }
 
 void PresentationFrame::Reset() {
   for (const auto& pid_route_id : presentation_id_to_route_id_)
-    router_->OnPresentationSessionDetached(pid_route_id.second);
+    router_->DetachRoute(pid_route_id.second);
 
   presentation_id_to_route_id_.clear();
   sinks_observer_.reset();
   connection_state_subscriptions_.clear();
   session_messages_observers_.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);
+  // We no longer need to observe route messages.

[imcheng, 2015/12/10 19:50:46]

nit: new line above comment

[mark a. foltz, 2015/12/10 23:46:49]

Done

+  auto observer_iter = std::find_if(
+      session_messages_observers_.begin(), session_messages_observers_.end(),
+      [&route_id](const PresentationSessionMessagesObserver* observer) {
+        return route_id == observer->route_id();
+      });
+  if (observer_iter != session_messages_observers_.end())
+    session_messages_observers_.erase(observer_iter);
+  // We keep the PresentationConnectionStateChangedCallback registered with MR

[imcheng, 2015/12/10 19:50:46]

nit: new line above comment

[mark a. foltz, 2015/12/10 23:46:49]

Done

+  // so the MRP can tell us when terminate() completed.
+  router_->DetachRoute(route_id);

[imcheng, 2015/12/10 19:50:46]

Looks like you are already calling DetachRoute in PSDImpl::CloseConnection(), so
no need to call it again here?

[mark a. foltz, 2015/12/10 23:46:49]

You're right, good catch.  

+}
+
 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()) {
     DLOG(ERROR) << __FUNCTION__ << "route id not found for presentation: "
                 << connection.presentation_id;
     return;
   }
@@ skipping 68 matching lines @@
   void AddDelegateObserver(const RenderFrameHostId& render_frame_host_id,
                            DelegateObserver* observer);
   void RemoveDelegateObserver(const RenderFrameHostId& render_frame_host_id);
   void AddDefaultPresentationRequestObserver(
       PresentationServiceDelegateImpl::DefaultPresentationRequestObserver*
           observer);
   void RemoveDefaultPresentationRequestObserver(
       PresentationServiceDelegateImpl::DefaultPresentationRequestObserver*
           observer);
   void Reset(const RenderFrameHostId& render_frame_host_id);
+  void RemoveConnection(const RenderFrameHostId& render_frame_host_id,
+                        const MediaRoute::Id& route_id,
+                        const std::string& presentation_id);
   bool HasScreenAvailabilityListenerForTest(
       const RenderFrameHostId& render_frame_host_id,
       const MediaSource::Id& source_id) const;
   void SetMediaRouterForTest(MediaRouter* router);
 
   void OnPresentationSessionStarted(
       const RenderFrameHostId& render_frame_host_id,
       const content::PresentationSessionInfo& session,
       const MediaRoute::Id& route_id);
   void OnDefaultPresentationSessionStarted(
@@ skipping 205 matching lines @@
   if (presentation_frame)
     presentation_frame->Reset();
 
   if (default_presentation_request_ &&
       render_frame_host_id ==
           default_presentation_request_->render_frame_host_id()) {
     ClearDefaultPresentationRequest();
   }
 }
 
+void PresentationFrameManager::RemoveConnection(
+    const RenderFrameHostId& render_frame_host_id,
+    const MediaRoute::Id& route_id,
+    const std::string& presentation_id) {
+  auto presentation_frame = presentation_frames_.get(render_frame_host_id);
+  if (presentation_frame)

[mlamouri (slow - plz ping), 2015/12/10 15:39:51]

Is `presentation_frame == nullptr` even possible? Should that be a DCHECK
instead?

[mark a. foltz, 2015/12/10 23:46:49]

My current understanding is a frame can call connection.close() or .terminate()
multiple times on the same connection, until the connection state change update
comes from content/ and Blink changes the connection state to disconnected or
terminated.

There's a TODO elsewhere to have Blink make the state change for
connection.close(), and have the Mojo call be fire and forget to content/ to
notify the MRP.   For terminate() I think we want the event to be fired when the
actual side effect takes place (so we'll wait for the callback from the MRP). 
At least, that's my thinking based on a conversation I had with imcheng@
yesterday.

+    presentation_frame->RemoveConnection(route_id, presentation_id);
+}
+
 PresentationFrame* PresentationFrameManager::GetOrAddPresentationFrame(
     const RenderFrameHostId& render_frame_host_id) {
   if (!presentation_frames_.contains(render_frame_host_id)) {
     presentation_frames_.add(
         render_frame_host_id,
         scoped_ptr<PresentationFrame>(
             new PresentationFrame(web_contents_, router_)));
   }
   return presentation_frames_.get(render_frame_host_id);
 }
@@ skipping 190 matching lines @@
       content::PresentationSessionInfo(presentation_url, presentation_id),
       success_cb, error_cb));
   router_->JoinRoute(
       MediaSourceForPresentationUrl(presentation_url).id(), presentation_id,
       GetLastCommittedURLForFrame(
           RenderFrameHostId(render_process_id, render_frame_id))
           .GetOrigin(),
       web_contents_, route_response_callbacks);
 }
 
-void PresentationServiceDelegateImpl::CloseSession(
+void PresentationServiceDelegateImpl::CloseConnection(
     int render_process_id,
     int render_frame_id,
     const std::string& presentation_id) {
-  const MediaRoute::Id& route_id = frame_manager_->GetRouteId(
-      RenderFrameHostId(render_process_id, render_frame_id), presentation_id);
+  const RenderFrameHostId rfh_id(render_process_id, render_frame_id);
+  const MediaRoute::Id& route_id =
+      frame_manager_->GetRouteId(rfh_id, presentation_id);
   if (route_id.empty()) {
     DVLOG(1) << "No active route for: " << presentation_id;
     return;
   }
 
-  // TODO(mfoltz, mlamouri): implement CloseSession().
-  // This could call router_->OnPresentationSessionDetached(route_id).
-  // PresentationFrame::Reset() should probably call CloseSession() too.
-  // Rename CloseRoute() to something else to avoid confusion?
-  NOTIMPLEMENTED();
+  router_->DetachRoute(route_id);
+  frame_manager_->RemoveConnection(rfh_id, presentation_id, route_id);
+  // TODO(mfoltz): close() should always succeed so there is no need to keep the
+  // state_changed_cb around - remove it and fire the ChangeEvent on the
+  // PresentationConnection in Blink.
 }
 
-void PresentationServiceDelegateImpl::TerminateSession(
+void PresentationServiceDelegateImpl::Terminate(
     int render_process_id,
     int render_frame_id,
     const std::string& presentation_id) {
-  const MediaRoute::Id& route_id = frame_manager_->GetRouteId(
-      RenderFrameHostId(render_process_id, render_frame_id), presentation_id);
+  const RenderFrameHostId rfh_id(render_process_id, render_frame_id);
+  const MediaRoute::Id& route_id =
+      frame_manager_->GetRouteId(rfh_id, presentation_id);
   if (route_id.empty()) {
     DVLOG(1) << "No active route for: " << presentation_id;
     return;
   }
-  router_->CloseRoute(route_id);
+  router_->TerminateRoute(route_id);
+  frame_manager_->RemoveConnection(rfh_id, presentation_id, route_id);

[imcheng, 2015/12/10 19:50:46]

So we are calling TerminateRoute, and then DetachRoute in
PresentationFrame::RemoveConnection()? Do we only need to call TerminateRoute?

[mark a. foltz, 2015/12/10 23:46:49]

Yeah removed call to router_->DetachRoute() from RemoveConnection.

 }
 
 void PresentationServiceDelegateImpl::ListenForSessionMessages(
     int render_process_id,
     int render_frame_id,
     const content::PresentationSessionInfo& session,
     const content::PresentationSessionMessageCallback& message_cb) {
   frame_manager_->ListenForSessionMessages(
       RenderFrameHostId(render_process_id, render_frame_id), session,
       message_cb);
@@ skipping 81 matching lines @@
 bool PresentationServiceDelegateImpl::HasScreenAvailabilityListenerForTest(
     int render_process_id,
     int render_frame_id,
     const MediaSource::Id& source_id) const {
   RenderFrameHostId render_frame_host_id(render_process_id, render_frame_id);
   return frame_manager_->HasScreenAvailabilityListenerForTest(
       render_frame_host_id, source_id);
 }
 
 }  // namespace media_router