Add a flag to LaunchSelLdr to skip grabbing a routing_id.

Add a flag to LaunchSelLdr to skip grabbing a routing_id.

Finding the routing id requires LaunchSelLdr to run from the
main thread.  However, some sel_ldr instances host nexes that do
not actually need to use Pepper APIs (e.g., the PNaCl translator),
and should be run on a background thread.

BUG= http://code.google.com/p/nativeclient/issues/detail?id=3176


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

[bbudge, 2012-11-29 02:12:02]

LGTM w/comment

https://codereview.chromium.org/11428063/diff/1/ppapi/api/private/ppb_nacl_pr...
File ppapi/api/private/ppb_nacl_private.idl (right):

https://codereview.chromium.org/11428063/diff/1/ppapi/api/private/ppb_nacl_pr...
ppapi/api/private/ppb_nacl_private.idl:35: */
We should probably add a comment for |uses_ppapi| and how if it's false this can
be called off the main thread.

[brettw, 2012-11-29 04:57:18]

LGTM

https://codereview.chromium.org/11428063/diff/1/chrome/renderer/pepper/ppb_na...
File chrome/renderer/pepper/ppb_nacl_private_impl.cc (right):

https://codereview.chromium.org/11428063/diff/1/chrome/renderer/pepper/ppb_na...
chrome/renderer/pepper/ppb_nacl_private_impl.cc:75:
webkit::ppapi::PluginInstance* plugin_instance =
Can you add an assertion at the top here that content::RenderThread::Get() is
non-NULL (we should be on the render thread).

[brettw, 2012-11-29 04:59:03]

Wait, we should also change the browser process side of the nacl process host to
not register the PpapiHost and other pepper stuff when the routing ID is 0.

[jvoung (off chromium), 2012-11-29 22:46:46]

+cc sehr and dschuff, who might need to take over this CL since I'm on vacation
starting tomorrow.

The bit in nacl_process_host.cc that calls CreateExternalPluginProcess(), inside
OnPpapiChannelCreated() seems to be needed.  Otherwise, I see that our helper
nexes don't respond (and it does get called when setting up our helper nexes). 
Is that expected?

I did try disabling the PepperMessageFilter when given a bad ID, and with that
the pnacl helper nexes are still functional.

https://codereview.chromium.org/11428063/diff/1/chrome/renderer/pepper/ppb_na...
File chrome/renderer/pepper/ppb_nacl_private_impl.cc (right):

https://codereview.chromium.org/11428063/diff/1/chrome/renderer/pepper/ppb_na...
chrome/renderer/pepper/ppb_nacl_private_impl.cc:75:
webkit::ppapi::PluginInstance* plugin_instance =
On 2012/11/29 04:57:18, brettw wrote:
> Can you add an assertion at the top here that content::RenderThread::Get() is
> non-NULL (we should be on the render thread).

Done.

https://codereview.chromium.org/11428063/diff/1/ppapi/api/private/ppb_nacl_pr...
File ppapi/api/private/ppb_nacl_private.idl (right):

https://codereview.chromium.org/11428063/diff/1/ppapi/api/private/ppb_nacl_pr...
ppapi/api/private/ppb_nacl_private.idl:35: */
On 2012/11/29 02:12:02, bbudge1 wrote:
> We should probably add a comment for |uses_ppapi| and how if it's false this
can
> be called off the main thread.

Done.

[bbudge, 2012-11-29 23:18:34]

On 2012/11/29 22:46:46, jvoung (cr) wrote:
> +cc sehr and dschuff, who might need to take over this CL since I'm on
vacation
> starting tomorrow.
> 
> The bit in nacl_process_host.cc that calls CreateExternalPluginProcess(),
inside
> OnPpapiChannelCreated() seems to be needed.  Otherwise, I see that our helper
> nexes don't respond (and it does get called when setting up our helper nexes).

> Is that expected?
> 
> I did try disabling the PepperMessageFilter when given a bad ID, and with that
> the pnacl helper nexes are still functional.
> 
>
https://codereview.chromium.org/11428063/diff/1/chrome/renderer/pepper/ppb_na...
> File chrome/renderer/pepper/ppb_nacl_private_impl.cc (right):
> 
>
https://codereview.chromium.org/11428063/diff/1/chrome/renderer/pepper/ppb_na...
> chrome/renderer/pepper/ppb_nacl_private_impl.cc:75:
> webkit::ppapi::PluginInstance* plugin_instance =
> On 2012/11/29 04:57:18, brettw wrote:
> > Can you add an assertion at the top here that content::RenderThread::Get()
is
> > non-NULL (we should be on the render thread).
> 
> Done.
> 
>
https://codereview.chromium.org/11428063/diff/1/ppapi/api/private/ppb_nacl_pr...
> File ppapi/api/private/ppb_nacl_private.idl (right):
> 
>
https://codereview.chromium.org/11428063/diff/1/ppapi/api/private/ppb_nacl_pr...
> ppapi/api/private/ppb_nacl_private.idl:35: */
> On 2012/11/29 02:12:02, bbudge1 wrote:
> > We should probably add a comment for |uses_ppapi| and how if it's false this
> can
> > be called off the main thread.
> 
> Done.

Are you certain you need to call CreateExternalPluginProcess? I don't see why
anything it does is needed.

[jvoung (off chromium), 2012-11-29 23:44:25]

On 2012/11/29 23:18:34, bbudge1 wrote:
> On 2012/11/29 22:46:46, jvoung (cr) wrote:
> > +cc sehr and dschuff, who might need to take over this CL since I'm on
> vacation
> > starting tomorrow.
> > 
> > The bit in nacl_process_host.cc that calls CreateExternalPluginProcess(),
> inside
> > OnPpapiChannelCreated() seems to be needed.  Otherwise, I see that our
helper
> > nexes don't respond (and it does get called when setting up our helper
nexes).
> 
> > Is that expected?
> > 
> > I did try disabling the PepperMessageFilter when given a bad ID, and with
that
> > the pnacl helper nexes are still functional.
> > 
> >
>
https://codereview.chromium.org/11428063/diff/1/chrome/renderer/pepper/ppb_na...
> > File chrome/renderer/pepper/ppb_nacl_private_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/11428063/diff/1/chrome/renderer/pepper/ppb_na...
> > chrome/renderer/pepper/ppb_nacl_private_impl.cc:75:
> > webkit::ppapi::PluginInstance* plugin_instance =
> > On 2012/11/29 04:57:18, brettw wrote:
> > > Can you add an assertion at the top here that content::RenderThread::Get()
> is
> > > non-NULL (we should be on the render thread).
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/11428063/diff/1/ppapi/api/private/ppb_nacl_pr...
> > File ppapi/api/private/ppb_nacl_private.idl (right):
> > 
> >
>
https://codereview.chromium.org/11428063/diff/1/ppapi/api/private/ppb_nacl_pr...
> > ppapi/api/private/ppb_nacl_private.idl:35: */
> > On 2012/11/29 02:12:02, bbudge1 wrote:
> > > We should probably add a comment for |uses_ppapi| and how if it's false
this
> > can
> > > be called off the main thread.
> > 
> > Done.
> 
> Are you certain you need to call CreateExternalPluginProcess? I don't see why
> anything it does is needed.

Ooops, yeah that isn't needed.

I had bundled disabling that with disabling sending 
"PpapiMsg_CreateNaClChannel".

[jvoung (off chromium), 2012-11-30 00:32:57]

On 2012/11/29 23:44:25, jvoung (cr) wrote:
> On 2012/11/29 23:18:34, bbudge1 wrote:
> > On 2012/11/29 22:46:46, jvoung (cr) wrote:
> > > +cc sehr and dschuff, who might need to take over this CL since I'm on
> > vacation
> > > starting tomorrow.
> > > 
> > > The bit in nacl_process_host.cc that calls CreateExternalPluginProcess(),
> > inside
> > > OnPpapiChannelCreated() seems to be needed.  Otherwise, I see that our
> helper
> > > nexes don't respond (and it does get called when setting up our helper
> nexes).
> > 
> > > Is that expected?
> > > 
> > > I did try disabling the PepperMessageFilter when given a bad ID, and with
> that
> > > the pnacl helper nexes are still functional.
> > > 
> > >
> >
>
https://codereview.chromium.org/11428063/diff/1/chrome/renderer/pepper/ppb_na...
> > > File chrome/renderer/pepper/ppb_nacl_private_impl.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/11428063/diff/1/chrome/renderer/pepper/ppb_na...
> > > chrome/renderer/pepper/ppb_nacl_private_impl.cc:75:
> > > webkit::ppapi::PluginInstance* plugin_instance =
> > > On 2012/11/29 04:57:18, brettw wrote:
> > > > Can you add an assertion at the top here that
content::RenderThread::Get()
> > is
> > > > non-NULL (we should be on the render thread).
> > > 
> > > Done.
> > > 
> > >
> >
>
https://codereview.chromium.org/11428063/diff/1/ppapi/api/private/ppb_nacl_pr...
> > > File ppapi/api/private/ppb_nacl_private.idl (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/11428063/diff/1/ppapi/api/private/ppb_nacl_pr...
> > > ppapi/api/private/ppb_nacl_private.idl:35: */
> > > On 2012/11/29 02:12:02, bbudge1 wrote:
> > > > We should probably add a comment for |uses_ppapi| and how if it's false
> this
> > > can
> > > > be called off the main thread.
> > > 
> > > Done.
> > 
> > Are you certain you need to call CreateExternalPluginProcess? I don't see
why
> > anything it does is needed.
> 
> Ooops, yeah that isn't needed.
> 
> I had bundled disabling that with disabling sending 
> "PpapiMsg_CreateNaClChannel".

Changed to not CreateExternalPluginProcess, and instead ReplyToRenderer() in
SendStart().

[bbudge, 2012-11-30 00:49:45]

https://codereview.chromium.org/11428063/diff/5003/chrome/browser/nacl_host/n...
File chrome/browser/nacl_host/nacl_process_host.cc (right):

https://codereview.chromium.org/11428063/diff/5003/chrome/browser/nacl_host/n...
chrome/browser/nacl_host/nacl_process_host.cc:745: if (!enable_ipc_proxy_ ||
!render_view_id_) {
If you really don't need any IPC stuff, it would be simpler to check
render_view_id_ in the NaClProcessHost ctor and if it's 0, set enable_ipc_proxy_
to false.

[jvoung (off chromium), 2012-11-30 01:34:09]

https://codereview.chromium.org/11428063/diff/5003/chrome/browser/nacl_host/n...
File chrome/browser/nacl_host/nacl_process_host.cc (right):

https://codereview.chromium.org/11428063/diff/5003/chrome/browser/nacl_host/n...
chrome/browser/nacl_host/nacl_process_host.cc:745: if (!enable_ipc_proxy_ ||
!render_view_id_) {
On 2012/11/30 00:49:46, bbudge1 wrote:
> If you really don't need any IPC stuff, it would be simpler to check
> render_view_id_ in the NaClProcessHost ctor and if it's 0, set
enable_ipc_proxy_
> to false.

Ah ok.  Done.

[bbudge, 2012-12-03 21:06:33]

LGTM if it works for your use case.

[commit-bot: I haz the power, 2012-12-04 16:44:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/11428063/5007

[Derek Schuff, 2012-12-04 17:06:07]

On 2012/12/04 16:44:28, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/jvoung%40chromium.org/11428063/5007

Thanks for the reviews.
Jan is on vacation, so I'm going to land this patch. hopefully the CQ will work.

[commit-bot: I haz the power, 2012-12-04 19:20:47]

Change committed as 171020