Eliminate the 'build_ppapi_ipc_proxy_untrusted' build flag for untrusted PPAPI proxy builds.

Eliminate the 'build_ppapi_ipc_proxy_untrusted' build flag for untrusted PPAPI proxy builds.

This CL removes our custom GYP flag and modifies the build so both the SRPC and IPC proxies are built side by side. In addition, NaClBrowser now checks the '--enable-nacl-ipc-proxy' flag to determine which IRT to load. The IPC proxy's name is of the form 'nacl_ipc_irt_*'.

BUG=116317
TEST=builds and runs NaCl apps with --enable-nacl-ipc-proxy
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=151581

[bbudge, 2012-08-14 00:33:17]

http://codereview.chromium.org/10836220/diff/8002/ppapi/ppapi_chrome_proxy_un...
File ppapi/ppapi_chrome_proxy_untrusted.gyp (right):

http://codereview.chromium.org/10836220/diff/8002/ppapi/ppapi_chrome_proxy_un...
ppapi/ppapi_chrome_proxy_untrusted.gyp:49: }
This file had to be renamed because GYP can't handle .gyp files with the same
name, and the SRPC proxy already has a ppapi_proxy_untrusted.gyp file.

[Mark Seaborn, 2012-08-14 01:12:05]

http://codereview.chromium.org/10836220/diff/8002/chrome/browser/nacl_host/na...
File chrome/browser/nacl_host/nacl_browser.cc (right):

http://codereview.chromium.org/10836220/diff/8002/chrome/browser/nacl_host/na...
chrome/browser/nacl_host/nacl_browser.cc:54: return is64 ?
NACL_IRT_FILE_NAME("nacl_chrome_irt", "x86_64") :
Chrome's existing irt.nexe is Chrome-specific too, so this name could be
confusing.  How about "nacl_ipc_proxy_irt"?

http://codereview.chromium.org/10836220/diff/8002/chrome/browser/nacl_host/na...
chrome/browser/nacl_host/nacl_browser.cc:64: return NACL_IRT_FILE_NAME("arm");
You need to update the ARM case too.  It looks like the code would be cleaner if
you drop the macro and concatenate the architecture name at runtime.

[dmichael (off chromium), 2012-08-14 17:03:31]

lgtm

http://codereview.chromium.org/10836220/diff/8002/build/all.gyp
File build/all.gyp (right):

http://codereview.chromium.org/10836220/diff/8002/build/all.gyp#newcode249
build/all.gyp:249: # TODO(bbudge): drop this when we have switched the NaCl
proxy to IPC.
Is this TODO still valid? Aren't we going to still have this dependencey?

http://codereview.chromium.org/10836220/diff/8002/chrome/browser/nacl_host/na...
File chrome/browser/nacl_host/nacl_browser.cc (right):

http://codereview.chromium.org/10836220/diff/8002/chrome/browser/nacl_host/na...
chrome/browser/nacl_host/nacl_browser.cc:54: return is64 ?
NACL_IRT_FILE_NAME("nacl_chrome_irt", "x86_64") :
On 2012/08/14 01:12:05, Mark Seaborn wrote:
> Chrome's existing irt.nexe is Chrome-specific too, so this name could be
> confusing.  How about "nacl_ipc_proxy_irt"?
Bill and I had talked about that name, too. It would be nice to have a name that
we like _after_ the switch too (rather than renaming the new IRT to the old IRT
name when we pull the switch). nacl_chrome_irt seemed like a good compromise in
the absence of any perfect name.

[brettw, 2012-08-14 17:11:24]

LGTM rubberstamp if you needed it (not sure)

[bbudge, 2012-08-14 18:35:18]

Adding Brad to check all.gyp and nacl.gypi to make sure our build still runs on
the bots.

http://codereview.chromium.org/10836220/diff/8002/build/all.gyp
File build/all.gyp (right):

http://codereview.chromium.org/10836220/diff/8002/build/all.gyp#newcode249
build/all.gyp:249: # TODO(bbudge): drop this when we have switched the NaCl
proxy to IPC.
On 2012/08/14 17:03:31, dmichael wrote:
> Is this TODO still valid? Aren't we going to still have this dependencey?

Actually, now that we have side-by-side builds, we can remove these and add our
'nacl_ipc_irt' target to chrome/nacl.gypi's dependencies.

http://codereview.chromium.org/10836220/diff/8002/chrome/browser/nacl_host/na...
File chrome/browser/nacl_host/nacl_browser.cc (right):

http://codereview.chromium.org/10836220/diff/8002/chrome/browser/nacl_host/na...
chrome/browser/nacl_host/nacl_browser.cc:54: return is64 ?
NACL_IRT_FILE_NAME("nacl_chrome_irt", "x86_64") :
On 2012/08/14 01:12:05, Mark Seaborn wrote:
> Chrome's existing irt.nexe is Chrome-specific too, so this name could be
> confusing.  How about "nacl_ipc_proxy_irt"?

'nacl_ipc_irt' was both David's and my first idea. Since it's yours too Mark,
perhaps I'm over-thinking this. Done.

http://codereview.chromium.org/10836220/diff/8002/chrome/browser/nacl_host/na...
chrome/browser/nacl_host/nacl_browser.cc:64: return NACL_IRT_FILE_NAME("arm");
On 2012/08/14 01:12:05, Mark Seaborn wrote:
> You need to update the ARM case too.  It looks like the code would be cleaner
if
> you drop the macro and concatenate the architecture name at runtime.

Done.

[bbudge, 2012-08-14 19:27:36]

Fix Windows compile.

[bradn, 2012-08-14 20:07:48]

LGTM on the gyp

[Mark Seaborn, 2012-08-14 22:14:03]

LGTM.  Your commit message still refers to "nacl_chrome_irt" though, and there's
a typo ('--enable-nacl-ipc-prox').  Also putting BUG= and TEST= in the middle of
the message is odd. :-)

http://codereview.chromium.org/10836220/diff/7009/chrome/browser/nacl_host/na...
File chrome/browser/nacl_host/nacl_browser.cc (right):

http://codereview.chromium.org/10836220/diff/7009/chrome/browser/nacl_host/na...
chrome/browser/nacl_host/nacl_browser.cc:38: switches::kEnableNaClIPCProxy))
Indent the function argument by 4 more spaces?

[bbudge, 2012-08-14 22:22:43]

http://codereview.chromium.org/10836220/diff/7009/chrome/browser/nacl_host/na...
File chrome/browser/nacl_host/nacl_browser.cc (right):

http://codereview.chromium.org/10836220/diff/7009/chrome/browser/nacl_host/na...
chrome/browser/nacl_host/nacl_browser.cc:38: switches::kEnableNaClIPCProxy))
On 2012/08/14 22:14:04, Mark Seaborn wrote:
> Indent the function argument by 4 more spaces?

Done.