Allow the NaCl IPC proxy to support multiple instances.

Allow the NaCl IPC proxy to support multiple instances.
Create a new PluginModule for each instance of a NaCl plugin that
uses the IPC-based PPAPI proxy. This is necessary because a NaCl plugin instance begins using in-process PPAPI, then switches to the out-of-process PPAPI proxy (when using the IPC proxy rather than the NaCl SRPC proxy.)
BUG=116317
TEST=manual

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

[bbudge, 2012-09-04 17:52:42]

Ready!

[dmichael (off chromium), 2012-09-04 21:21:33]

http://codereview.chromium.org/10912060/diff/10001/chrome/renderer/pepper/ppb...
File chrome/renderer/pepper/ppb_nacl_private_impl.cc (right):

http://codereview.chromium.org/10912060/diff/10001/chrome/renderer/pepper/ppb...
chrome/renderer/pepper/ppb_nacl_private_impl.cc:227: // to create additional
instances.
Could you elaborate more as to why new instances can't be created otherwise?
I.e., that PluginModule can only talk to 1 process, and NaCl always creates a
new plugin process for each instance.

http://codereview.chromium.org/10912060/diff/10001/webkit/plugins/ppapi/ppapi...
File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):

http://codereview.chromium.org/10912060/diff/10001/webkit/plugins/ppapi/ppapi...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:2533: nacl_module_.swap(module_);
What's the benefit of using swap()? Are you just trying to save the reference
counting? I think making the code less readable probably isn't worth it here.

http://codereview.chromium.org/10912060/diff/10001/webkit/plugins/ppapi/ppapi...
File webkit/plugins/ppapi/ppapi_plugin_instance.h (right):

http://codereview.chromium.org/10912060/diff/10001/webkit/plugins/ppapi/ppapi...
webkit/plugins/ppapi/ppapi_plugin_instance.h:578: scoped_ptr<
::ppapi::PPP_Instance_Combined> nacl_instance_interface_;
I feel like nacl_instance_interface_ is just as ambiguous as w/plugin
(nacl_module_ is also ambiguous, IMHO).

Maybe nacl_runtime_module_, nacl_runtime_instance_interface_?

[bbudge, 2012-09-05 19:15:48]

Addressed David's comments. brettw, piman, this CL does some "creative" reuse of
PluginModule, so I'd be glad to get your comments too.

http://codereview.chromium.org/10912060/diff/10001/chrome/renderer/pepper/ppb...
File chrome/renderer/pepper/ppb_nacl_private_impl.cc (right):

http://codereview.chromium.org/10912060/diff/10001/chrome/renderer/pepper/ppb...
chrome/renderer/pepper/ppb_nacl_private_impl.cc:227: // to create additional
instances.
On 2012/09/04 21:21:33, dmichael wrote:
> Could you elaborate more as to why new instances can't be created otherwise?
> I.e., that PluginModule can only talk to 1 process, and NaCl always creates a
> new plugin process for each instance.

Done.

http://codereview.chromium.org/10912060/diff/10001/webkit/plugins/ppapi/ppapi...
File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):

http://codereview.chromium.org/10912060/diff/10001/webkit/plugins/ppapi/ppapi...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:2533: nacl_module_.swap(module_);
On 2012/09/04 21:21:33, dmichael wrote:
> What's the benefit of using swap()? Are you just trying to save the reference
> counting? I think making the code less readable probably isn't worth it here.

Done.

http://codereview.chromium.org/10912060/diff/10001/webkit/plugins/ppapi/ppapi...
File webkit/plugins/ppapi/ppapi_plugin_instance.h (right):

http://codereview.chromium.org/10912060/diff/10001/webkit/plugins/ppapi/ppapi...
webkit/plugins/ppapi/ppapi_plugin_instance.h:578: scoped_ptr<
::ppapi::PPP_Instance_Combined> nacl_instance_interface_;
On 2012/09/04 21:21:33, dmichael wrote:
> I feel like nacl_instance_interface_ is just as ambiguous as w/plugin
> (nacl_module_ is also ambiguous, IMHO).
> 
> Maybe nacl_runtime_module_, nacl_runtime_instance_interface_?

Done. How about original_module_ and original_instance_interface_? That way it's
clearer what they're for.

[brettw, 2012-09-05 20:58:49]

lgtm

[dmichael (off chromium), 2012-09-05 21:02:01]

lgtm

[commit-bot: I haz the power, 2012-09-05 21:32:50]

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

[commit-bot: I haz the power, 2012-09-06 00:22:13]

Change committed as 155068