[Presentation API] Fix potential callback leaks in PSImpl.

content/browser/presentation/presentation_service_impl.cc

Index: content/browser/presentation/presentation_service_impl.cc
diff --git a/content/browser/presentation/presentation_service_impl.cc b/content/browser/presentation/presentation_service_impl.cc
index 2d2a52c5dd309772f8dedb6951f286b3336c35aa..f0f2e0a2238db679a84e9fcd0a6f9584c24c7945 100644
--- a/content/browser/presentation/presentation_service_impl.cc
+++ b/content/browser/presentation/presentation_service_impl.cc
@@ -25,6 +25,7 @@ PresentationServiceImpl::PresentationServiceImpl(
     : 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);
@@ -133,10 +134,7 @@ void PresentationServiceImpl::StartSession(
     const NewSessionMojoCallback& callback) {
   DVLOG(2) << "StartSession";
   if (!delegate_) {
-    callback.Run(
-        presentation::PresentationSessionInfoPtr(),
-        presentation::PresentationError::From(
-            PresentationError(PRESENTATION_ERROR_UNKNOWN, "")));
+    InvokeNewSessionMojoCallbackWithError(callback);
     return;
   }
 
@@ -152,23 +150,22 @@ void PresentationServiceImpl::JoinSession(
     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() {
@@ -187,23 +184,26 @@ 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)
@@ -212,15 +212,29 @@ void PresentationServiceImpl::OnStartOrJoinSessionSucceeded(
 
 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) {
@@ -327,12 +341,26 @@ void PresentationServiceImpl::Reset() {
 
   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() {