[Presentation API] Fix potential callback leaks in PSImpl.

content/browser/presentation/presentation_service_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_PRESENTATION_PRESENTATION_SERVICE_IMPL_H_
 #define CONTENT_BROWSER_PRESENTATION_PRESENTATION_SERVICE_IMPL_H_
 
 #include <deque>
 
 #include "base/basictypes.h"
@@ skipping 163 matching lines @@
   // WebContentsObserver override.
   void DidNavigateAnyFrame(
       content::RenderFrameHost* render_frame_host,
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) override;
   void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
 
   // 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(
       const std::string& presentation_url,
       const std::string& presentation_id);
 
   // Removes all listeners and resets default presentation URL on this instance
   // and informs the PresentationServiceDelegate of such.
   void Reset();
 
   // These two functions are bound as base::Callbacks and passed to
   // embedder's implementation of PresentationServiceDelegate for later
   // 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.
   void DoStartSession(
       const std::string& presentation_url,
       const std::string& presentation_id,
       const NewSessionMojoCallback& callback);
 
   // Removes the head of the queue (which represents the request that has just
   // been processed).
   // Checks if there are any queued StartSession requests and if so, executes
   // 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(
       const std::string& presentation_url);
 
   RenderFrameHost* render_frame_host_;
   PresentationServiceDelegate* delegate_;
 
   // Map from presentation URL to its ScreenAvailabilityContext state machine.
   base::hash_map<std::string, linked_ptr<ScreenAvailabilityContext>>
       availability_contexts_;
 
   std::string default_presentation_url_;
   std::string default_presentation_id_;
 
   // We only allow one StartSession request to be processed at a time.
   // StartSession requests are queued here. When a request has been processed,
   // 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_;
 
   DISALLOW_COPY_AND_ASSIGN(PresentationServiceImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_PRESENTATION_PRESENTATION_SERVICE_IMPL_H_