* Change C++ serialization/deserialization to not be move-only operations (with the except of |Ha…

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

 // Copyright 2014 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_LIB_ARRAY_SERIALIZATION_H_
 #define MOJO_PUBLIC_CPP_BINDINGS_LIB_ARRAY_SERIALIZATION_H_
 
 #include <string.h>  // For |memcpy()|.
-
 #include <vector>
 
 #include "mojo/public/c/system/macros.h"
 #include "mojo/public/cpp/bindings/lib/array_internal.h"
-#include "mojo/public/cpp/bindings/lib/map_serialization.h"
+#include "mojo/public/cpp/bindings/lib/bindings_internal.h"
+#include "mojo/public/cpp/bindings/lib/iterator_util.h"
+#include "mojo/public/cpp/bindings/lib/map_data_internal.h"
 #include "mojo/public/cpp/bindings/lib/string_serialization.h"
 #include "mojo/public/cpp/bindings/lib/template_util.h"
 #include "mojo/public/cpp/bindings/lib/validation_errors.h"
 
 namespace mojo {
 
 template <typename E>
 inline size_t GetSerializedSize_(const Array<E>& input);
 
 template <typename E, typename F>
 inline void SerializeArray_(
-    Array<E> input,
+    Array<E>* input,
     internal::Buffer* buf,
     internal::Array_Data<F>** output,
     const internal::ArrayValidateParams* validate_params);
 
 template <typename E, typename F>
 inline void Deserialize_(internal::Array_Data<F>* data, Array<E>* output);
 
+// To avoid a circular dependency on map_serialization.h, we forward declare:

[viettrungluu, 2015/09/23 18:15:55]

Maybe we should have a map_serialization_forward.h?

(Possibly this applies to other things which we have forward declarations for.)

Side point: I wonder if we should have the trailing _, for things that are
"exposed" (and if they're not exposed, maybe they should be in the |internal|
namespace).

[vardhan, 2015/09/23 22:07:00]

I think SerializeMap_ should be internal (not available to mojo apps to use), 
because SerializeMap_ is not enough to serialize a map and write it to disk --
you also need to encode handles and pointers.

But  if we had a public version and our internal version, I think having a
mojo::SerializeMap() version and an internal mojo::internal::SerializeMap() is a
little unambiguous, so I was thinking we should keep the trailing '_'?

LMK what you think, I'll do a follow up CL for this.

[vardhan, 2015/09/23 22:12:16]

Sorry, I meant its a little ambigious*

[viettrungluu, 2015/09/23 23:20:24]

Probably it should be moved to internal:: (without the trailing _).

(The only reason for trailing _s, is when we generate stuff in someone else's
namespace, or similarly generating members/methods in a class whose
members/methods are controlled by someone else.)

[vardhan, 2015/09/25 19:33:05]

Another thought is that I would have to leave Deserialize_ (with the
underscore), which creates an underscore inconsistency..

Would you mind if i left this renaming to another CL?

[viettrungluu, 2015/09/25 20:08:08]

OK

+template <typename Key, typename Value>
+class Map;
+
+template <typename MapKey,
+          typename MapValue,
+          typename DataKey,
+          typename DataValue>
+void SerializeMap_(Map<MapKey, MapValue>* input,
+                   internal::Buffer* buf,
+                   internal::Map_Data<DataKey, DataValue>** output,
+                   const internal::ArrayValidateParams* value_validate_params);
+template <typename MapKey, typename MapValue>
+size_t GetSerializedSize_(const Map<MapKey, MapValue>& input);
+
+template <typename MapKey,
+          typename MapValue,
+          typename DataKey,
+          typename DataValue>
+void Deserialize_(internal::Map_Data<DataKey, DataValue>* input,
+                  Map<MapKey, MapValue>* output);
+
 namespace internal {
 
+// ArraySerializer template contains static methods for serializing |Array|s of

[viettrungluu, 2015/09/23 18:15:55]

*The* ArraySerializer template ...

[vardhan, 2015/09/23 22:07:00]

Done.

+// various types.  These methods include:
+//   * size_t GetSerializedSize(..)
+//       Computes the size of the serialized version of the |Array|.
+//   * void SerializeElementsWithIterator(..)
+//       Takes an iterator and |num_elements| representing an |Array| (or
+//       possibly a |Map|), and serializes it.
+//   * void SerializeElements(..)
+//       Takes an |Array| directly and serializes it.  This usually calls into
+//       SerializeElementsWithIterator(), except for an |Array| of PODs which
+//       will directly memcpy the PODs as an optimization.
 template <typename E,
           typename F,
           bool is_union =
               IsUnionDataType<typename RemovePointer<F>::type>::value>
 struct ArraySerializer;
 
 // Handles serialization and deserialization of arrays of pod types.
 template <typename E, typename F>
 struct ArraySerializer<E, F, false> {
   static_assert(sizeof(E) == sizeof(F), "Incorrect array serializer");
   static size_t GetSerializedSize(const Array<E>& input) {
     return sizeof(Array_Data<F>) + Align(input.size() * sizeof(E));
   }
 
-  static void SerializeElements(Array<E> input,
+  template <typename Iterator>
+  static void SerializeElementsWithIterator(
+      Iterator it,
+      size_t num_elements,
+      Buffer* buf,
+      Array_Data<F>* output,
+      const ArrayValidateParams* validate_params) {
+    MOJO_DCHECK(!validate_params->element_is_nullable)
+        << "Primitive type should be non-nullable";
+    MOJO_DCHECK(!validate_params->element_validate_params)
+        << "Primitive type should not have array validate params";
+    if (num_elements) {
+      for (size_t i = 0; i < num_elements; ++i, ++it) {
+        output->at(i) = *it;
+      }
+    }
+  }
+
+  // We can optimize serializing PODs by |memcpy|ing directly.
+  static void SerializeElements(Array<E>* input,
                                 Buffer* buf,
                                 Array_Data<F>* output,
                                 const ArrayValidateParams* validate_params) {
     MOJO_DCHECK(!validate_params->element_is_nullable)
         << "Primitive type should be non-nullable";
     MOJO_DCHECK(!validate_params->element_validate_params)
         << "Primitive type should not have array validate params";
+    if (input->size()) {
+      memcpy(output->storage(), &input->storage()[0],
+             input->size() * sizeof(E));
+    }
+  }
 
-    if (input.size())
-      memcpy(output->storage(), &input.storage()[0], input.size() * sizeof(E));
-  }
   static void DeserializeElements(Array_Data<F>* input, Array<E>* output) {
     std::vector<E> result(input->size());
     if (input->size())
       memcpy(&result[0], input->storage(), input->size() * sizeof(E));
     output->Swap(&result);
   }
 };
 
 // Serializes and deserializes arrays of bools.
 template <>
 struct ArraySerializer<bool, bool, false> {
   static size_t GetSerializedSize(const Array<bool>& input) {
     return sizeof(Array_Data<bool>) + Align((input.size() + 7) / 8);
   }
 
-  static void SerializeElements(Array<bool> input,
-                                Buffer* buf,
-                                Array_Data<bool>* output,
-                                const ArrayValidateParams* validate_params) {
+  template <typename Iterator>
+  static void SerializeElementsWithIterator(
+      Iterator it,
+      size_t num_elements,
+      Buffer* buf,
+      Array_Data<bool>* output,
+      const ArrayValidateParams* validate_params) {
     MOJO_DCHECK(!validate_params->element_is_nullable)
         << "Primitive type should be non-nullable";
     MOJO_DCHECK(!validate_params->element_validate_params)
         << "Primitive type should not have array validate params";
 
     // TODO(darin): Can this be a memcpy somehow instead of a bit-by-bit copy?
-    for (size_t i = 0; i < input.size(); ++i)
-      output->at(i) = input[i];
+    for (size_t i = 0; i < num_elements; ++i, ++it) {
+      output->at(i) = *it;
+    }
   }
+
+  static void SerializeElements(Array<bool>* input,
+                                Buffer* buf,
+                                Array_Data<bool>* output,
+                                const ArrayValidateParams* validate_params) {
+    SerializeElementsWithIterator(input->begin(), input->size(), buf, output,
+                                  validate_params);
+  }
+
   static void DeserializeElements(Array_Data<bool>* input,
                                   Array<bool>* output) {
     Array<bool> result(input->size());
     // TODO(darin): Can this be a memcpy somehow instead of a bit-by-bit copy?
     for (size_t i = 0; i < input->size(); ++i)
       result.at(i) = input->at(i);
     output->Swap(&result);
   }
 };
 
 // Serializes and deserializes arrays of handles.
 template <typename H>
 struct ArraySerializer<ScopedHandleBase<H>, H, false> {
   static size_t GetSerializedSize(const Array<ScopedHandleBase<H>>& input) {
     return sizeof(Array_Data<H>) + Align(input.size() * sizeof(H));
   }
 
-  static void SerializeElements(Array<ScopedHandleBase<H>> input,
-                                Buffer* buf,
-                                Array_Data<H>* output,
-                                const ArrayValidateParams* validate_params) {
+  template <typename Iterator>
+  static void SerializeElementsWithIterator(
+      Iterator it,
+      size_t num_elements,
+      Buffer* buf,
+      Array_Data<H>* output,
+      const ArrayValidateParams* validate_params) {
     MOJO_DCHECK(!validate_params->element_validate_params)
         << "Handle type should not have array validate params";
 
-    for (size_t i = 0; i < input.size(); ++i) {
-      output->at(i) = input[i].release();  // Transfer ownership of the handle.
+    for (size_t i = 0; i < num_elements; ++i, ++it) {
+      // Transfer ownership of the handle.
+      output->at(i) = (*it).release();
       MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
           !validate_params->element_is_nullable && !output->at(i).is_valid(),
           VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE,
           MakeMessageWithArrayIndex(
-              "invalid handle in array expecting valid handles", input.size(),
+              "invalid handle in array expecting valid handles", num_elements,
               i));
     }
   }
+
+  static void SerializeElements(Array<ScopedHandleBase<H>>* input,

[viettrungluu, 2015/09/23 18:15:55]

Do you really need this? (Not having it would also allow you to keep the
"SerializeElements" name for the one that takes an iterator.)

[vardhan, 2015/09/23 22:06:59]

The only reason we have both SerializeElements and SerializeIterableElements is
so that SerializeElements can memcpy() Array<POD type> (minus bool) since it's
just a vector instead of having to work with an iterator (see line 114)

[viettrungluu, 2015/09/23 23:20:24]

Is it possible to do this via some appropriate specialization? All the extra
boilerplate is really unfortunate.

[vardhan, 2015/09/25 19:33:05]

Done :(

+                                Buffer* buf,
+                                Array_Data<H>* output,
+                                const ArrayValidateParams* validate_params) {
+    SerializeElementsWithIterator(input->begin(), input->size(), buf, output,
+                                  validate_params);
+  }
+
   static void DeserializeElements(Array_Data<H>* input,
                                   Array<ScopedHandleBase<H>>* output) {
     Array<ScopedHandleBase<H>> result(input->size());
     for (size_t i = 0; i < input->size(); ++i)
       result.at(i) = MakeScopedHandle(FetchAndReset(&input->at(i)));
     output->Swap(&result);
   }
 };
 
-// This template must only apply to pointer mojo entity (structs and arrays).
-// This is done by ensuring that WrapperTraits<S>::DataType is a pointer.
+// SerializeCaller template is used by |ArraySerializer| to dispatch a serialize
+// call on a non-POD type.  This template is defined outside |ArraySerializer|
+// since you cannot specialize a struct within a class definition.

[viettrungluu, 2015/09/23 18:15:55]

Are you allowed to specialize static member functions?

[vardhan, 2015/09/23 22:06:59]

Yeah, but explicitly specialized definitions of a member function needs to live
outside of the struct definition still.  Most of this trouble is because I'm
fully specializing for String (ie SerializeCaller<String>)

[viettrungluu, 2015/09/23 23:20:24]

Well, the definition would be declared outside, but at least you wouldn't be
dumping all of this inside the global namespace?

(And not having the extra struct would also reduce verbiage too.)

[vardhan, 2015/09/25 19:33:05]

Done.

+template <typename T>
+struct SerializeCaller {
+  static void Run(T* input,
+                  Buffer* buf,
+                  typename WrapperTraits<T>::DataType* output,
+                  const ArrayValidateParams* validate_params) {
+    MOJO_DCHECK(!validate_params)
+        << "Struct type should not have array validate params";
+    Serialize_(UnwrapStructPtr<T>::value(*input), buf, output);
+  }
+};
+
+template <>
+struct SerializeCaller<String> {
+  static void Run(const String* input,
+                  Buffer* buf,
+                  String_Data** output,
+                  const ArrayValidateParams* validate_params) {
+    MOJO_DCHECK(validate_params && !validate_params->element_validate_params &&
+                !validate_params->element_is_nullable &&
+                validate_params->expected_num_elements == 0)
+        << "String type has unexpected array validate params";
+    SerializeString_(*input, buf, output);
+  }
+};
+
+template <typename T>
+struct SerializeCaller<Array<T>> {
+  static void Run(Array<T>* input,
+                  Buffer* buf,
+                  typename Array<T>::Data_** output,
+                  const ArrayValidateParams* validate_params) {
+    SerializeArray_(input, buf, output, validate_params);
+  }
+};
+
+template <typename Key, typename Value>
+struct SerializeCaller<Map<Key, Value>> {
+  static void Run(Map<Key, Value>* input,
+                  Buffer* buf,
+                  typename Map<Key, Value>::Data_** output,
+                  const ArrayValidateParams* validate_params) {
+    SerializeMap_(input, buf, output, validate_params);
+  }
+};
+
+template <typename T,
+          bool IsStructPtr = IsSpecializationOf<StructPtr, T>::value ||
+                             IsSpecializationOf<InlinedStructPtr, T>::value>
+struct DeserializeCaller;
+
+template <typename T>
+struct DeserializeCaller<T, false> {
+  static void Run(typename WrapperTraits<T>::DataType input, T* output) {
+    Deserialize_(input, output);
+  }
+};
+
+template <typename T>
+struct DeserializeCaller<T, true> {
+  static void Run(typename WrapperTraits<T>::DataType input, T* output) {
+    // Since Deserialize_ takes in a |Struct*| (not |StructPtr|), we need to
+    // initialize the |StructPtr| here before deserializing into its underlying
+    // data.
+    // TODO(vardhan):  Either all containers, or just Deserialize_(), should
+    // support taking in an allocator.
+    using UnwrapedStructType = typename RemoveStructPtr<T>::type;
+    *output = UnwrapedStructType::New();
+    Deserialize_(input, output->get());
+  }
+};
+
+// This template must only apply to pointer mojo entity (structs, arrays,
+// strings).  This is done by ensuring that WrapperTraits<S>::DataType is a
+// pointer.
 template <typename S>
 struct ArraySerializer<
     S,
     typename EnableIf<IsPointer<typename WrapperTraits<S>::DataType>::value,
                       typename WrapperTraits<S>::DataType>::type,
     false> {
   typedef
       typename RemovePointer<typename WrapperTraits<S>::DataType>::type S_Data;
   static size_t GetSerializedSize(const Array<S>& input) {
     size_t size = sizeof(Array_Data<S_Data*>) +
                   input.size() * sizeof(StructPointer<S_Data>);
     for (size_t i = 0; i < input.size(); ++i)
-      size += GetSerializedSize_(input[i]);
+      size += GetSerializedSize_(*(UnwrapConstStructPtr<S>::value(input[i])));
     return size;
   }
 
-  static void SerializeElements(Array<S> input,
-                                Buffer* buf,
-                                Array_Data<S_Data*>* output,
-                                const ArrayValidateParams* validate_params) {
-    for (size_t i = 0; i < input.size(); ++i) {
+  template <typename Iterator>
+  static void SerializeElementsWithIterator(
+      Iterator it,
+      size_t num_elements,
+      Buffer* buf,
+      Array_Data<S_Data*>* output,
+      const ArrayValidateParams* validate_params) {
+    for (size_t i = 0; i < num_elements; ++i, ++it) {
       S_Data* element;
-      SerializeCaller<S>::Run(input[i].Pass(), buf, &element,
+      SerializeCaller<S>::Run(&(*it), buf, &element,
                               validate_params->element_validate_params);
       output->at(i) = element;
       MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
           !validate_params->element_is_nullable && !element,
           VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
           MakeMessageWithArrayIndex("null in array expecting valid pointers",
-                                    input.size(), i));
+                                    num_elements, i));
     }
   }
+
+  static void SerializeElements(Array<S>* input,
+                                Buffer* buf,
+                                Array_Data<S_Data*>* output,
+                                const ArrayValidateParams* validate_params) {
+    SerializeElementsWithIterator(input->begin(), input->size(), buf, output,
+                                  validate_params);
+  }
+
   static void DeserializeElements(Array_Data<S_Data*>* input,
                                   Array<S>* output) {
     Array<S> result(input->size());
     for (size_t i = 0; i < input->size(); ++i) {
-      Deserialize_(input->at(i), &result[i]);
+      DeserializeCaller<S>::Run(input->at(i), &result[i]);
     }
     output->Swap(&result);
   }
-
- private:
-  template <typename T>
-  struct SerializeCaller {
-    static void Run(T input,
-                    Buffer* buf,
-                    typename WrapperTraits<T>::DataType* output,
-                    const ArrayValidateParams* validate_params) {
-      MOJO_DCHECK(!validate_params)
-          << "Struct type should not have array validate params";
-
-      Serialize_(input.Pass(), buf, output);
-    }
-  };
-
-  template <typename T>
-  struct SerializeCaller<Array<T>> {
-    static void Run(Array<T> input,
-                    Buffer* buf,
-                    typename Array<T>::Data_** output,
-                    const ArrayValidateParams* validate_params) {
-      SerializeArray_(input.Pass(), buf, output, validate_params);
-    }
-  };
-
-  template <typename T, typename U>
-  struct SerializeCaller<Map<T, U>> {
-    static void Run(Map<T, U> input,
-                    Buffer* buf,
-                    typename Map<T, U>::Data_** output,
-                    const ArrayValidateParams* validate_params) {
-      SerializeMap_(input.Pass(), buf, output, validate_params);
-    }
-  };
 };
 
 // Handles serialization and deserialization of arrays of unions.
 template <typename U, typename U_Data>
 struct ArraySerializer<U, U_Data, true> {
   static size_t GetSerializedSize(const Array<U>& input) {
     size_t size = sizeof(Array_Data<U_Data>);
     for (size_t i = 0; i < input.size(); ++i) {
       // GetSerializedSize_ will account for both the data in the union and the
       // space in the array used to hold the union.
       size += GetSerializedSize_(input[i], false);
     }
     return size;
   }
 
-  static void SerializeElements(Array<U> input,
-                                Buffer* buf,
-                                Array_Data<U_Data>* output,
-                                const ArrayValidateParams* validate_params) {
-    for (size_t i = 0; i < input.size(); ++i) {
+  template <typename Iterator>
+  static void SerializeElementsWithIterator(
+      Iterator it,
+      size_t num_elements,
+      Buffer* buf,
+      Array_Data<U_Data>* output,
+      const ArrayValidateParams* validate_params) {
+    for (size_t i = 0; i < num_elements; ++i, ++it) {
       U_Data* result = output->storage() + i;
-      SerializeUnion_(input[i].Pass(), buf, &result, true);
+      SerializeUnion_((*it).get(), buf, &result, true);
       MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
           !validate_params->element_is_nullable && output->at(i).is_null(),
           VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
           MakeMessageWithArrayIndex("null in array expecting valid unions",
-                                    input.size(), i));
+                                    num_elements, i));
     }
   }
 
+  static void SerializeElements(Array<U>* input,
+                                Buffer* buf,
+                                Array_Data<U_Data>* output,
+                                const ArrayValidateParams* validate_params) {
+    SerializeElementsWithIterator(input->begin(), input->size(), buf, output,
+                                  validate_params);
+  }
+
   static void DeserializeElements(Array_Data<U_Data>* input, Array<U>* output) {
     Array<U> result(input->size());
     for (size_t i = 0; i < input->size(); ++i) {
-      Deserialize_(&input->at(i), &result[i]);
+      auto& elem = input->at(i);
+      if (!elem.is_null()) {
+        using UnwrapedUnionType = typename RemoveStructPtr<U>::type;
+        result[i] = UnwrapedUnionType::New();
+        Deserialize_(&elem, result[i].get());
+      }
     }
     output->Swap(&result);
   }
 };
-
-// Handles serialization and deserialization of arrays of strings.
-template <>
-struct ArraySerializer<String, String_Data*> {
-  static size_t GetSerializedSize(const Array<String>& input) {
-    size_t size =
-        sizeof(Array_Data<String_Data*>) + input.size() * sizeof(StringPointer);
-    for (size_t i = 0; i < input.size(); ++i)
-      size += GetSerializedSize_(input[i]);
-    return size;
-  }
-
-  static void SerializeElements(Array<String> input,
-                                Buffer* buf,
-                                Array_Data<String_Data*>* output,
-                                const ArrayValidateParams* validate_params) {
-    MOJO_DCHECK(
-        validate_params->element_validate_params &&
-        !validate_params->element_validate_params->element_validate_params &&
-        !validate_params->element_validate_params->element_is_nullable &&
-        validate_params->element_validate_params->expected_num_elements == 0)
-        << "String type has unexpected array validate params";
-
-    for (size_t i = 0; i < input.size(); ++i) {
-      String_Data* element;
-      Serialize_(input[i], buf, &element);
-      output->at(i) = element;
-      MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
-          !validate_params->element_is_nullable && !element,
-          VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
-          MakeMessageWithArrayIndex("null in array expecting valid strings",
-                                    input.size(), i));
-    }
-  }
-  static void DeserializeElements(Array_Data<String_Data*>* input,
-                                  Array<String>* output) {
-    Array<String> result(input->size());
-    for (size_t i = 0; i < input->size(); ++i)
-      Deserialize_(input->at(i), &result[i]);
-    output->Swap(&result);
-  }
-};
 
 }  // namespace internal
 
 template <typename E>
 inline size_t GetSerializedSize_(const Array<E>& input) {
   if (!input)
     return 0;
   typedef typename internal::WrapperTraits<E>::DataType F;
   return internal::ArraySerializer<E, F>::GetSerializedSize(input);
 }
 
 template <typename E, typename F>
 inline void SerializeArray_(
-    Array<E> input,
+    Array<E>* input,
     internal::Buffer* buf,
     internal::Array_Data<F>** output,
     const internal::ArrayValidateParams* validate_params) {
-  if (input) {
+  if (input && *input) {
     MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
         validate_params->expected_num_elements != 0 &&
-            input.size() != validate_params->expected_num_elements,
+            input->size() != validate_params->expected_num_elements,
         internal::VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER,
         internal::MakeMessageWithExpectedArraySize(
-            "fixed-size array has wrong number of elements", input.size(),
+            "fixed-size array has wrong number of elements", input->size(),
             validate_params->expected_num_elements));
 
     internal::Array_Data<F>* result =
-        internal::Array_Data<F>::New(input.size(), buf);
+        internal::Array_Data<F>::New(input->size(), buf);
     if (result) {
-      internal::ArraySerializer<E, F>::SerializeElements(
-          internal::Forward(input), buf, result, validate_params);
+      internal::ArraySerializer<E, F>::SerializeElements(input, buf, result,
+                                                         validate_params);
     }
     *output = result;
   } else {
     *output = nullptr;
   }
 }
 
 template <typename E, typename F>
 inline void Deserialize_(internal::Array_Data<F>* input, Array<E>* output) {
   if (input) {
     internal::ArraySerializer<E, F>::DeserializeElements(input, output);
   } else {
     output->reset();
   }
 }
 
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_LIB_ARRAY_SERIALIZATION_H_