Make sandboxed Flash work under UIPI

Make Flash work properly when started in low-integrity mode so that UIPI restrictions take effect. The change works by adding a new IPC from from plugin to browser, which tells the browser to reparent plugin windows when needed (since UIPI blacks parenting from the low-integrity plugin process).

See UIPI for reference: http://en.wikipedia.org/wiki/User_Interface_Privilege_Isolation

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

[jschuh, 2011-06-06 16:35:43]

This is the meat of the change (although I have an ugly layering violation in
plugin_process_host.cc that I have to fix). Early comments are appreciated.

[ananta, 2011-06-06 22:37:10]

Adding jam for his comments.

Looks great overall.

http://codereview.chromium.org/7054068/diff/2022/webkit/plugins/npapi/webplug...
File webkit/plugins/npapi/webplugin_delegate_impl_win.cc (right):

http://codereview.chromium.org/7054068/diff/2022/webkit/plugins/npapi/webplug...
webkit/plugins/npapi/webplugin_delegate_impl_win.cc:362:
::DestroyWindow(::GetParent(dummy_window_for_activation_));
We could maintain the dummy parent as a member variable. Feels creepy to destroy
a window via the GetParent call. Imagine a case where a window somehow sneaks
between us and the dummy activation window. :(

http://codereview.chromium.org/7054068/diff/2022/webkit/plugins/npapi/webplug...
webkit/plugins/npapi/webplugin_delegate_impl_win.cc:749: GetModuleHandle(NULL),
We could create the dummy parent window before creating the activation window?.
We could then pass the dummy hwnd as the parent in the CreateWindow call.

[jschuh, 2011-06-06 22:42:28]

On 2011/06/06 22:37:10, ananta wrote:
> Adding jam for his comments.
> 
> Looks great overall.
> 
>
http://codereview.chromium.org/7054068/diff/2022/webkit/plugins/npapi/webplug...
> File webkit/plugins/npapi/webplugin_delegate_impl_win.cc (right):
> 
>
http://codereview.chromium.org/7054068/diff/2022/webkit/plugins/npapi/webplug...
> webkit/plugins/npapi/webplugin_delegate_impl_win.cc:362:
> ::DestroyWindow(::GetParent(dummy_window_for_activation_));
> We could maintain the dummy parent as a member variable. Feels creepy to
destroy
> a window via the GetParent call. Imagine a case where a window somehow sneaks
> between us and the dummy activation window. :(

Yeah, good point. I'll store it off

>
http://codereview.chromium.org/7054068/diff/2022/webkit/plugins/npapi/webplug...
> webkit/plugins/npapi/webplugin_delegate_impl_win.cc:749:
GetModuleHandle(NULL),
> We could create the dummy parent window before creating the activation
window?.
> We could then pass the dummy hwnd as the parent in the CreateWindow call.

Yeah, I'll flip it. The current order was a result of code evolution rather than
an intentional decision.

[ananta, 2011-06-07 00:40:08]

LGTM

http://codereview.chromium.org/7054068/diff/12002/webkit/plugins/npapi/webplu...
File webkit/plugins/npapi/webplugin_delegate_impl_win.cc (right):

http://codereview.chromium.org/7054068/diff/12002/webkit/plugins/npapi/webplu...
webkit/plugins/npapi/webplugin_delegate_impl_win.cc:502: 0,
Please add some comments here which explain what we are trying to achieve with
the SetParent magic.

[jam, 2011-06-07 00:47:42]

lgtm (with boolean replaced with quirk just for consistency with the rest of the
file)

[cpu_(ooo_6.6-7.5), 2011-06-07 01:03:36]

lgtm after the two comments

http://codereview.chromium.org/7054068/diff/12002/content/browser/plugin_proc...
File content/browser/plugin_process_host.cc (right):

http://codereview.chromium.org/7054068/diff/12002/content/browser/plugin_proc...
content/browser/plugin_process_host.cc:73: 
If IsWindow() returns false you continue?

http://codereview.chromium.org/7054068/diff/12002/webkit/plugins/npapi/webplu...
File webkit/plugins/npapi/webplugin_delegate_impl_win.cc (right):

http://codereview.chromium.org/7054068/diff/12002/webkit/plugins/npapi/webplu...
webkit/plugins/npapi/webplugin_delegate_impl_win.cc:502: 0,
what Ananta said.

On 2011/06/07 00:40:08, ananta wrote:
> Please add some comments here which explain what we are trying to achieve with
> the SetParent magic.

[jschuh, 2011-06-07 01:25:25]

Okay, all issues fixed. Flagging for commit.

On 2011/06/07 01:03:36, cpu wrote:
> lgtm after the two comments
> 
>
http://codereview.chromium.org/7054068/diff/12002/content/browser/plugin_proc...
> File content/browser/plugin_process_host.cc (right):
> 
>
http://codereview.chromium.org/7054068/diff/12002/content/browser/plugin_proc...
> content/browser/plugin_process_host.cc:73: 
> If IsWindow() returns false you continue?
> 
>
http://codereview.chromium.org/7054068/diff/12002/webkit/plugins/npapi/webplu...
> File webkit/plugins/npapi/webplugin_delegate_impl_win.cc (right):
> 
>
http://codereview.chromium.org/7054068/diff/12002/webkit/plugins/npapi/webplu...
> webkit/plugins/npapi/webplugin_delegate_impl_win.cc:502: 0,
> what Ananta said.
> 
> On 2011/06/07 00:40:08, ananta wrote:
> > Please add some comments here which explain what we are trying to achieve
with
> > the SetParent magic.