Introduce IPC::Channel::Create*() to ensure it being heap-allocated.

Introduce IPC::Channel::Create*() to ensure it being heap-allocated.

This change introduces IPC::Channel::Create*() API to turn
IPC::Channel into a heap allocated object. This will allow us to
make Channel a polymorphic class.

This change also tries to hide Channel::Mode from public API
so that we can simplify channel creation code paths cleaner in
following changes. ChannelProxy has to follow same pattern to
finish this cleanup. Such changes will follow.

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

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273575

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273713

[Hajime Morrita, 2014-05-28 17:45:42]

PTAL?

[darin (slow to review), 2014-05-28 19:47:14]

LGTM, just nits:

https://codereview.chromium.org/307653003/diff/1/ipc/ipc_channel.h
File ipc/ipc_channel.h (right):

https://codereview.chromium.org/307653003/diff/1/ipc/ipc_channel.h#newcode124
ipc/ipc_channel.h:124: // FIXME(morrita): Replace CreateByModeForProxy() with
one of above Create*().
nit: in Chromium unlike Blink, we use TODO(user) instead of FIXME(user)

https://codereview.chromium.org/307653003/diff/1/ipc/ipc_channel.h#newcode126
ipc/ipc_channel.h:126: static Channel* CreateByModeForProxy(const
IPC::ChannelHandle &channel_handle,
nit: these should return scoped_ptr<Channel>.

https://codereview.chromium.org/307653003/diff/1/ipc/ipc_channel.h#newcode126
ipc/ipc_channel.h:126: static Channel* CreateByModeForProxy(const
IPC::ChannelHandle &channel_handle,
Perhaps the documentation that currently exists for the Mode enums should be
moved to these methods? You could also add a TODO(..) next to the enums saying
that they are deprecated.

https://codereview.chromium.org/307653003/diff/1/ipc/ipc_channel.h#newcode134
ipc/ipc_channel.h:134: static Channel* CreateOpenNamedServer(
It seems like CreateOpenNamedServer should be guarded by #if defined(OS_POSIX)
to match MODE_OPEN_NAMED_SERVER.

[Hajime Morrita, 2014-05-28 21:34:40]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-05-28 21:34:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/morrita@chromium.org/307653003/20001

[Hajime Morrita, 2014-05-28 21:36:06]

The CQ bit was unchecked by morrita@chromium.org

[Hajime Morrita, 2014-05-28 22:32:08]

The CQ bit was checked by morrita@chromium.org

[Hajime Morrita, 2014-05-28 22:32:23]

Thanks for review!

https://codereview.chromium.org/307653003/diff/1/ipc/ipc_channel.h
File ipc/ipc_channel.h (right):

https://codereview.chromium.org/307653003/diff/1/ipc/ipc_channel.h#newcode124
ipc/ipc_channel.h:124: // FIXME(morrita): Replace CreateByModeForProxy() with
one of above Create*().
On 2014/05/28 19:47:14, darin wrote:
> nit: in Chromium unlike Blink, we use TODO(user) instead of FIXME(user)

Done.

https://codereview.chromium.org/307653003/diff/1/ipc/ipc_channel.h#newcode126
ipc/ipc_channel.h:126: static Channel* CreateByModeForProxy(const
IPC::ChannelHandle &channel_handle,
On 2014/05/28 19:47:14, darin wrote:
> Perhaps the documentation that currently exists for the Mode enums should be
> moved to these methods? You could also add a TODO(..) next to the enums saying
> that they are deprecated.

Done.

https://codereview.chromium.org/307653003/diff/1/ipc/ipc_channel.h#newcode134
ipc/ipc_channel.h:134: static Channel* CreateOpenNamedServer(
On 2014/05/28 19:47:14, darin wrote:
> It seems like CreateOpenNamedServer should be guarded by #if defined(OS_POSIX)
> to match MODE_OPEN_NAMED_SERVER.

Done.

[commit-bot: I haz the power, 2014-05-28 22:32:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/morrita@chromium.org/307653003/40001

[commit-bot: I haz the power, 2014-05-29 09:18:54]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-29 10:58:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-29 10:58:41]

Try jobs failed on following builders:
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)

[Hajime Morrita, 2014-05-29 16:01:17]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-05-29 16:03:41]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/morrita@chromium.org/307653003/60001

[cpu_(ooo_6.6-7.5), 2014-05-29 17:06:11]

lgtm

[commit-bot: I haz the power, 2014-05-29 20:30:20]

Change committed as 273575

[Hajime Morrita, 2014-05-29 21:16:41]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-05-29 21:21:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/morrita@chromium.org/307653003/80001

[commit-bot: I haz the power, 2014-05-30 02:18:56]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-30 03:59:07]

Change committed as 273713