ChannelMojo: Handle when ChannelMojo outlives ChannelMojoHost

ipc/mojo/ipc_channel_mojo_host.h

Index: ipc/mojo/ipc_channel_mojo_host.h
diff --git a/ipc/mojo/ipc_channel_mojo_host.h b/ipc/mojo/ipc_channel_mojo_host.h
index e0d94beddc119903cac771a1e04bbf08f00f943f..378a71bff75a01145254aba3c325fb9f1db8bc8f 100644
--- a/ipc/mojo/ipc_channel_mojo_host.h
+++ b/ipc/mojo/ipc_channel_mojo_host.h
@@ -22,28 +22,46 @@ class ChannelMojo;
 // its client provides, including the child process's process handle. Every
 // server process that uses ChannelMojo must have a ChannelMojoHost
 // instance and call OnClientLaunched().
+//
+// Thread safety:
+//   ChannelMojoHost is accessed from both UI thread (from its clients)
+//   and IO thread (from hosting channels).
+//   The public API including OnClientLaunched is available for UI thread,
+//   some API which touches |channel_| is safe only in IO thread.

[viettrungluu, 2014/09/25 19:38:34]

You should explicitly say which things are I/O-thread-only.

+//   As its weak ptr is held by the hosting channel, clients should delete
+//   ChannelMojoHost in IO thread using ChannelMojoHostDeleter.
+//
 class IPC_MOJO_EXPORT ChannelMojoHost {
  public:
   explicit ChannelMojoHost(scoped_refptr<base::TaskRunner> task_runner);

[viettrungluu, 2014/09/25 19:38:34]

You should document what task runner this is (or maybe rename "task_runner" to
"io_task_runner" or something like that).

[Hajime Morrita, 2014/09/25 22:00:04]

Done.

   ~ChannelMojoHost();
 
   void OnClientLaunched(base::ProcessHandle process);
+  scoped_refptr<base::TaskRunner> task_runner() const { return task_runner_; }

[viettrungluu, 2014/09/25 19:38:34]

Does this need to be public? Why?

[Hajime Morrita, 2014/09/25 22:00:03]

Removed. Garbage made by try-n-error :-(

 
  private:
   friend class ChannelMojo;
+  friend class ChannelMojoHostDeleter;
 
-  void OnChannelCreated(ChannelMojo* channel);
-  void OnChannelDestroyed();
-
-  void InvokeOnClientLaunched(base::ProcessHandle process);
+  base::WeakPtr<ChannelMojoHost> ToWeakPtr();
+  void OnChannelCreated(base::WeakPtr<ChannelMojo> channel);
+  void DeleteThisSoon();
 
   base::WeakPtrFactory<ChannelMojoHost> weak_factory_;
   scoped_refptr<base::TaskRunner> task_runner_;

[viettrungluu, 2014/09/25 19:38:34]

Can this be made const?

[Hajime Morrita, 2014/09/25 22:00:03]

Done.

-  ChannelMojo* channel_;
+  base::WeakPtr<ChannelMojo> channel_;

[viettrungluu, 2014/09/25 19:38:34]

You should say that this should only be used on the I/O thread.

 
   DISALLOW_COPY_AND_ASSIGN(ChannelMojoHost);
 };
 
+// Specialized to post the deletion to the ChannelMojoHost::task_runner()
+// thread.
+// This is needed as IPC::ChannelMojo touches its host when it dies.
+class IPC_MOJO_EXPORT ChannelMojoHostDeleter {
+ public:
+  void operator()(IPC::ChannelMojoHost* ptr) const;
+};
+
 }  // namespace IPC
 
 #endif  // IPC_IPC_CHANNEL_MOJO_HOST_H_