Allow proxy channels to be created without initializing the underlying channel.

Allow proxy channels to be created without initializing the underlying channel.
This fixes a bug where a client needed to guarantee a message filter was in
place before any messages were received.
It also follows the style of not having constructors that do complex
initialization.
BUG=102894
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110229

[Paweł Hajdan Jr., 2011-11-04 07:58:17]

Automation LGTM. I didn't review the ipc code.

[kkania, 2011-11-04 18:12:12]

thanks Pawel, dmac will look at the ipc stuff

[dmac, 2011-11-04 22:30:44]

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.cc
File ipc/ipc_channel_proxy.cc (right):

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.cc#new...
ipc/ipc_channel_proxy.cc:301: void ChannelProxy::Init(const IPC::ChannelHandle&
channel_handle,
anyway to enforce that this is only called once?

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.h
File ipc/ipc_channel_proxy.h (right):

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.h#newc...
ipc/ipc_channel_proxy.h:131: // Initializes the channel proxy. Only call once
this to initialize a channel
fix up the comment. Only call this once perhaps.

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel.cc
File ipc/ipc_sync_channel.cc (right):

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel.cc#newc...
ipc/ipc_sync_channel.cc:384: // message loop running under it or not, so we
wouldn't know whether to
would be nice to get rid of this duplicated code block.comment. Can we get a
common init here as well?

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel_unittes...
File ipc/ipc_sync_channel_unittest.cc (right):

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel_unittes...
ipc/ipc_sync_channel_unittest.cc:315: 
could you add a more detailed comment for this set of classes?

[kkania, 2011-11-07 20:25:16]

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.cc
File ipc/ipc_channel_proxy.cc (right):

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.cc#new...
ipc/ipc_channel_proxy.cc:301: void ChannelProxy::Init(const IPC::ChannelHandle&
channel_handle,
On 2011/11/04 22:30:44, dmac wrote:
> anyway to enforce that this is only called once?

added did_init_

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.h
File ipc/ipc_channel_proxy.h (right):

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.h#newc...
ipc/ipc_channel_proxy.h:131: // Initializes the channel proxy. Only call once
this to initialize a channel
On 2011/11/04 22:30:44, dmac wrote:
> fix up the comment. Only call this once perhaps.

Done.

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel.cc
File ipc/ipc_sync_channel.cc (right):

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel.cc#newc...
ipc/ipc_sync_channel.cc:384: // message loop running under it or not, so we
wouldn't know whether to
On 2011/11/04 22:30:44, dmac wrote:
> would be nice to get rid of this duplicated code block.comment. Can we get a
> common init here as well?

Done.

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel_unittes...
File ipc/ipc_sync_channel_unittest.cc (right):

http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel_unittes...
ipc/ipc_sync_channel_unittest.cc:315: 
On 2011/11/04 22:30:44, dmac wrote:
> could you add a more detailed comment for this set of classes?

Done.

[kkania, 2011-11-10 16:43:13]

On 2011/11/07 20:25:16, kkania wrote:
> http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.cc
> File ipc/ipc_channel_proxy.cc (right):
> 
>
http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.cc#new...
> ipc/ipc_channel_proxy.cc:301: void ChannelProxy::Init(const
IPC::ChannelHandle&
> channel_handle,
> On 2011/11/04 22:30:44, dmac wrote:
> > anyway to enforce that this is only called once?
> 
> added did_init_
> 
> http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.h
> File ipc/ipc_channel_proxy.h (right):
> 
>
http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_channel_proxy.h#newc...
> ipc/ipc_channel_proxy.h:131: // Initializes the channel proxy. Only call once
> this to initialize a channel
> On 2011/11/04 22:30:44, dmac wrote:
> > fix up the comment. Only call this once perhaps.
> 
> Done.
> 
> http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel.cc
> File ipc/ipc_sync_channel.cc (right):
> 
>
http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel.cc#newc...
> ipc/ipc_sync_channel.cc:384: // message loop running under it or not, so we
> wouldn't know whether to
> On 2011/11/04 22:30:44, dmac wrote:
> > would be nice to get rid of this duplicated code block.comment. Can we get a
> > common init here as well?
> 
> Done.
> 
>
http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel_unittes...
> File ipc/ipc_sync_channel_unittest.cc (right):
> 
>
http://codereview.chromium.org/8417054/diff/2001/ipc/ipc_sync_channel_unittes...
> ipc/ipc_sync_channel_unittest.cc:315: 
> On 2011/11/04 22:30:44, dmac wrote:
> > could you add a more detailed comment for this set of classes?
> 
> Done.

ping

[dmac, 2011-11-10 17:01:26]

LGTM

[kkania, 2011-11-11 18:29:37]

only changed name of SyncChannel::Init -> SyncChannel::StartWatching
(also sync code)

automation_proxy.cc wasn't compiling because SyncChannel::Init hides the
ProxyChannel::Init. As an alternative solution, I could have used "using
ProxyChannel::Init" in the header to unhide it, but I'm not sure if this will
work on all compilers and if it fits the style guide. Plus I realized it could
be confusing having an Init function that doesn't actually Init the channel, but
just performs some extra custom sync non-channel initialization.

[commit-bot: I haz the power, 2011-11-15 23:30:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kkania@chromium.org/8417054/28002

[commit-bot: I haz the power, 2011-11-16 01:06:48]

Change committed as 110229