[Presentation API] Fix potential callback leaks in PSImpl.

content/browser/presentation/presentation_service_impl.h

Index: content/browser/presentation/presentation_service_impl.h
diff --git a/content/browser/presentation/presentation_service_impl.h b/content/browser/presentation/presentation_service_impl.h
index 104b3ae9b92b5d79fb731a844629e0e9f1f0d941..363fca433b8b4a8e1ca95d1ddda7f7d3e9f5217e 100644
--- a/content/browser/presentation/presentation_service_impl.h
+++ b/content/browser/presentation/presentation_service_impl.h
@@ -181,6 +181,14 @@ class CONTENT_EXPORT PresentationServiceImpl
   // PresentationServiceDelegate::Observer
   void OnDelegateDestroyed() override;
 
+  // Finds the callback from |pending_session_cbs_| using |request_session_id|.
+  // If it exists, invoke it with |session| and |error|, then erase it from
+  // |pending_session_cbs_|.
+  void RunAndEraseNewMojoCallback(

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

s/New/NewSession?

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

Done.

+    int request_session_id,
+    presentation::PresentationSessionInfoPtr session,
+    presentation::PresentationErrorPtr error);
+
   // Sets |default_presentation_url_| to |presentation_url| and informs the
   // delegate of new default presentation URL and ID.
   void DoSetDefaultPresentationUrl(
@@ -196,11 +204,11 @@ class CONTENT_EXPORT PresentationServiceImpl
   // invocation.
   void OnStartOrJoinSessionSucceeded(
       bool is_start_session,
-      const NewSessionMojoCallback& callback,
+      int request_session_id,
       const PresentationSessionInfo& session_info);
   void OnStartOrJoinSessionError(
       bool is_start_session,
-      const NewSessionMojoCallback& callback,
+      int request_session_id,
       const PresentationError& error);
 
   // Requests delegate to start a session.
@@ -215,6 +223,10 @@ class CONTENT_EXPORT PresentationServiceImpl
   // the first one in the queue.
   void HandleQueuedStartSessionRequests();
 
+  // Invokes |callback| with an error.
+  void InvokeNewSessionMojoCallbackWithError(
+      const NewSessionMojoCallback& callback);
+
   // Gets the ScreenAvailabilityContext for |presentation_url|, or creates one
   // if it does not exist.
   ScreenAvailabilityContext* GetOrCreateAvailabilityContext(
@@ -235,6 +247,9 @@ class CONTENT_EXPORT PresentationServiceImpl
   // it is removed from head of the queue.
   std::deque<linked_ptr<StartSessionRequest>> queued_start_session_requests_;
 
+  int next_request_session_id_;
+  base::hash_map<int, linked_ptr<NewSessionMojoCallback>> pending_session_cbs_;

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

Is it possible to just keep a set of Callback pointers or something? Then upon
running a callback you would erase it from the set and upon service shutdown,
iterate over the set and fire the remaining callbacks with errors?

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

I think that will be quite complicated to implement. If we bind
NewSessionMojoCallback* instead of request ids to OnStartOrJoinSession*() we
would have to somehow make sure the pointer is still valid by the time they are
invoked.

+
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<PresentationServiceImpl> weak_factory_;