Add IPC::ChannelProxy::Create() and IPC::SyncChannel::Create()

This change replaces constructors with Create() methods of
ChannelProxy and SyncChannel. This open the possibility to introduce
polymorphism to these classes.

This is a revision of r274310 (https://codereview.chromium.org/301973003/)
in which I added bunch of Create*() method variants.
The chagne was reverted. This change no longer does it and just keeps
using Channel::Mode to specify the channel type.

TEST=none
BUG=377980
R=darin@chromium.org,jam@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275140 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 91 insertions, 63 deletions

diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index e88bb5d..ceffb94 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -304,13 +304,15 @@ void ChannelProxy::Context::OnDispatchBadMessage(const Message& message) {
 
 //-----------------------------------------------------------------------------
 
-ChannelProxy::ChannelProxy(const IPC::ChannelHandle& channel_handle,
-                           Channel::Mode mode,
-                           Listener* listener,
-                           base::SingleThreadTaskRunner* ipc_task_runner)
-    : context_(new Context(listener, ipc_task_runner)),
-      did_init_(false) {
-  Init(channel_handle, mode, true);
+// static
+scoped_ptr<ChannelProxy> ChannelProxy::Create(
+    const IPC::ChannelHandle& channel_handle,
+    Channel::Mode mode,
+    Listener* listener,
+    base::SingleThreadTaskRunner* ipc_task_runner) {
+  scoped_ptr<ChannelProxy> channel(new ChannelProxy(listener, ipc_task_runner));
+  channel->Init(channel_handle, mode, true);
+  return channel.Pass();
 }
 
 ChannelProxy::ChannelProxy(Context* context)
@@ -318,6 +320,11 @@ ChannelProxy::ChannelProxy(Context* context)
       did_init_(false) {
 }
 
+ChannelProxy::ChannelProxy(Listener* listener,
+                           base::SingleThreadTaskRunner* ipc_task_runner)
+    : context_(new Context(listener, ipc_task_runner)), did_init_(false) {
+}
+
 ChannelProxy::~ChannelProxy() {
   DCHECK(CalledOnValidThread());
 
diff --git a/ipc/ipc_channel_proxy.h b/ipc/ipc_channel_proxy.h
index 0a3a5d2..31ca211 100644
--- a/ipc/ipc_channel_proxy.h
+++ b/ipc/ipc_channel_proxy.h
@@ -64,10 +64,11 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe {
   // on the background thread.  Any message not handled by the filter will be
   // dispatched to the listener.  The given task runner correspond to a thread
   // on which IPC::Channel is created and used (e.g. IO thread).
-  ChannelProxy(const IPC::ChannelHandle& channel_handle,
-               Channel::Mode mode,
-               Listener* listener,
-               base::SingleThreadTaskRunner* ipc_task_runner);
+  static scoped_ptr<ChannelProxy> Create(
+      const IPC::ChannelHandle& channel_handle,
+      Channel::Mode mode,
+      Listener* listener,
+      base::SingleThreadTaskRunner* ipc_task_runner);
 
   virtual ~ChannelProxy();
 
@@ -124,6 +125,9 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe {
   // to the internal state.
   ChannelProxy(Context* context);
 
+  ChannelProxy(Listener* listener,
+               base::SingleThreadTaskRunner* ipc_task_runner);
+
   // Used internally to hold state that is referenced on the IPC thread.
   class Context : public base::RefCountedThreadSafe<Context>,
                   public Listener {
diff --git a/ipc/ipc_sync_channel.cc b/ipc/ipc_sync_channel.cc
index 0e0018c..a7ed230 100644
--- a/ipc/ipc_sync_channel.cc
+++ b/ipc/ipc_sync_channel.cc
@@ -404,19 +404,27 @@ base::WaitableEventWatcher::EventCallback
   return base::Bind(&SyncChannel::SyncContext::OnWaitableEventSignaled, this);
 }
 
-SyncChannel::SyncChannel(
+// static
+scoped_ptr<SyncChannel> SyncChannel::Create(
     const IPC::ChannelHandle& channel_handle,
     Channel::Mode mode,
     Listener* listener,
     base::SingleThreadTaskRunner* ipc_task_runner,
     bool create_pipe_now,
-    WaitableEvent* shutdown_event)
-    : ChannelProxy(new SyncContext(listener, ipc_task_runner, shutdown_event)) {
-  // The current (listener) thread must be distinct from the IPC thread, or else
-  // sending synchronous messages will deadlock.
-  DCHECK_NE(ipc_task_runner, base::ThreadTaskRunnerHandle::Get());
-  ChannelProxy::Init(channel_handle, mode, create_pipe_now);
-  StartWatching();
+    base::WaitableEvent* shutdown_event) {
+  scoped_ptr<SyncChannel> channel =
+      Create(listener, ipc_task_runner, shutdown_event);
+  channel->Init(channel_handle, mode, create_pipe_now);
+  return channel.Pass();
+}
+
+// static
+scoped_ptr<SyncChannel> SyncChannel::Create(
+    Listener* listener,
+    base::SingleThreadTaskRunner* ipc_task_runner,
+    WaitableEvent* shutdown_event) {
+  return make_scoped_ptr(
+      new SyncChannel(listener, ipc_task_runner, shutdown_event));
 }
 
 SyncChannel::SyncChannel(
diff --git a/ipc/ipc_sync_channel.h b/ipc/ipc_sync_channel.h
index f18fcff9..8984184 100644
--- a/ipc/ipc_sync_channel.h
+++ b/ipc/ipc_sync_channel.h
@@ -66,19 +66,22 @@ class IPC_EXPORT SyncChannel : public ChannelProxy {
 
   // Creates and initializes a sync channel. If create_pipe_now is specified,
   // the channel will be initialized synchronously.
-  SyncChannel(const IPC::ChannelHandle& channel_handle,
-              Channel::Mode mode,
-              Listener* listener,
-              base::SingleThreadTaskRunner* ipc_task_runner,
-              bool create_pipe_now,
-              base::WaitableEvent* shutdown_event);
+  // The naming pattern follows IPC::Channel.
+  static scoped_ptr<SyncChannel> Create(
+      const IPC::ChannelHandle& channel_handle,
+      IPC::Channel::Mode mode,
+      Listener* listener,
+      base::SingleThreadTaskRunner* ipc_task_runner,
+      bool create_pipe_now,
+      base::WaitableEvent* shutdown_event);
 
   // Creates an uninitialized sync channel. Call ChannelProxy::Init to
   // initialize the channel. This two-step setup allows message filters to be
   // added before any messages are sent or received.
-  SyncChannel(Listener* listener,
-              base::SingleThreadTaskRunner* ipc_task_runner,
-              base::WaitableEvent* shutdown_event);
+  static scoped_ptr<SyncChannel> Create(
+      Listener* listener,
+      base::SingleThreadTaskRunner* ipc_task_runner,
+      base::WaitableEvent* shutdown_event);
 
   virtual ~SyncChannel();
 
@@ -188,6 +191,10 @@ class IPC_EXPORT SyncChannel : public ChannelProxy {
   };
 
  private:
+  SyncChannel(Listener* listener,
+              base::SingleThreadTaskRunner* ipc_task_runner,
+              base::WaitableEvent* shutdown_event);
+
   void OnWaitableEventSignaled(base::WaitableEvent* arg);
 
   SyncContext* sync_context() {
diff --git a/ipc/ipc_sync_channel_unittest.cc b/ipc/ipc_sync_channel_unittest.cc
index ca8d5d7..817f17e 100644
--- a/ipc/ipc_sync_channel_unittest.cc
+++ b/ipc/ipc_sync_channel_unittest.cc
@@ -151,12 +151,10 @@ class Worker : public Listener, public Sender {
   }
 
   virtual SyncChannel* CreateChannel() {
-    return new SyncChannel(channel_name_,
-                           mode_,
-                           this,
-                           ipc_thread_.message_loop_proxy().get(),
-                           true,
-                           &shutdown_event_);
+    scoped_ptr<SyncChannel> channel = SyncChannel::Create(
+        channel_name_, mode_, this, ipc_thread_.message_loop_proxy().get(),
+        true, &shutdown_event_);
+    return channel.release();
   }
 
   base::Thread* ListenerThread() {
@@ -324,9 +322,11 @@ class TwoStepServer : public Worker {
   }
 
   virtual SyncChannel* CreateChannel() OVERRIDE {
-    SyncChannel* channel = new SyncChannel(
-        this, ipc_thread().message_loop_proxy().get(), shutdown_event());
-    channel->Init(channel_name(), mode(), create_pipe_now_);
+    SyncChannel* channel =
+        SyncChannel::Create(channel_name(), mode(), this,
+                            ipc_thread().message_loop_proxy().get(),
+                            create_pipe_now_,
+                            shutdown_event()).release();
     return channel;
   }
 
@@ -345,9 +345,11 @@ class TwoStepClient : public Worker {
   }
 
   virtual SyncChannel* CreateChannel() OVERRIDE {
-    SyncChannel* channel = new SyncChannel(
-        this, ipc_thread().message_loop_proxy().get(), shutdown_event());
-    channel->Init(channel_name(), mode(), create_pipe_now_);
+    SyncChannel* channel =
+        SyncChannel::Create(channel_name(), mode(), this,
+                            ipc_thread().message_loop_proxy().get(),
+                            create_pipe_now_,
+                            shutdown_event()).release();
     return channel;
   }
 
@@ -1135,13 +1137,13 @@ class RestrictedDispatchClient : public Worker {
     else
       LOG(ERROR) << "Send failed to dispatch incoming message on same channel";
 
-    non_restricted_channel_.reset(
-        new SyncChannel("non_restricted_channel",
-                        Channel::MODE_CLIENT,
-                        this,
-                        ipc_thread().message_loop_proxy().get(),
-                        true,
-                        shutdown_event()));
+    non_restricted_channel_ =
+        SyncChannel::Create("non_restricted_channel",
+                            IPC::Channel::MODE_CLIENT,
+                            this,
+                            ipc_thread().message_loop_proxy().get(),
+                            true,
+                            shutdown_event());
 
     server_->ListenerThread()->message_loop()->PostTask(
         FROM_HERE, base::Bind(&RestrictedDispatchServer::OnDoPing, server_, 2));
@@ -1526,13 +1528,13 @@ class RestrictedDispatchPipeWorker : public Worker {
     if (is_first())
       event1_->Signal();
     event2_->Wait();
-    other_channel_.reset(
-        new SyncChannel(other_channel_name_,
-                        Channel::MODE_CLIENT,
-                        this,
-                        ipc_thread().message_loop_proxy().get(),
-                        true,
-                        shutdown_event()));
+    other_channel_ =
+        SyncChannel::Create(other_channel_name_,
+                            IPC::Channel::MODE_CLIENT,
+                            this,
+                            ipc_thread().message_loop_proxy().get(),
+                            true,
+                            shutdown_event());
     other_channel_->SetRestrictDispatchChannelGroup(group_);
     if (!is_first()) {
       event1_->Signal();
@@ -1606,13 +1608,13 @@ class ReentrantReplyServer1 : public Worker {
         server_ready_(server_ready) { }
 
   virtual void Run() OVERRIDE {
-    server2_channel_.reset(
-        new SyncChannel("reentrant_reply2",
-                        Channel::MODE_CLIENT,
-                        this,
-                        ipc_thread().message_loop_proxy().get(),
-                        true,
-                        shutdown_event()));
+    server2_channel_ =
+        SyncChannel::Create("reentrant_reply2",
+                            IPC::Channel::MODE_CLIENT,
+                            this,
+                            ipc_thread().message_loop_proxy().get(),
+                            true,
+                            shutdown_event());
     server_ready_->Signal();
     Message* msg = new SyncChannelTestMsg_Reentrant1();
     server2_channel_->Send(msg);
diff --git a/ipc/ipc_test_base.cc b/ipc/ipc_test_base.cc
index ca45d16..4e7133b 100644
--- a/ipc/ipc_test_base.cc
+++ b/ipc/ipc_test_base.cc
@@ -77,10 +77,10 @@ void IPCTestBase::CreateChannelProxy(
     base::SingleThreadTaskRunner* ipc_task_runner) {
   CHECK(!channel_.get());
   CHECK(!channel_proxy_.get());
-  channel_proxy_.reset(new IPC::ChannelProxy(GetChannelName(test_client_name_),
+  channel_proxy_ = IPC::ChannelProxy::Create(GetChannelName(test_client_name_),
                                              IPC::Channel::MODE_SERVER,
                                              listener,
-                                             ipc_task_runner));
+                                             ipc_task_runner);
 }
 
 void IPCTestBase::DestroyChannelProxy() {