Broker out PPAPI handle duplication

Broker out PPAPI handle duplication

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

[jschuh, 2012-05-10 00:10:56]

I needed to get privileged process handles out of the sandboxed processes. This
meant converting the handle duplication over to BrokerDuplicateHandle. To avoid
introducing even more #ifdefs I added a new wrapper function to content/public.

@brettw - Most of this code is just plumbing where I strip out the code that was
passing the handle around. I left comments in the important places so they don't
get lost in the plumbing.

@cpu - Sanity check on Windows internals.

@piman - Can I get a rubberstamp for the webkit/ stuff?

http://codereview.chromium.org/10378057/diff/70/content/ppapi_plugin/ppapi_th...
File content/ppapi_plugin/ppapi_thread.cc (right):

http://codereview.chromium.org/10378057/diff/70/content/ppapi_plugin/ppapi_th...
content/ppapi_plugin/ppapi_thread.cc:122: void
PpapiThread::OnChannelConnected(int32 peer_pid) {
We cache a handle to the other end here, because it's a broker process.

http://codereview.chromium.org/10378057/diff/70/content/ppapi_plugin/ppapi_th...
content/ppapi_plugin/ppapi_thread.cc:137: IPC::PlatformFileForTransit
PpapiThread::ShareHandleWithRemote(
Here's the handle duping logic.

http://codereview.chromium.org/10378057/diff/70/content/public/common/sandbox...
File content/public/common/sandbox_init.cc (right):

http://codereview.chromium.org/10378057/diff/70/content/public/common/sandbox...
content/public/common/sandbox_init.cc:9: IPC::PlatformFileForTransit
BrokerGetFileHandleForProcess(
Cross platform function to make an exact copy of the handle.

http://codereview.chromium.org/10378057/diff/70/content/renderer/pepper/peppe...
File content/renderer/pepper/pepper_proxy_channel_delegate_impl.cc (right):

http://codereview.chromium.org/10378057/diff/70/content/renderer/pepper/peppe...
content/renderer/pepper/pepper_proxy_channel_delegate_impl.cc:26:
IPC::PlatformFileForTransit
Just wraps the new content method.

http://codereview.chromium.org/10378057/diff/70/ppapi/proxy/proxy_channel.h
File ppapi/proxy/proxy_channel.h (right):

http://codereview.chromium.org/10378057/diff/70/ppapi/proxy/proxy_channel.h#n...
ppapi/proxy/proxy_channel.h:41: 
New delegate interface here.

[jschuh, 2012-05-10 02:13:45]

@jamesr - Turns out @piman can't give me a webkit/ rubberstamp. Do you mind?

[jamesr, 2012-05-10 18:06:30]

webkit/ and content/renderer/(non-pepper) LGTM

[cpu_(ooo_6.6-7.5), 2012-05-10 22:10:43]

http://codereview.chromium.org/10378057/diff/70/content/ppapi_plugin/ppapi_th...
File content/ppapi_plugin/ppapi_thread.cc (right):

http://codereview.chromium.org/10378057/diff/70/content/ppapi_plugin/ppapi_th...
content/ppapi_plugin/ppapi_thread.cc:125: peer_handle_ =
::OpenProcess(PROCESS_DUP_HANDLE, FALSE, peer_pid);
do we ever close the peer handle? base::ProcessHandle is just a typedef right?

[jschuh, 2012-05-10 22:16:28]

http://codereview.chromium.org/10378057/diff/70/content/ppapi_plugin/ppapi_th...
File content/ppapi_plugin/ppapi_thread.cc (right):

http://codereview.chromium.org/10378057/diff/70/content/ppapi_plugin/ppapi_th...
content/ppapi_plugin/ppapi_thread.cc:125: peer_handle_ =
::OpenProcess(PROCESS_DUP_HANDLE, FALSE, peer_pid);
On 2012/05/10 22:10:43, cpu wrote:
> do we ever close the peer handle? base::ProcessHandle is just a typedef right?

D'oh. I thought I'd made that a scoped handle. Fixing it now.

[jschuh, 2012-05-11 17:40:28]

@brettw - ping

[jschuh, 2012-05-11 18:13:07]

@bbudge - fyi for your pending cl

[brettw, 2012-05-11 20:54:20]

lgtm

[commit-bot: I haz the power, 2012-05-11 20:59:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jschuh@chromium.org/10378057/11066

[commit-bot: I haz the power, 2012-05-11 21:18:28]

Try job failure for 10378057-11066 (retry) on android for steps "compile, build"
(clobber build).
It's a second try, previously, steps "compile, build" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android&nu...