Reverting in an attempt to fix Win Builder 2010 (dbg).

Reverting in an attempt to fix Win Builder 2010 (dbg).


Revert 122689 - 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@)

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

Review URL: http://codereview.chromium.org/9425006

TBR=thakis@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=122698

[Nico, 2012-02-20 20:53:09]

LGTM

On Sat, Feb 18, 2012 at 3:40 PM,  <jhawkins@chromium.org> wrote:
> Reviewers: Nico,
>
> Description:
> Reverting in an attempt to fix Win Builder 2010 (dbg).
>
>
> Revert 122689 - 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@)
>
> BUG=90078
> TEST=No linker errors about IPC messages when doing components build on mac.
> (Other linker errors remain for now.)
> TBR=brettw
>
> Review URL: http://codereview.chromium.org/9425006
>
> TBR=thakis@chromium.org
>
> Please review this at https://chromiumcodereview.appspot.com/9355029/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     ipc/ipc_message_macros.h
>
>
> Index: ipc/ipc_message_macros.h
> ===================================================================
> --- ipc/ipc_message_macros.h    (revision 122697)
> +++ ipc/ipc_message_macros.h    (working copy)
> @@ -1,4 +1,4 @@
> -// Copyright (c) 2012 The Chromium Authors. All rights reserved.
> +// Copyright (c) 2011 The Chromium Authors. All rights reserved.
>  // Use of this source code is governed by a BSD-style license that can be
>  // found in the LICENSE file.
>
> @@ -173,14 +173,6 @@
>  //     ViewHostMsg_SyncMessageName::WriteReplyParams(reply_msg, out1,
> out2);
>  //     Send(reply_msg);
>
> -// Files that want to export their ipc messages should do
> -//   #undef IPC_MESSAGE_EXPORT
> -//   #define IPC_MESSAGE_EXPORT VISIBILITY_MACRO
> -// after including this header, but before using any of the macros below.
> -// (This needs to be before the include guard.)
> -#undef IPC_MESSAGE_EXPORT
> -#define IPC_MESSAGE_EXPORT
> -
>  #ifndef IPC_IPC_MESSAGE_MACROS_H_
>  #define IPC_IPC_MESSAGE_MACROS_H_
>
> @@ -192,6 +184,11 @@
>  #include "ipc/ipc_message_utils_impl.h"
>  #endif
>
> +// Override this to force message classes to be exported.
> +#ifndef IPC_MESSAGE_EXPORT
> +#define IPC_MESSAGE_EXPORT
> +#endif
> +
>  // Macros for defining structs.  May be subsequently redefined.
>  #define IPC_STRUCT_BEGIN(struct_name) \
>   IPC_STRUCT_BEGIN_WITH_PARENT(struct_name, IPC::NoParams)
>
>