Fix bug with using enums as default values in mojom. We were previously

generating invalid JavaScript bindings. We now translate an enum value like
FOO_VALUE in a mojom file to proper_namespace.EnumName.FOO_VALUE in JS.

I also fixed enum declarations in JS so an enum value can be defined in terms
of itself, e.g.
  enum Foo { FOO_1, FOO_2 = FOO_1 * 2 };

BUG=320082
R=darin@chromium.org

Review URL: https://codereview.chromium.org/164873002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251417 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 117 insertions, 47 deletions

diff --git a/mojo/apps/js/bindings/sample_service_unittests.js b/mojo/apps/js/bindings/sample_service_unittests.js
index 2992926..0bcaa30 100644
--- a/mojo/apps/js/bindings/sample_service_unittests.js
+++ b/mojo/apps/js/bindings/sample_service_unittests.js
@@ -8,7 +8,8 @@ define([
     "gin/test/expect",
     "mojom/sample_service",
     "mojom/sample_import",
-  ], function(console, hexdump, expect, sample, imported) {
+    "mojom/sample_import2",
+  ], function(console, hexdump, expect, sample, imported, imported2) {
 
   var global = this;
 
@@ -117,12 +118,10 @@ define([
     expect(full.point.y).toBe(15);
 
     expect(full.shape_masks.length).toBe(1);
-    // TODO(mpcomplete): This is broken.
-    // http://crbug.com/320082
-    // expect(full.shape_masks[0]).toBe(1 << imported.Shape.SHAPE_RECTANGLE);
+    expect(full.shape_masks[0]).toBe(1 << imported.Shape.SHAPE_RECTANGLE);
 
-    //expect(full.thing.shape).toBe(imported.Shape.SHAPE_RECTANGLE);
-    //expect(full.thing.color).toBe(imported.Color.COLOR_RED);
+    expect(full.thing.shape).toBe(imported.Shape.SHAPE_CIRCLE);
+    expect(full.thing.color).toBe(imported2.Color.COLOR_BLACK);
   }
 
   function ServiceImpl() {
diff --git a/mojo/public/bindings/generators/cpp_templates/enum_declaration.tmpl b/mojo/public/bindings/generators/cpp_templates/enum_declaration.tmpl
index 08c12c5..1c80eee 100644
--- a/mojo/public/bindings/generators/cpp_templates/enum_declaration.tmpl
+++ b/mojo/public/bindings/generators/cpp_templates/enum_declaration.tmpl
@@ -1,7 +1,7 @@
 enum {{enum.name}} {
 {%- for field in enum.fields %}
 {%-    if field.value %}
-  {{field.name}} = {{field.value}},
+  {{field.name}} = {{field.value|expression_to_text(module)}},
 {%-    else %}
   {{field.name}},
 {%-    endif %}
diff --git a/mojo/public/bindings/generators/cpp_templates/struct_builder_definition.tmpl b/mojo/public/bindings/generators/cpp_templates/struct_builder_definition.tmpl
index 86f4c21..ea9c0e9 100644
--- a/mojo/public/bindings/generators/cpp_templates/struct_builder_definition.tmpl
+++ b/mojo/public/bindings/generators/cpp_templates/struct_builder_definition.tmpl
@@ -9,7 +9,7 @@
 {%- macro set_default(kind, value, depth) -%}
 {#--- Strings ---#}
 {%-   if kind|is_string_kind -%}
-{{caller("mojo::String(" ~ value ~ ")")}}
+{{caller("mojo::String(" ~ value|expression_to_text(module) ~ ")")}}
 {#--- Arrays ---#}
 {%-   elif kind|is_array_kind %}
 {%-     set _ = value|verify_token_type("ARRAY") %}
@@ -43,7 +43,7 @@ tmp{{depth}}.set_{{subfield.name}}({{result}});
 }
 {#--- POD types ---#}
 {%-   else -%}
-{{caller(value|substitute_namespace(imports))}}
+{{caller(value|expression_to_text(module))}}
 {%-   endif %}
 {%- endmacro %}
 
diff --git a/mojo/public/bindings/generators/js_templates/enum_definition.tmpl b/mojo/public/bindings/generators/js_templates/enum_definition.tmpl
index a87db5c..653cfaa 100644
--- a/mojo/public/bindings/generators/js_templates/enum_definition.tmpl
+++ b/mojo/public/bindings/generators/js_templates/enum_definition.tmpl
@@ -1,12 +1,14 @@
-{%- macro enum_def(enum_init, enum) %}
-  {{enum_init}} = {
-{%- set next_value = 0 %}
+{%- macro enum_def(enum_name, enum, module) -%}
+  {{enum_name}} = {};
+
+{%- set prev_enum = 0 %}
 {%- for field in enum.fields %}
 {%-   if field.value %}
-{%-     set next_value = field.value|int  %}
+  {{enum_name}}.{{field.name}} = {{field.value|expression_to_text(module)}};
+{%-   elif loop.first %}
+  {{enum_name}}.{{field.name}} = 0;
+{%-   else %}
+  {{enum_name}}.{{field.name}} = {{enum_name}}.{{enum.fields[loop.index0 - 1].name}} + 1;
 {%-   endif %}
-    {{field.name}}: {{next_value}},
-{%-   set next_value = next_value + 1 %}
 {%- endfor %}
-  };
 {%- endmacro %}
diff --git a/mojo/public/bindings/generators/js_templates/interface_definition.tmpl b/mojo/public/bindings/generators/js_templates/interface_definition.tmpl
index fd5e27a..df1483d 100644
--- a/mojo/public/bindings/generators/js_templates/interface_definition.tmpl
+++ b/mojo/public/bindings/generators/js_templates/interface_definition.tmpl
@@ -46,8 +46,8 @@ params.{{parameter.name}}{% if not loop.last %}, {% endif %}
   };
 
 {#--- Enums #}
-{%- from "enum_definition.tmpl" import enum_def -%}
+{%  from "enum_definition.tmpl" import enum_def -%}
 {%  for enum in interface.enums %}
-{{  enum_def("%sProxy.%s"|format(interface.name, enum.name), enum)}}
+  {{enum_def("%sProxy.%s"|format(interface.name, enum.name), enum, module)}}
   {{interface.name}}Stub.{{enum.name}} = {{interface.name}}Proxy.{{enum.name}};
 {%-  endfor %}
diff --git a/mojo/public/bindings/generators/js_templates/module.js.tmpl b/mojo/public/bindings/generators/js_templates/module.js.tmpl
index 2fb3c0e..3bce117 100644
--- a/mojo/public/bindings/generators/js_templates/module.js.tmpl
+++ b/mojo/public/bindings/generators/js_templates/module.js.tmpl
@@ -15,14 +15,14 @@ define([
 ) {
 
 {#--- Enums #}
-{%- from "enum_definition.tmpl" import enum_def -%}
+{%  from "enum_definition.tmpl" import enum_def -%}
 {%  for enum in enums %}
-{{    enum_def("var %s"|format(enum.name), enum) }}
+  var {{ enum_def(enum.name, enum, module) }}
 {%-  endfor %}
 
 {#--- Struct definitions #}
 {%  for struct in structs %}
-{%    include "struct_definition.tmpl" %}
+{%-   include "struct_definition.tmpl" %}
 {%- endfor %}
 
 {#--- Interface definitions #}
diff --git a/mojo/public/bindings/generators/js_templates/struct_definition.tmpl b/mojo/public/bindings/generators/js_templates/struct_definition.tmpl
index 4c66df2..e80e675 100644
--- a/mojo/public/bindings/generators/js_templates/struct_definition.tmpl
+++ b/mojo/public/bindings/generators/js_templates/struct_definition.tmpl
@@ -1,7 +1,7 @@
 {%- macro set_default(kind, value, depth) -%}
 {#--- Strings ---#}
 {%-   if kind|is_string_kind -%}
-{{caller(value)}}
+{{caller(value|expression_to_text(module))}}
 {#--- Arrays ---#}
 {%-   elif kind|is_array_kind %}
 {%-     set _ = value|verify_token_type("ARRAY") %}
@@ -35,11 +35,12 @@ tmp{{depth}}.{{subfield.name}} = {{result}};
 }
 {#--- POD types ---#}
 {%-   else -%}
-{{caller(value|substitute_namespace(imports))}}
+{{caller(value|expression_to_text(module))}}
 {%-   endif %}
 {%- endmacro %}
 
 
+{#--- Begin #}
   function {{struct.name}}() {
 {%- for packed_field in struct.packed.packed_fields %}
 {%- if packed_field.field.default %}
diff --git a/mojo/public/bindings/generators/mojom_cpp_generator.py b/mojo/public/bindings/generators/mojom_cpp_generator.py
index b0592a8..4420aaf 100644
--- a/mojo/public/bindings/generators/mojom_cpp_generator.py
+++ b/mojo/public/bindings/generators/mojom_cpp_generator.py
@@ -102,11 +102,17 @@ def IsStructWithHandles(struct):
       return True
   return False
 
-def SubstituteNamespace(value, imports):
-  for import_item in imports:
-    value = value.replace(import_item['namespace'] + ".",
-        import_item['namespace'] + "::")
-  return value
+def SubstituteNamespace(token, module):
+  for import_item in module.imports:
+    token = token.replace(import_item["namespace"] + ".",
+        import_item["namespace"] + "::")
+  return token
+
+def ExpressionToText(value, module):
+  if value[0] != "EXPRESSION":
+    raise Exception("Expected EXPRESSION, got" + value)
+  return "".join(mojom_generator.ExpressionMapper(value,
+      lambda token: SubstituteNamespace(token, module)))
 
 _HEADER_SIZE = 8
 
@@ -118,6 +124,7 @@ class Generator(mojom_generator.Generator):
     "cpp_field_type": GetCppFieldType,
     "cpp_type": GetCppType,
     "cpp_wrapper_type": GetCppWrapperType,
+    "expression_to_text": ExpressionToText,
     "get_pad": mojom_pack.GetPad,
     "is_handle_kind": mojom_generator.IsHandleKind,
     "is_object_kind": mojom_generator.IsObjectKind,
@@ -127,12 +134,12 @@ class Generator(mojom_generator.Generator):
     "struct_size": lambda ps: ps.GetTotalSize() + _HEADER_SIZE,
     "struct_from_method": mojom_generator.GetStructFromMethod,
     "stylize_method": mojom_generator.StudlyCapsToCamel,
-    "substitute_namespace": SubstituteNamespace,
     "verify_token_type": mojom_generator.VerifyTokenType,
   }
 
   def GetJinjaExports(self):
     return {
+      "module": self.module,
       "module_name": self.module.name,
       "namespace": self.module.namespace,
       "imports": self.module.imports,
diff --git a/mojo/public/bindings/generators/mojom_js_generator.py b/mojo/public/bindings/generators/mojom_js_generator.py
index bfa443a..252e4ce 100644
--- a/mojo/public/bindings/generators/mojom_js_generator.py
+++ b/mojo/public/bindings/generators/mojom_js_generator.py
@@ -10,7 +10,6 @@ from generate import mojom_generator
 
 from generate.template_expander import UseJinja
 
-
 _kind_to_javascript_default_value = {
   mojom.BOOL:    "false",
   mojom.INT8:    "0",
@@ -148,13 +147,59 @@ def JavaScriptEncodeSnippet(kind):
     return JavaScriptEncodeSnippet(mojom.MSGPIPE)
 
 
-def SubstituteNamespace(value, imports):
-  for import_item in imports:
-    value = value.replace(import_item["namespace"] + ".",
-        import_item["unique_name"] + ".")
+def GetConstants(module):
+  """Returns a generator that enumerates all constants that can be referenced
+  from this module."""
+  class Constant:
+    pass
+
+  for enum in module.enums:
+    for field in enum.fields:
+      constant = Constant()
+      constant.namespace = module.namespace
+      constant.is_current_namespace = True
+      constant.import_item = None
+      constant.name = (enum.name, field.name)
+      yield constant
+
+  for each in module.imports:
+    for enum in each["module"].enums:
+      for field in enum.fields:
+        constant = Constant()
+        constant.namespace = each["namespace"]
+        constant.is_current_namespace = constant.namespace == module.namespace
+        constant.import_item = each
+        constant.name = (enum.name, field.name)
+        yield constant
+
+
+def TranslateConstants(value, module):
+  # We're assuming we're dealing with an identifier, but that may not be
+  # the case. If we're not, we just won't find any matches.
+  if value.find(".") != -1:
+    namespace, identifier = value.split(".")
+  else:
+    namespace, identifier = "", value
+
+  for constant in GetConstants(module):
+    if namespace == constant.namespace or (
+        namespace == "" and constant.is_current_namespace):
+      if constant.name[1] == identifier:
+        if constant.import_item:
+          return "%s.%s.%s" % (constant.import_item["unique_name"],
+              constant.name[0], constant.name[1])
+        else:
+          return "%s.%s" % (constant.name[0], constant.name[1])
   return value
 
 
+def ExpressionToText(value, module):
+  if value[0] != "EXPRESSION":
+    raise Exception("Expected EXPRESSION, got" + value)
+  return "".join(mojom_generator.ExpressionMapper(value,
+      lambda token: TranslateConstants(token, module)))
+
+
 def JavascriptType(kind):
   if kind.imported_from:
     return kind.imported_from["unique_name"] + "." + kind.name
@@ -169,12 +214,12 @@ class Generator(mojom_generator.Generator):
     "payload_size": JavaScriptPayloadSize,
     "decode_snippet": JavaScriptDecodeSnippet,
     "encode_snippet": JavaScriptEncodeSnippet,
+    "expression_to_text": ExpressionToText,
     "is_object_kind": mojom_generator.IsObjectKind,
     "is_string_kind": mojom_generator.IsStringKind,
     "is_array_kind": lambda kind: isinstance(kind, mojom.Array),
     "js_type": JavascriptType,
     "stylize_method": mojom_generator.StudlyCapsToCamel,
-    "substitute_namespace": SubstituteNamespace,
     "verify_token_type": mojom_generator.VerifyTokenType,
   }
 
@@ -184,6 +229,7 @@ class Generator(mojom_generator.Generator):
       "imports": self.GetImports(),
       "kinds": self.module.kinds,
       "enums": self.module.enums,
+      "module": self.module,
       "structs": self.GetStructs() + self.GetStructsFromMethods(),
       "interfaces": self.module.interfaces,
     }
diff --git a/mojo/public/bindings/mojom_bindings_generator.gypi b/mojo/public/bindings/mojom_bindings_generator.gypi
index 0df8d4e..6a46dc2 100644
--- a/mojo/public/bindings/mojom_bindings_generator.gypi
+++ b/mojo/public/bindings/mojom_bindings_generator.gypi
@@ -16,6 +16,7 @@
       },
       'inputs': [
         '<(mojom_bindings_generator)',
+        '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/enum_declaration.tmpl',
         '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/interface_declaration.tmpl',
         '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/interface_definition.tmpl',
         '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/interface_proxy_declaration.tmpl',
@@ -30,6 +31,7 @@
         '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/struct_destructor.tmpl',
         '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/struct_macros.tmpl',
         '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl',
+        '<(DEPTH)/mojo/public/bindings/generators/js_templates/enum_definition.tmpl',
         '<(DEPTH)/mojo/public/bindings/generators/js_templates/interface_definition.tmpl',
         '<(DEPTH)/mojo/public/bindings/generators/js_templates/module.js.tmpl',
         '<(DEPTH)/mojo/public/bindings/generators/js_templates/struct_definition.tmpl',
diff --git a/mojo/public/bindings/pylib/generate/mojom_data.py b/mojo/public/bindings/pylib/generate/mojom_data.py
index 4972b689..9a575a8 100644
--- a/mojo/public/bindings/pylib/generate/mojom_data.py
+++ b/mojo/public/bindings/pylib/generate/mojom_data.py
@@ -60,6 +60,7 @@ def ImportFromData(module, data):
   import_item = {}
   import_item['module_name'] = import_module.name
   import_item['namespace'] = import_module.namespace
+  import_item['module'] = import_module
 
   # Copy the struct kinds from our imports into the current module.
   for kind in import_module.kinds.itervalues():
diff --git a/mojo/public/bindings/pylib/generate/mojom_generator.py b/mojo/public/bindings/pylib/generate/mojom_generator.py
index 24901ac..1148764 100644
--- a/mojo/public/bindings/pylib/generate/mojom_generator.py
+++ b/mojo/public/bindings/pylib/generate/mojom_generator.py
@@ -53,6 +53,14 @@ def VerifyTokenType(token, expected):
     raise Exception("Expected token type '%s'. Got '%s'." %
         (expected, token[0]))
 
+def ExpressionMapper(expression, mapper):
+  if isinstance(expression, tuple) and expression[0] == 'EXPRESSION':
+    result = []
+    for each in expression[1]:
+      result.extend(ExpressionMapper(each, mapper))
+    return result
+  return [mapper(expression)]
+
 class Generator(object):
   # Pass |output_dir| to emit files to disk. Omit |output_dir| to echo all
   # files to stdout.
diff --git a/mojo/public/bindings/pylib/parse/mojo_parser.py b/mojo/public/bindings/pylib/parse/mojo_parser.py
index 654814a..4db5cd2 100755
--- a/mojo/public/bindings/pylib/parse/mojo_parser.py
+++ b/mojo/public/bindings/pylib/parse/mojo_parser.py
@@ -243,7 +243,7 @@ class Parser(object):
 
   def p_expression(self, p):
     """expression : conditional_expression"""
-    p[0] = p[1]
+    p[0] = ('EXPRESSION', p[1])
 
   def p_conditional_expression(self, p):
     """conditional_expression : binary_expression
@@ -251,7 +251,7 @@ class Parser(object):
                                     conditional_expression"""
     # Just pass the arguments through. I don't think it's possible to preserve
     # the spaces of the original, so just put a single space between them.
-    p[0] = ' '.join(p[1:])
+    p[0] = ListFromConcat(*p[1:])
 
   # PLY lets us specify precedence of operators, but since we don't actually
   # evaluate them, we don't need that here.
@@ -259,7 +259,7 @@ class Parser(object):
     """binary_expression : unary_expression
                          | binary_expression binary_operator \
                                binary_expression"""
-    p[0] = ' '.join(p[1:])
+    p[0] = ListFromConcat(*p[1:])
 
   def p_binary_operator(self, p):
     """binary_operator : TIMES
@@ -285,7 +285,7 @@ class Parser(object):
   def p_unary_expression(self, p):
     """unary_expression : primary_expression
                         | unary_operator expression"""
-    p[0] = ''.join(p[1:])
+    p[0] = ListFromConcat(*p[1:])
 
   def p_unary_operator(self, p):
     """unary_operator : PLUS
@@ -296,9 +296,13 @@ class Parser(object):
 
   def p_primary_expression(self, p):
     """primary_expression : constant
-                          | NAME
-                          | NAME DOT NAME
+                          | identifier
                           | LPAREN expression RPAREN"""
+    p[0] = ListFromConcat(*p[1:])
+
+  def p_identifier(self, p):
+    """identifier : NAME
+                  | NAME DOT NAME"""
     p[0] = ''.join(p[1:])
 
   def p_constant(self, p):
@@ -311,7 +315,7 @@ class Parser(object):
                 | WCHAR_CONST
                 | STRING_LITERAL
                 | WSTRING_LITERAL"""
-    p[0] = ''.join(p[1:])
+    p[0] = ListFromConcat(*p[1:])
 
   def p_error(self, e):
     print('error: %s'%e)
diff --git a/mojo/public/bindings/tests/sample_import2.mojom b/mojo/public/bindings/tests/sample_import2.mojom
index 07c80f0..32be752 100644
--- a/mojo/public/bindings/tests/sample_import2.mojom
+++ b/mojo/public/bindings/tests/sample_import2.mojom
@@ -20,8 +20,8 @@ struct Size {
 };
 
 struct Thing {
-  int32 shape;
-  int32 color;
+  int32 shape = SHAPE_RECTANGLE;
+  int32 color = COLOR_BLACK;
   Point location = {0, 0};
   Size size;
 };
diff --git a/mojo/public/bindings/tests/sample_service.mojom b/mojo/public/bindings/tests/sample_service.mojom
index 0a15257..7bdd171 100644
--- a/mojo/public/bindings/tests/sample_service.mojom
+++ b/mojo/public/bindings/tests/sample_service.mojom
@@ -48,7 +48,7 @@ struct DefaultsTest {
   uint8[] data = [1, 2, 3] @2;
   imported.Point point = {7, 15} @3;
   int32[] shape_masks = [1 << imported.SHAPE_RECTANGLE] @4;
-  imported.Thing thing = {imported.SHAPE_RECTANGLE, imported.COLOR_RED};
+  imported.Thing thing = {imported.SHAPE_CIRCLE, imported.COLOR_BLACK};
 };
 
 interface Port {
diff --git a/mojo/public/bindings/tests/sample_service_unittest.cc b/mojo/public/bindings/tests/sample_service_unittest.cc
index 8f5590b..4c8a9a0 100644
--- a/mojo/public/bindings/tests/sample_service_unittest.cc
+++ b/mojo/public/bindings/tests/sample_service_unittest.cc
@@ -343,8 +343,8 @@ TEST(BindingsSampleTest, DefaultValues) {
   EXPECT_EQ(1u, full.shape_masks().size());
   EXPECT_EQ(1 << imported::SHAPE_RECTANGLE, full.shape_masks()[0]);
 
-  EXPECT_EQ(imported::SHAPE_RECTANGLE, full.thing().shape());
-  EXPECT_EQ(imported::COLOR_RED, full.thing().color());
+  EXPECT_EQ(imported::SHAPE_CIRCLE, full.thing().shape());
+  EXPECT_EQ(imported::COLOR_BLACK, full.thing().color());
 }
 
 }  // namespace sample