New issue
Advanced search Search tips

Issue 911330 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 5
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Clean up export macros in Mojo code

Project Member Reported by rockot@google.com, Dec 3

Issue description

In Chrome we have component builds for developers to speed up build time by linking the executable as a bunch of smaller shared library targets. To support this we have a special "component" build target type which build as a static library when is_component_build = false but build as a shared library when is_component_build = true.[1]

Linking against shared libraries requires dealing with symbol imports and exports, and we use preprocessor macros to deal with this as painlessly as possible. The old way of doing things required writing a header specifically for your component target and annotating all your symbols with a macro defined in that header.[2]

The new way is to use the generic COMPONENT_EXPORT macro defined in base/component_export.h, parameterized with a token that is derived from your build target definition.[3] This is nice because it avoids the need for special headers scattered all over the tree just to support component builds.

There are still tons of targets around the tree using the old thing instead of the new thing. It would be nice to use the new thing for the //mojo/public/cpp/bindings:bindings target.

An example CL which does this for the //ipc:ipc target can be found here: https://chromium-review.googlesource.com/c/chromium/src/+/876884

[1] https://chromium.googlesource.com/chromium/src/+/HEAD/docs/component_build.md
[2] https://chromium.googlesource.com/chromium/src/+/HEAD/docs/component_build.md#chrome_s-deprecated-pattern-for-exports
[3] https://chromium.googlesource.com/chromium/src/+/HEAD/docs/component_build.md#exporting-and-importing-symbols
 
Owner: staphany@chromium.org
Status: Assigned (was: Available)
This is a purely mechanical change that would a nice cleanup to have. Maybe an OK thing to start with?
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d3698bab565eea6130600978e694d5f04fa6ed75

commit d3698bab565eea6130600978e694d5f04fa6ed75
Author: Staphany Park <staphany@chromium.org>
Date: Tue Dec 04 21:15:07 2018

mojo: Clean up export macros.

Migrates the bindings library to COMPONENT_EXPORT macro.

Bug:  911330 
Change-Id: Ibe26f0a6d124dbdfe739d423810eb547631b8162
Reviewed-on: https://chromium-review.googlesource.com/c/1359869
Commit-Queue: Staphany Park <staphany@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#613693}
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/BUILD.gn
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/associated_binding.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/associated_interface_ptr.h
[delete] https://crrev.com/acc79f1cd39645d800757d7cfb09e4740a89ceab/mojo/public/cpp/bindings/bindings_export.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/connector.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/filter_chain.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/interface_endpoint_client.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/lib/binding_state.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/lib/control_message_handler.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/lib/control_message_proxy.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/lib/interface_ptr_state.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/lib/multiplex_router.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/lib/native_struct_serialization.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/lib/test_random_mojo_delays.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/pipe_control_message_handler.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/pipe_control_message_proxy.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/sequence_local_sync_event_watcher.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/sync_call_restrictions.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/sync_event_watcher.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/sync_handle_registry.h
[modify] https://crrev.com/d3698bab565eea6130600978e694d5f04fa6ed75/mojo/public/cpp/bindings/sync_handle_watcher.h

Status: Fixed (was: Assigned)
Fixed in https://crrev.com/c/1359869.

Sign in to add a comment