[Media Router] Erase PresentationFrame entry when corresponding RenderFrameHost is gone

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..e88756588e64db324dcdb7f48d61b56d7e612006 100644
--- a/chrome/browser/media/router/presentation_service_delegate_impl.cc
+++ b/chrome/browser/media/router/presentation_service_delegate_impl.cc
@@ -160,10 +160,6 @@ class PresentationFrame {
       const MediaRoute& route);
   void OnPresentationServiceDelegateDestroyed() const;
 
-  void set_delegate_observer(DelegateObserver* observer) {
-    delegate_observer_ = observer;
-  }
-
  private:
   MediaSource GetMediaSourceFromListener(
       content::PresentationScreenAvailabilityListener* listener) const;
@@ -186,8 +182,6 @@ class PresentationFrame {
   // References to the owning WebContents, and the corresponding MediaRouter.
   content::WebContents* web_contents_;
   MediaRouter* router_;
-
-  DelegateObserver* delegate_observer_;
 };
 
 PresentationFrame::PresentationFrame(
@@ -397,6 +391,8 @@ class PresentationFrameManager {
       const PresentationRequest& request,
       const content::PresentationSessionInfo& session,
       const MediaRoute& route);
+  void AddPresentationFrame(const RenderFrameHostId& render_frame_host_id);
+  void RemovePresentationFrame(const RenderFrameHostId& render_frame_host_id);
 
   const MediaRoute::Id GetRouteId(const RenderFrameHostId& render_frame_host_id,
                                   const std::string& presentation_id) const;
@@ -479,6 +475,18 @@ void PresentationFrameManager::OnDefaultPresentationSessionStarted(
   }
 }
 
+void PresentationFrameManager::AddPresentationFrame(
+    const RenderFrameHostId& render_frame_host_id) {
+  GetOrAddPresentationFrame(render_frame_host_id);
+}
+
+void PresentationFrameManager::RemovePresentationFrame(
+    const RenderFrameHostId& render_frame_host_id) {
+  const auto it = presentation_frames_.find(render_frame_host_id);

[imcheng, 2017/01/27 20:00:01]

This can just be presentation_frames_.erase(render_frame_host_id);

[zhaobin, 2017/01/27 23:42:50]

Code removed.

+  if (it != presentation_frames_.end())
+    presentation_frames_.erase(it);
+}
+
 const MediaRoute::Id PresentationFrameManager::GetRouteId(
     const RenderFrameHostId& render_frame_host_id,
     const std::string& presentation_id) const {
@@ -667,11 +675,15 @@ void PresentationServiceDelegateImpl::AddObserver(int render_process_id,
                                                   DelegateObserver* observer) {
   DCHECK(observer);
   observers_.AddObserver(render_process_id, render_frame_id, observer);
+  frame_manager_->AddPresentationFrame(

[imcheng, 2017/01/27 20:00:01]

It seems to me we don't need to explicitly add a PresentationFrame because it
gets created on demand.

[zhaobin, 2017/01/27 23:42:50]

Done.

+      RenderFrameHostId(render_process_id, render_frame_id));
 }
 
 void PresentationServiceDelegateImpl::RemoveObserver(int render_process_id,
                                                      int render_frame_id) {
   observers_.RemoveObserver(render_process_id, render_frame_id);
+  frame_manager_->RemovePresentationFrame(

[imcheng, 2017/01/27 20:00:01]

I think it would make more sense to erase the PresentationFrame on Reset(). It
would change the lifetime of PresentationFrame slightly but it would align it
with the lifetime of a document, which is more intuitive. WDYT?

[zhaobin, 2017/01/27 23:42:50]

Done.

+      RenderFrameHostId(render_process_id, render_frame_id));
 }
 
 bool PresentationServiceDelegateImpl::AddScreenAvailabilityListener(