Commit 6ddaf0e7 authored by Ken Rockot's avatar Ken Rockot Committed by Commit Bot

Inline new typemap configs within mojom targets

This removes the recently introduced `mojom_cpp_typemap` GN template in
favor of simply inlining typemap configs directly within the `mojom`
target they affect.

A few motivations here:

- The separate target approach required traits sources to always belong
  to a different target than the mojom variant's own sources. This has
  insurmountable dependency issues in some corner cases like
  //content/common.
- A typemap can only realistically affect a single mojom target in the
  build under most circumstances.
- A significant portion of the variables within a typemap target were
  only used by forwarding them to the affected mojom target. Inlining
  everything makes the direct impact on the mojom target more clear.
- This approach is exactly as powerful (from the perspective of build
  expressiveness) as typemap files, but without the typemap files.

Bug: 1059389
Change-Id: I8218312374e86a0c02dcaea630b05f61c7aa6d61
Tbr: changwan@chromium.org
Tbr: haraken@chromium.org
Tbr: dcheng@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2106887
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: default avatarOksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751449}
parent 6dfb67d3
......@@ -5,20 +5,6 @@
import("//build/config/android/rules.gni")
import("//mojo/public/tools/bindings/mojom.gni")
mojom_cpp_typemap("common_typemap") {
types = [
{
mojom = "android_webview.mojom.AwOriginMatcher"
cpp = "::android_webview::AwOriginMatcher"
},
]
public_headers = [
"//android_webview/common/aw_origin_matcher.h",
"//android_webview/common/aw_origin_matcher_mojom_traits.h",
]
}
mojom("common_mojom") {
sources = [
"aw_origin_matcher.mojom",
......@@ -30,7 +16,18 @@ mojom("common_mojom") {
"//services/network/public/mojom:mojom",
]
cpp_typemaps = [ ":common_typemap" ]
cpp_typemaps = [
{
types = [
{
mojom = "android_webview.mojom.AwOriginMatcher"
cpp = "::android_webview::AwOriginMatcher"
},
]
traits_headers =
[ "//android_webview/common/aw_origin_matcher_mojom_traits.h" ]
},
]
}
source_set("common") {
......
......@@ -192,39 +192,33 @@ mojom_component("mojom") {
"//mojo/public/mojom/base",
]
cpp_typemaps = [ ":message_typemap" ]
cpp_typemaps = [
{
types = [
{
mojom = "IPC.mojom.Message"
cpp = "::IPC::MessageView"
move_only = true
},
]
traits_headers = [ "//ipc/message_mojom_traits.h" ]
traits_sources = [
"//ipc/message_mojom_traits.cc",
"//ipc/message_view.cc",
"//ipc/message_view.h",
]
traits_public_deps = [
"//ipc:message_support",
"//mojo/public/cpp/base:shared_typemap_traits",
]
},
]
# Don't generate a variant sources since we depend on generated internal
# bindings types and we don't generate or build variants of those.
disable_variants = true
}
mojom_cpp_typemap("message_typemap") {
types = [
{
mojom = "IPC.mojom.Message"
cpp = "::IPC::MessageView"
move_only = true
},
]
public_headers = [
"//ipc/message_mojom_traits.h",
"//ipc/message_view.h",
]
traits_sources = [
"//ipc/message_mojom_traits.cc",
"//ipc/message_mojom_traits.h",
"//ipc/message_view.cc",
"//ipc/message_view.h",
]
traits_public_deps = [
":mojom_shared",
"//ipc:message_support",
"//mojo/public/cpp/base:shared_typemap_traits",
]
traits_defines = [ "IS_IPC_MOJOM_IMPL" ]
}
mojom("mojom_constants") {
sources = [ "constants.mojom" ]
}
......
......@@ -1420,69 +1420,85 @@ 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.
must add some more information to our `mojom` target in GN:
```
mojom_cpp_typemap("geometry_typemap") {
types = [
{
mojom = "gfx.mojom.Rect"
cpp = "::gfx::Rect"
}
# Without a typemap
mojom("mojom") {
sources = [
"rect.mojom",
]
}
public_headers = [
"//ui/gfx/geometry/mojo/geometry_mojom_traits.h",
"//ui/gfx/geometry/rect.h",
# With a typemap.
mojom("mojom") {
sources = [
"rect.mojom",
]
# 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",
cpp_typemaps = [
{
# NOTE: A single typemap entry can list multiple individual type mappings.
# Each mapping assumes the same values for |traits_headers| etc below.
#
# To typemap a type with separate |traits_headers| etc, add a separate
# entry to |cpp_typemaps|.
types = [
{
mojom = "gfx.mojom.Rect"
cpp = "::gfx::Rect"
},
]
traits_headers = [ "//ui/gfx/geometry/mojo/geometry_mojom_traits.h" ]
traits_sources = [ "//ui/gfx/geometry/mojo/geometry_mojom_traits.cc" ]
traits_public_deps = [ "//ui/gfx/geometry" ]
},
]
public_deps = [ "//ui/gfx/geometry" ]
}
```
See the `mojom_cpp_typemap` documentation in
See 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" ]
...
}
With this extra configuration present, any mojom references to `gfx.mojom.Rect`
(e.g. for method parameters or struct fields) will be `gfx::Rect` references in
generated C++ code.
```
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 if you want to use the same typemap for both Blink and non-Blink
bindings.
Using `traits_sources` in a typemap configuration means that the listed sources
will be baked directly into the corresponding `mojom` target's own sources. This
can be problematic if you want to use the same typemap for both Blink and
non-Blink bindings.
For such 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:
for your typemap traits, and reference this in the `traits_public_deps` of the
typemap:
```
mojom("mojom") {
sources = [
"rect.mojom",
]
cpp_typemaps = [
{
types = [
{
mojom = "gfx.mojom.Rect"
cpp = "::gfx::Rect"
},
]
traits_headers = [ "//ui/gfx/geometry/mojo/geometry_mojom_traits.h" ]
traits_public_deps = [ ":geometry_mojom_traits" ]
},
]
}
component("geometry_mojom_traits") {
sources = [
"//ui/gfx/geometry/mojo/geometry_mojom_traits.cc",
......@@ -1491,68 +1507,9 @@ component("geometry_mojom_traits") {
# 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",
]
}
```
### Type Mapping For `mojom_component` Targets
Due to how component builds manage symbol exports, `mojom_component` targets
require some special consideration when applying a C++ typemap. The simplest
rule to apply here is to just **define your traits in a separate `component`
target and reference that target in the typemap's `public_deps`**, as
demonstrated in the previous section.
If you must use `traits_sources` for your traits definitions, you will need to
tag traits symbols with the same generated export macro that your mojom uses,
and you will need to assign `traits_defines` to define an appropriate macro
in the typemap rule itself. Example:
```
mojom_component("mojom") {
output_prefix = "my_mojom"
macro_prefix = "MY_MOJOM"
sources = [ "foo.mojom" ]
cpp_typemaps = [ ":typemap" ]
}
mojom_cpp_typemap("typemap") {
types = [ ... ]
public_headers = [ ... ]
traits_sources = [
"foo_mojom_traits.cc",
"foo_mojom_traits.h",
]
traits_defines = [ "IS_MY_MOJOM_IMPL" ]
}
```
And the C++ code would look like:
``` cpp
template <>
struct COMPONENT_EXPORT(MY_MOJOM) StructTraits<...> {
...
```
If this were a Blink typemap, the traits would instead define
`IS_MY_MOJOM_BLINK_IMPL` and tag symbols with
`COMPONENT_EXPORT(MY_MOJOM_BLINK)`.
### StructTraits Reference
Each of a `StructTraits` specialization's static getter methods -- one per
......
This diff is collapsed.
......@@ -83,15 +83,15 @@ def ParseTypemapArgs(args):
return result
def LoadCppTypemapConfigs(paths):
def LoadCppTypemapConfig(path):
configs = {}
for path in paths:
with open(path) as f:
config = json.load(f)
with open(path) as f:
for config in json.load(f):
for entry in config['types']:
configs[entry['mojom']] = {
'typename': entry['cpp'],
'public_headers': config.get('public_headers', []),
'public_headers': config.get('traits_headers', []),
'traits_headers': config.get('traits_private_headers', []),
'copyable_pass_by_value': entry.get('copyable_pass_by_value',
False),
'force_serialize': entry.get('force_serialize', False),
......@@ -162,9 +162,8 @@ def main():
parser.add_argument(
'--cpp-typemap-config',
type=str,
action='append',
default=[],
dest='cpp_config_paths',
action='store',
dest='cpp_config_path',
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',
......@@ -173,7 +172,8 @@ def main():
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))
if params.cpp_config_path:
typemaps.update(LoadCppTypemapConfig(params.cpp_config_path))
missing = [path for path in params.dependency if not os.path.exists(path)]
if missing:
raise IOError('Missing dependencies: %s' % ', '.join(missing))
......
This diff is collapsed.
......@@ -10,33 +10,47 @@ import re
import sys
def CheckCppTypemapConfig(target_name, config_filename, out_filename):
_SUPPORTED_KEYS = set([
'mojom', 'cpp', 'public_headers', 'copyable_pass_by_value',
'force_serialize', 'hashable', 'move_only', 'nullable_is_same_type'
def CheckCppTypemapConfigs(target_name, config_filename, out_filename):
_SUPPORTED_CONFIG_KEYS = set([
'types', 'traits_headers', 'traits_private_headers', 'traits_sources',
'traits_deps', 'traits_public_deps'
])
_SUPPORTED_TYPE_KEYS = set([
'mojom', 'cpp', 'copyable_pass_by_value', 'force_serialize', 'hashable',
'move_only', 'nullable_is_same_type'
])
with open(config_filename, 'r') 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 type property "%s" when processing %s' %
(key, target_name))
for config in json.load(f):
for key in config.keys():
if key not in _SUPPORTED_CONFIG_KEYS:
raise ValueError('Invalid typemap property "%s" when processing %s' %
(key, target_name))
types = config.get('types')
if not types:
raise ValueError('Typemap for %s must specify at least one type to map'
% target_name)
for entry in types:
for key in entry.keys():
if key not in _SUPPORTED_TYPE_KEYS:
raise IOError(
'Invalid type property "%s" in typemap for "%s" on target %s' %
(key, entry.get('mojom', '(unknown)'), target_name))
with open(out_filename, 'w') as f:
f.truncate(0)
json.dump(config, f)
def main():
parser = argparse.ArgumentParser()
_, args = parser.parse_known_args()
if len(args) != 3:
print('Usage: check_typemap_config.py target_name config_filename '
'validated_config_filename')
print('Usage: validate_typemap_config.py target_name config_filename '
'stamp_filename')
sys.exit(1)
CheckCppTypemapConfig(args[0], args[1], args[2])
CheckCppTypemapConfigs(args[0], args[1], args[2])
if __name__ == '__main__':
......
# Copyright 2020 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.
import("//mojo/public/tools/bindings/mojom.gni")
mojom_cpp_typemap("big_string_typemap") {
types = [
{
mojom = "mojo_base.mojom.BigString"
cpp = "::WTF::String"
nullable_is_same_type = true
},
]
public_headers = [
"//third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h",
"//third_party/blink/renderer/platform/wtf/text/wtf_string.h",
]
public_deps = [ "//mojo/public/cpp/base" ]
}
mojom_cpp_typemap("url_typemap") {
types = [
{
mojom = "url.mojom.Url"
cpp = "::blink::KURL"
force_serialize = true
},
]
public_headers = [
"//third_party/blink/renderer/platform/mojo/kurl_mojom_traits.h",
"//third_party/blink/renderer/platform/weborigin/kurl.h",
"//third_party/blink/renderer/platform/weborigin/kurl_hash.h",
]
# NOTE: Consumers of this typemap must themselves depend on platform.
public_deps = [ "//url" ]
}
mojom_cpp_typemap("origin_typemap") {
types = [
{
mojom = "url.mojom.Origin"
cpp = "::scoped_refptr<const ::blink::SecurityOrigin>"
nullable_is_same_type = true
},
]
public_headers = [
"//third_party/blink/renderer/platform/mojo/security_origin_mojom_traits.h",
"//third_party/blink/renderer/platform/weborigin/security_origin.h",
]
# NOTE: Consumers of this typemap must themselves depend on platform.
public_deps = [ "//url" ]
}
......@@ -7,9 +7,39 @@ import("//mojo/public/tools/bindings/mojom.gni")
mojom("url_mojom_gurl") {
generate_java = true
sources = [ "url.mojom" ]
cpp_typemaps = [ ":url_typemap" ]
blink_cpp_typemaps =
[ "//third_party/blink/renderer/platform/mojo:url_typemap" ]
cpp_typemaps = [
{
types = [
{
mojom = "url.mojom.Url"
cpp = "::GURL"
},
]
traits_headers = [ "//url/mojom/url_gurl_mojom_traits.h" ]
traits_public_deps = [
":mojom_traits",
"//url",
]
},
]
blink_cpp_typemaps = [
{
types = [
{
mojom = "url.mojom.Url"
cpp = "::blink::KURL"
force_serialize = true
},
]
traits_headers = [
"//third_party/blink/renderer/platform/mojo/kurl_mojom_traits.h",
"//third_party/blink/renderer/platform/weborigin/kurl_hash.h",
]
traits_public_deps = [ "//url" ]
},
]
}
mojom("url_mojom_origin") {
......@@ -23,9 +53,35 @@ mojom("url_mojom_origin") {
check_includes_blink = false
cpp_typemaps = [ ":origin_typemap" ]
blink_cpp_typemaps =
[ "//third_party/blink/renderer/platform/mojo:origin_typemap" ]
cpp_typemaps = [
{
types = [
{
mojom = "url.mojom.Origin"
cpp = "::url::Origin"
},
]
traits_headers = [ "//url/mojom/origin_mojom_traits.h" ]
traits_public_deps = [
":mojom_traits",
"//url",
]
},
]
blink_cpp_typemaps = [
{
types = [
{
mojom = "url.mojom.Origin"
cpp = "::scoped_refptr<const ::blink::SecurityOrigin>"
nullable_is_same_type = true
},
]
traits_headers = [ "//third_party/blink/renderer/platform/mojo/security_origin_mojom_traits.h" ]
traits_public_deps = [ "//url" ]
},
]
}
mojom("test_url_mojom_gurl") {
......@@ -57,37 +113,3 @@ component("mojom_traits") {
"//url",
]
}
mojom_cpp_typemap("url_typemap") {
types = [
{
mojom = "url.mojom.Url"
cpp = "::GURL"
},
]
public_headers = [
"//url/gurl.h",
"//url/mojom/url_gurl_mojom_traits.h",
]
public_deps = [
":mojom_traits",
"//url",
]
}
mojom_cpp_typemap("origin_typemap") {
types = [
{
mojom = "url.mojom.Origin"
cpp = "::url::Origin"
},
]
public_headers = [
"//url/origin.h",
"//url/mojom/origin_mojom_traits.h",
]
public_deps = [
":mojom_traits",
"//url",
]
}
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