Fuchsia: Implement vector<string>, fix traversal order

Previously the body of structs was traversed in-order, but out-of-line
items were deferred to a queue, making traversal mostly breadth-first.
Instead, fix encoding by emitting any out-of-line items by allocating
(and filling) directly following the current item.

Bug: 883496
Change-Id: If157930521e4cf5f89f702207defe91beb862def
Reviewed-on: https://chromium-review.googlesource.com/c/1316429
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605745}

build/fuchsia/fidlgen_js/gen.py

@@ -70,8 +70,10 @@ def _InlineSizeOfType(t):
         'uint64': 8,
         'uint8': 1,
     }[t.subtype]
+  elif t.kind == fidl.TypeKind.STRING:
+    return 16
   else:
-    raise NotImplementedError()
+    raise NotImplementedError(t.kind)
 
 
 def _CompileConstant(val, assignment_type):
@@ -223,10 +225,8 @@ const _kTT_%(name)s_Nullable = {
   enc: function(e, o, v) {
     e.data.setUint32(o, v ? 0xffffffff : 0, $fidl__kLE);
     e.data.setUint32(o + 4, v ? 0xffffffff : 0, $fidl__kLE);
-    e.outOfLine.push([function() {
-      var start = e.alloc(%(size)s);
-      _kTT_%(name)s.enc(e, start, v);
-    }]);
+    var start = e.alloc(%(size)s);
+    _kTT_%(name)s.enc(e, start, v);
   },
   dec: function(d, o) {
     if (d.data.getUint32(o, $fidl__kLE) === 0) {
@@ -344,7 +344,7 @@ function %(name)s(%(param_names)s) {
     if t.kind == fidl.TypeKind.PRIMITIVE:
       return t.subtype
     elif t.kind == fidl.TypeKind.STRING:
-      return 'String' + ('_Nullable' if t.nullable else '_Nonnull')
+      return 'String' + ('_Nullable' if t.nullable else '')
     elif t.kind == fidl.TypeKind.IDENTIFIER:
       compound = _ParseCompoundIdentifier(t.identifier)
       name = _CompileCompoundIdentifier(compound)
@@ -382,7 +382,7 @@ const _kTT_%(ttname)s = {
     elif t.kind == fidl.TypeKind.VECTOR:
       element_ttname = self._CompileType(t.element_type)
       ttname = (
-          'VEC_' + ('Nullable_' if t.nullable else 'Nonnull_') + element_ttname)
+          'VEC_' + ('Nullable_' if t.nullable else '') + element_ttname)
       pointer_set = '''    if (v === null || v === undefined) {
       e.data.setUint32(o + 8, 0, $fidl__kLE);
       e.data.setUint32(o + 12, 0, $fidl__kLE);
@@ -407,13 +407,10 @@ const _kTT_%(ttname)s = {
     e.data.setUint32(o, v.length, $fidl__kLE);
     e.data.setUint32(o + 4, 0, $fidl__kLE);
 %(pointer_set)s
-    e.outOfLine.push([function(e, body) {
-        var start = e.alloc(body.length * %(element_size)s);
-        for (var i = 0; i < body.length; i++) {
-          _kTT_%(element_ttname)s.enc(e, start + (i * %(element_size)s), body[i]);
-        }
-      },
-      v]);
+    var start = e.alloc(v.length * %(element_size)s);
+    for (var i = 0; i < v.length; i++) {
+      _kTT_%(element_ttname)s.enc(e, start + (i * %(element_size)s), v[i]);
+    }
   },
   dec: function(d, o) {
     var len = d.data.getUint32(o, $fidl__kLE);

build/fuchsia/fidlgen_js/runtime/fidl.mjs

@@ -36,7 +36,6 @@ function $fidl_Encoder(ordinal) {
   this.data = new DataView(buf);
   this.extent = 0;
   this.handles = [];
-  this.outOfLine = [];
   this._encodeMessageHeader(ordinal);
 }
 
@@ -88,13 +87,6 @@ $fidl_Encoder.prototype.addHandle = function(handle) {
 };
 
 $fidl_Encoder.prototype.messageData = function() {
-  // Add all out of line data.
-  var len = this.outOfLine.length;
-  for (var i = 0; i < len; i++) {
-    this.outOfLine[i][0](this, this.outOfLine[i][1]);
-  }
-
-  // Return final result.
   return new DataView(this.data.buffer, 0, this.extent);
 };
 
@@ -189,17 +181,21 @@ const _kTT_Handle = {
   },
 };
 
-const _kTT_String_Nonnull = {
+const _kTT_String = {
   enc: function(e, o, v) {
     if (v === null || v === undefined) throw "non-null string required";
     // Both size and data are uint64, but that's awkward in JS, so for now only
-    // support a maximum of 32b lengths.
+    // support a maximum of 32b lengths. The maximum length of a FIDL message is
+    // shorter than 32b in any case.
     var asUtf8 = $FidlJsStrToUtf8Array(v);
     e.data.setUint32(o, asUtf8.length, $fidl__kLE);
     e.data.setUint32(o + 4, 0, $fidl__kLE);
     e.data.setUint32(o + 8, 0xffffffff, $fidl__kLE);
     e.data.setUint32(o + 12, 0xffffffff, $fidl__kLE);
-    e.outOfLine.push([$fidl_OutOfLineStringEnc, asUtf8]);
+    var body = e.alloc(asUtf8.length);
+    for (var i = 0; i < asUtf8.length; i++) {
+      e.data.setUint8(body + i, asUtf8[i], $fidl__kLE);
+    }
   },
   dec: function(d, o) {
     var len = d.data.getUint32(o, $fidl__kLE);
@@ -210,9 +206,25 @@ const _kTT_String_Nonnull = {
   }
 };
 
-function $fidl_OutOfLineStringEnc(e, strAsUtf8Array) {
-  var start = e.alloc(strAsUtf8Array.length);
-  for (var i = 0; i < strAsUtf8Array.length; i++) {
-    e.data.setUint8(start + i, strAsUtf8Array[i], $fidl__kLE);
+const _kTT_String_Nullable = {
+  enc: function(e, o, v) {
+    if (v === null || v === undefined) {
+      e.data.setUint32(o, 0, $fidl__kLE);
+      e.data.setUint32(o + 4, 0, $fidl__kLE);
+      e.data.setUint32(o + 8, 0, $fidl__kLE);
+      e.data.setUint32(o + 12, 0, $fidl__kLE);
+    } else {
+      _kTT_String.enc(e, o, v);
+    }
+  },
+  dec: function(d, o) {
+    if (v === null || v === undefined) {
+      var pointer = d.data.getUint32(o + 8, $fidl__kLE);
+      if (pointer === 0) {
+        return null;
+      }
+    } else {
+      return _kTT_String.dec(e, o, v);
+    }
   }
-}
+};

build/fuchsia/fidlgen_js/test/fidlgen_js_unittest.cc

@@ -251,6 +251,27 @@ class TestolaImpl : public fidljstest::Testola {
     callback(std::move(resp));
   }
 
+  void SendVectorsOfString(fidl::VectorPtr<fidl::StringPtr> unsized,
+                           fidl::VectorPtr<fidl::StringPtr> nullable,
+                           fidl::VectorPtr<fidl::StringPtr> sized10) override {
+    ASSERT_EQ(unsized->size(), 3u);
+    EXPECT_EQ((*unsized)[0], "str0");
+    EXPECT_EQ((*unsized)[1], "str1");
+    EXPECT_EQ((*unsized)[2], "str2");
+
+    ASSERT_EQ(nullable->size(), 5u);
+    EXPECT_EQ((*nullable)[0], "str3");
+    EXPECT_TRUE((*nullable)[1].is_null());
+    EXPECT_TRUE((*nullable)[2].is_null());
+    EXPECT_TRUE((*nullable)[3].is_null());
+    EXPECT_EQ((*nullable)[4], "str4");
+
+    ASSERT_EQ(sized10->size(), 1u);
+    EXPECT_EQ((*sized10)[0], "0123456789");
+
+    did_get_vectors_of_string_ = true;
+  }
+
   bool was_do_something_called() const { return was_do_something_called_; }
   int32_t received_int() const { return received_int_; }
   const std::string& received_msg() const { return received_msg_; }
@@ -265,6 +286,8 @@ class TestolaImpl : public fidljstest::Testola {
 
   bool did_receive_union() const { return did_receive_union_; }
 
+  bool did_get_vectors_of_string() const { return did_get_vectors_of_string_; }
+
   void CallResponseCallbacks() {
     for (auto& cb : response_callbacks_) {
       std::move(cb).Run();
@@ -283,6 +306,7 @@ class TestolaImpl : public fidljstest::Testola {
   std::vector<base::OnceClosure> response_callbacks_;
   zx_handle_t unowned_log_handle_;
   bool did_receive_union_ = false;
+  bool did_get_vectors_of_string_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(TestolaImpl);
 };
@@ -723,6 +747,52 @@ TEST_F(FidlGenJsTest, DefaultUsingIdentifier) {
             static_cast<int>(fidljstest::Blorp::BETA));
 }
 
+TEST_F(FidlGenJsTest, VectorOfStrings) {
+  v8::Isolate* isolate = instance_->isolate();
+  BindingsSetupHelper helper(isolate);
+
+  TestolaImpl testola_impl;
+  fidl::Binding<fidljstest::Testola> binding(&testola_impl);
+  binding.Bind(std::move(helper.server()));
+
+  std::string source = R"(
+    var proxy = new TestolaProxy();
+    proxy.$bind(testHandle);
+
+    var v1 = ['str0', 'str1', 'str2'];
+    var v2 = ['str3', null, null, null, 'str4'];
+    var v3 = ['0123456789'];  // This is the maximum allowed length.
+    proxy.SendVectorsOfString(v1, v2, v3);
+  )";
+  helper.runner().Run(source, "test.js");
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(testola_impl.did_get_vectors_of_string());
+}
+
+TEST_F(FidlGenJsTest, VectorOfStringsTooLongString) {
+  v8::Isolate* isolate = instance_->isolate();
+  BindingsSetupHelper helper(isolate);
+
+  TestolaImpl testola_impl;
+  fidl::Binding<fidljstest::Testola> binding(&testola_impl);
+  binding.Bind(std::move(helper.server()));
+
+  std::string source = R"(
+    var proxy = new TestolaProxy();
+    proxy.$bind(testHandle);
+
+    var too_long = ['this string is longer than allowed'];
+    proxy.SendVectorsOfString([], [], too_long);
+    this.tried_to_send = true;
+  )";
+  helper.runner().Run(source, "test.js");
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(helper.Get<bool>("tried_to_send"));
+  EXPECT_FALSE(testola_impl.did_get_vectors_of_string());
+}
+
 int main(int argc, char** argv) {
   base::TestSuite test_suite(argc, argv);
 

build/fuchsia/fidlgen_js/test/simple.fidl

@@ -81,4 +81,8 @@ interface Testola {
   9: ReceiveUnions(StructOfMultipleUnions somu);
 
   10: SendUnions() -> (StructOfMultipleUnions somu);
+
+  11: SendVectorsOfString(vector<string> unsized,
+                          vector<string?> nullable,
+                          vector<string:10> sized10);
 };