Skip to content

Commit

Permalink
Mojo C++ bindings: remove support for generating code with mojo::Arra…
Browse files Browse the repository at this point in the history
…y/String/Map/WTFArray/WTFMap.

BUG=624136

Review-Url: https://codereview.chromium.org/2584763002
Cr-Commit-Position: refs/heads/master@{#438992}
  • Loading branch information
yzshen authored and Commit bot committed Dec 16, 2016
1 parent 15a7ab4 commit db95936
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 97 deletions.
27 changes: 0 additions & 27 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -1542,32 +1542,6 @@ def _CheckIpcOwners(input_api, output_api):
return results


def _CheckMojoUsesNewWrapperTypes(input_api, output_api):
"""Checks to make sure that all newly added mojom targets map array/map/string
to STL (for chromium) or WTF (for blink) types.
TODO(yzshen): remove this check once crbug.com/624136 is completed.
"""
files = []
pattern = input_api.re.compile(r'use_new_wrapper_types.*false',
input_api.re.MULTILINE)

for f in input_api.AffectedFiles():
if not f.LocalPath().endswith(('.gyp', '.gypi', 'gn', 'gni')):
continue

for _, line in f.ChangedContents():
if pattern.search(line):
files.append(f)
break

if len(files):
return [output_api.PresubmitError(
'Do not introduce new mojom targets with use_new_wrapper_types set to '
'false. The mode is deprecated and will be removed soon.',
files)]
return []


def _CheckUselessForwardDeclarations(input_api, output_api):
"""Checks that added or removed lines in non third party affected
header files do not lead to new useless class or struct forward
Expand Down Expand Up @@ -2084,7 +2058,6 @@ def _CommonChecks(input_api, output_api):
results.extend(_CheckPydepsNeedsUpdating(input_api, output_api))
results.extend(_CheckJavaStyle(input_api, output_api))
results.extend(_CheckIpcOwners(input_api, output_api))
results.extend(_CheckMojoUsesNewWrapperTypes(input_api, output_api))
results.extend(_CheckUselessForwardDeclarations(input_api, output_api))
results.extend(_CheckForRiskyJsFeatures(input_api, output_api))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class {{export_attribute}} {{interface.name}}
{%- endif %}

using {{method.name}}Callback = {{interface_macros.declare_callback(method,
for_blink, use_new_wrapper_types, use_once_callback)}};
for_blink, use_once_callback)}};
{%- endif %}
virtual void {{method.name}}({{interface_macros.declare_request_params("", method, use_once_callback)}}) = 0;
{%- endfor %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,7 @@ class {{class_name}}_{{method.name}}_ProxyToResponder {

void Run(
{{interface_macros.declare_responder_params(
"in_", method.response_parameters, for_blink,
use_new_wrapper_types)}});
"in_", method.response_parameters, for_blink)}});

uint64_t request_id_;
bool is_sync_;
Expand All @@ -283,8 +282,7 @@ class {{class_name}}_{{method.name}}_ProxyToResponder {

void {{class_name}}_{{method.name}}_ProxyToResponder::Run(
{{interface_macros.declare_responder_params(
"in_", method.response_parameters, for_blink,
use_new_wrapper_types)}}) {
"in_", method.response_parameters, for_blink)}}) {
{{struct_macros.get_serialized_size(response_params_struct, "in_%s",
"&serialization_context_")}}
mojo::internal::ResponseMessageBuilder builder(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,21 @@
{%- endfor %}
{%- endmacro %}

{%- macro declare_responder_params(prefix, parameters, for_blink, use_new_wrapper_types) %}
{%- macro declare_responder_params(prefix, parameters, for_blink) %}
{%- for param in parameters -%}
{%- if (not param.kind|is_string_kind) or for_blink or
use_new_wrapper_types -%}
{{param.kind|cpp_wrapper_param_type}} {{prefix}}{{param.name}}
{%- else %}
mojo::String {{prefix}}{{param.name}}
{%- endif %}
{%- if not loop.last %}, {% endif %}
{%- endfor %}
{%- endmacro %}

{%- macro declare_callback(method, for_blink, use_new_wrapper_types, use_once_callback) -%}
{%- macro declare_callback(method, for_blink, use_once_callback) -%}
{%- if use_once_callback -%}
base::OnceCallback<void(
{%- else -%}
base::Callback<void(
{%- endif -%}
{%- for param in method.response_parameters -%}
{#- TODO(yzshen): For historical reasons, we use mojo::String here (instead of
const mojo::String&) inconsistently. Preserve the behavior temporarily. #}
{%- if (not param.kind|is_string_kind) or for_blink or
use_new_wrapper_types -%}
{{param.kind|cpp_wrapper_param_type}}
{%- else -%}
mojo::String
{%- endif %}
{%- if not loop.last %}, {% endif %}
{%- endfor -%}
)>
Expand Down
37 changes: 9 additions & 28 deletions mojo/public/tools/bindings/generators/mojom_cpp_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
# generator library code so that filters can use the generator as context.
_current_typemap = {}
_for_blink = False
_use_new_wrapper_types = False
# TODO(rockot, yzshen): The variant handling is kind of a hack currently. Make
# it right.
_variant = None
Expand Down Expand Up @@ -142,11 +141,6 @@ def DefaultValue(field):
if not IsTypemappedKind(field.kind):
return "%s::New()" % GetNameForKind(field.kind)
return ExpressionToText(field.default, kind=field.kind)
if not _use_new_wrapper_types:
if mojom.IsArrayKind(field.kind) or mojom.IsMapKind(field.kind):
return "nullptr";
if mojom.IsStringKind(field.kind):
return "" if _for_blink else "nullptr"
return ""

def NamespaceToArray(namespace):
Expand Down Expand Up @@ -245,24 +239,16 @@ def _AddOptional(type_name):
return "%sPtr" % GetNameForKind(
kind, add_same_module_namespaces=add_same_module_namespaces)
if mojom.IsArrayKind(kind):
pattern = None
if _use_new_wrapper_types:
pattern = "WTF::Vector<%s>" if _for_blink else "std::vector<%s>"
if mojom.IsNullableKind(kind):
pattern = _AddOptional(pattern)
else:
pattern = "mojo::WTFArray<%s>" if _for_blink else "mojo::Array<%s>"
pattern = "WTF::Vector<%s>" if _for_blink else "std::vector<%s>"
if mojom.IsNullableKind(kind):
pattern = _AddOptional(pattern)
return pattern % GetCppWrapperType(
kind.kind, add_same_module_namespaces=add_same_module_namespaces)
if mojom.IsMapKind(kind):
pattern = None
if _use_new_wrapper_types:
pattern = ("WTF::HashMap<%s, %s>" if _for_blink else
"std::unordered_map<%s, %s>")
if mojom.IsNullableKind(kind):
pattern = _AddOptional(pattern)
else:
pattern = "mojo::WTFMap<%s, %s>" if _for_blink else "mojo::Map<%s, %s>"
pattern = ("WTF::HashMap<%s, %s>" if _for_blink else
"std::unordered_map<%s, %s>")
if mojom.IsNullableKind(kind):
pattern = _AddOptional(pattern)
return pattern % (
GetCppWrapperType(
kind.key_kind,
Expand All @@ -285,8 +271,6 @@ def _AddOptional(type_name):
if mojom.IsStringKind(kind):
if _for_blink:
return "WTF::String"
if not _use_new_wrapper_types:
return "mojo::String"
type_name = "std::string"
return _AddOptional(type_name) if mojom.IsNullableKind(kind) else type_name
if mojom.IsGenericHandleKind(kind):
Expand All @@ -311,9 +295,9 @@ def IsMoveOnlyKind(kind):
if mojom.IsStructKind(kind) or mojom.IsUnionKind(kind):
return True
if mojom.IsArrayKind(kind):
return IsMoveOnlyKind(kind.kind) if _use_new_wrapper_types else True
return IsMoveOnlyKind(kind.kind)
if mojom.IsMapKind(kind):
return IsMoveOnlyKind(kind.value_kind) if _use_new_wrapper_types else True
return IsMoveOnlyKind(kind.value_kind)
if mojom.IsAnyHandleOrInterfaceKind(kind):
return True
return False
Expand Down Expand Up @@ -621,7 +605,6 @@ def GetJinjaExports(self):
"extra_traits_headers": self.GetExtraTraitsHeaders(),
"extra_public_headers": self.GetExtraPublicHeaders(),
"for_blink": self.for_blink,
"use_new_wrapper_types": self.use_new_wrapper_types,
"use_once_callback": self.use_once_callback,
"export_attribute": self.export_attribute,
"export_header": self.export_header,
Expand Down Expand Up @@ -669,8 +652,6 @@ def GenerateFiles(self, args):
_current_typemap = self.typemap
global _for_blink
_for_blink = self.for_blink
global _use_new_wrapper_types
_use_new_wrapper_types = self.use_new_wrapper_types
global _use_once_callback
_use_once_callback = self.use_once_callback
global _variant
Expand Down
14 changes: 0 additions & 14 deletions mojo/public/tools/bindings/mojom.gni
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,6 @@ foreach(configuration, _bindings_configurations) {
#
# visibility (optional)
#
# use_new_wrapper_types (optional)
# If set to true, mojom array/map/string will be mapped to STL (for
# chromium variant) or WTF (for blink) types. Otherwise, they will be
# mapped to mojo::Array/Map/String/etc.
# Default value is true.
# TODO(yzshen):
# - convert all users to use the new mode;
# - remove support for the old mode.
#
# use_once_callback (optional)
# If set to true, generated classes will use base::OnceCallback instead of
# base::RepeatingCallback.
Expand Down Expand Up @@ -363,11 +354,6 @@ template("mojom") {
}
}

if (!defined(invoker.use_new_wrapper_types) ||
invoker.use_new_wrapper_types) {
args += [ "--use_new_wrapper_types" ]
}

if (defined(invoker.use_once_callback) && invoker.use_once_callback) {
args += [ "--use_once_callback" ]
}
Expand Down
5 changes: 0 additions & 5 deletions mojo/public/tools/bindings/mojom_bindings_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ def _GenerateModule(self, args, remaining_args, generator_modules,
module, args.output_dir, typemap=self._typemap.get(language, {}),
variant=args.variant, bytecode_path=args.bytecode_path,
for_blink=args.for_blink,
use_new_wrapper_types=args.use_new_wrapper_types,
use_once_callback=args.use_once_callback,
export_attribute=args.export_attribute,
export_header=args.export_header,
Expand Down Expand Up @@ -289,10 +288,6 @@ def main():
generate_parser.add_argument("--for_blink", action="store_true",
help="Use WTF types as generated types for mojo "
"string/array/map.")
generate_parser.add_argument(
"--use_new_wrapper_types", action="store_true",
help="Map mojom array/map/string to STL (for chromium variant) or WTF "
"(for blink variant) types directly.")
generate_parser.add_argument(
"--use_once_callback", action="store_true",
help="Use base::OnceCallback instead of base::RepeatingCallback.")
Expand Down
7 changes: 3 additions & 4 deletions mojo/public/tools/bindings/pylib/mojom/generate/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,15 @@ class Generator(object):
# Pass |output_dir| to emit files to disk. Omit |output_dir| to echo all
# files to stdout.
def __init__(self, module, output_dir=None, typemap=None, variant=None,
bytecode_path=None, for_blink=False, use_new_wrapper_types=False,
use_once_callback=False, export_attribute=None,
export_header=None, generate_non_variant_code=False):
bytecode_path=None, for_blink=False, use_once_callback=False,
export_attribute=None, export_header=None,
generate_non_variant_code=False):
self.module = module
self.output_dir = output_dir
self.typemap = typemap or {}
self.variant = variant
self.bytecode_path = bytecode_path
self.for_blink = for_blink
self.use_new_wrapper_types = use_new_wrapper_types
self.use_once_callback = use_once_callback
self.export_attribute = export_attribute
self.export_header = export_header
Expand Down

0 comments on commit db95936

Please sign in to comment.