Fuchsia: Fix handling of nullable vectors in FIDL/JS

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

build/fuchsia/fidlgen_js/gen.py

@@ -395,30 +395,30 @@ const _kTT_%(ttname)s = {
     elif t.kind == fidl.TypeKind.VECTOR:
       element_ttname = self._CompileType(t.element_type)
       ttname = ('VEC_' + ('Nullable_' if t.nullable else '') + element_ttname)
-      pointer_set = '''    if (v === null || v === undefined) {
+      if t.nullable:
+        handle_null_enc = '''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 {
-      e.data.setUint32(o + 8, 0xffffffff, $fidl__kLE);
-      e.data.setUint32(o + 12, 0xffffffff, $fidl__kLE);
-    }'''
-      if not t.nullable:
-        throw_if_null_enc = ('if (v === null || v === undefined) '
-                             'throw "non-null vector required";')
-        throw_if_null_dec = ('if (pointer === 0) '
-                             'throw "non-null vector required";')
-        pointer_set = '''    e.data.setUint32(o + 8, 0xffffffff, $fidl__kLE);
-    e.data.setUint32(o + 12, 0xffffffff, $fidl__kLE);'''
+      return;
+'''
+        handle_null_dec = 'return null;'
+      else:
+        handle_null_enc = 'throw "non-null vector required";'
+        handle_null_dec = 'throw "non-null vector required";'
 
       if ttname not in self.type_table_defined:
         self.type_table_defined.add(ttname)
         self.output_deferred_to_eof += ('''\
 const _kTT_%(ttname)s = {
   enc: function(e, o, v) {
-    %(throw_if_null_enc)s
+    if (v === null || v === undefined) {
+      %(handle_null_enc)s
+    }
     e.data.setUint32(o, v.length, $fidl__kLE);
     e.data.setUint32(o + 4, 0, $fidl__kLE);
-%(pointer_set)s
+    e.data.setUint32(o + 8, 0xffffffff, $fidl__kLE);
+    e.data.setUint32(o + 12, 0xffffffff, $fidl__kLE);
     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]);
@@ -427,7 +427,9 @@ const _kTT_%(ttname)s = {
   dec: function(d, o) {
     var len = d.data.getUint32(o, $fidl__kLE);
     var pointer = d.data.getUint32(o + 8, $fidl__kLE);
-    %(throw_if_null_dec)s
+    if (pointer === 0) {
+      %(handle_null_dec)s
+    }
     var dataOffset = d.claimMemory(len * %(element_size)s);
     var result = [];
     for (var i = 0; i < len; i++) {
@@ -442,9 +444,8 @@ const _kTT_%(ttname)s = {
             'ttname': ttname,
             'element_ttname': element_ttname,
             'element_size': self._InlineSizeOfType(t.element_type),
-            'pointer_set': pointer_set,
-            'throw_if_null_enc': throw_if_null_enc,
-            'throw_if_null_dec': throw_if_null_dec
+            'handle_null_enc': handle_null_enc,
+            'handle_null_dec': handle_null_dec,
         })
       return ttname
     else:

build/fuchsia/fidlgen_js/test/fidlgen_js_unittest.cc

@@ -174,6 +174,10 @@ class BindingsSetupHelper {
     return result;
   }
 
+  bool IsNull(const std::string& name) {
+    return runner_.global()->Get(gin::StringToV8(isolate_, name))->IsNull();
+  }
+
   void DestroyBindingsForTesting() { zx_bindings_.reset(); }
 
   zx::channel& server() { return server_; }
@@ -255,6 +259,11 @@ class TestolaImpl : public fidljstest::Testola {
     for (uint64_t i = 0; i < fidljstest::ARRRR_SIZE; ++i) {
       sat.arrrr[i] = static_cast<int32_t>(i * 5) - 10;
     }
+    sat.nullable_vector_of_string0 = nullptr;
+    fidl::VectorPtr<fidl::StringPtr> vector_of_str;
+    vector_of_str.push_back("passed_str0");
+    vector_of_str.push_back("passed_str1");
+    sat.nullable_vector_of_string1 = std::move(vector_of_str);
 
     resp(std::move(sat));
   }
@@ -758,6 +767,8 @@ TEST_F(FidlGenJsTest, RawReceiveFidlNestedStructsAndRespond) {
              this.result_basic_u32 = sat.basic.u32;
              this.result_later_string = sat.later_string;
              this.result_arrrr = sat.arrrr;
+             this.result_vs0 = sat.nullable_vector_of_string0;
+             this.result_vs1 = sat.nullable_vector_of_string1;
            })
            .catch((e) => log('FAILED: ' + e));
     )";
@@ -790,6 +801,12 @@ TEST_F(FidlGenJsTest, RawReceiveFidlNestedStructsAndRespond) {
   for (uint64_t i = 0; i < fidljstest::ARRRR_SIZE; ++i) {
     EXPECT_EQ(result_arrrr[i], static_cast<int32_t>(i * 5) - 10);
   }
+  EXPECT_TRUE(helper.IsNull("result_vs0"));
+  EXPECT_FALSE(helper.IsNull("result_vs1"));
+  auto result_vs1 = helper.Get<std::vector<std::string>>("result_vs1");
+  ASSERT_EQ(result_vs1.size(), 2u);
+  EXPECT_EQ(result_vs1[0], "passed_str0");
+  EXPECT_EQ(result_vs1[1], "passed_str1");
 }
 
 TEST_F(FidlGenJsTest, HandlePassing) {

build/fuchsia/fidlgen_js/test/simple.fidl

@@ -31,6 +31,8 @@ struct StuffAndThings {
   BasicStruct basic;
   string later_string;
   array<int32>:ARRRR_SIZE arrrr;
+  vector<string>? nullable_vector_of_string0;
+  vector<string>? nullable_vector_of_string1;
 };
 
 struct StructWithBool {