First pass at making the proxy handle multiple renderers. This associates the...

First pass at making the proxy handle multiple renderers. This associates the
instance with resources and has most callers retrieve the dispatcher
according to the appropriate instance. This isn't hooked up to anything yet.

This changes some PPB_Flash interface methods to use PP_Bool.

The most challenging part of the change is in the plugin_var_tracker which
now needs to track which dispatcher each var object came from, and remap var
IDs since each renderer will be generating var IDs in its own space, which
will likely overlap. A similar system will need to be done for resources
which is not implemented yet.

I added some null checks in audio_impl because audio_ can be NULL in some
cases when using the trusted API. I discovered this when testing NaCl for
this patch.

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

[brettw, 2011-01-19 20:56:09]

PluginVarTracker is badly in need of unit tests. I started one but need to move
some test infrastructure out of Chrome and into IPC, and I didn't want to make
this patch any larger. I'll do that as a followup.

[piman, 2011-01-20 00:26:23]

http://codereview.chromium.org/6282007/diff/116001/ppapi/c/dev/ppb_testing_dev.h
File ppapi/c/dev/ppb_testing_dev.h (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/c/dev/ppb_testing_de...
ppapi/c/dev/ppb_testing_dev.h:15: #define PPB_TESTING_DEV_INTERFACE
"PPB_Testing(Dev);0.4"
rev (API change)

http://codereview.chromium.org/6282007/diff/116001/ppapi/c/private/ppb_flash.h
File ppapi/c/private/ppb_flash.h (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/c/private/ppb_flash....
ppapi/c/private/ppb_flash.h:22: #define PPB_FLASH_INTERFACE "PPB_Flash;3"
rev (ABI change for bool->PP_Bool)

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/plugin_resourc...
File ppapi/proxy/plugin_resource_tracker.h (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/plugin_resourc...
ppapi/proxy/plugin_resource_tracker.h:27: static PluginResourceTracker*
GetInstance();
Isn't it a problem that there is only a global resource tracker, but PP_Resource
are scoped by the renderer, and so a PP_Resource from one could collide with a
PP_Resource from another one ?

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_audio_conf...
File ppapi/proxy/ppb_audio_config_proxy.cc (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_audio_conf...
ppapi/proxy/ppb_audio_config_proxy.cc:60: uint32_t requested_sample_frame_count)
{
It sounds like this API needs a PP_Instance...

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_image_data...
File ppapi/proxy/ppb_image_data_proxy.cc (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_image_data...
ppapi/proxy/ppb_image_data_proxy.cc:27: PP_ImageDataFormat
GetNativeImageDataFormat() {
Here too, it sounds like we should pass a PP_Instance (and below).

http://codereview.chromium.org/6282007/diff/116001/ppapi/tests/test_var_depre...
File ppapi/tests/test_var_deprecated.cc (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/tests/test_var_depre...
ppapi/tests/test_var_deprecated.cc:51: pp::Module::Get()->pp_module());
pp::Module::Get()->pp_module() -> instance_->pp_instance()

[brettw, 2011-01-20 00:48:45]

New snap up.

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/plugin_resourc...
File ppapi/proxy/plugin_resource_tracker.h (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/plugin_resourc...
ppapi/proxy/plugin_resource_tracker.h:27: static PluginResourceTracker*
GetInstance();
Yes, we will unfortunately need the same type of ID renaming for resources as we
do for var IDs. I'm going to do that in a separate pass.

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_audio_conf...
File ppapi/proxy/ppb_audio_config_proxy.cc (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_audio_conf...
ppapi/proxy/ppb_audio_config_proxy.cc:60: uint32_t requested_sample_frame_count)
{
Yes, this one is difficult. I already sent an email to the team about this.

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_image_data...
File ppapi/proxy/ppb_image_data_proxy.cc (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_image_data...
ppapi/proxy/ppb_image_data_proxy.cc:27: PP_ImageDataFormat
GetNativeImageDataFormat() {
I was less sure what to do about these ones. Since this represents "global
machine state" that we easily know in advance, we can easily have the renderer
send all of this global settings to the plugin when it starts and have them
available here. This doesn't work as well for the audio config since that might
be complicated to retrieve.

That seemed like kind of a lot of work for just these simple two cases, I put
the code in a shared place. Since this can be figured out at compile time, it
seemed OK for the proxy to "just know" what the right values are.

[piman, 2011-01-20 01:05:11]

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_image_data...
File ppapi/proxy/ppb_image_data_proxy.cc (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_image_data...
ppapi/proxy/ppb_image_data_proxy.cc:27: PP_ImageDataFormat
GetNativeImageDataFormat() {
On 2011/01/20 00:48:45, brettw wrote:
> I was less sure what to do about these ones. Since this represents "global
> machine state" that we easily know in advance, we can easily have the renderer
> send all of this global settings to the plugin when it starts and have them
> available here. This doesn't work as well for the audio config since that
might
> be complicated to retrieve.
> 
> That seemed like kind of a lot of work for just these simple two cases, I put
> the code in a shared place. Since this can be figured out at compile time, it
> seemed OK for the proxy to "just know" what the right values are.

I just want to make sure we don't design ourselves into a corner. For example if
in the future we decide that accelerated compositing expects a different format
from regular painting, then it becomes context-dependent and we would need the
instance.

[brettw, 2011-01-20 05:52:13]

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_image_data...
File ppapi/proxy/ppb_image_data_proxy.cc (right):

http://codereview.chromium.org/6282007/diff/116001/ppapi/proxy/ppb_image_data...
ppapi/proxy/ppb_image_data_proxy.cc:27: PP_ImageDataFormat
GetNativeImageDataFormat() {
This flag means more than just the fast compositing mode, it also tells you what
format you need to use to get the platform-specific software font drawing to
work. It seems like this will always be a specific value for an OS. If
compositing requires a different order, I think we'd probably just need to make
it work somehow.

[piman, 2011-01-20 21:31:22]

LGTM