Initial audio implementation.

Implement audio proxy for Pepper.

In addition to the basic proxying, this required some changes to the dispatcher
and process infrastructure to get the handles of the processes available when
I need them so I can duplicate the shared memory handles properly into the
different processes.

TEST=none
BUG=none

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

[brettw, 2010-12-01 00:23:05]

Sorry the proxy is so complicated. This is by far the worst one. Partially
because it has different types of handles that need to be shared in different
ways, and partially because some of the types implementing these handles vary by
platform as well. I tried to factor it as best I could.

http://codereview.chromium.org/4985001/diff/38001/chrome/browser/ppapi_plugin...
File chrome/browser/ppapi_plugin_process_host.cc (right):

http://codereview.chromium.org/4985001/diff/38001/chrome/browser/ppapi_plugin...
chrome/browser/ppapi_plugin_process_host.cc:82: void
PpapiPluginProcessHost::OnChannelConnected(int32 peer_pid) {
The changes in this component are to support sending the renderer handle to the
plugin, and the plugin handle to the renderer.

[piman, 2010-12-01 01:44:51]

Overall it looks reasonable. I have a few nits.

http://codereview.chromium.org/4985001/diff/64001/ppapi/proxy/ppapi_messages_...
File ppapi/proxy/ppapi_messages_internal.h (right):

http://codereview.chromium.org/4985001/diff/64001/ppapi/proxy/ppapi_messages_...
ppapi/proxy/ppapi_messages_internal.h:47: // The handler of this message should
always code all of the handles passed
"should always code" ? You mean check ? or close ?

http://codereview.chromium.org/4985001/diff/64001/ppapi/proxy/ppapi_messages_...
ppapi/proxy/ppapi_messages_internal.h:56: #endif
Using IPC::PlatformFileForTransit (typedef'd to HANDLE on Windows, and
FileDescriptor on Posix) will save you a lot of #ifdefs.

http://codereview.chromium.org/4985001/diff/64001/ppapi/proxy/ppb_audio_proxy.cc
File ppapi/proxy/ppb_audio_proxy.cc (right):

http://codereview.chromium.org/4985001/diff/64001/ppapi/proxy/ppb_audio_proxy...
ppapi/proxy/ppb_audio_proxy.cc:52: }
Most of the threading logic and I/O on the socket is almost identical to
pepper::Audio.
Is there any way to share most of the code between them ?

http://codereview.chromium.org/4985001/diff/64001/ppapi/proxy/ppb_audio_proxy...
ppapi/proxy/ppb_audio_proxy.cc:84: "plugin_audio_thread"));
the thread should have the same name wherever it's created ("audio" vs
"plugin_audio_thread")

http://codereview.chromium.org/4985001/diff/64001/ppapi/proxy/ppb_audio_proxy...
ppapi/proxy/ppb_audio_proxy.cc:317: ::DuplicateHandle(GetCurerntProcess(),
static_cast<HANDLE>(socket_handle),
GetCurerntProcess -> GetCurrentProcess

http://codereview.chromium.org/4985001/diff/64001/ppapi/proxy/ppb_audio_proxy...
ppapi/proxy/ppb_audio_proxy.cc:340:
base::FileDescriptor(static_cast<int>(shared_memory_handle), false),
why the static_cast ? it's an int already.

[brettw, 2010-12-01 22:45:13]

Please take another look. I moved the shared code into a new directory/lib that
I made so we can share implementation stuff between the proxy and the renderer
impl. I suspect we can move some more stuff here as well later.

[brettw, 2010-12-01 22:50:17]

+nicholas: You don't really need to review anything but I changed some stuff
around in pepper_audio you should be aware of. I will have to update some of the
types I use in this patch to match your latest patch, I think.

[piman, 2010-12-01 23:27:10]

LGTM except for a nit and a question. I don't think volatile should ever be used
in the context of chrome, except maybe if we use setjmp/longjmp.

http://codereview.chromium.org/4985001/diff/136001/ppapi/proxy/ppb_audio_prox...
File ppapi/proxy/ppb_audio_proxy.cc (right):

http://codereview.chromium.org/4985001/diff/136001/ppapi/proxy/ppb_audio_prox...
ppapi/proxy/ppb_audio_proxy.cc:256: *foreign_socket_handle =
base::FileDescriptor(static_cast<int>(socket_handle),
I don't think you need this static cast, int32_t->int should be implicit.

http://codereview.chromium.org/4985001/diff/136001/ppapi/shared_impl/audio_im...
File ppapi/shared_impl/audio_impl.h (right):

http://codereview.chromium.org/4985001/diff/136001/ppapi/shared_impl/audio_im...
ppapi/shared_impl/audio_impl.h:76: volatile PPB_Audio_Callback callback_;
Why volatile ?

[nfullagar, 2010-12-01 23:34:21]

http://codereview.chromium.org/4985001/diff/136001/webkit/glue/plugins/pepper...
File webkit/glue/plugins/pepper_audio.h (right):

http://codereview.chromium.org/4985001/diff/136001/webkit/glue/plugins/pepper...
webkit/glue/plugins/pepper_audio.h:103: // invalid all other times.
check last sentence above.  "These will are..."

[brettw, 2010-12-02 00:00:53]

http://codereview.chromium.org/4985001/diff/136001/ppapi/proxy/ppb_audio_prox...
File ppapi/proxy/ppb_audio_proxy.cc (right):

http://codereview.chromium.org/4985001/diff/136001/ppapi/proxy/ppb_audio_prox...
ppapi/proxy/ppb_audio_proxy.cc:256: *foreign_socket_handle =
base::FileDescriptor(static_cast<int>(socket_handle),
Ah yes, this was left over from when the value used to be 64-bits.

http://codereview.chromium.org/4985001/diff/136001/ppapi/shared_impl/audio_im...
File ppapi/shared_impl/audio_impl.h (right):

http://codereview.chromium.org/4985001/diff/136001/ppapi/shared_impl/audio_im...
ppapi/shared_impl/audio_impl.h:76: volatile PPB_Audio_Callback callback_;
On 2010/12/01 23:27:10, piman wrote:
> Why volatile ?

I just copied this from the old code. I don't think it actually gets us anything
in this case so I removed it.

[piman, 2010-12-02 00:07:25]

LGTM