Enable ChannelMojo for non-renderer child processes

Enable ChannelMojo for non-renderer child processes

This CL enables ChannelMojo for ChidThread subclasses.
It refers the same flag as one the render thread sees,
so it can be disabled through --disable-mojo-channel

Note that this doesn't touch the channel to NaCL as
it needs more plumbing to make it work.

BUG=377980
R=creis@chromium.org, jam@chromium.org

[jam, 2015-02-26 16:23:02]

it's a bit hard to review in isolation; it would be good to see what at least
one child process change looks like. also why not do all at once instead of one
by one?

one more question: it seems error prone that the browser and child process
callbacks need to match. why not have one single method on ContentClient or
somewhere else which defines this behavior? or it might just be simpler to turn
them all on or off together.

[Hajime Morrita, 2015-02-26 19:41:29]

Thanks for the review John!
PTAL at the updated change?

On 2015/02/26 16:23:02, jam wrote:
> it's a bit hard to review in isolation; it would be good to see what at least
> one child process change looks like. also why not do all at once instead of
one
> by one?
> 
> one more question: it seems error prone that the browser and child process
> callbacks need to match. why not have one single method on ContentClient or
> somewhere else which defines this behavior? or it might just be simpler to
turn
> them all on or off together.

This makes sense.
I thought it'd be easy to revert if I commit each flag flip as an individual
commit,
but probably we can just revert the whole change or tweak the code to disable
it.

The updated CL sees content::ShouldUseMojoChannel() so it should be less error
prone.

[jam, 2015-03-03 21:18:48]

it seems that the internal code should call ShouldUseMojoChannel directly
instead of going through all the delegates.

i've also left a comment on https://codereview.chromium.org/886153003/

https://codereview.chromium.org/960693003/diff/100001/content/browser/browser...
File content/browser/browser_child_process_host_impl.cc (right):

https://codereview.chromium.org/960693003/diff/100001/content/browser/browser...
content/browser/browser_child_process_host_impl.cc:267: return
delegate_->ShouldUseMojoChannel();
actually, why do we have this callback. why doesn't the caller jus call the new
content method directly?

https://codereview.chromium.org/960693003/diff/100001/content/browser/plugin_...
File content/browser/plugin_process_host.cc (right):

https://codereview.chromium.org/960693003/diff/100001/content/browser/plugin_...
content/browser/plugin_process_host.cc:333: return
content::ShouldUseMojoChannel();
nit: content:: not necessary

https://codereview.chromium.org/960693003/diff/100001/content/browser/ppapi_p...
File content/browser/ppapi_plugin_process_host.cc (right):

https://codereview.chromium.org/960693003/diff/100001/content/browser/ppapi_p...
content/browser/ppapi_plugin_process_host.cc:427: return
content::ShouldUseMojoChannel();
ditto

[Hajime Morrita, 2015-03-03 21:31:21]

Thanks for the review John!

> it seems that the internal code should call ShouldUseMojoChannel directly
instead of going through all the delegates.

There is a BrowserChildProcessHostDelegate subclass 
(NaclProcessHost) which doesn't do this.
We cannot turn it on there yet
as ChannelMojo isn't ready for NaCL.
Once it become ready, we can safely remove the hook and call
content::ShouldUseMojoChannel() directly.

https://codereview.chromium.org/960693003/diff/100001/content/browser/plugin_...
File content/browser/plugin_process_host.cc (right):

https://codereview.chromium.org/960693003/diff/100001/content/browser/plugin_...
content/browser/plugin_process_host.cc:333: return
content::ShouldUseMojoChannel();
On 2015/03/03 21:18:47, jam wrote:
> nit: content:: not necessary
The method name shadows this?

[Hajime Morrita, 2015-03-03 21:32:12]

BTW, I'm preparing a fix for the win_chromium_rel_ng redness.
It is orthogonal to this so I'll land it separately before landing this.

[jam, 2015-03-05 18:19:26]

should i review patchset 7 or 6?

[Hajime Morrita, 2015-03-05 18:22:52]

On 2015/03/05 18:19:26, jam wrote:
> should i review patchset 7 or 6?

Oops, no. I uploaded a wrong CL. Sorry for the noise.
I'll re-upload the correct one and run a try, then ping you once it gets green.

[Hajime Morrita, 2015-03-31 17:14:37]

morrita@chromium.org changed reviewers:
+ viettrungluu@chromium.org

[Hajime Morrita, 2015-03-31 17:14:38]

This had been blocked other issues but now I'm back.

Jam: Could you take a look at content/ ? It basically hooks up Channel creation
in various places and passes around the flag to enable ChannelMojo.


Trung: PTAL for ipc/ ? It's a one-line change.

[viettrungluu, 2015-03-31 22:29:10]

//ipc lgtm

[Hajime Morrita, 2015-04-01 17:24:45]

John: Could you take a look at content/ as an OWNER?

[Hajime Morrita, 2015-04-03 22:22:47]

This CL now becomes a pure content/ CL.
Can any owner take a look at this?