Move child-common classes to content/common_child

Move child-common classes to content/common_child

We need a place to put code that is shared between child processes of different
types but not used in the browser process. For instance, the NPObject bindings
code is used in the plugin and renderer processes but depends on WebBindings
which the browser shouldn't depend on. Some web platform features require shared
code between renderer and worker processe. The WebKit image decoders are used
by worker, renderer and utility processes.

This creates a content/common_child directory for code shared by more than one
child process type. content/common_child can depend on content/common and all
content/ subdirs except for content/browser and content/common can depend on it.

The java bridge code is (more than a) bit busted since it pulls the NPObject
bindings in to the browser, but since this code is only intended for use on
android single-process configurations I've just created DEPS exceptions for
this bit of code.

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

[jamesr, 2013-05-17 00:49:38]

This appears to work - PTAL.  I had to split the plugin-related IPCs sent
to/from the browser process into a new header I called
content/common/plugin_process_messages.h and I had to fix up some includes,
otherwise this is about as close to a straight code move as there is.

[michaeln, 2013-05-20 20:43:26]

Yo... wassup with this one? Fwiw, seems cut-n-dry like a good CL? The reason i
ask is that we're getting some back log gated in this change.

[Jói, 2013-05-20 20:55:26]

FWIW, I've reviewed the whole change and it LGTM.

https://codereview.chromium.org/15047014/diff/12001/content/common/DEPS
File content/common/DEPS (right):

https://codereview.chromium.org/15047014/diff/12001/content/common/DEPS#newco...
content/common/DEPS:11: # Java bridge code passes NPAPI types to the browser
process. Crazy!
indent +2

[piman, 2013-05-20 21:06:36]

Mostly nits, but I think the start value for messages could be a real bug.

https://codereview.chromium.org/15047014/diff/12001/content/browser/renderer_...
File content/browser/renderer_host/render_widget_host_view_win.cc (right):

https://codereview.chromium.org/15047014/diff/12001/content/browser/renderer_...
content/browser/renderer_host/render_widget_host_view_win.cc:10: #include
<wtsapi32.h>
nit: I assume this is an intentional move, c headers vs c++ ones
Maybe this should be grouped together with the c headers above, and move the
#pragma below together with it?

https://codereview.chromium.org/15047014/diff/12001/content/common/plugin_pro...
File content/common/plugin_process_messages.h (right):

https://codereview.chromium.org/15047014/diff/12001/content/common/plugin_pro...
content/common/plugin_process_messages.h:19: #define IPC_MESSAGE_START
PluginMsgStart
I think this needs to be a different start value, otherwise we could have
accidental collision of messages IDs (ID is based on the start_value and the
header line), and that could be a very bad thing...

https://codereview.chromium.org/15047014/diff/12001/content/renderer/render_t...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/15047014/diff/12001/content/renderer/render_t...
content/renderer/render_thread_impl.cc:38: #include
"content/common_child/plugin_messages.h"
nit: include order

[jamesr, 2013-05-20 21:18:50]

On 2013/05/20 21:06:36, piman wrote:
> Mostly nits, but I think the start value for messages could be a real bug.
> 
>
https://codereview.chromium.org/15047014/diff/12001/content/browser/renderer_...
> File content/browser/renderer_host/render_widget_host_view_win.cc (right):
> 
>
https://codereview.chromium.org/15047014/diff/12001/content/browser/renderer_...
> content/browser/renderer_host/render_widget_host_view_win.cc:10: #include
> <wtsapi32.h>
> nit: I assume this is an intentional move, c headers vs c++ ones
> Maybe this should be grouped together with the c headers above, and move the
> #pragma below together with it?

Right - this is just to make the #include order checker happy.  Moved the
<wtsapi32.h> include with the other C includes and moved the #pragma to be just
after it.

> 
>
https://codereview.chromium.org/15047014/diff/12001/content/common/plugin_pro...
> File content/common/plugin_process_messages.h (right):
> 
>
https://codereview.chromium.org/15047014/diff/12001/content/common/plugin_pro...
> content/common/plugin_process_messages.h:19: #define IPC_MESSAGE_START
> PluginMsgStart
> I think this needs to be a different start value, otherwise we could have
> accidental collision of messages IDs (ID is based on the start_value and the
> header line), and that could be a very bad thing...

Good catch!  Set this to the (new) value PluginProcessMsgStart and added that to
ipc/ipc_message_start.h

> 
>
https://codereview.chromium.org/15047014/diff/12001/content/renderer/render_t...
> File content/renderer/render_thread_impl.cc (right):
> 
>
https://codereview.chromium.org/15047014/diff/12001/content/renderer/render_t...
> content/renderer/render_thread_impl.cc:38: #include
> "content/common_child/plugin_messages.h"
> nit: include order

D'oh. Fixed.

[jamesr, 2013-05-20 21:39:40]

piman@ - PTAL

cevans@ - could you look at the IPC changes?  I've moved the plugin<->renderer
messages into content/common_child, redefined the SiteData IPCs as
PluginProcess(Host)Msg since they're sent from the browser to the plugin
process, and added a new start type for PluginProcessMsg.

[piman, 2013-05-20 21:44:59]

lgtm

[Chris Evans, 2013-05-20 22:03:12]

On 2013/05/20 21:39:40, jamesr wrote:
> piman@ - PTAL
> 
> cevans@ - could you look at the IPC changes?  I've moved the plugin<->renderer
> messages into content/common_child, redefined the SiteData IPCs as
> PluginProcess(Host)Msg since they're sent from the browser to the plugin
> process, and added a new start type for PluginProcessMsg.

LGTM

[commit-bot: I haz the power, 2013-05-20 22:11:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/15047014/27001

[commit-bot: I haz the power, 2013-05-21 08:16:10]

Change committed as 201252

[jam, 2013-05-22 00:31:13]

naming nit: the two terms in common_child seem redundant. why not just "child"?

[jamesr, 2013-05-22 07:39:58]

On 2013/05/22 00:31:13, jam wrote:
> naming nit: the two terms in common_child seem redundant. why not just
"child"?

That was my first instict for the name as well, but it sounds a lot like it's
another process type instead of a place for code shared between lots of
different process types.  I'd be happy to do the work to rename if we decide
content/child/ is a definitely better place for this stuff.

[jam, 2013-05-22 11:51:41]

"child" is used in many places as an abstract term for child processes, and
not any specific one. i.e. see content/common/child*


On Wed, May 22, 2013 at 12:39 AM, <jamesr@chromium.org> wrote:

> On 2013/05/22 00:31:13, jam wrote:
>
>> naming nit: the two terms in common_child seem redundant. why not just
>>
> "child"?
>
> That was my first instict for the name as well, but it sounds a lot like
> it's
> another process type instead of a place for code shared between lots of
> different process types.  I'd be happy to do the work to rename if we
> decide
> content/child/ is a definitely better place for this stuff.
>
>
https://codereview.chromium.**org/15047014/<https://codereview.chromium.org/1...
>