Mojo JavaScript bindings should be binary compatible with C++

After this CL, the JavaScript bindings we autogenerate from
sample_service.mojom files are binary compatible with the C++ bindings. I've
also replaced the hand-written sample_service.js file with the auto-generated
version.

I've also moved the logic for assigning implicit ordinals to methods into
common code that's shared between the CPP and JS bindings generators.

BUG=317398

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

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

9 files changed, 123 insertions, 92 deletions

diff --git a/mojo/public/bindings/generators/js_templates/module.js.tmpl b/mojo/public/bindings/generators/js_templates/module.js.tmpl
index b13aaab7..9394d8b 100644
--- a/mojo/public/bindings/generators/js_templates/module.js.tmpl
+++ b/mojo/public/bindings/generators/js_templates/module.js.tmpl
@@ -73,7 +73,7 @@ define([
   }
 
 {%- for method in interface.methods %}
-  {{interface.name}}Proxy.prototype.{{method.name}} = function(
+  {{interface.name}}Proxy.prototype.{{method.name|stylize_method}} = function(
 {%- for parameter in method.parameters -%}
 {{parameter.name}}{% if not loop.last %}, {% endif %}
 {%- endfor %}) {
@@ -100,7 +100,7 @@ define([
 {%- for method in interface.methods %}
     case k{{interface.name}}_{{method.name}}_Name:
       var params = reader.decodeStruct({{interface.name}}_{{method.name}}_Params);
-      this.{{method.name}}(
+      this.{{method.name|stylize_method}}(
 {%- for parameter in method.parameters -%}
 params.{{parameter.name}}{% if not loop.last %}, {% endif %}
 {%- endfor %});
diff --git a/mojo/public/bindings/generators/mojom_cpp_generator.py b/mojo/public/bindings/generators/mojom_cpp_generator.py
index 813491c..0d329bb 100644
--- a/mojo/public/bindings/generators/mojom_cpp_generator.py
+++ b/mojo/public/bindings/generators/mojom_cpp_generator.py
@@ -191,8 +191,7 @@ class CPPGenerator(mojom_generator.Generator):
 
   def WriteTemplateToFile(self, template_name, **substitutions):
     template = self.GetTemplate(template_name)
-    filename = template_name.replace(
-        "module", mojom_generator.CamelToUnderscores(self.module.name))
+    filename = template_name.replace("module", self.module.name)
     substitutions['YEAR'] = datetime.date.today().year
     substitutions['NAMESPACE'] = self.module.namespace
     if self.output_dir is None:
@@ -321,17 +320,16 @@ class CPPGenerator(mojom_generator.Generator):
         INTERFACE_PROXY_DECLARATIONS = self.GetInterfaceProxyDeclarations(),
         INTERFACE_STUB_DECLARATIONS = self.GetInterfaceStubDeclarations())
 
-  def GetParamsDefinition(self, interface, method, next_id):
+  def GetParamsDefinition(self, interface, method):
     struct = mojom_generator.GetStructFromMethod(interface, method)
     method_name = "k%s_%s_Name" % (interface.name, method.name)
-    if method.ordinal is not None:
-      next_id = method.ordinal
+    assert method.ordinal is not None
     params_def = self.GetStructDeclaration(
         struct.name,
         mojom_pack.PackedStruct(struct),
         self.GetTemplate("params_definition"),
-        {'METHOD_NAME': method_name, 'METHOD_ID': next_id})
-    return params_def, next_id + 1
+        {'METHOD_NAME': method_name, 'METHOD_ID': method.ordinal})
+    return params_def
 
   def GetStructDefinitions(self):
     template = self.GetTemplate("struct_definition")
@@ -425,10 +423,8 @@ class CPPGenerator(mojom_generator.Generator):
   def GetParamsDefinitions(self):
     params_defs = []
     for interface in self.module.interfaces:
-      next_id = 0
       for method in interface.methods:
-        (params_def, next_id) = \
-            self.GetParamsDefinition(interface, method, next_id)
+        params_def = self.GetParamsDefinition(interface, method)
         params_defs.append(params_def)
     return '\n'.join(params_defs)
 
diff --git a/mojo/public/bindings/generators/mojom_data.py b/mojo/public/bindings/generators/mojom_data.py
index d9b800a..7aa150c 100644
--- a/mojo/public/bindings/generators/mojom_data.py
+++ b/mojo/public/bindings/generators/mojom_data.py
@@ -161,6 +161,13 @@ def ModuleFromData(data):
     lambda interface: InterfaceFromData(kinds, interface), data['interfaces'])
   return module
 
-
-
-
+def OrderedModuleFromData(data):
+  module = ModuleFromData(data)
+  next_interface_ordinal = 0
+  for interface in module.interfaces:
+    next_ordinal = 0
+    for method in interface.methods:
+      if method.ordinal is None:
+        method.ordinal = next_ordinal
+      next_ordinal = method.ordinal + 1
+  return module
diff --git a/mojo/public/bindings/generators/mojom_generator.py b/mojo/public/bindings/generators/mojom_generator.py
index 06c59e4..ee83ea1 100644
--- a/mojo/public/bindings/generators/mojom_generator.py
+++ b/mojo/public/bindings/generators/mojom_generator.py
@@ -22,6 +22,9 @@ def CamelToUnderscores(camel):
   s = re.sub('(.)([A-Z][a-z]+)', r'\1_\2', camel)
   return re.sub('([a-z0-9])([A-Z])', r'\1_\2', s).lower()
 
+def StudlyCapsToCamel(studly):
+  return studly[0].lower() + studly[1:]
+
 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/generators/mojom_js_generator.py b/mojo/public/bindings/generators/mojom_js_generator.py
index e1099be..2bfe9e8 100644
--- a/mojo/public/bindings/generators/mojom_js_generator.py
+++ b/mojo/public/bindings/generators/mojom_js_generator.py
@@ -7,10 +7,10 @@
 import os
 import mojom
 import mojom_pack
+import mojom_generator
 
 from functools import partial
 from template_expander import UseJinja
-from mojom_generator import Generator, GetStructFromMethod, CamelToUnderscores
 
 _kind_to_default_value = {
   mojom.BOOL:   "false",
@@ -21,7 +21,7 @@ _kind_to_default_value = {
   mojom.INT32:  "0",
   mojom.UINT32: "0",
   mojom.FLOAT:  "0",
-  mojom.HANDLE: "codec.kInvalidHandle",
+  mojom.HANDLE: "core.kInvalidHandle",
   mojom.INT64:  "0",
   mojom.UINT64: "0",
   mojom.DOUBLE: "0",
@@ -90,7 +90,7 @@ _kind_to_decode_snippet = {
   mojom.INT64:  "read64()",
   mojom.UINT64: "read64()",
   mojom.DOUBLE: "decodeDouble()",
-  mojom.STRING: "decodeString()",
+  mojom.STRING: "decodeStringPointer()",
 }
 
 
@@ -116,7 +116,7 @@ _kind_to_encode_snippet = {
   mojom.INT64:  "write64(",
   mojom.UINT64: "write64(",
   mojom.DOUBLE: "encodeDouble(",
-  mojom.STRING: "encodeString(",
+  mojom.STRING: "encodeStringPointer(",
 }
 
 
@@ -139,19 +139,20 @@ def GetStructInfo(exported, struct):
   }
 
 
-class JSGenerator(Generator):
+class JSGenerator(mojom_generator.Generator):
   filters = {
     "default_value": DefaultValue,
     "payload_size": PayloadSize,
     "decode_snippet": DecodeSnippet,
     "encode_snippet": EncodeSnippet,
+    "stylize_method": mojom_generator.StudlyCapsToCamel,
   }
 
   def GetStructsFromMethods(self):
     result = []
     for interface in self.module.interfaces:
       for method in interface.methods:
-        result.append(GetStructFromMethod(interface, method))
+        result.append(mojom_generator.GetStructFromMethod(interface, method))
     return map(partial(GetStructInfo, False), result)
 
   def GetStructs(self):
@@ -168,7 +169,7 @@ class JSGenerator(Generator):
     if self.output_dir is None:
       print contents
       return
-    filename = "%s.js" % CamelToUnderscores(self.module.name)
+    filename = "%s.js" % self.module.name
     with open(os.path.join(self.output_dir, filename), "w+") as f:
       f.write(contents)
 
diff --git a/mojo/public/bindings/generators/mojom_pack.py b/mojo/public/bindings/generators/mojom_pack.py
index 3bf47ff..f5abbd9 100644
--- a/mojo/public/bindings/generators/mojom_pack.py
+++ b/mojo/public/bindings/generators/mojom_pack.py
@@ -128,6 +128,9 @@ def GetByteLayout(packed_struct):
     bytes[packed_field.offset].packed_fields.append(packed_field)
     limit_of_previous_field = packed_field.offset + packed_field.size
 
+  for i in xrange(limit_of_previous_field, len(bytes)):
+    bytes[i].is_padding = True
+
   for byte in bytes:
     # A given byte cannot both be padding and have a fields packed into it.
     assert not (byte.is_padding and byte.packed_fields)
diff --git a/mojo/public/bindings/js/codec_unittests.js b/mojo/public/bindings/js/codec_unittests.js
index 8311f2c..e177f8c 100644
--- a/mojo/public/bindings/js/codec_unittests.js
+++ b/mojo/public/bindings/js/codec_unittests.js
@@ -142,11 +142,11 @@ define([
     gtest.expectEqual(foo2.y, foo.y,
         "foo2.y is " + foo2.y);
 
-    gtest.expectEqual(foo2.a, foo.a & 1,
+    gtest.expectEqual(foo2.a, foo.a & 1 ? true : false,
         "foo2.a is " + foo2.a);
-    gtest.expectEqual(foo2.b, foo.b & 1,
+    gtest.expectEqual(foo2.b, foo.b & 1 ? true : false,
         "foo2.b is " + foo2.b);
-    gtest.expectEqual(foo2.c, foo.c & 1,
+    gtest.expectEqual(foo2.c, foo.c & 1 ? true : false,
         "foo2.c is " + foo2.c);
 
     barMatches(foo2.bar, foo.bar);
diff --git a/mojo/public/bindings/mojom_bindings_generator.py b/mojo/public/bindings/mojom_bindings_generator.py
index 41e2615..6ea1754 100755
--- a/mojo/public/bindings/mojom_bindings_generator.py
+++ b/mojo/public/bindings/mojom_bindings_generator.py
@@ -37,7 +37,7 @@ def Main():
     # at least avoid generating the serialized Mojom IR.
     tree = mojo_parser.Parse(filename)
     mojom = mojo_translate.Translate(tree, name)
-    module = mojom_data.ModuleFromData(mojom)
+    module = mojom_data.OrderedModuleFromData(mojom)
     cpp = mojom_cpp_generator.CPPGenerator(
         module, options.include_dir, options.output_dir)
     cpp.GenerateFiles()
diff --git a/mojo/public/bindings/sample/mojom/sample_service.js b/mojo/public/bindings/sample/mojom/sample_service.js
index 4957039..853b0ca 100644
--- a/mojo/public/bindings/sample/mojom/sample_service.js
+++ b/mojo/public/bindings/sample/mojom/sample_service.js
@@ -2,14 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// "Generated" code for sample_service.mojom
 define([
     "mojo/public/bindings/js/core",
     "mojo/public/bindings/js/codec",
   ], function(core, codec) {
 
-  // Bar ----------------------------------------------------------------------
-
   function Bar() {
     this.alpha = 0;
     this.beta = 0;
@@ -19,34 +16,41 @@ define([
   Bar.encodedSize = codec.kStructHeaderSize + 8;
 
   Bar.decode = function(decoder) {
+    var packed;
     var val = new Bar();
     var numberOfBytes = decoder.read32();
     var numberOfFields = decoder.read32();
-    // TODO(abarth): We need to support optional fields.
     val.alpha = decoder.read8();
     val.beta = decoder.read8();
     val.gamma = decoder.read8();
-    decoder.skip(5);
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
     return val;
   };
 
   Bar.encode = function(encoder, val) {
+    var packed;
     encoder.write32(Bar.encodedSize);
     encoder.write32(3);
     encoder.write8(val.alpha);
     encoder.write8(val.beta);
     encoder.write8(val.gamma);
-    encoder.skip(5);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
   };
 
-  // Foo ----------------------------------------------------------------------
-
   function Foo() {
     this.x = 0;
     this.y = 0;
-    this.a = 0;
-    this.b = 0;
-    this.c = 0;
+    this.a = false;
+    this.b = false;
+    this.c = false;
     this.bar = null;
     this.data = [];
     this.extra_bars = [];
@@ -57,17 +61,23 @@ define([
   Foo.encodedSize = codec.kStructHeaderSize + 56;
 
   Foo.decode = function(decoder) {
+    var packed;
     var val = new Foo();
     var numberOfBytes = decoder.read32();
     var numberOfFields = decoder.read32();
-    // TODO(abarth): We need to support optional fields.
     val.x = decoder.read32();
     val.y = decoder.read32();
-    var packed = decoder.read8();
-    val.a = (packed >> 0) & 1;
-    val.b = (packed >> 1) & 1;
-    val.c = (packed >> 2) & 1;
-    decoder.skip(7);
+    packed = decoder.read8();
+    val.a = (packed >> 0) & 1 ? true : false;
+    val.b = (packed >> 1) & 1 ? true : false;
+    val.c = (packed >> 2) & 1 ? true : false;
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
     val.bar = decoder.decodeStructPointer(Bar);
     val.data = decoder.decodeArrayPointer(codec.Uint8);
     val.extra_bars = decoder.decodeArrayPointer(new codec.PointerTo(Bar));
@@ -77,15 +87,23 @@ define([
   };
 
   Foo.encode = function(encoder, val) {
+    var packed;
     encoder.write32(Foo.encodedSize);
     encoder.write32(10);
     encoder.write32(val.x);
     encoder.write32(val.y);
-    var packed = (val.a & 1) << 0 |
-                 (val.b & 1) << 1 |
-                 (val.c & 1) << 2;
+    packed = 0;
+    packed |= (val.a & 1) << 0
+    packed |= (val.b & 1) << 1
+    packed |= (val.c & 1) << 2
     encoder.write8(packed);
-    encoder.skip(7);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
     encoder.encodeStructPointer(Bar, val.bar);
     encoder.encodeArrayPointer(codec.Uint8, val.data);
     encoder.encodeArrayPointer(new codec.PointerTo(Bar), val.extra_bars);
@@ -93,51 +111,83 @@ define([
     encoder.encodeArrayPointer(codec.Handle, val.files);
   };
 
-  // Service ------------------------------------------------------------------
-
-  var kService_Frobinate_Name = 0;
-
   function Service_Frobinate_Params() {
     this.foo = null;
     this.baz = false;
-    this.port = core.kInvalidHandle;
+    this.port = codec.kInvalidHandle;
   }
 
   Service_Frobinate_Params.encodedSize = codec.kStructHeaderSize + 16;
 
   Service_Frobinate_Params.decode = function(decoder) {
+    var packed;
     var val = new Service_Frobinate_Params();
     var numberOfBytes = decoder.read32();
     var numberOfFields = decoder.read32();
     val.foo = decoder.decodeStructPointer(Foo);
     val.baz = decoder.read8() & 1;
-    decoder.skip(3);
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
     val.port = decoder.decodeHandle();
     return val;
   };
 
   Service_Frobinate_Params.encode = function(encoder, val) {
+    var packed;
     encoder.write32(Service_Frobinate_Params.encodedSize);
     encoder.write32(3);
     encoder.encodeStructPointer(Foo, val.foo);
-    encoder.write8(val.baz & 1);
-    encoder.skip(3);
+    encoder.write8(1 & val.baz);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
     encoder.encodeHandle(val.port);
   };
 
+  function ServiceClient_DidFrobinate_Params() {
+    this.result = 0;
+  }
+
+  ServiceClient_DidFrobinate_Params.encodedSize = codec.kStructHeaderSize + 8;
+
+  ServiceClient_DidFrobinate_Params.decode = function(decoder) {
+    var packed;
+    var val = new ServiceClient_DidFrobinate_Params();
+    var numberOfBytes = decoder.read32();
+    var numberOfFields = decoder.read32();
+    val.result = decoder.read32();
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
+    decoder.skip(1);
+    return val;
+  };
+
+  ServiceClient_DidFrobinate_Params.encode = function(encoder, val) {
+    var packed;
+    encoder.write32(ServiceClient_DidFrobinate_Params.encodedSize);
+    encoder.write32(1);
+    encoder.write32(val.result);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
+    encoder.skip(1);
+  };
+  var kService_Frobinate_Name = 0;
+
   function ServiceProxy(receiver) {
     this.receiver_ = receiver;
   }
-
   ServiceProxy.prototype.frobinate = function(foo, baz, port) {
     var params = new Service_Frobinate_Params();
     params.foo = foo;
     params.baz = baz;
     params.port = port;
 
-    var payloadSize = codec.align(Service_Frobinate_Params.encodedSize);
-    var builder = new codec.MessageBuilder(kService_Frobinate_Name,
-                                           payloadSize);
+    var builder = new codec.MessageBuilder(
+        kService_Frobinate_Name,
+        codec.align(Service_Frobinate_Params.encodedSize));
     builder.encodeStruct(Service_Frobinate_Params, params);
     var message = builder.finish();
     this.receiver_.accept(message);
@@ -157,45 +207,18 @@ define([
       return false;
     }
   };
-
-  // ServiceClient ------------------------------------------------------------
-
   var kServiceClient_DidFrobinate_Name = 0;
 
-  function ServiceClient_DidFrobinate_Params() {
-    this.result = 0;
-  }
-
-  ServiceClient_DidFrobinate_Params.encodedSize = codec.kStructHeaderSize + 8;
-
-  ServiceClient_DidFrobinate_Params.decode = function(decoder) {
-    var val = new ServiceClient_DidFrobinate_Params();
-    var numberOfBytes = decoder.read32();
-    var numberOfFields = decoder.read32();
-    val.result = decoder.read32();
-    decoder.skip(4);
-    return val;
-  };
-
-  ServiceClient_DidFrobinate_Params.encode = function(encoder, val) {
-    encoder.write32(ServiceClient_DidFrobinate_Params.encodedSize);
-    encoder.write32(1);
-    encoder.write32(val.result);
-    encoder.skip(4);
-  };
-
   function ServiceClientProxy(receiver) {
     this.receiver_ = receiver;
   }
-
   ServiceClientProxy.prototype.didFrobinate = function(result) {
     var params = new ServiceClient_DidFrobinate_Params();
     params.result = result;
 
-    var payloadSize = codec.align(
-        ServiceClient_DidFrobinate_Params.encodedSize);
-    var builder = new codec.MessageBuilder(kServiceClient_DidFrobinate_Name,
-                                           payloadSize);
+    var builder = new codec.MessageBuilder(
+        kServiceClient_DidFrobinate_Name,
+        codec.align(ServiceClient_DidFrobinate_Params.encodedSize));
     builder.encodeStruct(ServiceClient_DidFrobinate_Params, params);
     var message = builder.finish();
     this.receiver_.accept(message);
@@ -208,7 +231,7 @@ define([
     var reader = new codec.MessageReader(message);
     switch (reader.messageName) {
     case kServiceClient_DidFrobinate_Name:
-      var params = reader.decodeStruct(kServiceClient_DidFrobinate_Name);
+      var params = reader.decodeStruct(ServiceClient_DidFrobinate_Params);
       this.didFrobinate(params.result);
       return true;
     default:
@@ -216,8 +239,6 @@ define([
     }
   };
 
-  // Exports ------------------------------------------------------------------
-
   var exports = {};
   exports.Bar = Bar;
   exports.Foo = Foo;