Mojo bindings: Add string support to StructTraits

This CL enables string-typed fields in structs to be
serialized and deserialized via StructTraits using
base::StringPiece as an intermediate interface between
the native type and the message buffer.

BUG=577686
R=yzshen@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#376952}

mojo/mojo_edk_tests.gyp

@@ -56,6 +56,7 @@
         'mojo_public.gyp:mojo_public_test_interfaces',
         'mojo_public.gyp:mojo_public_test_interfaces_blink',
         'mojo_public.gyp:mojo_public_test_interfaces_chromium',
+        'mojo_public.gyp:mojo_public_test_interfaces_struct_traits',
         'mojo_public.gyp:mojo_public_test_utils',
         'mojo_public.gyp:mojo_utility',
       ],
@@ -94,6 +95,8 @@
         'public/cpp/bindings/tests/string_unittest.cc',
         'public/cpp/bindings/tests/struct_traits_unittest.cc',
         'public/cpp/bindings/tests/struct_unittest.cc',
+        'public/cpp/bindings/tests/struct_with_traits_impl.cc',
+        'public/cpp/bindings/tests/struct_with_traits_impl.h',
         'public/cpp/bindings/tests/sync_method_unittest.cc',
         'public/cpp/bindings/tests/type_conversion_unittest.cc',
         'public/cpp/bindings/tests/union_unittest.cc',

mojo/mojo_public.gyp

@@ -375,6 +375,19 @@
       },
       'includes': [ 'mojom_bindings_generator_explicit.gypi' ],
     },
+    {
+      'target_name': 'mojo_public_test_interfaces_struct_traits',
+      'type': 'static_library',
+      'variables': {
+        'mojom_extra_generator_args': [
+          '--typemap', '<(DEPTH)/mojo/public/interfaces/bindings/tests/struct_with_traits.typemap',
+        ],
+      },
+      'sources': [
+        'public/interfaces/bindings/tests/struct_with_traits.mojom',
+      ],
+      'includes': [ 'mojom_bindings_generator.gypi' ],
+    },
     {
       'target_name': 'mojo_public_test_interfaces_mojom_blink',
       'type': 'none',

mojo/public/cpp/bindings/tests/BUILD.gn

@@ -42,6 +42,8 @@ source_set("tests") {
     "string_unittest.cc",
     "struct_traits_unittest.cc",
     "struct_unittest.cc",
+    "struct_with_traits_impl.cc",
+    "struct_with_traits_impl.h",
     "sync_method_unittest.cc",
     "type_conversion_unittest.cc",
     "union_unittest.cc",
@@ -62,6 +64,7 @@ source_set("tests") {
     "//mojo/public/interfaces/bindings/tests:test_interfaces_blink",
     "//mojo/public/interfaces/bindings/tests:test_interfaces_chromium",
     "//mojo/public/interfaces/bindings/tests:test_interfaces_experimental",
+    "//mojo/public/interfaces/bindings/tests:test_struct_traits_interfaces",
     "//testing/gtest",
   ]
 }

mojo/public/cpp/bindings/tests/struct_traits_unittest.cc

@@ -10,7 +10,9 @@
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "mojo/public/cpp/bindings/tests/rect_blink.h"
 #include "mojo/public/cpp/bindings/tests/rect_chromium.h"
+#include "mojo/public/cpp/bindings/tests/struct_with_traits_impl.h"
 #include "mojo/public/cpp/bindings/weak_binding_set.h"
+#include "mojo/public/interfaces/bindings/tests/struct_with_traits.mojom.h"
 #include "mojo/public/interfaces/bindings/tests/test_native_types.mojom-blink.h"
 #include "mojo/public/interfaces/bindings/tests/test_native_types.mojom-chromium.h"
 #include "mojo/public/interfaces/bindings/tests/test_native_types.mojom.h"
@@ -95,10 +97,12 @@ class BlinkRectServiceImpl : public blink::RectService {
 };
 
 // A test which runs both Chromium and Blink implementations of a RectService.
-class StructTraitsTest : public testing::Test {
+class StructTraitsTest : public testing::Test,
+                         public TraitsTestService {
  public:
   StructTraitsTest() {}
 
+ protected:
   void BindToChromiumService(mojo::InterfaceRequest<RectService> request) {
     chromium_bindings_.AddBinding(&chromium_service_, std::move(request));
   }
@@ -107,14 +111,27 @@ class StructTraitsTest : public testing::Test {
     blink_bindings_.AddBinding(&blink_service_, std::move(request));
   }
 
+  TraitsTestServicePtr GetTraitsTestProxy() {
+    return traits_test_bindings_.CreateInterfacePtrAndBind(this);
+  }
+
  private:
+  // TraitsTestService:
+  void PassStructWithTraits(
+      const StructWithTraitsImpl& s,
+      const PassStructWithTraitsCallback& callback) override {
+    callback.Run(s);
+  }
+
   base::MessageLoop loop_;
 
   ChromiumRectServiceImpl chromium_service_;
   mojo::WeakBindingSet<chromium::RectService> chromium_bindings_;
 
   BlinkRectServiceImpl blink_service_;
   mojo::WeakBindingSet<blink::RectService> blink_bindings_;
+
+  mojo::WeakBindingSet<TraitsTestService> traits_test_bindings_;
 };
 
 }  // namespace
@@ -181,5 +198,26 @@ TEST_F(StructTraitsTest, BlinkProxyToChromiumService) {
   }
 }
 
+TEST_F(StructTraitsTest, FieldTypes) {
+  StructWithTraitsImpl input;
+  input.set_bool(true);
+  input.set_uint32(7);
+  input.set_uint64(42);
+  input.set_string("hello world!");
+
+  base::RunLoop loop;
+  TraitsTestServicePtr proxy = GetTraitsTestProxy();
+  proxy->PassStructWithTraits(
+      input,
+      [&] (const StructWithTraitsImpl& passed) {
+        EXPECT_EQ(input.get_bool(), passed.get_bool());
+        EXPECT_EQ(input.get_uint32(), passed.get_uint32());
+        EXPECT_EQ(input.get_uint64(), passed.get_uint64());
+        EXPECT_EQ(input.get_string(), passed.get_string());
+        loop.Quit();
+      });
+  loop.Run();
+}
+
 }  // namespace test
 }  // namespace mojo

mojo/public/cpp/bindings/tests/struct_with_traits_impl.cc

@@ -0,0 +1,29 @@
+// Copyright 2016 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.
+
+#include "mojo/public/cpp/bindings/tests/struct_with_traits_impl.h"
+
+#include "mojo/public/interfaces/bindings/tests/struct_with_traits.mojom.h"
+
+namespace mojo {
+namespace test {
+
+StructWithTraitsImpl::StructWithTraitsImpl() {}
+
+StructWithTraitsImpl::~StructWithTraitsImpl() {}
+
+}  // namespace test
+
+// static
+bool StructTraits<test::StructWithTraits, test::StructWithTraitsImpl>::Read(
+    test::StructWithTraits_Reader r,
+    test::StructWithTraitsImpl* out) {
+  out->set_bool(r.f_bool());
+  out->set_uint32(r.f_uint32());
+  out->set_uint64(r.f_uint64());
+  out->set_string(r.f_string().as_string());
+  return true;
+}
+
+}  // namespace mojo

mojo/public/cpp/bindings/tests/struct_with_traits_impl.h

@@ -0,0 +1,78 @@
+// Copyright 2016 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.
+
+#ifndef MOJO_PUBLIC_CPP_BINDINGS_TESTS_STRUCT_WITH_TRAITS_IMPL_H_
+#define MOJO_PUBLIC_CPP_BINDINGS_TESTS_STRUCT_WITH_TRAITS_IMPL_H_
+
+#include <stdint.h>
+
+#include <string>
+
+#include "base/strings/string_piece.h"
+#include "mojo/public/cpp/bindings/struct_traits.h"
+
+namespace mojo {
+namespace test {
+
+// The mojom types are forward-declared to avoid circular dependencies between
+// this and generated headers.
+class StructWithTraits;
+class StructWithTraits_Reader;
+
+// A type which knows how to look like a mojo::test::StructWithTraits mojom type
+// by way of mojo::StructTraits.
+class StructWithTraitsImpl {
+ public:
+  StructWithTraitsImpl();
+  ~StructWithTraitsImpl();
+
+  void set_bool(bool value) { bool_ = value; }
+  bool get_bool() const { return bool_; }
+
+  void set_uint32(uint32_t value) { uint32_ = value; }
+  uint32_t get_uint32() const { return uint32_; }
+
+  void set_uint64(uint64_t value) { uint64_ = value; }
+  uint64_t get_uint64() const { return uint64_; }
+
+  void set_string(std::string value) { string_ = value; }
+  base::StringPiece get_string() const { return string_; }
+
+ private:
+  bool bool_ = false;
+  uint32_t uint32_ = 0;
+  uint64_t uint64_ = 0;
+  std::string string_;
+};
+
+}  // namespace test
+
+template <>
+struct StructTraits<test::StructWithTraits, test::StructWithTraitsImpl> {
+  // Deserialization to test::StructTraitsImpl.
+  static bool Read(test::StructWithTraits_Reader r,
+                   test::StructWithTraitsImpl* out);
+
+  // Fields in test::StructWithTraits.
+  // See src/mojo/public/interfaces/bindings/tests/test_native_types.mojom.
+  static bool f_bool(const test::StructWithTraitsImpl& value) {
+    return value.get_bool();
+  }
+
+  static uint32_t f_uint32(const test::StructWithTraitsImpl& value) {
+    return value.get_uint32();
+  }
+
+  static uint64_t f_uint64(const test::StructWithTraitsImpl& value) {
+    return value.get_uint64();
+  }
+
+  static base::StringPiece f_string(const test::StructWithTraitsImpl& value) {
+    return value.get_string();
+  }
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_BINDINGS_TESTS_STRUCT_WITH_TRAITS_IMPL_H_

mojo/public/interfaces/bindings/tests/BUILD.gn

@@ -29,6 +29,17 @@ mojom("test_interfaces") {
   with_environment = false
 }
 
+mojom("test_struct_traits_interfaces") {
+  testonly = true
+  sources = [
+    "struct_with_traits.mojom",
+  ]
+
+  typemaps = [ "struct_with_traits.typemap" ]
+
+  with_environment = false
+}
+
 mojom("test_interfaces_experimental") {
   testonly = true
   sources = [

mojo/public/interfaces/bindings/tests/struct_with_traits.mojom

@@ -0,0 +1,16 @@
+// Copyright 2016 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 mojo.test;
+
+struct StructWithTraits {
+  bool f_bool;
+  uint32 f_uint32;
+  uint64 f_uint64;
+  string f_string;
+};
+
+interface TraitsTestService {
+  PassStructWithTraits(StructWithTraits s) => (StructWithTraits passed);
+};

mojo/public/interfaces/bindings/tests/struct_with_traits.typemap

@@ -0,0 +1,14 @@
+// Copyright 2015 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.
+
+{
+  "c++": {
+    "mojo.test.StructWithTraits": {
+      "typename": "mojo::test::StructWithTraitsImpl",
+      "headers": [
+        "mojo/public/cpp/bindings/tests/struct_with_traits_impl.h"
+      ]
+    }
+  }
+}

mojo/public/interfaces/bindings/tests/test_associated_interfaces.mojom

@@ -7,7 +7,7 @@ module mojo.test;
 interface FooInterface {};
 
 struct StructContainsAssociated {
-  associated FooInterface foo_interface;
+  associated FooInterface? foo_interface;
   associated FooInterface& foo_request;
 };
 

mojo/public/interfaces/bindings/tests/test_native_types.mojom

@@ -23,11 +23,10 @@ interface PicklePasser {
       => (array<array<PickledStruct>> passed);
 };
 
-// Used to verify support for native serialization of mojom-defined structs.
-// This is tested with a typemap applied to the Rect type from rect.mojom.
+// Used to verify support for native serialization of mojom-defined structs
+// using StrucTraits with different variants of the Rect type from rect.mojom.
 
 interface RectService {
   AddRect(Rect r);
   GetLargestRect() => (Rect largest);
 };
-

mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl

@@ -18,6 +18,7 @@
 #include <stdint.h>
 #include <ostream>
 
+#include "base/strings/string_piece.h"
 #include "mojo/public/cpp/bindings/array.h"
 #include "mojo/public/cpp/bindings/associated_interface_ptr.h"
 #include "mojo/public/cpp/bindings/associated_interface_ptr_info.h"

mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_declaration.tmpl

@@ -53,8 +53,13 @@ class {{struct.name}}_Reader {
 {%-   for pf in struct.packed.packed_fields_in_ordinal_order %}
 {%-     set kind = pf.field.kind -%}
 {%-     set name = pf.field.name -%}
+{%-     if kind|is_nullable_kind %}
+  bool has_{{name}}() const;
+{%-     endif %}
 {%-     if kind|is_struct_kind and not kind|is_native_only_kind %}
   {{kind|get_name_for_kind}}::Reader {{name}}() const;
+{%-     elif kind|is_string_kind %}
+  base::StringPiece {{name}}() const;
 {%-     elif kind|is_union_kind %}
   // TODO(rockot): Support reading unions. ({{name}}() omitted)
 {%-     elif kind|is_object_kind %}
@@ -86,10 +91,21 @@ struct {{struct.name}}_SerializerTraits_ {
   static size_t GetSize(const NativeType& input) {
     size_t size = sizeof(internal::{{struct.name}}_Data);
 {%- for pf in struct.packed.packed_fields_in_ordinal_order %}
-{%-   if pf.field.kind|is_object_kind %}
-    NOTREACHED() << "Mojom struct traits only support POD fields at this time. "
-        << "Cannot determine the size of field {{pf.field.name}}";
+    do {
+{%-   if pf.field.kind|is_nullable_kind %}
+      if (!mojo::StructTraits<{{struct.name}}, NativeType>
+              ::has_{{pf.field.name}}(input))
+        break;
+{%-   endif %}
+{%-   if pf.field.kind|is_string_kind %}
+      size += mojo::internal::String_Data::Traits::GetStorageSize(
+          static_cast<uint32_t>(mojo::StructTraits<{{struct.name}}, NativeType>
+              ::{{pf.field.name}}(input).size()));
+{%-   elif pf.field.kind|is_object_kind %}
+      NOTREACHED() << "Unsupported field type for StructTraits: "
+                   << "{{pf.field.name}}";
 {%-   endif %}
+    } while (false);
 {%- endfor %}
     return size;
   }
@@ -99,11 +115,30 @@ struct {{struct.name}}_SerializerTraits_ {
                         internal::{{struct.name}}_Data** output) {
     internal::{{struct.name}}_Data* result =
         internal::{{struct.name}}_Data::New(buffer);
-{%- for pf in struct.packed.packed_fields_in_ordinal_order %}
+{%- for pf in struct.packed.packed_fields_in_ordinal_order -%}
 {%-   set name = pf.field.name -%}
 {%-   set kind = pf.field.kind %}
-    result->{{name}} = mojo::StructTraits<{{struct.name}}, NativeType>
-        ::{{name}}(input);
+    do {
+{%-   if kind|is_nullable_kind %}
+      if (!mojo::StructTraits<{{struct.name}}, NativeType>
+              ::has_{{name}}(input))
+        break;
+{%-   endif %}
+{%-   if kind|is_string_kind %}
+      base::StringPiece input_{{name}} =
+          mojo::StructTraits<{{struct.name}}, NativeType>::{{name}}(input);
+      result->{{name}}.ptr =
+          mojo::internal::String_Data::New(input_{{name}}.size(), buffer);
+      memcpy(result->{{name}}.ptr->storage(), input_{{name}}.data(),
+             input_{{name}}.size());
+{%-   elif kind|is_object_kind %}
+      NOTREACHED() << "Unsupported field type for StructTraits: "
+                   << "{{pf.field.name}}";
+{%-   else %}
+      result->{{name}} = mojo::StructTraits<{{struct.name}}, NativeType>
+          ::{{name}}(input);
+{%-   endif %}
+    } while (false);
 {%- endfor %}
     *output = result;
   }