Remove the NaCl SRPC proxy from Chrome.

Remove the NaCl SRPC proxy from Chrome.

Removes GYP targets for untrusted and trusted parts of the proxy (SRPC proxy
source files will be removed in a follow-on, to keep this patch manageable.)
Removes SRPC proxy startup code from NaCl trusted plugin.
Removes PPAPI PPP* interface adapters from the Plugin class.
Removes a bunch of GYP external dependency declarations that are no longer
needed.

TBR=brettw@chromium.org for OWNERS

BUG=169152
TEST=none


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

[bbudge, 2013-01-16 23:56:52]

Mark for chrome/
David and David for ppapi/

[Mark Seaborn, 2013-01-17 00:25:58]

LGTM

https://codereview.chromium.org/11783112/diff/31075/ppapi/c/private/ppb_flash...
File ppapi/c/private/ppb_flash_font_file.h (right):

https://codereview.chromium.org/11783112/diff/31075/ppapi/c/private/ppb_flash...
ppapi/c/private/ppb_flash_font_file.h:6: /* From private/ppb_flash_font_file.idl
modified Wed Jan  2 14:34:38 2013. */
The file hasn't changed otherwise.  Please undo this file's change.

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/nativ...
File ppapi/native_client/native_client.gyp (left):

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/nativ...
ppapi/native_client/native_client.gyp:313:
'<(SHARED_INTERMEDIATE_DIR)/tc_newlib/lib64/libbase_untrusted.a',
Why are you removing all deps for the IPC proxy?  It looks like some of these
should stay.

Whether "extra_deps" actually done anything is debatable:
https://code.google.com/p/nativeclient/issues/detail?id=2690  I think it
doesn't, but others disagreed.

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
ppapi/native_client/src/trusted/plugin/plugin.cc:421: // Try to start the PPAPI
proxy to the untrusted plugin instance.
Is "to" the wrong preposition?  "for the untrusted plugin instance"?

[dmichael (off chromium), 2013-01-17 17:43:15]

Awesome!

couple nits, o/w lgtm

I assume you are planning to delete the actual ppapi_proxy code in a separate
CL?

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/nativ...
File ppapi/native_client/native_client.gyp (left):

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/nativ...
ppapi/native_client/native_client.gyp:313:
'<(SHARED_INTERMEDIATE_DIR)/tc_newlib/lib64/libbase_untrusted.a',
On 2013/01/17 00:25:58, Mark Seaborn wrote:
> Why are you removing all deps for the IPC proxy?  It looks like some of these
> should stay.
> 
> Whether "extra_deps" actually done anything is debatable:
> https://code.google.com/p/nativeclient/issues/detail?id=2690  I think it
> doesn't, but others disagreed.
+1
I'd support trying to get rid of them, but it should probably be a different CL.

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
File ppapi/native_client/src/trusted/plugin/plugin.cc (left):

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
ppapi/native_client/src/trusted/plugin/plugin.cc:888: (shutdown_start -
ready_time_) / NACL_MICROS_PER_MILLI);
Do we have this Histogram call in the right place for the IPC proxy now?

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
ppapi/native_client/src/trusted/plugin/plugin.cc:737: return true;
You're sure this still gets called here? (I assume it does, but if not, it would
be better to remove the method)

[bbudge, 2013-01-17 19:07:09]

David, I left deleting the source files for a separate patch, as it's very
large. There is a tricky issue in that one file, ppruntime.h, is used outside of
the proxy code. I need to figure out where that should go.

https://codereview.chromium.org/11783112/diff/31075/ppapi/c/private/ppb_flash...
File ppapi/c/private/ppb_flash_font_file.h (right):

https://codereview.chromium.org/11783112/diff/31075/ppapi/c/private/ppb_flash...
ppapi/c/private/ppb_flash_font_file.h:6: /* From private/ppb_flash_font_file.idl
modified Wed Jan  2 14:34:38 2013. */
On 2013/01/17 00:25:58, Mark Seaborn wrote:
> The file hasn't changed otherwise.  Please undo this file's change.

Done.

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/nativ...
File ppapi/native_client/native_client.gyp (left):

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/nativ...
ppapi/native_client/native_client.gyp:313:
'<(SHARED_INTERMEDIATE_DIR)/tc_newlib/lib64/libbase_untrusted.a',
On 2013/01/17 00:25:58, Mark Seaborn wrote:
> Why are you removing all deps for the IPC proxy?  It looks like some of these
> should stay.
> 
> Whether "extra_deps" actually done anything is debatable:
> https://code.google.com/p/nativeclient/issues/detail?id=2690  I think it
> doesn't, but others disagreed.

It builds without them. Brad Nelson doesn't think they're needed any more.

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/nativ...
ppapi/native_client/native_client.gyp:313:
'<(SHARED_INTERMEDIATE_DIR)/tc_newlib/lib64/libbase_untrusted.a',
On 2013/01/17 17:43:15, dmichael wrote:
> On 2013/01/17 00:25:58, Mark Seaborn wrote:
> > Why are you removing all deps for the IPC proxy?  It looks like some of
these
> > should stay.
> > 
> > Whether "extra_deps" actually done anything is debatable:
> > https://code.google.com/p/nativeclient/issues/detail?id=2690  I think it
> > doesn't, but others disagreed.
> +1
> I'd support trying to get rid of them, but it should probably be a different
CL.

OK, this is complicated enough without them. I'll split this out.

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
File ppapi/native_client/src/trusted/plugin/plugin.cc (left):

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
ppapi/native_client/src/trusted/plugin/plugin.cc:888: (shutdown_start -
ready_time_) / NACL_MICROS_PER_MILLI);
On 2013/01/17 17:43:15, dmichael wrote:
> Do we have this Histogram call in the right place for the IPC proxy now?

Good catch. This is the best place to report this so I put back the histogram
call using the nexe_error_reported_ field, which looks like a reasonable
replacement for the ppapi_proxy_ check.

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
ppapi/native_client/src/trusted/plugin/plugin.cc:421: // Try to start the PPAPI
proxy to the untrusted plugin instance.
On 2013/01/17 00:25:58, Mark Seaborn wrote:
> Is "to" the wrong preposition?  "for the untrusted plugin instance"?

Bad comment. I don't think it's really necessary anymore. Deleted.

https://codereview.chromium.org/11783112/diff/31075/ppapi/native_client/src/t...
ppapi/native_client/src/trusted/plugin/plugin.cc:737: return true;
On 2013/01/17 17:43:15, dmichael wrote:
> You're sure this still gets called here? (I assume it does, but if not, it
would
> be better to remove the method)

I tried removing it but that caused the MIME type handler test to fail. I
believe this must be called before the proxy starts, and the default returns
false, which causes the loader to close.

[commit-bot: I haz the power, 2013-01-17 20:27:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bbudge@chromium.org/11783112/23068

[commit-bot: I haz the power, 2013-01-17 20:27:31]

Presubmit check for 11783112-23068 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

Presubmit checks took 3.1s to calculate.

[bbudge, 2013-01-17 20:32:01]

+jam for chrome/

This is a simple removal of the NaCl PPAPI proxy flag.

[sehr (please use chromium), 2013-01-17 20:35:50]

lgtm

[sehr, 2013-01-17 20:38:22]

lgtm

[commit-bot: I haz the power, 2013-01-17 21:08:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bbudge@chromium.org/11783112/23068

[commit-bot: I haz the power, 2013-01-17 21:37:17]

Retried try job too often on mac_rel for step(s) sync_integration_tests

[commit-bot: I haz the power, 2013-01-17 21:44:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bbudge@chromium.org/11783112/23068

[commit-bot: I haz the power, 2013-01-17 23:39:44]

Change committed as 177531