[Presentation API] Fix potential callback leaks in PSImpl.

content/browser/presentation/presentation_service_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 "content/browser/presentation/presentation_service_impl.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "content/browser/presentation/presentation_type_converters.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/frame_navigate_params.h"
 
 namespace content {
 
 PresentationServiceImpl::PresentationServiceImpl(
     RenderFrameHost* render_frame_host,
     WebContents* web_contents,
     PresentationServiceDelegate* delegate)
     : WebContentsObserver(web_contents),
       render_frame_host_(render_frame_host),
       delegate_(delegate),
+      next_request_session_id_(0),
       weak_factory_(this) {
   DCHECK(render_frame_host_);
   DCHECK(web_contents);
   DVLOG(2) << "PresentationServiceImpl: "
            << render_frame_host_->GetProcess()->GetID() << ", "
            << render_frame_host_->GetRoutingID();
   if (delegate_)
     delegate_->AddObserver(this);
 }
 
@@ skipping 88 matching lines @@
     const DefaultSessionMojoCallback& callback) {
   NOTIMPLEMENTED();
 }
 
 void PresentationServiceImpl::StartSession(
     const mojo::String& presentation_url,
     const mojo::String& presentation_id,
     const NewSessionMojoCallback& callback) {
   DVLOG(2) << "StartSession";
   if (!delegate_) {
-    callback.Run(
-        presentation::PresentationSessionInfoPtr(),
-        presentation::PresentationError::From(
-            PresentationError(PRESENTATION_ERROR_UNKNOWN, "")));
+    InvokeNewSessionMojoCallbackWithError(callback);
     return;
   }
 
   queued_start_session_requests_.push_back(make_linked_ptr(
       new StartSessionRequest(presentation_url, presentation_id, callback)));
   if (queued_start_session_requests_.size() == 1)
     DoStartSession(presentation_url, presentation_id, callback);
 }
 
 void PresentationServiceImpl::JoinSession(
     const mojo::String& presentation_url,
     const mojo::String& presentation_id,
     const NewSessionMojoCallback& callback) {
   DVLOG(2) << "JoinSession";
   if (!delegate_) {
-    callback.Run(
-        presentation::PresentationSessionInfoPtr(),
-        presentation::PresentationError::From(
-            PresentationError(PRESENTATION_ERROR_UNKNOWN, "")));
+    InvokeNewSessionMojoCallbackWithError(callback);
     return;
   }
 
+  ++next_request_session_id_;
+  pending_session_cbs_[next_request_session_id_].reset(
+      new NewSessionMojoCallback(callback));
   delegate_->JoinSession(
       render_frame_host_->GetProcess()->GetID(),
       render_frame_host_->GetRoutingID(),
       presentation_url,
       presentation_id,
-      // TODO(imcheng): These callbacks may be dropped. http://crbug.com/468575
       base::Bind(&PresentationServiceImpl::OnStartOrJoinSessionSucceeded,
-                 weak_factory_.GetWeakPtr(), false, callback),
+                 weak_factory_.GetWeakPtr(), false, next_request_session_id_),
       base::Bind(&PresentationServiceImpl::OnStartOrJoinSessionError,
-                 weak_factory_.GetWeakPtr(), false, callback));
+                 weak_factory_.GetWeakPtr(), false, next_request_session_id_));
 }
 
 void PresentationServiceImpl::HandleQueuedStartSessionRequests() {
   DCHECK(!queued_start_session_requests_.empty());
   queued_start_session_requests_.pop_front();
   if (!queued_start_session_requests_.empty()) {
     const linked_ptr<StartSessionRequest>& request =
         queued_start_session_requests_.front();
     DoStartSession(request->presentation_url,
                    request->presentation_id,
                    request->callback);
   }
 }
 
 void PresentationServiceImpl::DoStartSession(
     const std::string& presentation_url,
     const std::string& presentation_id,
     const NewSessionMojoCallback& callback) {
+  ++next_request_session_id_;

[whywhat, 2015/03/25 17:05:54]

I'd encapsulate this logic into something like:

int request_session_id = RegisterNewSessionCallback(callback);

since it's repeated in two places here.

[imcheng, 2015/03/25 23:03:42]

Done.

+  pending_session_cbs_[next_request_session_id_].reset(
+      new NewSessionMojoCallback(callback));
   delegate_->StartSession(
       render_frame_host_->GetProcess()->GetID(),
       render_frame_host_->GetRoutingID(),
       presentation_url,
       presentation_id,
-      // TODO(imcheng): These callbacks may be dropped. http://crbug.com/468575
       base::Bind(&PresentationServiceImpl::OnStartOrJoinSessionSucceeded,
-                 weak_factory_.GetWeakPtr(), true, callback),
+                 weak_factory_.GetWeakPtr(), true, next_request_session_id_),
       base::Bind(&PresentationServiceImpl::OnStartOrJoinSessionError,
-                 weak_factory_.GetWeakPtr(), true, callback));
+                 weak_factory_.GetWeakPtr(), true, next_request_session_id_));
 }
 
 void PresentationServiceImpl::OnStartOrJoinSessionSucceeded(
     bool is_start_session,
-    const NewSessionMojoCallback& callback,
+    int request_session_id,
     const PresentationSessionInfo& session_info) {
-  callback.Run(
+  RunAndEraseNewMojoCallback(
+      request_session_id,
       presentation::PresentationSessionInfo::From(session_info),
       presentation::PresentationErrorPtr());
   if (is_start_session)
     HandleQueuedStartSessionRequests();
 }
 
 void PresentationServiceImpl::OnStartOrJoinSessionError(
     bool is_start_session,
-    const NewSessionMojoCallback& callback,
+    int request_session_id,
     const PresentationError& error) {
-  callback.Run(
+  RunAndEraseNewMojoCallback(
+      request_session_id,
       presentation::PresentationSessionInfoPtr(),
       presentation::PresentationError::From(error));
   if (is_start_session)
     HandleQueuedStartSessionRequests();
 }
 
+void PresentationServiceImpl::RunAndEraseNewMojoCallback(
+    int request_session_id,
+    presentation::PresentationSessionInfoPtr session,
+    presentation::PresentationErrorPtr error) {
+  auto it = pending_session_cbs_.find(request_session_id);
+  if (it == pending_session_cbs_.end())
+    return;
+
+  DCHECK(it->second.get());
+  it->second->Run(session.Pass(), error.Pass());
+  pending_session_cbs_.erase(it);
+}
+    
 void PresentationServiceImpl::DoSetDefaultPresentationUrl(
     const std::string& default_presentation_url,
     const std::string& default_presentation_id) {
   DCHECK(delegate_);
   delegate_->SetDefaultPresentationUrl(
       render_frame_host_->GetProcess()->GetID(),
       render_frame_host_->GetRoutingID(),
       default_presentation_url,
       default_presentation_id);
   default_presentation_url_ = default_presentation_url;
@@ skipping 86 matching lines @@
 void PresentationServiceImpl::Reset() {
   DVLOG(2) << "PresentationServiceImpl::Reset";
   if (delegate_) {
     delegate_->Reset(
         render_frame_host_->GetProcess()->GetID(),
         render_frame_host_->GetRoutingID());
   }
 
   default_presentation_url_.clear();
   default_presentation_id_.clear();
-  for (const auto& context : availability_contexts_) {
-    context.second->OnScreenAvailabilityChanged(false);
+  for (const auto& context_entry : availability_contexts_) {
+    context_entry.second->OnScreenAvailabilityChanged(false);
   }
   availability_contexts_.clear();
-  // TODO(imcheng): This may drop callbacks. See http://crbug.com/468575.
+  for (auto& request_ptr : queued_start_session_requests_) {
+    InvokeNewSessionMojoCallbackWithError(request_ptr->callback);
+  }
   queued_start_session_requests_.clear();
+  for (auto& pending_entry : pending_session_cbs_) {
+    InvokeNewSessionMojoCallbackWithError(*pending_entry.second);
+  }
+  pending_session_cbs_.clear();
+}
+
+void PresentationServiceImpl::InvokeNewSessionMojoCallbackWithError(
+    const NewSessionMojoCallback& callback) {
+  callback.Run(
+        presentation::PresentationSessionInfoPtr(),
+        presentation::PresentationError::From(
+            PresentationError(PRESENTATION_ERROR_UNKNOWN, "Internal error")));
 }
 
 void PresentationServiceImpl::OnDelegateDestroyed() {
   DVLOG(2) << "PresentationServiceImpl::OnDelegateDestroyed";
   delegate_ = nullptr;
   Reset();
 }
 
 PresentationServiceImpl::ScreenAvailabilityContext::ScreenAvailabilityContext(
     const std::string& presentation_url)
@@ skipping 61 matching lines @@
     : presentation_url(presentation_url),
       presentation_id(presentation_id),
       callback(callback) {
 }
 
 PresentationServiceImpl::StartSessionRequest::~StartSessionRequest() {
 }
 
 }  // namespace content