Wire up a PPAPI host in content/browser.

Wire up a PPAPI host in content/browser.

Nothing actually uses this yet. I plumbed through a PpapiHost and the permissions through the message filter.

BUG=
TEST=

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

[brettw, 2012-07-08 23:55:41]

Should be easy.

[dmichael (off chromium), 2012-07-09 02:14:46]

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/pepper/content_browser_pepper_host_factory.h
(right):

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/pepper/content_browser_pepper_host_factory.h:17:
ContentBrowserPepperHostFactory(PepperMessageFilter* filter);
explicit ;-)

Maybe worth mentioning the ownership semantics (I'm assuming the filter is not
owned and must outlive the ContentBrowserPepperHostFactory)

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/pepper_message_filter.cc (right):

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/pepper_message_filter.cc:82: permissions_),
optional: I would almost rather you did ppapi_host_ with a scoped pointer so you
don't have to rely on initialization order. I know what you have is correct, but
I find it more obvious if you separate them. Take it or leave it :)

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/pepper_message_filter.h (right):

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/pepper_message_filter.h:27: #include
"ppapi/shared_impl/ppapi_permissions.h"
I don't see this file locally...  did you forget to add it?

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/pepper_message_filter.h:28: #include
"ppapi/host/ppapi_host.h"
nit: include order

[brettw, 2012-07-09 04:41:46]

New snap up

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/pepper_message_filter.cc (right):

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/pepper_message_filter.cc:82: permissions_),
I know what you mean, but I kind of hate pointers :)

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/pepper_message_filter.h (right):

http://codereview.chromium.org/10696132/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/pepper_message_filter.h:27: #include
"ppapi/shared_impl/ppapi_permissions.h"
It's in this patch:
http://codereview.chromium.org/10735011/
that isn't checked in yet.

[dmichael (off chromium), 2012-07-09 15:00:26]

Everything I understand lgtm. I was hoping you could explain the bit about
permissions and the renderer (and maybe improve that comment if you think it's
warranted).

http://codereview.chromium.org/10696132/diff/5001/content/browser/renderer_ho...
File content/browser/renderer_host/pepper_message_filter.cc (right):

http://codereview.chromium.org/10696132/diff/5001/content/browser/renderer_ho...
content/browser/renderer_host/pepper_message_filter.cc:130: // than the renderer
might have.
I was hoping that reading the permissions class would help me understand this
comment. If there's a problem whereby renderers could get elevated permissions
if you pass through ppapi_host_...  why isn't that also a problem for a plugin
process? I clearly don't quite understand what the problem is here. Is it that
the plugin process will only have 1 module in it, while the renderer might have
multiple with different permissions (so this is an anti-spoofing measure)?

[brettw, 2012-07-12 05:18:28]

http://codereview.chromium.org/10696132/diff/5001/content/browser/renderer_ho...
File content/browser/renderer_host/pepper_message_filter.cc (right):

http://codereview.chromium.org/10696132/diff/5001/content/browser/renderer_ho...
content/browser/renderer_host/pepper_message_filter.cc:130: // than the renderer
might have.
I rephrased this a bit. Plugins have more permissions than we want to give to
the renderers, like opening TCP sockets. If we responded to "open TCP socket"
from the renderer, we would allow an exploited renderer to talk to anywhere on
the internet. As-is, we only allow this for exploited plugins which limits
surface area (the TCP example is more complicated in practice, but that's the
jist for this type of thing).

[dmichael (off chromium), 2012-07-12 16:12:04]

Thanks for clarifying. LGTM

http://codereview.chromium.org/10696132/diff/9/content/browser/renderer_host/...
File content/browser/renderer_host/pepper/pepper_message_filter.cc (right):

http://codereview.chromium.org/10696132/diff/9/content/browser/renderer_host/...
content/browser/renderer_host/pepper/pepper_message_filter.cc:129: // renderers
to send these messages since they have fewer capabilities for
nit: I think it would be clearer to say something like:
"Ignore messages from renderers, since..."
since the message has already been sent.