Make ChannelMojoHost::ChannelDelegate a RefContedThreadSafe

ChannelDelegate::GetWeakPtr() was called from the UI thread but
it was racy as the weak ptr is also used in the IO thread.
This CL turns ChannelDelegate a ThreadSafeRefCounted so that
we can pass ChannelDelegate itself to the task runner, instead
of using its weak ptr on the UI thread.

This change also turns some TaskRunner declarations to
SequencedTaskRunner to access its DeleteSoon() API from
ChannelDelegate.

TBR=creis@chromium.org
R=viettrungluu@chrormium.org, agl@chromium.org
BUG=460243

Review URL: https://codereview.chromium.org/955813002

Cr-Commit-Position: refs/heads/master@{#318994}

6 files changed, 35 insertions, 33 deletions

diff --git a/build/sanitizers/tsan_suppressions.cc b/build/sanitizers/tsan_suppressions.cc
index d1e2223..4659a3f 100644
--- a/build/sanitizers/tsan_suppressions.cc
+++ b/build/sanitizers/tsan_suppressions.cc
@@ -325,9 +325,6 @@ char kTSanDefaultSuppressions[] =
 // https://crbug.com/459429
 "race:randomnessPid\n"
 
-// https://crbug.com/460243
-"race:IPC::ChannelMojoHost::OnClientLaunched\n"
-
 // https://crbug.com/454655
 "race:content::BrowserTestBase::PostTaskToInProcessRendererAndWait\n"
 
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index 16aa989..ca08bfc 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -665,7 +665,7 @@ scoped_ptr<IPC::ChannelProxy> RenderProcessHostImpl::CreateChannelProxy(
       BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO);
   if (ShouldUseMojoChannel()) {
     VLOG(1) << "Mojo Channel is enabled on host";
-    scoped_refptr<base::TaskRunner> io_task_runner =
+    scoped_refptr<base::SequencedTaskRunner> io_task_runner =
         BrowserThread::UnsafeGetMessageLoopForThread(BrowserThread::IO)
               ->task_runner();
     if (!channel_mojo_host_) {
diff --git a/ipc/ipc_test_base.cc b/ipc/ipc_test_base.cc
index 745d0b4..e8fa6a4 100644
--- a/ipc/ipc_test_base.cc
+++ b/ipc/ipc_test_base.cc
@@ -153,7 +153,7 @@ IPC::ChannelHandle IPCTestBase::GetTestChannelHandle() {
   return GetChannelName(test_client_name_);
 }
 
-scoped_refptr<base::TaskRunner> IPCTestBase::task_runner() {
+scoped_refptr<base::SequencedTaskRunner> IPCTestBase::task_runner() {
   return message_loop_->message_loop_proxy();
 }
 
diff --git a/ipc/ipc_test_base.h b/ipc/ipc_test_base.h
index 5ed7dd5..b3c6bf9 100644
--- a/ipc/ipc_test_base.h
+++ b/ipc/ipc_test_base.h
@@ -101,7 +101,7 @@ class IPCTestBase : public base::MultiProcessTest {
   IPC::ChannelProxy* channel_proxy() { return channel_proxy_.get(); }
 
   const base::Process& client_process() const { return client_process_; }
-  scoped_refptr<base::TaskRunner> task_runner();
+  scoped_refptr<base::SequencedTaskRunner> task_runner();
 
   virtual scoped_ptr<IPC::ChannelFactory> CreateChannelFactory(
       const IPC::ChannelHandle& handle, base::TaskRunner* runner);
diff --git a/ipc/mojo/ipc_channel_mojo_host.cc b/ipc/mojo/ipc_channel_mojo_host.cc
index beb18ee..7e1bff9 100644
--- a/ipc/mojo/ipc_channel_mojo_host.cc
+++ b/ipc/mojo/ipc_channel_mojo_host.cc
@@ -10,14 +10,22 @@
 
 namespace IPC {
 
+class ChannelMojoHost::ChannelDelegateTraits {
+ public:
+  static void Destruct(const ChannelMojoHost::ChannelDelegate* ptr);
+};
+
 // The delete class lives on the IO thread to talk to ChannelMojo on
 // behalf of ChannelMojoHost.
 //
 // The object must be touched only on the IO thread.
-class ChannelMojoHost::ChannelDelegate : public ChannelMojo::Delegate {
+class ChannelMojoHost::ChannelDelegate
+    : public base::RefCountedThreadSafe<ChannelMojoHost::ChannelDelegate,
+                                        ChannelMojoHost::ChannelDelegateTraits>,
+      public ChannelMojo::Delegate {
  public:
-  explicit ChannelDelegate(scoped_refptr<base::TaskRunner> io_task_runner);
-  ~ChannelDelegate() override;
+  explicit ChannelDelegate(
+      scoped_refptr<base::SequencedTaskRunner> io_task_runner);
 
   // ChannelMojo::Delegate
   base::WeakPtr<Delegate> ToWeakPtr() override;
@@ -27,10 +35,14 @@ class ChannelMojoHost::ChannelDelegate : public ChannelMojo::Delegate {
   // Returns an weak ptr of ChannelDelegate instead of Delegate
   base::WeakPtr<ChannelDelegate> GetWeakPtr();
   void OnClientLaunched(base::ProcessHandle process);
-  void DeleteThisSoon();
+  void DeleteThisSoon() const;
 
  private:
-  scoped_refptr<base::TaskRunner> io_task_runner_;
+  friend class base::DeleteHelper<ChannelDelegate>;
+
+  ~ChannelDelegate() override;
+
+  scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
   base::WeakPtr<ChannelMojo> channel_;
   base::WeakPtrFactory<ChannelDelegate> weak_factory_;
 
@@ -38,7 +50,7 @@ class ChannelMojoHost::ChannelDelegate : public ChannelMojo::Delegate {
 };
 
 ChannelMojoHost::ChannelDelegate::ChannelDelegate(
-    scoped_refptr<base::TaskRunner> io_task_runner)
+    scoped_refptr<base::SequencedTaskRunner> io_task_runner)
     : io_task_runner_(io_task_runner), weak_factory_(this) {
 }
 
@@ -72,18 +84,16 @@ void ChannelMojoHost::ChannelDelegate::OnClientLaunched(
     channel_->OnClientLaunched(process);
 }
 
-void ChannelMojoHost::ChannelDelegate::DeleteThisSoon() {
-  io_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&base::DeletePointer<ChannelMojoHost::ChannelDelegate>,
-                 base::Unretained(this)));
+void ChannelMojoHost::ChannelDelegate::DeleteThisSoon() const {
+  io_task_runner_->DeleteSoon(FROM_HERE, this);
 }
 
 //
 // ChannelMojoHost
 //
 
-ChannelMojoHost::ChannelMojoHost(scoped_refptr<base::TaskRunner> io_task_runner)
+ChannelMojoHost::ChannelMojoHost(
+    scoped_refptr<base::SequencedTaskRunner> io_task_runner)
     : io_task_runner_(io_task_runner),
       channel_delegate_(new ChannelDelegate(io_task_runner)),
       weak_factory_(this) {
@@ -98,8 +108,7 @@ void ChannelMojoHost::OnClientLaunched(base::ProcessHandle process) {
   } else {
     io_task_runner_->PostTask(FROM_HERE,
                               base::Bind(&ChannelDelegate::OnClientLaunched,
-                                         channel_delegate_->GetWeakPtr(),
-                                         process));
+                                         channel_delegate_, process));
   }
 }
 
@@ -107,8 +116,9 @@ ChannelMojo::Delegate* ChannelMojoHost::channel_delegate() const {
   return channel_delegate_.get();
 }
 
-void ChannelMojoHost::DelegateDeleter::operator()(
-    ChannelMojoHost::ChannelDelegate* ptr) const {
+// static
+void ChannelMojoHost::ChannelDelegateTraits::Destruct(
+    const ChannelMojoHost::ChannelDelegate* ptr) {
   ptr->DeleteThisSoon();
 }
 
diff --git a/ipc/mojo/ipc_channel_mojo_host.h b/ipc/mojo/ipc_channel_mojo_host.h
index 8289515..db60b12 100644
--- a/ipc/mojo/ipc_channel_mojo_host.h
+++ b/ipc/mojo/ipc_channel_mojo_host.h
@@ -12,7 +12,7 @@
 #include "ipc/mojo/ipc_channel_mojo.h"
 
 namespace base {
-class TaskRunner;
+class SequencedTaskRunner;
 }
 
 namespace IPC {
@@ -23,7 +23,8 @@ namespace IPC {
 // instance and call OnClientLaunched().
 class IPC_MOJO_EXPORT ChannelMojoHost {
  public:
-  explicit ChannelMojoHost(scoped_refptr<base::TaskRunner> io_task_runner);
+  explicit ChannelMojoHost(
+      scoped_refptr<base::SequencedTaskRunner> io_task_runner);
   ~ChannelMojoHost();
 
   void OnClientLaunched(base::ProcessHandle process);
@@ -31,16 +32,10 @@ class IPC_MOJO_EXPORT ChannelMojoHost {
 
  private:
   class ChannelDelegate;
+  class ChannelDelegateTraits;
 
-  // Delegate talks to ChannelMojo, whch lives in IO thread, thus
-  // the Delegate should also live and dies in the IO thread as well.
-  class DelegateDeleter {
-   public:
-    void operator()(ChannelDelegate* ptr) const;
-  };
-
-  const scoped_refptr<base::TaskRunner> io_task_runner_;
-  scoped_ptr<ChannelDelegate, DelegateDeleter> channel_delegate_;
+  const scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
+  scoped_refptr<ChannelDelegate> channel_delegate_;
   base::WeakPtrFactory<ChannelMojoHost> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ChannelMojoHost);