Mojo C++ bindings: switch the existing usage of StructTraits to use t…

…he new data view interface.

BUG=577686

Review-Url: https://codereview.chromium.org/1966933002
Cr-Commit-Position: refs/heads/master@{#393448}

mojo/public/cpp/bindings/lib/array_serialization.h

@@ -390,10 +390,9 @@ struct Serializer<Array<Element>, MaybeConstUserType> {
   static bool Deserialize(Data* input,
                           UserType* output,
                           SerializationContext* context) {
-    if (input)
-      return Impl::DeserializeElements(input, output, context);
-    Traits::SetToNull(*output);
-    return true;
+    if (!input)
+      return CallSetToNullIfExists<Traits>(output);
+    return Impl::DeserializeElements(input, output, context);
   }
 };
 

mojo/public/cpp/bindings/lib/array_traits_standard.h

@@ -15,6 +15,7 @@ struct ArrayTraits<Array<T>> {
   using Element = T;
 
   static bool IsNull(const Array<T>& input) { return input.is_null(); }
+  static void SetToNull(Array<T>* output) { *output = nullptr; }
 
   static size_t GetSize(const Array<T>& input) { return input.size(); }
 
@@ -32,8 +33,6 @@ struct ArrayTraits<Array<T>> {
   }
 
   static void Resize(Array<T>& input, size_t size) { input.resize(size); }
-
-  static void SetToNull(Array<T>& input) { input = nullptr; }
 };
 
 }  // namespace mojo

mojo/public/cpp/bindings/lib/array_traits_wtf.h

@@ -15,6 +15,7 @@ struct ArrayTraits<WTFArray<U>> {
   using Element = U;
 
   static bool IsNull(const WTFArray<U>& input) { return input.is_null(); }
+  static void SetToNull(WTFArray<U>* output) { *output = nullptr; }
 
   static size_t GetSize(const WTFArray<U>& input) { return input.size(); }
 
@@ -29,8 +30,6 @@ struct ArrayTraits<WTFArray<U>> {
   }
 
   static void Resize(WTFArray<U>& input, size_t size) { input.resize(size); }
-
-  static void SetToNull(WTFArray<U>& input) { input = nullptr; }
 };
 
 }  // namespace mojo

mojo/public/cpp/bindings/lib/serialization_util.h

@@ -102,20 +102,6 @@ inline void InterfaceDataToPointer(Interface_Data* input,
       input->version));
 }
 
-// TODO(yzshen): Unify StructTraits::Read*() methods and remove
-// HasReadFromDataViewMethod.
-template <typename T>
-struct HasReadFromDataViewMethod {
-  template <typename U>
-  static char Test(decltype(U::ReadFromDataView)*);
-  template <typename U>
-  static int Test(...);
-  static const bool value = sizeof(Test<T>(0)) == sizeof(char);
-
- private:
-  EnsureTypeIsComplete<T> check_t_;
-};
-
 template <typename T>
 struct HasIsNullMethod {
   template <typename U>
@@ -143,6 +129,37 @@ template <
 bool CallIsNullIfExists(const UserType& input) {
   return false;
 }
+template <typename T>
+struct HasSetToNullMethod {
+  template <typename U>
+  static char Test(decltype(U::SetToNull)*);
+  template <typename U>
+  static int Test(...);
+  static const bool value = sizeof(Test<T>(0)) == sizeof(char);
+
+ private:
+  EnsureTypeIsComplete<T> check_t_;
+};
+
+template <
+    typename Traits,
+    typename UserType,
+    typename std::enable_if<HasSetToNullMethod<Traits>::value>::type* = nullptr>
+bool CallSetToNullIfExists(UserType* output) {
+  Traits::SetToNull(output);
+  return true;
+}
+
+template <typename Traits,
+          typename UserType,
+          typename std::enable_if<!HasSetToNullMethod<Traits>::value>::type* =
+              nullptr>
+bool CallSetToNullIfExists(UserType* output) {
+  LOG(ERROR) << "A null value is received. But the Struct/Array/StringTraits "
+             << "class doesn't define a SetToNull() function and therefore is "
+             << "unable to deserialize the value.";
+  return false;
+}
 
 template <typename T>
 struct HasSetUpContextMethod {

mojo/public/cpp/bindings/lib/string_serialization.h

@@ -58,6 +58,8 @@ struct Serializer<String, MaybeConstUserType> {
   static bool Deserialize(String_Data* input,
                           UserType* output,
                           SerializationContext* context) {
+    if (!input)
+      return CallSetToNullIfExists<Traits>(output);
     return Traits::Read(StringDataView(input), output);
   }
 };

mojo/public/cpp/bindings/lib/string_traits_wtf.cc

@@ -42,6 +42,15 @@ UTF8AdaptorInfo* ToAdaptor(const WTF::String& input, void* context) {
 
 }  // namespace
 
+// static
+void StringTraits<WTF::String>::SetToNull(WTF::String* output) {
+  if (output->isNull())
+    return;
+
+  WTF::String result;
+  output->swap(result);
+}
+
 // static
 void* StringTraits<WTF::String>::SetUpContext(const WTF::String& input) {
   return new UTF8AdaptorInfo(input);
@@ -68,13 +77,8 @@ const char* StringTraits<WTF::String>::GetData(const WTF::String& input,
 // static
 bool StringTraits<WTF::String>::Read(StringDataView input,
                                      WTF::String* output) {
-  if (!input.is_null()) {
-    WTF::String result = WTF::String::fromUTF8(input.storage(), input.size());
-    output->swap(result);
-  } else if (!output->isNull()) {
-    WTF::String result;
-    output->swap(result);
-  }
+  WTF::String result = WTF::String::fromUTF8(input.storage(), input.size());
+  output->swap(result);
   return true;
 }
 

mojo/public/cpp/bindings/string_traits.h

@@ -13,21 +13,13 @@ namespace mojo {
 // Access to a string inside a mojo message.
 class StringDataView {
  public:
-  explicit StringDataView(internal::String_Data* data) : data_(data) {}
-
-  // Whether the data represents a null string.
-  // Note: Must not access the following methods if this method returns true.
-  bool is_null() const { return !data_; }
-
-  const char* storage() const {
-    DCHECK(!is_null());
-    return data_->storage();
+  explicit StringDataView(internal::String_Data* data) : data_(data) {
+    DCHECK(data_);
   }
 
-  size_t size() const {
-    DCHECK(!is_null());
-    return data_->size();
-  }
+  const char* storage() const { return data_->storage(); }
+
+  size_t size() const { return data_->size(); }
 
  private:
   internal::String_Data* data_;

mojo/public/cpp/bindings/string_traits_standard.h

@@ -13,18 +13,15 @@ namespace mojo {
 template <>
 struct StringTraits<String> {
   static bool IsNull(const String& input) { return input.is_null(); }
+  static void SetToNull(String* output) { *output = nullptr; }
 
   static size_t GetSize(const String& input) { return input.size(); }
 
   static const char* GetData(const String& input) { return input.data(); }
 
   static bool Read(StringDataView input, String* output) {
-    if (!input.is_null()) {
-      String result(input.storage(), input.size());
-      result.Swap(output);
-    } else {
-      *output = nullptr;
-    }
+    String result(input.storage(), input.size());
+    result.Swap(output);
     return true;
   }
 };

mojo/public/cpp/bindings/string_traits_string_piece.h

@@ -12,12 +12,19 @@ namespace mojo {
 
 template <>
 struct StringTraits<base::StringPiece> {
-  // No IsNull() function, which means that base::StringPiece is always
-  // considered as non-null mojom string. We could have let StringPiece
-  // containing a null data pointer map to null mojom string, but
-  // StringPiece::empty() returns true in this case. It seems confusing to mix
-  // the concept of empty and null strings, especially because they mean
-  // different things in mojom.
+  static bool IsNull(const base::StringPiece& input) {
+    // base::StringPiece is always converted to non-null mojom string. We could
+    // have let StringPiece containing a null data pointer map to null mojom
+    // string, but StringPiece::empty() returns true in this case. It seems
+    // confusing to mix the concept of empty and null strings, especially
+    // because they mean different things in mojom.
+    return false;
+  }
+
+  static void SetToNull(base::StringPiece* output) {
+    // Convert null to an "empty" base::StringPiece.
+    output->set(nullptr, 0);
+  }
 
   static size_t GetSize(const base::StringPiece& input) { return input.size(); }
 
@@ -26,10 +33,7 @@ struct StringTraits<base::StringPiece> {
   }
 
   static bool Read(StringDataView input, base::StringPiece* output) {
-    if (!input.is_null())
-      output->set(input.storage(), input.size());
-    else
-      output->set(nullptr, 0);
+    output->set(input.storage(), input.size());
     return true;
   }
 };

mojo/public/cpp/bindings/string_traits_wtf.h

@@ -14,6 +14,7 @@ namespace mojo {
 template <>
 struct StringTraits<WTF::String> {
   static bool IsNull(const WTF::String& input) { return input.isNull(); }
+  static void SetToNull(WTF::String* output);
 
   static void* SetUpContext(const WTF::String& input);
   static void TearDownContext(const WTF::String& input, void* context);

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

@@ -18,7 +18,7 @@ struct StructTraits<test::blink::TypemappedRect, test::RectBlink> {
   static int width(const test::RectBlink& r) { return r.width(); }
   static int height(const test::RectBlink& r) { return r.height(); }
 
-  static bool Read(test::blink::TypemappedRect::Reader r,
+  static bool Read(test::blink::TypemappedRectDataView r,
                    test::RectBlink* out) {
     if (r.x() < 0 || r.y() < 0 || r.width() < 0 || r.height() < 0) {
       return false;

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

@@ -18,7 +18,7 @@ struct StructTraits<test::TypemappedRect, test::RectChromium> {
   static int width(const test::RectChromium& r) { return r.width(); }
   static int height(const test::RectChromium& r) { return r.height(); }
 
-  static bool Read(test::TypemappedRect::Reader r, test::RectChromium* out) {
+  static bool Read(test::TypemappedRectDataView r, test::RectChromium* out) {
     if (r.width() < 0 || r.height() < 0)
       return false;
     out->set_x(r.x());

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

@@ -3,19 +3,21 @@
 // found in the LICENSE file.
 
 #include "mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.h"
-#include "mojo/public/interfaces/bindings/tests/struct_with_traits.mojom.h"
 
 namespace mojo {
 
 // static
 bool StructTraits<test::StructWithTraits, test::StructWithTraitsImpl>::Read(
-    test::StructWithTraits_Reader r,
+    test::StructWithTraitsDataView data,
     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 r.f_string() == r.f_string2();
+  out->set_bool(data.f_bool());
+  out->set_uint32(data.f_uint32());
+  out->set_uint64(data.f_uint64());
+  base::StringPiece f_string, f_string2;
+  if (!data.ReadFString(&f_string) || !data.ReadFString2(&f_string2))
+    return false;
+  out->set_string(f_string.as_string());
+  return f_string == f_string2;
 }
 
 }  // namespace mojo

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

@@ -12,17 +12,14 @@
 #include "base/strings/string_piece.h"
 #include "mojo/public/cpp/bindings/struct_traits.h"
 #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 {
-class StructWithTraits;
-class StructWithTraits_Reader;
-}
 
 template <>
 struct StructTraits<test::StructWithTraits, test::StructWithTraitsImpl> {
   // Deserialization to test::StructTraitsImpl.
-  static bool Read(test::StructWithTraits_Reader r,
+  static bool Read(test::StructWithTraitsDataView data,
                    test::StructWithTraitsImpl* out);
 
   // Fields in test::StructWithTraits.

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

@@ -125,7 +125,6 @@ using {{struct.name}} = mojo::NativeStruct;
 using {{struct.name}}Ptr = mojo::NativeStructPtr;
 {%-   else %}
 class {{struct.name}};
-class {{struct.name}}_Reader;
 class {{struct.name}}DataView;
 {%-     if struct|should_inline %}
 using {{struct.name}}Ptr = mojo::InlinedStructPtr<{{struct.name}}>;

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

@@ -4,18 +4,12 @@ class {{struct.name}}DataView {
       internal::{{struct.name}}_Data* data,
       mojo::internal::SerializationContext* context);
 
-  // Whether the data represents a null struct.
-  // Note: Must not access the following methods if is_null() returns true.
-  bool is_null() const { return !data_; }
-
 {%- for pf in struct.packed.packed_fields_in_ordinal_order %}
 {%-   set kind = pf.field.kind -%}
 {%-   set name = pf.field.name -%}
 {%-   if kind|is_struct_kind or kind|is_array_kind or kind|is_string_kind %}
   template <typename UserType>
   bool Read{{name|under_to_camel}}(UserType* value) {
-    DCHECK(!is_null());
-
 {%-  if pf.min_version != 0 %}
     auto pointer = data_->header_.version >= {{pf.min_version}}
                    ? data_->{{name}}.ptr : nullptr;