From 9f187aa3de7e2f5b7ab759594e7b3c4e3c4083de Mon Sep 17 00:00:00 2001 From: qsr Date: Mon, 29 Sep 2014 08:24:30 -0700 Subject: [PATCH] mojo: Fix java generator Fix issue with method with empty response. Fix issue with local variable shadowing user parameters. R=ppi@chromium.org Review URL: https://codereview.chromium.org/607933002 Cr-Commit-Position: refs/heads/master@{#297176} --- build/get_landmines.py | 2 +- mojo/mojo_public_tests.gypi | 2 + mojo/public/interfaces/bindings/tests/BUILD.gn | 2 + .../interfaces/bindings/tests/no_module.mojom | 9 +++ .../bindings/tests/regression_tests.mojom | 37 +++++++++ .../src/org/chromium/mojo/bindings/Callbacks.java | 94 ++++++++++++---------- .../generators/java_templates/enum_definition.tmpl | 4 +- .../java_templates/interface_definition.tmpl | 34 ++++---- .../bindings/generators/mojom_java_generator.py | 18 +++-- mojo/tools/generate_java_callback_interfaces.py | 22 +++-- 10 files changed, 151 insertions(+), 73 deletions(-) create mode 100644 mojo/public/interfaces/bindings/tests/no_module.mojom create mode 100644 mojo/public/interfaces/bindings/tests/regression_tests.mojom diff --git a/build/get_landmines.py b/build/get_landmines.py index 69564581a7fd..47f4479154b7 100755 --- a/build/get_landmines.py +++ b/build/get_landmines.py @@ -28,7 +28,7 @@ def print_landmines(): builder() == 'ninja'): print 'Need to clobber winja goma due to backend cwd cache fix.' if platform() == 'android': - print 'Clobber: To delete generated mojo class files.' + print 'Clobber: To delete newly generated mojo class files.' if platform() == 'win' and builder() == 'ninja': print 'Compile on cc_unittests fails due to symbols removed in r185063.' if platform() == 'linux' and builder() == 'ninja': diff --git a/mojo/mojo_public_tests.gypi b/mojo/mojo_public_tests.gypi index 10bd212e7e22..69d8cf769f0a 100644 --- a/mojo/mojo_public_tests.gypi +++ b/mojo/mojo_public_tests.gypi @@ -193,7 +193,9 @@ 'type': 'static_library', 'sources': [ 'public/interfaces/bindings/tests/math_calculator.mojom', + 'public/interfaces/bindings/tests/no_module.mojom', 'public/interfaces/bindings/tests/rect.mojom', + 'public/interfaces/bindings/tests/regression_tests.mojom', 'public/interfaces/bindings/tests/sample_factory.mojom', 'public/interfaces/bindings/tests/sample_import.mojom', 'public/interfaces/bindings/tests/sample_import2.mojom', diff --git a/mojo/public/interfaces/bindings/tests/BUILD.gn b/mojo/public/interfaces/bindings/tests/BUILD.gn index d8d6f24615c7..e56f9b5de8c2 100644 --- a/mojo/public/interfaces/bindings/tests/BUILD.gn +++ b/mojo/public/interfaces/bindings/tests/BUILD.gn @@ -8,7 +8,9 @@ mojom("test_interfaces") { testonly = true sources = [ "math_calculator.mojom", + "no_module.mojom", "rect.mojom", + "regression_tests.mojom", "sample_factory.mojom", "sample_import.mojom", "sample_import2.mojom", diff --git a/mojo/public/interfaces/bindings/tests/no_module.mojom b/mojo/public/interfaces/bindings/tests/no_module.mojom new file mode 100644 index 000000000000..f3800112904e --- /dev/null +++ b/mojo/public/interfaces/bindings/tests/no_module.mojom @@ -0,0 +1,9 @@ +// Copyright 2014 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. + +// Entities without module + +enum EnumWithoutModule { + A +}; diff --git a/mojo/public/interfaces/bindings/tests/regression_tests.mojom b/mojo/public/interfaces/bindings/tests/regression_tests.mojom new file mode 100644 index 000000000000..49ab69abdcd3 --- /dev/null +++ b/mojo/public/interfaces/bindings/tests/regression_tests.mojom @@ -0,0 +1,37 @@ +// Copyright 2014 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. + +// Module containing entities for regression tests of the generator. Entities +// must never be modified, instead new entity must be added to add new tests. +[JavaPackage="org.chromium.mojo.bindings.test.mojom.regression_tests"] +module regression_tests { + +interface CheckMethodWithEmptyResponse { +WithouParameterAndEmptyResponse() => (); +WithParameterAndEmptyResponse(bool b) => (); +}; + +interface CheckNameCollision { +WithNameCollision(bool message, bool response) => (bool message, bool response); +}; + +enum EnumWithReference { + k_STEREO_AND_KEYBOARD_MIC = 30, + k_MAX = k_STEREO_AND_KEYBOARD_MIC +}; + +enum EnumWithLowercase { + PlanarF16, + PlanarF32 +}; + +enum EnumWithNumbers { + k_2_1 = 4 +}; + +enum EnumWithK { + K = 0 +}; + +} // module imported diff --git a/mojo/public/java/bindings/src/org/chromium/mojo/bindings/Callbacks.java b/mojo/public/java/bindings/src/org/chromium/mojo/bindings/Callbacks.java index 87b9abdcd731..c6b14c14944d 100644 --- a/mojo/public/java/bindings/src/org/chromium/mojo/bindings/Callbacks.java +++ b/mojo/public/java/bindings/src/org/chromium/mojo/bindings/Callbacks.java @@ -13,107 +13,117 @@ package org.chromium.mojo.bindings; public interface Callbacks { /** - * A generic 1-argument callback. - * - * @param the type of argument 1. - */ + * A generic callback. + */ + interface Callback0 { + /** + * Call the callback. + */ + public void call(); + } + + /** + * A generic 1-argument callback. + * + * @param the type of argument 1. + */ interface Callback1 { /** - * Call the callback. - */ + * Call the callback. + */ public void call(T1 arg1); } /** - * A generic 2-argument callback. - * - * @param the type of argument 1. + * A generic 2-argument callback. + * + * @param the type of argument 1. * @param the type of argument 2. - */ + */ interface Callback2 { /** - * Call the callback. - */ + * Call the callback. + */ public void call(T1 arg1, T2 arg2); } /** - * A generic 3-argument callback. - * - * @param the type of argument 1. + * A generic 3-argument callback. + * + * @param the type of argument 1. * @param the type of argument 2. * @param the type of argument 3. - */ + */ interface Callback3 { /** - * Call the callback. - */ + * Call the callback. + */ public void call(T1 arg1, T2 arg2, T3 arg3); } /** - * A generic 4-argument callback. - * - * @param the type of argument 1. + * A generic 4-argument callback. + * + * @param the type of argument 1. * @param the type of argument 2. * @param the type of argument 3. * @param the type of argument 4. - */ + */ interface Callback4 { /** - * Call the callback. - */ + * Call the callback. + */ public void call(T1 arg1, T2 arg2, T3 arg3, T4 arg4); } /** - * A generic 5-argument callback. - * - * @param the type of argument 1. + * A generic 5-argument callback. + * + * @param the type of argument 1. * @param the type of argument 2. * @param the type of argument 3. * @param the type of argument 4. * @param the type of argument 5. - */ + */ interface Callback5 { /** - * Call the callback. - */ + * Call the callback. + */ public void call(T1 arg1, T2 arg2, T3 arg3, T4 arg4, T5 arg5); } /** - * A generic 6-argument callback. - * - * @param the type of argument 1. + * A generic 6-argument callback. + * + * @param the type of argument 1. * @param the type of argument 2. * @param the type of argument 3. * @param the type of argument 4. * @param the type of argument 5. * @param the type of argument 6. - */ + */ interface Callback6 { /** - * Call the callback. - */ + * Call the callback. + */ public void call(T1 arg1, T2 arg2, T3 arg3, T4 arg4, T5 arg5, T6 arg6); } /** - * A generic 7-argument callback. - * - * @param the type of argument 1. + * A generic 7-argument callback. + * + * @param the type of argument 1. * @param the type of argument 2. * @param the type of argument 3. * @param the type of argument 4. * @param the type of argument 5. * @param the type of argument 6. * @param the type of argument 7. - */ + */ interface Callback7 { /** - * Call the callback. - */ + * Call the callback. + */ public void call(T1 arg1, T2 arg2, T3 arg3, T4 arg4, T5 arg5, T6 arg6, T7 arg7); } diff --git a/mojo/public/tools/bindings/generators/java_templates/enum_definition.tmpl b/mojo/public/tools/bindings/generators/java_templates/enum_definition.tmpl index 7273287b6567..a16c178e67ee 100644 --- a/mojo/public/tools/bindings/generators/java_templates/enum_definition.tmpl +++ b/mojo/public/tools/bindings/generators/java_templates/enum_definition.tmpl @@ -4,7 +4,7 @@ {%- elif index == 0 -%} 0 {%- else -%} -{{enum.fields[index - 1].name}} + 1 +{{enum.fields[index - 1]|name}} + 1 {%- endif -%} {%- endmacro -%} @@ -12,7 +12,7 @@ public {{ 'static ' if not top_level }}final class {{enum|name}} { {% for field in enum.fields %} - public static final int {{field.name}} = {{enum_value(enum, field, loop.index0)}}; + public static final int {{field|name}} = {{enum_value(enum, field, loop.index0)}}; {% endfor %} private {{enum|name}}() {} diff --git a/mojo/public/tools/bindings/generators/java_templates/interface_definition.tmpl b/mojo/public/tools/bindings/generators/java_templates/interface_definition.tmpl index 058e31db4add..941fec777f33 100644 --- a/mojo/public/tools/bindings/generators/java_templates/interface_definition.tmpl +++ b/mojo/public/tools/bindings/generators/java_templates/interface_definition.tmpl @@ -19,12 +19,12 @@ {%- macro declare_callback(method) -%} -interface {{method|interface_response_name}} extends org.chromium.mojo.bindings.Callbacks.Callback{{method.response_parameters|length}}< +interface {{method|interface_response_name}} extends org.chromium.mojo.bindings.Callbacks.Callback{{method.response_parameters|length}}{% if method.response_parameters %}< {%- for param in method.response_parameters -%} {{param.kind|java_type(True)}} {%- if not loop.last %}, {% endif %} {%- endfor -%} -> { } +>{% endif %} { } {%- endmacro -%} {%- macro run_callback(variable, parameters) -%} @@ -105,8 +105,8 @@ try { } switch(header.getType()) { {% for method in interface.methods %} -{% if (with_response and method.response_parameters) or - (not with_response and not method.response_parameters) %} +{% if (with_response and method.response_parameters != None) or + (not with_response and method.response_parameters == None) %} {% set request_struct = method|struct_from_method %} {% if with_response %} {% set response_struct = method|response_struct_from_method %} @@ -152,7 +152,7 @@ public interface {{interface|name}} extends {{super_class(client)}} { {% for method in interface.methods %} void {{method|name}}({{declare_request_params(method)}}); -{% if method.response_parameters %} +{% if method.response_parameters != None %} {{declare_callback(method)|indent(4)}} {% endif %} {% endfor %} @@ -179,13 +179,13 @@ class {{interface|name}}_Internal { @Override public void {{method|name}}({{declare_request_params(method)}}) { {% set request_struct = method|struct_from_method %} - {{request_struct|name}} message = new {{request_struct|name}}(); + {{request_struct|name}} _message = new {{request_struct|name}}(); {% for param in method.parameters %} - message.{{param|name}} = {{param|name}}; + _message.{{param|name}} = {{param|name}}; {% endfor %} -{% if method.response_parameters %} +{% if method.response_parameters != None %} getMessageReceiver().acceptWithResponder( - message.serializeWithHeader( + _message.serializeWithHeader( getCore(), new org.chromium.mojo.bindings.MessageHeader( {{method|method_ordinal_name}}, @@ -194,7 +194,7 @@ class {{interface|name}}_Internal { new {{method|response_struct_from_method|name}}ForwardToCallback(callback)); {% else %} getMessageReceiver().accept( - message.serializeWithHeader( + _message.serializeWithHeader( getCore(), new org.chromium.mojo.bindings.MessageHeader({{method|method_ordinal_name}}))); {% endif %} @@ -222,7 +222,7 @@ class {{interface|name}}_Internal { {% for method in interface.methods %} {{ struct_def(method|struct_from_method, True)|indent(4) }} -{% if method.response_parameters %} +{% if method.response_parameters != None %} {% set response_struct = method|response_struct_from_method %} {{ struct_def(response_struct, True)|indent(4) }} @@ -245,7 +245,9 @@ class {{interface|name}}_Internal { {{flags_for_method(method, False)}})) { return false; } +{% if method.response_parameters|length %} {{response_struct|name}} response = {{response_struct|name}}.deserialize(messageWithHeader.getPayload()); +{% endif %} mCallback.call({{run_callback('response', method.response_parameters)}}); return true; } catch (org.chromium.mojo.bindings.DeserializationException e) { @@ -271,18 +273,18 @@ class {{interface|name}}_Internal { @Override public void call({{declare_params(method.response_parameters, true)}}) { - {{response_struct|name}} response = new {{response_struct|name}}(); + {{response_struct|name}} _response = new {{response_struct|name}}(); {% for param in method.response_parameters %} - response.{{param|name}} = {{param|name}}; + _response.{{param|name}} = {{param|name}}; {% endfor %} - org.chromium.mojo.bindings.ServiceMessage message = - response.serializeWithHeader( + org.chromium.mojo.bindings.ServiceMessage _message = + _response.serializeWithHeader( mCore, new org.chromium.mojo.bindings.MessageHeader( {{method|method_ordinal_name}}, {{flags_for_method(method, False)}}, mRequestId)); - mMessageReceiver.accept(message); + mMessageReceiver.accept(_message); } } {% endif %} diff --git a/mojo/public/tools/bindings/generators/mojom_java_generator.py b/mojo/public/tools/bindings/generators/mojom_java_generator.py index 678c3e5ea846..043d9e3acf49 100644 --- a/mojo/public/tools/bindings/generators/mojom_java_generator.py +++ b/mojo/public/tools/bindings/generators/mojom_java_generator.py @@ -108,8 +108,11 @@ def CamelCase(name): def ConstantStyle(name): components = NameToComponent(name) - if components[0] == 'k': + if components[0] == 'k' and len(components) > 1: components = components[1:] + # variable cannot starts with a digit. + if components[0][0].isdigit(): + components[0] = '_' + components[0] return '_'.join([x.upper() for x in components]) def GetNameForElement(element): @@ -126,9 +129,10 @@ def GetNameForElement(element): return (GetNameForElement(element.enum) + '.' + ConstantStyle(element.name)) if isinstance(element, (mojom.NamedValue, - mojom.Constant)): + mojom.Constant, + mojom.EnumField)): return ConstantStyle(element.name) - raise Exception('Unexpected element: ' % element) + raise Exception('Unexpected element: %s' % element) def GetInterfaceResponseName(method): return UpperCamelCase(method.name + 'Response') @@ -215,7 +219,9 @@ def GetPackage(module): if 'JavaPackage' in module.attributes: return ParseStringAttribute(module.attributes['JavaPackage']) # Default package. - return 'org.chromium.mojom.' + module.namespace + if module.namespace: + return 'org.chromium.mojom.' + module.namespace + return 'org.chromium.mojom' def GetNameForKind(context, kind): def _GetNameHierachy(kind): @@ -344,13 +350,13 @@ def GetMethodOrdinalName(method): def HasMethodWithResponse(interface): for method in interface.methods: - if method.response_parameters: + if method.response_parameters is not None: return True return False def HasMethodWithoutResponse(interface): for method in interface.methods: - if not method.response_parameters: + if method.response_parameters is None: return True return False diff --git a/mojo/tools/generate_java_callback_interfaces.py b/mojo/tools/generate_java_callback_interfaces.py index eb0f738c703c..257a5403e07a 100644 --- a/mojo/tools/generate_java_callback_interfaces.py +++ b/mojo/tools/generate_java_callback_interfaces.py @@ -5,14 +5,14 @@ import sys CALLBACK_TEMPLATE = (""" /** - * A generic %d-argument callback. - * - * %s - */ + * A generic %d-argument callback. + * + * %s + */ interface Callback%d<%s> { /** - * Call the callback. - */ + * Call the callback. + */ public void call(%s); } """) @@ -31,6 +31,16 @@ package org.chromium.mojo.bindings; * Contains a generic interface for callbacks. */ public interface Callbacks { + + /** + * A generic callback. + */ + interface Callback0 { + /** + * Call the callback. + */ + public void call(); + } %s }""") -- 2.11.4.GIT