Pepper: Split PpapiHostMsg_ChannelCreated use.

Pepper: Split PpapiHostMsg_ChannelCreated use.

PpapiHostMsg_ChannelCreated is used for two purposes:
  * Notifying the browser that a browser<->plugin channel has been created, and
  * Notifying that browser that a renderer<->plugin channel has been created.

In this case renderer<->plugin channel is for NaCl-based plugins, and is
special because it does some inspection of message contents and passes on the
message to the untrusted side of the plugin.

I've changed this because I think it makes the code easier to read, and I'm
adding an additional renderer<->plugin channel anyway.

BUG=
TBR=mseaborn,jschuh

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

[bbudge, 2014-01-03 16:52:14]

https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
File components/nacl/browser/nacl_process_host.h (right):

https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
components/nacl/browser/nacl_process_host.h:182: void
OnPpapiNaClChannelCreated(const IPC::ChannelHandle& channel_handle);
In theory we may create more than one renderer channel. This is the normal PPAPI
setup, where there is only a single plugin process connected to multiple
renderers. That may never be supported for NaCl though.

How about calling these OnPpapiRendererChannelCreated and
OnPpapiBrowserChannelCreated?

[teravest, 2014-01-03 16:59:41]

On Fri, Jan 3, 2014 at 9:52 AM,  <bbudge@chromium.org> wrote:
>
>
https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
> File components/nacl/browser/nacl_process_host.h (right):
>
>
https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
> components/nacl/browser/nacl_process_host.h:182: void
> OnPpapiNaClChannelCreated(const IPC::ChannelHandle& channel_handle);
> In theory we may create more than one renderer channel. This is the
> normal PPAPI setup, where there is only a single plugin process
> connected to multiple renderers. That may never be supported for NaCl
> though.
>
> How about calling these OnPpapiRendererChannelCreated and
> OnPpapiBrowserChannelCreated?

Done. Do you want me to change the message names too?

I don't want to make anything incorrect in the case of renderer-plugin
channels that aren't using NaCl, though.

>
> https://codereview.chromium.org/98183012/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[bbudge, 2014-01-03 17:06:14]

On 2014/01/03 16:59:41, teravest wrote:
> On Fri, Jan 3, 2014 at 9:52 AM,  <mailto:bbudge@chromium.org> wrote:
> >
> >
>
https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
> > File components/nacl/browser/nacl_process_host.h (right):
> >
> >
>
https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
> > components/nacl/browser/nacl_process_host.h:182: void
> > OnPpapiNaClChannelCreated(const IPC::ChannelHandle& channel_handle);
> > In theory we may create more than one renderer channel. This is the
> > normal PPAPI setup, where there is only a single plugin process
> > connected to multiple renderers. That may never be supported for NaCl
> > though.
> >
> > How about calling these OnPpapiRendererChannelCreated and
> > OnPpapiBrowserChannelCreated?
> 
> Done. Do you want me to change the message names too?
> 
> I don't want to make anything incorrect in the case of renderer-plugin
> channels that aren't using NaCl, though.
> 
> >
> > https://codereview.chromium.org/98183012/
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

I think these are all NaCl channels for PPAPI.

[bbudge, 2014-01-03 17:06:56]

On 2014/01/03 17:06:14, bbudge1 wrote:
> On 2014/01/03 16:59:41, teravest wrote:
> > On Fri, Jan 3, 2014 at 9:52 AM,  <mailto:bbudge@chromium.org> wrote:
> > >
> > >
> >
>
https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
> > > File components/nacl/browser/nacl_process_host.h (right):
> > >
> > >
> >
>
https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
> > > components/nacl/browser/nacl_process_host.h:182: void
> > > OnPpapiNaClChannelCreated(const IPC::ChannelHandle& channel_handle);
> > > In theory we may create more than one renderer channel. This is the
> > > normal PPAPI setup, where there is only a single plugin process
> > > connected to multiple renderers. That may never be supported for NaCl
> > > though.
> > >
> > > How about calling these OnPpapiRendererChannelCreated and
> > > OnPpapiBrowserChannelCreated?
> > 
> > Done. Do you want me to change the message names too?
> > 
> > I don't want to make anything incorrect in the case of renderer-plugin
> > channels that aren't using NaCl, though.
> > 
> > >
> > > https://codereview.chromium.org/98183012/
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> 
> I think these are all NaCl channels for PPAPI.

Oh, I think the message names should match too.

[teravest, 2014-01-03 17:18:22]

So, as it stands in the current change:
NaClProcessMsg_Start - creates a browser-plugin channel when nacl is used
PpapiMsg_CreateChannel - creates a renderer-plugin channel for ppapi_plugin
PpapiMsg_CreateNaClChannel - creates a renderer-plugin channel when nacl is
used

NaClProcessHostMsg_PpapiChannelCreated - a browser-plugin channel was
created (nacl)
PpapiHostMsg_ChannelCreated - a renderer-plugin channel was created,
but nacl isn't used
PpapiHostMsg_NaClChannelCreated - a renderer-plugin channel was
created, nacl is used

I'm not sure that the message naming is too bad, actually.

On Fri, Jan 3, 2014 at 10:06 AM,  <bbudge@chromium.org> wrote:
> On 2014/01/03 17:06:14, bbudge1 wrote:
>>
>> On 2014/01/03 16:59:41, teravest wrote:
>> > On Fri, Jan 3, 2014 at 9:52 AM,  <mailto:bbudge@chromium.org> wrote:
>> > >
>> > >
>> >
>
>
>
https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
>>
>> > > File components/nacl/browser/nacl_process_host.h (right):
>> > >
>> > >
>> >
>
>
>
https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
>>
>> > > components/nacl/browser/nacl_process_host.h:182: void
>> > > OnPpapiNaClChannelCreated(const IPC::ChannelHandle& channel_handle);
>> > > In theory we may create more than one renderer channel. This is the
>> > > normal PPAPI setup, where there is only a single plugin process
>> > > connected to multiple renderers. That may never be supported for NaCl
>> > > though.
>> > >
>> > > How about calling these OnPpapiRendererChannelCreated and
>> > > OnPpapiBrowserChannelCreated?
>> >
>> > Done. Do you want me to change the message names too?
>> >
>> > I don't want to make anything incorrect in the case of renderer-plugin
>> > channels that aren't using NaCl, though.
>> >
>> > >
>> > > https://codereview.chromium.org/98183012/
>> >
>> > To unsubscribe from this group and stop receiving emails from it, send
>> > an
>> email
>> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>
>
>> I think these are all NaCl channels for PPAPI.
>
>
> Oh, I think the message names should match too.
>
> https://codereview.chromium.org/98183012/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[bbudge, 2014-01-03 17:24:21]

On 2014/01/03 17:18:22, teravest wrote:
> So, as it stands in the current change:
> NaClProcessMsg_Start - creates a browser-plugin channel when nacl is used
> PpapiMsg_CreateChannel - creates a renderer-plugin channel for ppapi_plugin
> PpapiMsg_CreateNaClChannel - creates a renderer-plugin channel when nacl is
> used
> 
> NaClProcessHostMsg_PpapiChannelCreated - a browser-plugin channel was
> created (nacl)
> PpapiHostMsg_ChannelCreated - a renderer-plugin channel was created,
> but nacl isn't used
> PpapiHostMsg_NaClChannelCreated - a renderer-plugin channel was
> created, nacl is used
> 
> I'm not sure that the message naming is too bad, actually.
> 
> On Fri, Jan 3, 2014 at 10:06 AM,  <mailto:bbudge@chromium.org> wrote:
> > On 2014/01/03 17:06:14, bbudge1 wrote:
> >>
> >> On 2014/01/03 16:59:41, teravest wrote:
> >> > On Fri, Jan 3, 2014 at 9:52 AM,  <mailto:bbudge@chromium.org> wrote:
> >> > >
> >> > >
> >> >
> >
> >
> >
>
https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
> >>
> >> > > File components/nacl/browser/nacl_process_host.h (right):
> >> > >
> >> > >
> >> >
> >
> >
> >
>
https://codereview.chromium.org/98183012/diff/1/components/nacl/browser/nacl_...
> >>
> >> > > components/nacl/browser/nacl_process_host.h:182: void
> >> > > OnPpapiNaClChannelCreated(const IPC::ChannelHandle& channel_handle);
> >> > > In theory we may create more than one renderer channel. This is the
> >> > > normal PPAPI setup, where there is only a single plugin process
> >> > > connected to multiple renderers. That may never be supported for NaCl
> >> > > though.
> >> > >
> >> > > How about calling these OnPpapiRendererChannelCreated and
> >> > > OnPpapiBrowserChannelCreated?
> >> >
> >> > Done. Do you want me to change the message names too?
> >> >
> >> > I don't want to make anything incorrect in the case of renderer-plugin
> >> > channels that aren't using NaCl, though.
> >> >
> >> > >
> >> > > https://codereview.chromium.org/98183012/
> >> >
> >> > To unsubscribe from this group and stop receiving emails from it, send
> >> > an
> >> email
> >> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> >
> >
> >> I think these are all NaCl channels for PPAPI.
> >
> >
> > Oh, I think the message names should match too.
> >
> > https://codereview.chromium.org/98183012/
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

You're right. LGTM

[commit-bot: I haz the power, 2014-01-03 17:34:38]

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

[commit-bot: I haz the power, 2014-01-03 18:20:48]

Retried try job too often on linux_rel for step(s) chromedriver2_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2014-01-06 15:36:26]

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

[commit-bot: I haz the power, 2014-01-06 16:54:17]

Change committed as 243101

[Mark Seaborn, 2014-01-06 16:56:13]

Can you fill out the BUG= field in these changes, please?

https://codereview.chromium.org/98183012/diff/60001/ppapi/proxy/ppapi_messages.h
File ppapi/proxy/ppapi_messages.h (right):

https://codereview.chromium.org/98183012/diff/60001/ppapi/proxy/ppapi_message...
ppapi/proxy/ppapi_messages.h:733: // These are from the plugin to the renderer.
Can you update the comments in line with the explanation in the commit message? 
PpapiHostMsg_NaClChannelCreated is received by the browser, not the renderer.