Skip to content

Commit

Permalink
Revert "[mojo] Enable use of forward declarations of type-mapped mojo…
Browse files Browse the repository at this point in the history
…m types"

This reverts commit 63b02ed.

Reason for revert: Looks like this broke compile on linux-chromeos-chrome:

https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-chrome/15690
https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-chrome/15691
https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-chrome/15692

Original change's description:
> [mojo] Enable use of forward declarations of type-mapped mojom types
>
> This makes it possible to reduce generated header sizes significantly
> when the full type is not necessary.
>
> For example, url_loader_factory.mojom defines an interface using the
> URLRequest mojom type, which is type-mapped to the C++ type
> ResourceRequest.
>
> Making it possible to forward declare ResourceRequest avoids the need
> to include url_request_mojom.h in url_loader_factory.mojom.h, which
> reduces the compiler input size for the chrome target on Linux by
> 1.3 GB, saving 19 CPU-minutes of compile time on my machine.
>
> Bug: 1226821, 242216
> Change-Id: I7ba187f714183ef5a7bc909458276aa4aeb05b2f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3008756
> Commit-Queue: Hans Wennborg <hans@chromium.org>
> Reviewed-by: Ken Rockot <rockot@google.com>
> Cr-Commit-Position: refs/heads/master@{#899267}

Bug: 1226821, 242216
Change-Id: I412b23efbe70b6e17c9d0c7e3ef031e3d33d6b07
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3011443
Auto-Submit: Scott Little <sclittle@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Trevor Perrier <perrier@chromium.org>
Reviewed-by: Scott Little <sclittle@chromium.org>
Reviewed-by: Trevor Perrier <perrier@chromium.org>
Owners-Override: Scott Little <sclittle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#899307}
  • Loading branch information
scott-little authored and Chromium LUCI CQ committed Jul 7, 2021
1 parent f7c8791 commit 04ad4de
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 30 deletions.
1 change: 0 additions & 1 deletion mojo/public/tools/bindings/generate_type_mappings.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ def LoadCppTypemapConfig(path):
for entry in config['types']:
configs[entry['mojom']] = {
'typename': entry['cpp'],
'forward_declaration': entry.get('forward_declaration', None),
'public_headers': config.get('traits_headers', []),
'traits_headers': config.get('traits_private_headers', []),
'copyable_pass_by_value': entry.get('copyable_pass_by_value',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,6 @@ namespace {{variant}} {
#endif
{%- endif %}

{%- for forward_declaration in typemap_forward_declarations %}
{{forward_declaration}}
{%- endfor %}

{#--- WTF enum hashing #}
{%- from "enum_macros.tmpl" import enum_hash_blink%}
{%- if for_blink %}
Expand Down
21 changes: 4 additions & 17 deletions mojo/public/tools/bindings/generators/mojom_cpp_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,12 +331,6 @@ def _GetJinjaExports(self):
for interface in self.module.interfaces:
all_enums.extend(interface.enums)

typemap_forward_declarations = []
for kind in self.module.imported_kinds.values():
forward_declaration = self._GetTypemappedForwardDeclaration(kind)
if forward_declaration:
typemap_forward_declarations.append(forward_declaration)

return {
"all_enums": all_enums,
"contains_only_enums": self._ContainsOnlyEnums(),
Expand All @@ -350,7 +344,6 @@ def _GetJinjaExports(self):
"extra_traits_headers": self._GetExtraTraitsHeaders(),
"for_blink": self.for_blink,
"imports": self.module.imports,
"typemap_forward_declarations": typemap_forward_declarations,
"interfaces": self.module.interfaces,
"kinds": self.module.kinds,
"module": self.module,
Expand Down Expand Up @@ -573,12 +566,6 @@ def _IsTypemappedKind(self, kind):
return hasattr(kind, "name") and \
self._GetFullMojomNameForKind(kind) in self.typemap

def _GetTypemappedForwardDeclaration(self, kind):
if not self._IsTypemappedKind(kind):
return None
return self.typemap[self._GetFullMojomNameForKind(
kind)]["forward_declaration"]

def _IsHashableKind(self, kind):
"""Check if the kind can be hashed.
Expand Down Expand Up @@ -742,11 +729,11 @@ def _IsFullHeaderRequiredForImport(self, imported_module):
"""Determines whether a given import module requires a full header include,
or if the forward header is sufficient."""

# Type-mapped kinds may not have forward declarations, and nested kinds
# cannot be forward declared.
# Type-mapped kinds don't have forward declarations, and nested kinds cannot
# be forward declared.
# TODO(hans): Use forward declarations for type-mapped kinds.
if any(kind.module == imported_module and (
(self._IsTypemappedKind(kind) and not self.
_GetTypemappedForwardDeclaration(kind)) or kind.parent_kind != None)
self._IsTypemappedKind(kind) or kind.parent_kind != None)
for kind in self.module.imported_kinds.values()):
return True

Expand Down
6 changes: 0 additions & 6 deletions mojo/public/tools/bindings/mojom.gni
Original file line number Diff line number Diff line change
Expand Up @@ -401,12 +401,6 @@ if (enable_scrambled_message_ids) {
# should be mapped in generated bindings. This is a string like
# "::base::Value" or "std::vector<::base::Value>".
#
# forward_declaration (optional)
# A forward declaration of the C++ type, which bindings that don't
# need the full type definition can use to reduce the size of
# the generated code. This is a string like
# "namespace base { class 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
Expand Down
2 changes: 1 addition & 1 deletion mojo/public/tools/bindings/validate_typemap_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def CheckCppTypemapConfigs(target_name, config_filename, out_filename):
])
_SUPPORTED_TYPE_KEYS = set([
'mojom', 'cpp', 'copyable_pass_by_value', 'force_serialize', 'hashable',
'move_only', 'nullable_is_same_type', 'forward_declaration'
'move_only', 'nullable_is_same_type'
])
with open(config_filename, 'r') as f:
for config in json.load(f):
Expand Down
1 change: 0 additions & 1 deletion services/network/public/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ mojom("url_loader_base") {
{
mojom = "network.mojom.URLRequest"
cpp = "::network::ResourceRequest"
forward_declaration = "namespace network { struct ResourceRequest; }"
},
{
mojom = "network.mojom.URLRequestBody"
Expand Down

0 comments on commit 04ad4de

Please sign in to comment.