Cache Nacl Plugin private interface instead of calling GetnaclInterface on every process launch

Cache Nacl Plugin private interface instead of calling GetnaclInterface on every process launch

This fixes nacl subprocess creation off the main thread for LoadHelperNaClModule (used by pnacl coordinator).
GetNaclInterface calls GetBrowserInterface, which is required to be called from the main thread. So we call it
once and cache the result for calling off the main thread
(only for starting nacl helper processes).

R=dmichael@chromium.org,jvoung@chromium.org
BUG=(broken nacl/chrome integration bots)
TEST=nacl_integration


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

[dmichael (off chromium), 2012-06-25 22:24:18]

https://chromiumcodereview.appspot.com/10665036/diff/1/ppapi/native_client/sr...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

https://chromiumcodereview.appspot.com/10665036/diff/1/ppapi/native_client/sr...
ppapi/native_client/src/trusted/plugin/plugin.cc:868: nacl_interface_ =
GetNaclInterface();
Since you're in the neighborhood, could you rename to GetNaClInterface?
(note the upper-case C)

https://chromiumcodereview.appspot.com/10665036/diff/1/ppapi/native_client/sr...
ppapi/native_client/src/trusted/plugin/plugin.cc:893: using_ipc_proxy_(false) {
please init nacl_interface_ to NULL.

I'd also suggest calling GetNaClInterface in the body of the constructor, and
maybe doing a CHECK that you get a valid pointer.

[Derek Schuff, 2012-06-25 22:33:10]

https://chromiumcodereview.appspot.com/10665036/diff/1/ppapi/native_client/sr...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

https://chromiumcodereview.appspot.com/10665036/diff/1/ppapi/native_client/sr...
ppapi/native_client/src/trusted/plugin/plugin.cc:868: nacl_interface_ =
GetNaclInterface();
On 2012/06/25 22:24:18, dmichael wrote:
> Since you're in the neighborhood, could you rename to GetNaClInterface?
> (note the upper-case C)

Done.

https://chromiumcodereview.appspot.com/10665036/diff/1/ppapi/native_client/sr...
ppapi/native_client/src/trusted/plugin/plugin.cc:893: using_ipc_proxy_(false) {
On 2012/06/25 22:24:18, dmichael wrote:
> please init nacl_interface_ to NULL.
> 
> I'd also suggest calling GetNaClInterface in the body of the constructor, and
> maybe doing a CHECK that you get a valid pointer.

Done.

[jvoung (off chromium), 2012-06-25 22:42:29]

https://chromiumcodereview.appspot.com/10665036/diff/1003/ppapi/native_client...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

https://chromiumcodereview.appspot.com/10665036/diff/1003/ppapi/native_client...
ppapi/native_client/src/trusted/plugin/plugin.cc:125: const PPB_NaCl_Private*
GetNaClInterface() {
Is there really a reason to still have this helper, if the preferred way is to
use the cached one?

[dmichael (off chromium), 2012-06-25 22:42:34]

lgtm

https://chromiumcodereview.appspot.com/10665036/diff/1003/ppapi/native_client...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

https://chromiumcodereview.appspot.com/10665036/diff/1003/ppapi/native_client...
ppapi/native_client/src/trusted/plugin/plugin.cc:899: CHECK(nacl_interface_ !=
NULL);
nit: In chromium, I'm used to just seeing/writing "CHECK(nacl_interface_);"

[Derek Schuff, 2012-06-25 22:49:07]

https://chromiumcodereview.appspot.com/10665036/diff/1003/ppapi/native_client...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

https://chromiumcodereview.appspot.com/10665036/diff/1003/ppapi/native_client...
ppapi/native_client/src/trusted/plugin/plugin.cc:125: const PPB_NaCl_Private*
GetNaClInterface() {
I don't know if it's the preferred way, exactly. Currently the only use is for
process creation, but if I understand correctly, not much other than process
creation will work off the main thread. so maybe for other uses it would be
better just to use this?

On 2012/06/25 22:42:29, jvoung (chromium) wrote:
> Is there really a reason to still have this helper, if the preferred way is to
> use the cached one?

https://chromiumcodereview.appspot.com/10665036/diff/1003/ppapi/native_client...
ppapi/native_client/src/trusted/plugin/plugin.cc:899: CHECK(nacl_interface_ !=
NULL);
all the CHECKs in this particular file seem to use the != NULL form, so I
decided to follow those.

On 2012/06/25 22:42:34, dmichael wrote:
> nit: In chromium, I'm used to just seeing/writing "CHECK(nacl_interface_);"

[jvoung (off chromium), 2012-06-25 22:50:28]

lgtm

[commit-bot: I haz the power, 2012-06-25 23:09:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dschuff@chromium.org/10665036/1003

[commit-bot: I haz the power, 2012-06-26 01:01:51]

Change committed as 144083