Make IPC_MESSAGE_EXPORT more robust.

Make IPC_MESSAGE_EXPORT more robust.

Currently, files that want to export ipc messages currently do

  #undef IPC_MESSAGE_EXPORT
  #define IPC_MESSAGE_EXPORT  CONTENT_EXPORT

at the top, and files that don't want to export ipc messages just do nothing. This is problematic if a cc file does

  #include "exported_messages.h"
  #include "not_exported_messages.h"

because the second header file picks up the #define from the first file and declares all its messages as exported. In other translation units, where not_exported_messages.h is #included without another header above it, the messages will get default visibility – so the same class ends up with different visibilities in different translation units.

Instead, let ipc_message_macros.h #undef IPC_MESSAGE_EXPORT outside of the include guard, so that all files that don't set the define see it as defined to nothing. (Idea from jam@)

Also disable about:ipc in the component build, since ipc logging adds a dependency from chrome on all ipc message classes, so they would all have to be exported.

BUG=90078
TEST=No linker errors about IPC messages when doing components build on mac. (Other linker errors remain for now.)
TBR=brettw

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

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

[Nico, 2012-02-17 19:48:48]

Look at the ipc_message_macros.h change and one random _messages.h file to get
the gist of this – the rest of the CL is mechanical.

[dmac, 2012-02-17 23:01:50]

lgtm

[Nico, 2012-02-17 23:18:01]

+brettw for OWNERS

[commit-bot: I haz the power, 2012-02-17 23:18:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/9425006/2001

[commit-bot: I haz the power, 2012-02-17 23:27:15]

Try job failure for 9425006-2001 (retry) on android for steps "build, Compile".
It's a second try, previously, steps "build, Compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android&nu...

[commit-bot: I haz the power, 2012-02-17 23:32:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/9425006/1053

[commit-bot: I haz the power, 2012-02-17 23:35:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/9425006/1055

[commit-bot: I haz the power, 2012-02-18 00:28:44]

Try job failure for 9425006-1055 (retry) on mac_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-02-18 00:33:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/9425006/7006

[Nico, 2012-02-18 01:26:16]

Patch set "jammify" is a lot simpler, thanks to a suggestion by jam. I'm going
to land that.

[commit-bot: I haz the power, 2012-02-18 01:27:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/9425006/111

[jam, 2012-02-18 01:44:24]

lgtm, thanks!

[commit-bot: I haz the power, 2012-02-18 08:20:30]

Change committed as 122689

[Nico, 2012-02-20 23:58:24]

This got reverted because it broke the windows shared builder.
about_ipc_dialog.cc includes a file called all_messages.h, which includes all
ipc messages. Since about_ipc_dialog.cc is in 'chrome', this means all ipc
messages not in chrome need to have visible symbols in the components build.

Options:
1.) Always export ipc message classes in the components build. This is what
effectively happened before this CL, accidentally.

2.) Disable about:ipc in the components build.

3.) Remove the about:ipc code (It's currently only enabled in debug builds, and
debug builds are under discussion to become component-build-built by default, so
this is somewhat similar to 2).

Opinions?

[Nico, 2012-02-21 02:56:48]

On 2012/02/20 23:58:24, Nico wrote:
> This got reverted because it broke the windows shared builder.
> about_ipc_dialog.cc includes a file called all_messages.h, which includes all
> ipc messages. Since about_ipc_dialog.cc is in 'chrome', this means all ipc
> messages not in chrome need to have visible symbols in the components build.
> 
> Options:
> 1.) Always export ipc message classes in the components build. This is what
> effectively happened before this CL, accidentally.

That's actually not possible on windows due to import/export.

> 2.) Disable about:ipc in the components build.

That's what I changed this CL to do, since it's the least intrusive change.
Please take another look.

[jam, 2012-02-21 03:42:57]

On 2012/02/21 02:56:48, Nico wrote:
> On 2012/02/20 23:58:24, Nico wrote:
> > This got reverted because it broke the windows shared builder.
> > about_ipc_dialog.cc includes a file called all_messages.h, which includes
all
> > ipc messages. Since about_ipc_dialog.cc is in 'chrome', this means all ipc
> > messages not in chrome need to have visible symbols in the components build.
> > 
> > Options:
> > 1.) Always export ipc message classes in the components build. This is what
> > effectively happened before this CL, accidentally.
> 
> That's actually not possible on windows due to import/export.
> 
> > 2.) Disable about:ipc in the components build.
> 
> That's what I changed this CL to do, since it's the least intrusive change.
> Please take another look.

I agree this sounds like the best solution (even if windows worked). but why do
you need to modify the message files?

[Nico, 2012-02-21 03:46:48]

> I agree this sounds like the best solution (even if windows worked). but why
do
> you need to modify the message files?

Because they are used by various unit tests. net/ has NET_EXPORT_PRIVATE for
tests, but content/ doesn't seem to make this distinction.

[jam, 2012-02-21 04:13:10]

On 2012/02/21 03:46:48, Nico wrote:
> > I agree this sounds like the best solution (even if windows worked). but why
> do
> > you need to modify the message files?
> 
> Because they are used by various unit tests. net/ has NET_EXPORT_PRIVATE for
> tests, but content/ doesn't seem to make this distinction.

lgtm

[jam, 2012-02-21 04:13:21]

http://codereview.chromium.org/9425006/diff/2019/ipc/ipc_message.h
File ipc/ipc_message.h (right):

http://codereview.chromium.org/9425006/diff/2019/ipc/ipc_message.h#newcode15
ipc/ipc_message.h:15: #if !defined(NDEBUG) && !defined(COMPONENT_BUILD)
nit: please add a comment about why this is done for component build

[commit-bot: I haz the power, 2012-02-21 14:36:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/9425006/1074

[commit-bot: I haz the power, 2012-02-21 16:04:52]

Change committed as 122828