Remove browser support for non-PPAPI nexes

Remove browser support for non-PPAPI nexes, plus related artifacts.
BUG= http://code.google.com/p/nativeclient/issues/detail?id=2587
TEST=none
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=122289

[sehr (please use chromium), 2012-02-14 21:36:15]

This is blocked by another CL, which removes the last tests that use non-PPAPI
nexes.  I have tested the CL with that change and things work, as well as
building in Chrome, so I would like to get the review started on this now.

[jvoung - send to chromium..., 2012-02-15 00:58:09]

http://codereview.chromium.org/9390028/diff/11005/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

http://codereview.chromium.org/9390028/diff/11005/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/plugin.cc:643: // TODO(jvoung): This next
bit is likely not needed...
Can this "StartSrpcServices()" be removed now too?  I think it used to be used
by the JS __startSrpcServices interface.

If it is removable, then StartSrpcServicesCommon can be renamed to
StartSrpcServices.

[sehr (please use chromium), 2012-02-15 01:16:38]

Thanks for the review.  PTAL.

http://codereview.chromium.org/9390028/diff/11005/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

http://codereview.chromium.org/9390028/diff/11005/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/plugin.cc:643: // TODO(jvoung): This next
bit is likely not needed...
On 2012/02/15 00:58:09, jvoung wrote:
> Can this "StartSrpcServices()" be removed now too?  I think it used to be used
> by the JS __startSrpcServices interface.
> 
> If it is removable, then StartSrpcServicesCommon can be renamed to
> StartSrpcServices.

Done.

[jvoung - send to chromium..., 2012-02-15 08:28:17]

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/plugin.cc:639: return
subprocess->StartJSObjectProxy(this, error_info);
Unless there is some value to keeping this abstraction, could just remove
plugin's StartJSObjectProxy and invoke main_subprocess.StartJSObjectProxy()
directly below.

Similarly, is it necessary for both plugin and nacl_subprocess to have a 
StartSrpcServices() method?

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/pnacl_resources.cc (left):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/pnacl_resources.cc:7: #include <utility>
misc: some of these other includes may be unnecessary too (no use of vector,
pair here anymore?)

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/scriptable_handle.cc (right):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/scriptable_handle.cc:13: #include <set>
<set> may be unnecessary now (was used for g_ValidHandles)

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/scriptable_handle.cc:48: // In
JavaScript, foo[1] is equivalent to foo["1"], so map both indexed and
Why not leave these in var_util.h?  I guess you don't expect any other users...

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/scriptable_handle.h (right):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/scriptable_handle.h:33: //
ScriptableHandle encapsulates objects that are scriptable from the browser.
It seems like from the implementation, most of the scripting is disabled, and
only getting of properties is supported.  Should that be revealed here in the
comment?

Or, I guess now the only scriptable part is the plugin?  Could ScriptableHandle
be called ScriptablePlugin, or something then?

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/service_runtime.h (right):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/service_runtime.h:34: class
BrowserInterface;
no need for this declaration of BrowserInterface anymore.

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/srpc_params.h (right):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/srpc_params.h:43: exception_string_ =
STRDUP(msg);
Should this check if a previous exception_string was already set to avoid
leaking mem?

Actually, I'm not sure if set_exception_string, etc. are even used anymore?
It looks like use of exception_string() from scriptable handle was removed.

[sehr (please use chromium), 2012-02-15 17:58:10]

Thanks for the review.  PTAL.

David

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/plugin.cc:639: return
subprocess->StartJSObjectProxy(this, error_info);
On 2012/02/15 08:28:17, jvoung wrote:
> Unless there is some value to keeping this abstraction, could just remove
> plugin's StartJSObjectProxy and invoke main_subprocess.StartJSObjectProxy()
> directly below.
> 
> Similarly, is it necessary for both plugin and nacl_subprocess to have a 
> StartSrpcServices() method?

I had sort of deferred this next level until next time because of all the
same-name-different-class stuff going on.  Done.

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/pnacl_resources.cc (left):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/pnacl_resources.cc:7: #include <utility>
On 2012/02/15 08:28:17, jvoung wrote:
> misc: some of these other includes may be unnecessary too (no use of vector,
> pair here anymore?)

Done.

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/scriptable_handle.cc (right):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/scriptable_handle.cc:13: #include <set>
On 2012/02/15 08:28:17, jvoung wrote:
> <set> may be unnecessary now (was used for g_ValidHandles)

Done.

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/scriptable_handle.cc:48: // In
JavaScript, foo[1] is equivalent to foo["1"], so map both indexed and
On 2012/02/15 08:28:17, jvoung wrote:
> Why not leave these in var_util.h?  I guess you don't expect any other
users...

This file has now become the only interface from PPAPI .  There's no real place
to use this interface otherwise, I think.

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/scriptable_handle.h (right):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/scriptable_handle.h:33: //
ScriptableHandle encapsulates objects that are scriptable from the browser.
On 2012/02/15 08:28:17, jvoung wrote:
> It seems like from the implementation, most of the scripting is disabled, and
> only getting of properties is supported.  Should that be revealed here in the
> comment?
> 
> Or, I guess now the only scriptable part is the plugin?  Could
ScriptableHandle
> be called ScriptablePlugin, or something then?

You're right.  It only wraps Plugins now and they only have properties, and only
getters for those. I've moved and renamed the class.

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/service_runtime.h (right):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/service_runtime.h:34: class
BrowserInterface;
On 2012/02/15 08:28:17, jvoung wrote:
> no need for this declaration of BrowserInterface anymore.

Done.

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/srpc_params.h (right):

http://codereview.chromium.org/9390028/diff/13064/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/srpc_params.h:43: exception_string_ =
STRDUP(msg);
On 2012/02/15 08:28:17, jvoung wrote:
> Should this check if a previous exception_string was already set to avoid
> leaking mem?
> 
> Actually, I'm not sure if set_exception_string, etc. are even used anymore?
> It looks like use of exception_string() from scriptable handle was removed.

Good catch.  Removed.

[jvoung - send to chromium..., 2012-02-15 19:02:28]

http://codereview.chromium.org/9390028/diff/18006/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

http://codereview.chromium.org/9390028/diff/18006/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/plugin.cc:643: if
(!(main_subprocess_.StartSrpcServices()
Should the error_info be passed along, to note any srpc connection failures?

http://codereview.chromium.org/9390028/diff/18006/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/plugin.cc:681: &&
nacl_subprocess->StartSrpcServices())) {
same

http://codereview.chromium.org/9390028/diff/18006/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/scriptable_plugin.cc (right):

http://codereview.chromium.org/9390028/diff/18006/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/scriptable_plugin.cc:13: // #include
<set>
remove fully?

[sehr (please use chromium), 2012-02-16 01:16:59]

Thanks for the review.  As I told you, I made a (hopefully successful) complete
trip through the included files.  PTAL.

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

https://chromiumcodereview.appspot.com/9390028/diff/18006/ppapi/native_client...
ppapi/native_client/src/trusted/plugin/plugin.cc:643: if
(!(main_subprocess_.StartSrpcServices()
On 2012/02/15 19:02:28, jvoung wrote:
> Should the error_info be passed along, to note any srpc connection failures?

I've replaced what was there.

https://chromiumcodereview.appspot.com/9390028/diff/18006/ppapi/native_client...
ppapi/native_client/src/trusted/plugin/plugin.cc:681: &&
nacl_subprocess->StartSrpcServices())) {
On 2012/02/15 19:02:28, jvoung wrote:
> same

Done.

https://chromiumcodereview.appspot.com/9390028/diff/18006/ppapi/native_client...
File ppapi/native_client/src/trusted/plugin/scriptable_plugin.cc (right):

https://chromiumcodereview.appspot.com/9390028/diff/18006/ppapi/native_client...
ppapi/native_client/src/trusted/plugin/scriptable_plugin.cc:13: // #include
<set>
On 2012/02/15 19:02:28, jvoung wrote:
> remove fully?

Done.

[jvoung - send to chromium..., 2012-02-16 01:22:36]

lgtm

http://codereview.chromium.org/9390028/diff/23017/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/plugin.h (right):

http://codereview.chromium.org/9390028/diff/23017/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/plugin.h:18: // #include <vector>
one more commented out header

[sehr (please use chromium), 2012-02-16 05:22:08]

Thanks again.  One more round of trybots before going in.

http://codereview.chromium.org/9390028/diff/23017/ppapi/native_client/src/tru...
File ppapi/native_client/src/trusted/plugin/plugin.h (right):

http://codereview.chromium.org/9390028/diff/23017/ppapi/native_client/src/tru...
ppapi/native_client/src/trusted/plugin/plugin.h:18: // #include <vector>
On 2012/02/16 01:22:36, jvoung wrote:
> one more commented out header

Ugh, three passes plus two greps missed this one...  Done.

[sehr (please use chromium), 2012-02-16 15:12:08]

Thanks again. r122289