Add support for passing an arbitrary parameter to an IPC message handler. The motivation is for Web…

Add support for passing an arbitrary parameter to an IPC message handler. The motivation is for WebContentsObserver to pass RenderFrameHost* to message handlers easily.

As an example, an observer would look like this:

bool FooWebContentsObserver::OnMessageReceived(
    const IPC::Message& message,
    RenderFrameHost* render_frame_host) {
IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(FooWebContentsObserver, message, RenderFrameHost, render_frame_host)
  IPC_MESSAGE_HANDLER(FooHostMsg_Bar, OnBar)
.
.
.

void FooWebContentsObserver::OnBar(RenderFrameHost* render_frame_host, ...

You can of course still have dispatchers without the extra parameter as before.


This is generalizing the existing code that allows an IPC message handler to have a "const IPC::Message& message) first parameter to get access to the IPC.

Sync IPCs don't support this yet. It's a lot more work because for them we conveniently reuse tuple's DispatchToMethod. This isn't urgent yet, since sync IPCs aren't dispatched on the UI thread for the most part because of NPAPI and Windows, so punting on this for now.

BUG=304341
R=tsepez@chromium.org

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

[jam, 2014-05-13 18:10:21]

btw all the pepper changes are because otherwise I get compiler errors because
of unused parameters, so that's why I added PPAPI_BEGIN_MESSAGE_MAP. It seems
that the the versions in IPC should only be used with IPC_MESSAGE_HANDLER as
they depend on each other's implementation closely.

[Tom Sepez, 2014-05-13 18:36:26]

Wondering if there's some clever way to make it so the additional parameter need
not be specified at the top of the map, but rather as an argument to each
specific hanlder, as in 

IPC_BEGIN_MESSAGE_MAP(FooWebContentsObserver, messge)
 IPC_MESSAGE_HANDLER_WITH_PARAM(FooHostMsg_Bar, OnBar, render_frame_host)
 IPC_MESSAGE_HANDLER(FooHostMsg_Normal, OnNormal)

in this example, render_frame_host is still in scope at the time of macro
expansion ...

[Tom Sepez, 2014-05-13 18:44:11]

https://codereview.chromium.org/283623002/diff/180090/ipc/ipc_message_macros.h
File ipc/ipc_message_macros.h (right):

https://codereview.chromium.org/283623002/diff/180090/ipc/ipc_message_macros....
ipc/ipc_message_macros.h:909: { \
nit: just noticed this.  Could this be:
#define IPC_BEGIN_MESSAGE_MAP(class_name, msg) \
  IPC_BEGIN_MESSAGE_MAP_EX(class_name, msg, true)

https://codereview.chromium.org/283623002/diff/180090/ipc/ipc_message_macros....
ipc/ipc_message_macros.h:917: #define
IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(class_name, msg, param_type, param) \
nit: maybe this take the message_is_ok parameter to enable the others to be
defined in terms of it.

[Tom Sepez, 2014-05-13 18:46:36]

missed one.

https://codereview.chromium.org/283623002/diff/180090/ipc/ipc_message_macros.h
File ipc/ipc_message_macros.h (right):

https://codereview.chromium.org/283623002/diff/180090/ipc/ipc_message_macros....
ipc/ipc_message_macros.h:901: typedef class_name _IpcMessageHandlerClass; \
nit: maybe
#define IPC_BEGIN_MESSAGE_MAP_EX(class_name, msg, msg_is_ok) \
  IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(class_name, msg, param_type, param,
msg_is_ok) \

[Tom Sepez, 2014-05-13 18:48:39]

https://codereview.chromium.org/283623002/diff/180090/chrome/renderer/pepper/...
File chrome/renderer/pepper/pepper_extensions_common_host.cc (right):

https://codereview.chromium.org/283623002/diff/180090/chrome/renderer/pepper/...
chrome/renderer/pepper/pepper_extensions_common_host.cc:81:
PPAPI_BEGIN_MESSAGE_MAP(PepperExtensionsCommonHost, msg)
Could we split this PPAPI_ cleanup into a separate CL that could go first, just
to reduce the heft of this one?

[jam, 2014-05-13 20:31:39]

On 2014/05/13 18:36:26, Tom Sepez wrote:
> Wondering if there's some clever way to make it so the additional parameter
need
> not be specified at the top of the map, but rather as an argument to each
> specific hanlder, as in 
> 
> IPC_BEGIN_MESSAGE_MAP(FooWebContentsObserver, messge)
>  IPC_MESSAGE_HANDLER_WITH_PARAM(FooHostMsg_Bar, OnBar, render_frame_host)
>  IPC_MESSAGE_HANDLER(FooHostMsg_Normal, OnNormal)
> 
> in this example, render_frame_host is still in scope at the time of macro
> expansion ...

I had considered that, but I much prefer changing one place per map instead of
each individual handler's line. To be clear, not because it's less work, but
because it's more similar to how we dispatch IPCs currently, and it's less
work/typing when adding or converting IPCs.

https://codereview.chromium.org/283623002/diff/180090/chrome/renderer/pepper/...
File chrome/renderer/pepper/pepper_extensions_common_host.cc (right):

https://codereview.chromium.org/283623002/diff/180090/chrome/renderer/pepper/...
chrome/renderer/pepper/pepper_extensions_common_host.cc:81:
PPAPI_BEGIN_MESSAGE_MAP(PepperExtensionsCommonHost, msg)
On 2014/05/13 18:48:40, Tom Sepez wrote:
> Could we split this PPAPI_ cleanup into a separate CL that could go first,
just
> to reduce the heft of this one?

Done: https://codereview.chromium.org/281803003/

https://codereview.chromium.org/283623002/diff/180090/ipc/ipc_message_macros.h
File ipc/ipc_message_macros.h (right):

https://codereview.chromium.org/283623002/diff/180090/ipc/ipc_message_macros....
ipc/ipc_message_macros.h:901: typedef class_name _IpcMessageHandlerClass; \
On 2014/05/13 18:46:36, Tom Sepez wrote:
> nit: maybe
> #define IPC_BEGIN_MESSAGE_MAP_EX(class_name, msg, msg_is_ok) \
>   IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(class_name, msg, param_type, param,
> msg_is_ok) \

Done.

https://codereview.chromium.org/283623002/diff/180090/ipc/ipc_message_macros....
ipc/ipc_message_macros.h:909: { \
On 2014/05/13 18:44:12, Tom Sepez wrote:
> nit: just noticed this.  Could this be:
> #define IPC_BEGIN_MESSAGE_MAP(class_name, msg) \
>   IPC_BEGIN_MESSAGE_MAP_EX(class_name, msg, true)

this one I couldn't do since the bool reference in
IPC_BEGIN_MESSAGE_MAP_WITH_PARAM can't be assigned to "true", so I've left as is

https://codereview.chromium.org/283623002/diff/180090/ipc/ipc_message_macros....
ipc/ipc_message_macros.h:917: #define
IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(class_name, msg, param_type, param) \
On 2014/05/13 18:44:12, Tom Sepez wrote:
> nit: maybe this take the message_is_ok parameter to enable the others to be
> defined in terms of it.

Done.

[Tom Sepez, 2014-05-13 20:44:03]

LGTM.