Create a PPAPI host for new resource message routing.

Create a PPAPI host for new resource message routing.

This infrastructure will be used in the renderer and in the browser as the backing for new resources.

The PpapiHost object doues the routing for the resource messages, and also has hooks for the embedder (the renderer or the browser) to create resources. This adds a content_renderer factory which currently does nothing (we'll add most of the resources here).

BUG=
TEST=

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

[brettw, 2012-06-21 05:52:46]

Low priority.

This is the next step on the new way to implement proxied reources. It doesn't
actually do anything yet, this just provides the routing for responding to the
new messages on the host side.

This builds on the patch Bill reviewed to add a new resource message:
https://chromiumcodereview.appspot.com/10560030/

You can see how it gets hooked up to content here:
https://chromiumcodereview.appspot.com/10578043/

[dmichael (off chromium), 2012-06-22 16:56:49]

Haven't looked at everything yet. Do you have an example you could upload
somewhere showing what a resource proxy will look like once migrated to this? I
don't have a completely clear picture yet of how it works. And I'm not sure how
much code it's going to save...  we'll still have to define roughly the  same
messages (with maybe fewer parameters, since they won't have to include
PP_Resource), and we'll still have to demultiplex the messages to roughly the
same set of methods. So I'd be interested in an example to get an idea of how
much of a win it might be.

http://codereview.chromium.org/10572040/diff/8001/ppapi/host/host_factory.h
File ppapi/host/host_factory.h (right):

http://codereview.chromium.org/10572040/diff/8001/ppapi/host/host_factory.h#n...
ppapi/host/host_factory.h:31: virtual scoped_ptr<ResourceHost>
CreateMessageReceived(
The name's slightly confusing, seeing as this is a factory and it's creating a
ResourceHost. It could be CreateResourceHost, since that's what it does.

(An alternate could be "DidReceiveCreateMessage", but this class doesn't really
need to know or care *why* it's creating a ResourceHost.)

http://codereview.chromium.org/10572040/diff/8001/ppapi/host/host_factory.h#n...
ppapi/host/host_factory.h:38: virtual ~HostFactory() {}
Is it important that we disallow deleting via a base pointer? I don't think
there's an advantage to making the destructor protected.

http://codereview.chromium.org/10572040/diff/8001/ppapi/host/ppapi_host.cc
File ppapi/host/ppapi_host.cc (right):

http://codereview.chromium.org/10572040/diff/8001/ppapi/host/ppapi_host.cc#ne...
ppapi/host/ppapi_host.cc:21: // Put a cap on the maximum number of resources so
we don't explode of the
nit: explode of->explode if

http://codereview.chromium.org/10572040/diff/8001/ppapi/host/ppapi_host.cc#ne...
ppapi/host/ppapi_host.cc:23: const size_t kMaxResourcesPerPlugin = 16384;
optional suggestion: 1 << 14 might look less arbitrary

http://codereview.chromium.org/10572040/diff/8001/ppapi/host/resource_host.h
File ppapi/host/resource_host.h (right):

http://codereview.chromium.org/10572040/diff/8001/ppapi/host/resource_host.h#...
ppapi/host/resource_host.h:46: // result code, just set |*optional_reply| to be
the message of your
s/optional_reply/context.reply_msg?

Or...  maybe split it out in to the params and the reply message? It seems like
maybe you had it that way at one point, and that might be less confusing. Right
now it's an in-out parameter, where the context struct has one pure (the params
object) in and one more-or-less pure out (the message)

http://codereview.chromium.org/10572040/diff/8001/ppapi/proxy/dispatcher.h
File ppapi/proxy/dispatcher.h (right):

http://codereview.chromium.org/10572040/diff/8001/ppapi/proxy/dispatcher.h#ne...
ppapi/proxy/dispatcher.h:101: std::vector<IPC::Channel::Listener*> filters_;
I don't think you're using this in this CL. Did it belong in a different one?

http://codereview.chromium.org/10572040/diff/8001/ppapi/shared_impl/resource.h
File ppapi/shared_impl/resource.h (right):

http://codereview.chromium.org/10572040/diff/8001/ppapi/shared_impl/resource....
ppapi/shared_impl/resource.h:175: // override this function.
nit: missing closing paren

http://codereview.chromium.org/10572040/diff/8001/ppapi/shared_impl/resource....
ppapi/shared_impl/resource.h:178: // need to call this function from general
code that doesn't know how ti
ti->to

[brettw, 2012-06-24 04:42:08]

New snap up

https://chromiumcodereview.appspot.com/10572040/diff/8001/ppapi/host/resource...
File ppapi/host/resource_host.h (right):

https://chromiumcodereview.appspot.com/10572040/diff/8001/ppapi/host/resource...
ppapi/host/resource_host.h:46: // result code, just set |*optional_reply| to be
the message of your
Why there's a weird in/out struct is a long story:

Right now none of the message macros work with resource messages. This means you
have to manually unpack and dispatch each messagee (see
OnResourceMessageReceived in:
http://codereview.chromium.org/10544089/diff/12001/content/renderer/pepper/pe...
)
rather than use IPC_MESSAGE_HANDLER which is a huge pain.

I'm planning (but haven't yet) on adding a new macro for dispatching IPC
messages that passes an additional parameter from the caller to the message
handler. I don't want to add more than one parameter because this will explode
some tuple stuff (I need to add some templates to concatenate tuples). So this
is why I put all the extra info each message handler will need into one param.

https://chromiumcodereview.appspot.com/10572040/diff/8001/ppapi/proxy/dispatc...
File ppapi/proxy/dispatcher.h (right):

https://chromiumcodereview.appspot.com/10572040/diff/8001/ppapi/proxy/dispatc...
ppapi/proxy/dispatcher.h:101: std::vector<IPC::Channel::Listener*> filters_;
Actually, I'm not using anything in this CL yet :)

This file was kind of the odd one out and I couldn't decide which patch to put
it in when I was splitting up this huge one:
http://codereview.chromium.org/10572040. I chose this patch because this is what
allows the hosts to be hooked up. Anyway, you can treat this file's change as a
somewhat separate thing.

[dmichael (off chromium), 2012-06-29 22:54:03]

Some nits, o/w lgtm

http://codereview.chromium.org/10572040/diff/15001/ppapi/host/host_message_co...
File ppapi/host/host_message_context.h (right):

http://codereview.chromium.org/10572040/diff/15001/ppapi/host/host_message_co...
ppapi/host/host_message_context.h:18: HostMessageContext(const
ppapi::proxy::ResourceMessageCallParams& cp)
nit: explicit

http://codereview.chromium.org/10572040/diff/15001/ppapi/host/ppapi_host.cc
File ppapi/host/ppapi_host.cc (right):

http://codereview.chromium.org/10572040/diff/15001/ppapi/host/ppapi_host.cc#n...
ppapi/host/ppapi_host.cc:101: host_factory_->CreateMessageReceived(this, params,
instance,
^^^ Does this compile? I think you mean CreateResourceHost now.

http://codereview.chromium.org/10572040/diff/15001/ppapi/host/resource_host.h
File ppapi/host/resource_host.h (right):

http://codereview.chromium.org/10572040/diff/15001/ppapi/host/resource_host.h...
ppapi/host/resource_host.h:45: // |optional_reply|. However, if you have a reply
more than just the int32_t
missed two |optional_reply|s (45 and 46)