Skip to content

GitLab

T
tangled
Commit 41449ea8 authored by Nico Weber's avatar Nico Weber
Browse files
Options

Revert "Support typemaps inlined as GN targets" 

This reverts commit 2714fec9.

Reason for revert: Breaks GN's MSVC generator, https://crbug.com/1059666

Original change's description:
> Support typemaps inlined as GN targets
> 
> This introduces support for typemaps specified as GN targets, with new
> cpp_typemaps and blink_cpp_typemaps variables that mojom() targets can
> used to reference their typemap rules.
> 
> The underlying work to use the typemap configuration is shared with the
> existing typemap infrastructure, but the net result is that we no longer
> need separate .typemap files or global "bindings configurations" once
> everything is converted to this approach.
> 
> Bug: 1059389
> Change-Id: Id2e5fe765d3c7600a3f50e337fb693f1b3a3cc0a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090716
> Commit-Queue: Ken Rockot <rockot@google.com>
> Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#747990}

TBR=rockot@google.com,oksamyt@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1059389
Change-Id: I060cf9d20c7d03c930296a9e777ab5e8c4e2a7b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2093136Reviewed-by: default avatarNico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748211}
parent 9a0bcaf8
Hide whitespace changes
Inline Side-by-side
Showing with 146 additions and 423 deletions
mojo/public/cpp/bindings/README.md
View file @ 41449ea8
......@@ -1307,6 +1307,22 @@ class Canvas {
The correct answer is, "Yes! That would be nice!" And fortunately, it can!
### Global Configuration
While this feature is quite powerful, it introduces some unavoidable complexity
into build system. This stems from the fact that type-mapping is an inherently
viral concept: if `gfx::mojom::Rect` is mapped to `gfx::Rect` anywhere, the
mapping needs to apply *everywhere*.
For this reason we have a few global typemap configurations defined in
[chromium_bindings_configuration.gni](https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/chromium_bindings_configuration.gni)
and
[blink_bindings_configuration.gni](https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/blink_bindings_configuration.gni). These configure the two supported [variants](#Variants) of Mojom generated
bindings in the repository. Read more on this in the sections that follow.
For now, let's take a look at how to express the mapping from `gfx::mojom::Rect`
to `gfx::Rect`.
### Defining `StructTraits`
In order to teach generated bindings code how to serialize an arbitrary native
......@@ -1319,11 +1335,11 @@ A valid specialization of `StructTraits` MUST define the following static
methods:
* A single static accessor for every field of the Mojom struct, with the exact
same name as the struct field. These accessors must all take a (preferably
const) ref to an object of the native type, and must return a value compatible
with the Mojom struct field's type. This is used to safely and consistently
extract data from the native type during message serialization without
incurring extra copying costs.
same name as the struct field. These accessors must all take a const ref to
an object of the native type, and must return a value compatible with the
Mojom struct field's type. This is used to safely and consistently extract
data from the native type during message serialization without incurring extra
copying costs.
* A single static `Read` method which initializes an instance of the the native
type given a serialized representation of the Mojom struct. The `Read` method
......@@ -1420,96 +1436,89 @@ struct StructTraits<url::mojom::UrlDataView, GURL> {
We've defined the `StructTraits` necessary, but we still need to teach the
bindings generator (and hence the build system) about the mapping. To do this we
must create a new `mojom_cpp_typemap` target in GN. We can define this
anywhere, but conventionally we will put it in the same BUILD.gn that defines
the `mojom` target which will use it.
```
mojom_cpp_typemap("geometry_typemap") {
types = [
{
mojom = "gfx.mojom.Rect"
cpp = "::gfx::Rect"
}
]
public_headers = [
"//ui/gfx/geometry/mojo/geometry_mojom_traits.h",
"//ui/gfx/geometry/rect.h",
]
# NOTE: We use |traits_sources| for simplicity in this example, but see the
# section below for a more robust alternative.
traits_sources = [
"//ui/gfx/geometry/mojo/geometry_mojom_traits.cc",
"//ui/gfx/geometry/mojo/geometry_mojom_traits.h",
]
public_deps = [ "//ui/gfx/geometry" ]
}
```
See the `mojom_cpp_typemap` documentation in
[mojom.gni](https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/mojom.gni)
for details on the above definition and other supported parameters.
Now that we have the typemap target defined we need to reference it in the
`mojom` GN target which defines gfx.mojom.Rect. We can do that trivially by
adding to the `cpp_typemaps` list:
```
mojom("mojom") {
sources = [
"rect.mojom",
...
]
cpp_typemaps = [ ":geometry_typemap" ]
...
}
```
For the Blink variant of bindings, add to the `blink_cpp_typemaps` list instead.
### Type Mapping Without `traits_sources`
Inlining the traits sources into the typemap definition means that the sources
get baked directly into the corresponding `mojom` target's sources. This can
be problematic in a few scenarios:
* You want to use the same typemap for both Blink and non-Blink bindings
* Your `mojom` target is actually a `mojom_component` target
For these cases, it is recommended that you define a separate `component` target
for your typemap traits, and reference this in the `public_deps` of the typemap:
```
component("geometry_mojom_traits") {
sources = [
"//ui/gfx/geometry/mojo/geometry_mojom_traits.cc",
"//ui/gfx/geometry/mojo/geometry_mojom_traits.h",
]
# The header of course needs corresponding COMPONENT_EXPORT() tags.
defines = [ "IS_GEOMETRY_MOJOM_TRAITS_IMPL" ]
...
}
```
mojom_cpp_typemap("geometry_typemap") {
# Same as the example above.
types = [ ... ]
public_headers = [ ... ]
# NOTE: No |traits_sources|.
public_deps = [
":geometry_mojom_traits",
"//ui/gfx/geometry",
]
}
must create a **typemap** file, which uses familiar GN syntax to describe the
new type mapping.
Let's place this `geometry.typemap` file alongside our Mojom file:
```
mojom = "//ui/gfx/geometry/mojo/geometry.mojom"
os_whitelist = [ "android" ]
public_headers = [ "//ui/gfx/geometry/rect.h" ]
traits_headers = [ "//ui/gfx/geometry/mojo/geometry_mojom_traits.h" ]
sources = [
"//ui/gfx/geometry/mojo/geometry_mojom_traits.cc",
"//ui/gfx/geometry/mojo/geometry_mojom_traits.h",
]
public_deps = [ "//ui/gfx/geometry" ]
type_mappings = [
"gfx.mojom.Rect=::gfx::Rect",
]
```
Let's look at each of the variables above:
* `mojom`: Specifies the `mojom` file to which the typemap applies. Many
typemaps may apply to the same `mojom` file, but any given typemap may only
apply to a single `mojom` file.
* `os_whitelist`: Optional list of specific platforms this typemap
should be constrained to.
* `public_headers`: Additional headers required by any code which would depend
on the Mojom definition of `gfx.mojom.Rect` now that the typemap is applied.
Any headers required for the native target type definition should be listed
here.
* `traits_headers`: Headers which contain the relevant `StructTraits`
specialization(s) for any type mappings described by this file.
* `sources`: Any implementation sources needed for the `StructTraits`
definition. These sources are compiled directly into the generated C++
bindings target for a `mojom` file applying this typemap.
* `public_deps`: Target dependencies exposed by the `public_headers` and
`traits_headers`.
* `deps`: Target dependencies exposed by `sources` but not already covered by
`public_deps`.
* `type_mappings`: A list of type mappings to be applied for this typemap. The
strings in this list are of the format `"MojomType=CppType"`, where
`MojomType` must be a fully qualified Mojom typename and `CppType` must be a
fully qualified C++ typename. Additional attributes may be specified in square
brackets following the `CppType`:
* `move_only`: The `CppType` is move-only and should be passed by value
in any generated method signatures. Note that `move_only` is transitive,
so containers of `MojomType` will translate to containers of `CppType`
also passed by value.
* `copyable_pass_by_value`: Forces values of type `CppType` to be passed by
value without moving them. Unlike `move_only`, this is not transitive.
* `nullable_is_same_type`: By default a non-nullable `MojomType` will be
mapped to `CppType` while a nullable `MojomType?` will be mapped to
`base::Optional<CppType>`. If this attribute is set, the `base::Optional`
wrapper is omitted for nullable `MojomType?` values, but the
`StructTraits` definition for this type mapping must define additional
`IsNull` and `SetToNull` methods.
* `force_serialize`: The typemap is incompatible with lazy serialization
(e.g. consider a typemap to a `base::StringPiece`, where retaining a
copy is unsafe). Any messages carrying the type will be forced down the
eager serailization path.
Now that we have the typemap file we need to add it to a local list of typemaps
that can be added to the global configuration. We create a new
`//ui/gfx/typemaps.gni` file with the following contents:
```
typemaps = [
"//ui/gfx/geometry/mojo/geometry.typemap",
]
```
And finally we can reference this file in the global default (Chromium) bindings
configuration by adding it to `_typemap_imports` in
[chromium_bindings_configuration.gni](https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/chromium_bindings_configuration.gni):
```
_typemap_imports = [
...,
"//ui/gfx/typemaps.gni",
...,
]
```
### StructTraits Reference
......@@ -1675,31 +1684,13 @@ class TableImpl : public db::mojom::blink::Table {
```
In addition to using different C++ types for builtin strings, arrays, and maps,
the custom typemaps applied to Blink bindings are managed separately from
regular bindings.
`mojom` targets support a `blink_cpp_typemaps` parameter in addition to the
regular `cpp_typemaps`. This lists the typemaps to apply to Blink bindings.
To depend specifically on generated Blink bindings, reference
`${target_name}_blink`. So for example, with the definition:
```
# In //foo/mojom
mojom("mojom") {
sources = [
"db.mojom",
]
}
```
C++ sources can depend on the Blink bindings by depending on
`"//foo/mojom:mojom_blink"`.
the global typemap configuration for default and "blink" variants are completely
separate. To add a typemap for the Blink configuration, you can modify
[blink_bindings_configuration.gni](https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/blink_bindings_configuration.gni).
Finally note that both bindings variants share some common definitions which are
unaffected by differences in the type-mapping configuration (like enums, and
structures describing serialized object formats). These definitions are
generated in *shared* sources:
All variants share some definitions which are unaffected by differences in the
type mapping configuration (enums, for example). These definitions are generated
in *shared* sources:
```
out/gen/sample/db.mojom-shared.cc
......@@ -1711,9 +1702,22 @@ Including either variant's header (`db.mojom.h` or `db.mojom-blink.h`)
implicitly includes the shared header, but may wish to include *only* the shared
header in some instances.
C++ sources can depend on shared sources only, by referencing the
`"${target_name}_shared"` target, e.g. `"//foo/mojom:mojom_shared"` in the
example above.
Finally, note that for `mojom` GN targets, there is implicitly a corresponding
`mojom_{variant}` target defined for any supported bindings configuration. So
for example if you've defined in `//sample/BUILD.gn`:
```
import("mojo/public/tools/bindings/mojom.gni")
mojom("mojom") {
sources = [
"db.mojom",
]
}
```
Code in Blink which wishes to use the generated Blink-variant definitions must
depend on `"//sample:mojom_blink"`.
## Versioning Considerations
......
mojo/public/tools/bindings/generate_type_mappings.py
View file @ 41449ea8
......@@ -81,36 +81,6 @@ def ParseTypemapArgs(args):
return result
def LoadCppTypemapConfigs(paths):
_SUPPORTED_KEYS = set([
'mojom', 'cpp', 'public_headers', 'copyable_pass_by_value',
'force_serialize', 'hashable', 'move_only', 'nullable_is_same_type'
])
configs = {}
for path in paths:
with open(path) as f:
config = json.load(f)
for entry in config['types']:
for key in entry.iterkeys():
if key not in _SUPPORTED_KEYS:
raise IOError('Invalid typemap property "%s" when processing %s' %
(key, path))
configs[entry['mojom']] = {
'typename': entry['cpp'],
'public_headers': config.get('public_headers', []),
'copyable_pass_by_value': entry.get('copyable_pass_by_value',
False),
'force_serialize': entry.get('force_serialize', False),
'hashable': entry.get('hashable', False),
'move_only': entry.get('move_only', False),
'nullable_is_same_type': entry.get('nullable_is_same_type', False),
'non_copyable_non_movable': False,
}
return configs
def ParseTypemap(typemap):
values = {'type_mappings': [], 'public_headers': [], 'traits_headers': []}
for line in typemap.split('\n'):
......@@ -167,21 +137,12 @@ def main():
default=[],
help=('A path to another JSON typemap to merge into the output. '
'This may be repeated to merge multiple typemaps.'))
parser.add_argument(
'--cpp-typemap-config',
type=str,
action='append',
default=[],
dest='cpp_config_paths',
help=('A path to a single JSON-formatted typemap config as emitted by'
'GN when processing a mojom_cpp_typemap build rule.'))
parser.add_argument('--output',
type=str,
required=True,
help='The path to which to write the generated JSON.')
params, typemap_params = parser.parse_known_args()
typemaps = ParseTypemapArgs(typemap_params)
typemaps.update(LoadCppTypemapConfigs(params.cpp_config_paths))
missing = [path for path in params.dependency if not os.path.exists(path)]
if missing:
raise IOError('Missing dependencies: %s' % ', '.join(missing))
......
mojo/public/tools/bindings/mojom.gni
View file @ 41449ea8
......@@ -220,14 +220,6 @@ if (enable_mojom_typemapping) {
# will still be generated even when |cpp_only| is set to |true|, unless
# you also set |enable_fuzzing| to |false| in your mojom target.
#
# cpp_typemaps (optional)
# A list of mojom_cpp_typemap targets to be applied to the generated
# C++ bindings for this mojom target. Note that this only applies to the
# non-Blink variant of generated C++ bindings.
#
# blink_cpp_typemaps (optional)
# Same as above, but for the Blink variant of generated C++ bindings.
#
# generate_java (optional)
# If set to true, Java bindings are generated for Android builds. If
# |cpp_only| is set to true, it overrides this to prevent generation of
......@@ -386,13 +378,6 @@ template("mojom") {
assert(defined(invoker.component_deps_blink))
}
# Type-mapping may be disabled or we may not generate C++ bindings.
not_needed(invoker,
[
"cpp_typemaps",
"blink_cpp_typemaps",
])
require_full_cpp_deps =
!defined(invoker.disallow_native_types) ||
!invoker.disallow_native_types || !defined(invoker.disallow_interfaces) ||
......@@ -859,57 +844,22 @@ template("mojom") {
cpp_only = true
}
cpp_typemap_targets = []
export_defines = []
export_defines_overridden = false
force_source_set = false
if (defined(bindings_configuration.for_blink) &&
bindings_configuration.for_blink) {
if (defined(invoker.blink_cpp_typemaps)) {
cpp_typemap_targets = invoker.blink_cpp_typemaps
}
if (defined(invoker.export_define_blink)) {
export_defines_overridden = true
export_defines = [ invoker.export_define_blink ]
force_source_set = true
}
} else {
if (defined(invoker.cpp_typemaps)) {
cpp_typemap_targets = invoker.cpp_typemaps
}
if (defined(invoker.export_define)) {
export_defines_overridden = true
export_defines = [ invoker.export_define ]
force_source_set = true
}
} else if (defined(invoker.export_define)) {
export_defines_overridden = true
export_defines = [ invoker.export_define ]
force_source_set = true
}
cpp_typemap_configs = []
foreach(typemap_target, cpp_typemap_targets) {
typemap_config_dir = get_label_info(typemap_target, "target_gen_dir")
typemap_config_name = get_label_info(typemap_target, "name")
_config = {
}
# Targets generated by the typemap target's template. These consolidate
# the typemap's specified deps so that our generated sources can inherit
# them.
_config.deps = [ "${typemap_target}__source_deps" ]
_config.public_deps = [ "${typemap_target}__source_public_deps" ]
_config.configs = [ "${typemap_target}__source_configs" ]
_config.public_configs = [ "${typemap_target}__source_public_configs" ]
# This is the file written out by GN when processing the typemap target's
# definition (during build generation, NOT during build). We feed this
# into the bindings generator.
_config.config_filename =
"$typemap_config_dir/${typemap_config_name}.typemap_config"
cpp_typemap_configs += [ _config ]
}
not_needed([ "cpp_typemap_configs" ])
if (!export_defines_overridden && defined(invoker.component_macro_prefix)) {
output_name_override =
"${invoker.component_output_prefix}${variant_suffix}"
......@@ -1011,13 +961,6 @@ template("mojom") {
}
}
foreach(config, cpp_typemap_configs) {
_typemap = {
}
_typemap.config = config
active_typemaps += [ _typemap ]
}
generator_target_name = "${target_name}${variant_suffix}__generator"
action(generator_target_name) {
visibility = [ ":*" ]
......@@ -1135,9 +1078,6 @@ template("mojom") {
dependency_path,
]
}
foreach(typemap_target, cpp_typemap_targets) {
deps += [ "${typemap_target}__json_config" ]
}
if (sources_list != []) {
# TODO(sammc): Pass the typemap description in a file to avoid command
......@@ -1147,38 +1087,25 @@ template("mojom") {
_typemap_config = {
}
_typemap_config = typemap.config
# Old style typemaps are passed inline on the command line
if (defined(_typemap_config.type_mappings)) {
typemap_description += [ "--start-typemap" ]
if (defined(_typemap_config.public_headers)) {
foreach(value, _typemap_config.public_headers) {
typemap_description += [ "public_headers=$value" ]
}
typemap_description += [ "--start-typemap" ]
if (defined(_typemap_config.public_headers)) {
foreach(value, _typemap_config.public_headers) {
typemap_description += [ "public_headers=$value" ]
}
if (defined(_typemap_config.traits_headers)) {
foreach(value, _typemap_config.traits_headers) {
typemap_description += [ "traits_headers=$value" ]
}
}
foreach(value, _typemap_config.type_mappings) {
typemap_description += [ "type_mappings=$value" ]
}
# The typemap configuration files are not actually used as inputs here
# but this establishes a necessary build dependency to ensure that
# typemap changes force a rebuild of affected targets.
if (defined(typemap.filename)) {
inputs += [ typemap.filename ]
}
if (defined(_typemap_config.traits_headers)) {
foreach(value, _typemap_config.traits_headers) {
typemap_description += [ "traits_headers=$value" ]
}
} else {
assert(defined(_typemap_config.config_filename))
inputs += [ _typemap_config.config_filename ]
typemap_description += [
"--cpp-typemap-config",
rebase_path(_typemap_config.config_filename, root_build_dir),
]
}
foreach(value, _typemap_config.type_mappings) {
typemap_description += [ "type_mappings=$value" ]
}
# The typemap configuration files are not actually used as inputs here
# but this establishes a necessary build dependency to ensure that
# typemap changes force a rebuild of affected targets.
inputs += [ typemap.filename ]
}
args += typemap_description
}
......@@ -1656,172 +1583,3 @@ template("mojom_component") {
component_macro_prefix = invoker.macro_prefix
}
}
# Specifies a type-mapping configuration for one or more mojom types to native
# C++ types. The mojom() target which defines the named mojom type map reference
# this typemap target in either (or both) its |cpp_typemaps| or
# |blink_cpp_typemaps| lists in order to apply the typemap to generated C++
# bindings.
#
# NOTE: If no mojom() target references a typemap target in its definition, the
# typemap has no interesting effect on the build.
#
# Required parameters for mojom_cpp_typemap():
#
# types
# A list of type specifications for this typemap. Each type specification
# is a GN scope, which can be expressed with the following syntax:
#
# {
# mojom = "foo.mojom.Bar"
# cpp = "::foo::LegitBar"
# move_only = true
# # etc...
# }
#
# Each type specification supports the following values:
#
# mojom (required)
# The fully qualified name of a mojom type to be mapped. This is a
# string like "foo.mojom.Bar".
#
# cpp (required)
# The fully qualified name of the C++ type to which the mojom type
# should be mapped in generated bindings. This is a string like
# "::base::Value" or "std::vector<::base::Value>".
#
# move_only (optional)
# A boolean value (default false) which indicates whether the C++
# type is move-only. If true, generated bindings will pass the type
# by value and use std::move() at call sites.
#
# copyable_pass_by_value (optional)
# A boolean value (default false) which effectively indicates
# whether the C++ type is very cheap to copy. If so, generated
# bindings will pass by value but not use std::move() at call sites.
#
# nullable_is_same_type (optional)
# A boolean value (default false) which indicates that the C++ type
# has some baked-in semantic notion of a "null" state. If true, the
# traits for the type must define IsNull and SetToNull methods.
#
# When false, nullable fields are represented by wrapping the C++
# type with base::Optional, and null values are simply
# base::nullopt.
#
# hashable (optional)
# A boolean value (default false) indicating whether the C++ type is
# hashable. Set to true if true AND needed (i.e. you need to use the
# type as the key of a mojom map).
#
# force_serialize (optional)
# A boolean value (default false) which disables lazy serialization
# of the typemapped type if lazy serialization is enabled for the
# mojom target applying this typemap.
#
#
# Optional parameters for mojom_cpp_typemap():
#
# public_headers (optional)
# Headers which must be included in the generated mojom in order for
# serialization to be possible. This generally means including at least
# the header for the corresponding mojom traits definitions.
#
# Note that even when the traits header is listed in |traits_sources| or
# within a target referenced by |public_deps|, it must still be listed
# here to be included properly by generated code.
#
# traits_sources (optional)
# The references to the source files (typically a single .cc and .h file)
# defining an appropriate set of EnumTraits or StructTraits, etc for the
# the type-mapping. Using this will cause the listed sources to be
# integrated directly into the dependent mojom's generated type-mapping
# targets.
#
# Prefer using |public_deps| over inlined |traits_sources|, as this will
# generally lead to easier build maintenance over time.
#
# NOTE: If a typemap is shared by Blink and non-Blink bindings, you cannot
# use this and MUST use |public_deps| to reference traits built within
# a separate target.
#
# traits_deps / traits_public_deps (optional)
# If any sources are listed in |traits_sources|, their dependences must be
# listed here.
#
# public_deps (optional)
# This work just like its equivalent for source_set or component targets
# and is incorporated into any C++ source targets generated by the
# dependent mojom target.
#
# NOTE: If relevant traits are defined in a separate target (as opposed to
# using inlined |traits_sources|), that target MUST be listed in
# |public_deps|.
#
template("mojom_cpp_typemap") {
assert(defined(invoker.types), "Missing 'types' value")
typemap_target_name = target_name
not_needed([ "typemap_target_name" ])
public_headers = []
if (defined(invoker.public_headers)) {
public_headers = invoker.public_headers
}
traits_sources = []
if (defined(invoker.traits_sources)) {
traits_sources = invoker.traits_sources
}
source_deps = []
if (defined(invoker.deps)) {
source_deps = invoker.deps
}
source_public_deps = []
if (defined(invoker.public_deps)) {
source_public_deps = invoker.public_deps
}
# Write out a JSON file that can be loaded by the generator.
typemap_types = invoker.types
generated_file("${target_name}__json_config") {
outputs = [ "$target_gen_dir/${typemap_target_name}.typemap_config" ]
contents = {
}
contents.types = typemap_types
contents.public_headers = rebase_path(public_headers, "//")
output_conversion = "json"
}
if (traits_sources != []) {
traits_deps = []
if (defined(invoker.traits_deps)) {
traits_deps = invoker.traits_deps
}
traits_public_deps = []
if (defined(invoker.traits_public_deps)) {
traits_public_deps = invoker.traits_public_deps
}
source_set("${typemap_target_name}__traits") {
sources = traits_sources
public_deps = traits_public_deps
deps = traits_deps
}
}
# These group targets are used to forward C++ sources deps and configs to the
# dependent mojom's generated source targets.
group("${target_name}__source_deps") {
public_deps = source_deps
}
group("${target_name}__source_public_deps") {
public_deps = source_public_deps
if (traits_sources != []) {
public_deps += [ ":${typemap_target_name}__traits" ]
}
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment