Simplify the magic required to create IPC message headers a bit.

ipc: Simplify the magic required to create IPC message headers.This gets rid of having to include the files in a magic place because of xcode dependency issues, and just makes it simpler to create new IPC message classes.  It also gets rid of including the X_messages_internal.h file multiple times, which simplifies things and should make the build a little faster.

In a future change, I will remove the "internal.h" files since they're no longer needed.

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

[jam, 2010-12-07 04:34:26]

actually hold off on this review for now, I think I found a way to get
rid of the multiple passes

On Monday, December 6, 2010,  <jam@chromium.org> wrote:
> Reviewers: brettw,
>
> Description:
> Simplify the magic required to create IPC message headers a bit.
>
> Please review this at http://codereview.chromium.org/5526008/
>
> SVN Base: svn://chrome-svn/chrome/trunk/src/
>
> Affected files:
>   M     chrome/browser/importer/importer_messages.h
>   M     chrome/browser/importer/importer_messages.cc
>   M     chrome/browser/importer/importer_messages_internal.h
>   chrome/common/automation_messages.h
>   M     chrome/common/automation_messages.cc
>   M     chrome/common/automation_messages_internal.h
>   M     chrome/common/devtools_messages.h
>   M     chrome/common/devtools_messages.cc
>   M     chrome/common/devtools_messages_internal.h
>   M     chrome/common/gpu_messages.h
>   M     chrome/common/gpu_messages.cc
>   M     chrome/common/gpu_messages_internal.h
>   M     chrome/common/nacl_messages.h
>   M     chrome/common/nacl_messages.cc
>   M     chrome/common/nacl_messages_internal.h
>   M     chrome/common/plugin_messages.h
>   M     chrome/common/plugin_messages.cc
>   M     chrome/common/plugin_messages_internal.h
>   M     chrome/common/render_messages.h
>   M     chrome/common/render_messages.cc
>   M     chrome/common/render_messages_internal.h
>   M     chrome/common/service_messages.h
>   M     chrome/common/service_messages.cc
>   M     chrome/common/service_messages_internal.h
>   M     chrome/common/utility_messages.h
>   M     chrome/common/utility_messages.cc
>   M     chrome/common/utility_messages_internal.h
>   M     chrome/common/worker_messages.h
>   M     chrome/common/worker_messages.cc
>   M     chrome/common/worker_messages_internal.h
>   M     ipc/ipc_fuzzing_tests.cc
>   M     ipc/ipc_message_impl_macros.h
>   M     ipc/ipc_message_macros.h
>   M     ipc/ipc_sync_channel_unittest.cc
>   M     ipc/ipc_sync_message_unittest.h
>   M     ipc/ipc_sync_message_unittest.cc
>   M     ipc/sync_socket_unittest.cc
>   M     ppapi/proxy/ppapi_messages.h
>   M     ppapi/proxy/ppapi_messages.cc
>   M     ppapi/proxy/ppapi_messages_internal.h
>
>
>

[jam, 2010-12-08 00:46:09]

changing reviewers since Matt/Elliot are the ones most familiar with this. 
guys, only one of you needs to review this, so whoever has time (sooner the
better since I have a lot of changes that depend on this :) ).

I plan on adding #line in automation_messages_internal.h so that the message IDs
match the old builds.  I want to get this reviewed in the meantime, but I'll
make sure to test with reference builds before commit.

[jam, 2010-12-08 01:44:10]

http://codereview.chromium.org/5526008/diff/40002/ipc/ipc_message_macros.h
File ipc/ipc_message_macros.h (right):

http://codereview.chromium.org/5526008/diff/40002/ipc/ipc_message_macros.h#ne...
ipc/ipc_message_macros.h:306: enum { ID = (IPC_MESSAGE_START << 16) + __LINE__
}; \
note: the only changes I made here are fixing spaces at the beginning of lines,
making the enum use IPC_MESSAGE_START, and adding the IPC_MESSAGE_X_EXTRA at the
end of each macro

[jam, 2010-12-08 18:19:24]

Evan, can you take a look?  Thanks.

[Elliot Glaysher, 2010-12-08 19:26:17]

In general, I like this change. You might want to:

http://codereview.chromium.org/5526008/diff/64003/ipc/ipc_message_macros.h
File ipc/ipc_message_macros.h (right):

http://codereview.chromium.org/5526008/diff/64003/ipc/ipc_message_macros.h#ne...
ipc/ipc_message_macros.h:47: #if defined(IPC_MESSAGE_IMPL)
Documentation on how this works would be appreciated. I was really confused that
this section is either supposed to pull in ipc_message_impl_macros.h OR your
_EXTRA methods that define logging. (That may have been my preconceptions about
how the IPC message definitions are supposed to work).

Maybe pull all the elifs into their own .h file? I dunno.

http://codereview.chromium.org/5526008/diff/64003/ipc/ipc_message_macros.h#ne...
ipc/ipc_message_macros.h:73: LoggerRegisterHelper##msg_class
g_LoggerRegisterHelper##msg_class;
Is there anyway that the method definition and the g_LoggerRegisterHelper##XXX
can be rolled into the impl macros?

[jam, 2010-12-08 19:36:27]

Thanks for taking a look, I know this is not the nicest change to look over
:)

On Wed, Dec 8, 2010 at 11:26 AM, <erg@chromium.org> wrote:

> In general, I like this change. You might want to:
>
>
> http://codereview.chromium.org/5526008/diff/64003/ipc/ipc_message_macros.h
>
> File ipc/ipc_message_macros.h (right):
>
>
>
http://codereview.chromium.org/5526008/diff/64003/ipc/ipc_message_macros.h#ne...
> ipc/ipc_message_macros.h:47: #if defined(IPC_MESSAGE_IMPL)
> Documentation on how this works would be appreciated. I was really
> confused that this section is either supposed to pull in
> ipc_message_impl_macros.h OR your _EXTRA methods that define logging.
> (That may have been my preconceptions about how the IPC message
> definitions are supposed to work).
>

Sure I can add a comment here.  I had thought that the comment at the top is
enough, but maybe not.


>
> Maybe pull all the elifs into their own .h file? I dunno.
>

I thought about it, but I didn't think it would make much of a difference.
 This code is just grungy, but it really doesn't need to be touched/updated.


>
>
>
http://codereview.chromium.org/5526008/diff/64003/ipc/ipc_message_macros.h#ne...
> ipc/ipc_message_macros.h:73: LoggerRegisterHelper##msg_class
> g_LoggerRegisterHelper##msg_class;
> Is there anyway that the method definition and the
> g_LoggerRegisterHelper##XXX can be rolled into the impl macros?


I tried to do this, but I don't think it's possible, since we don't want to
define any of the logging stuff unless logging is enabled.  That can only be
determined with the #if check, but we can't put that inside the macro.

>
>
> http://codereview.chromium.org/5526008/
>

[Elliot Glaysher, 2010-12-08 19:39:51]

LGTM