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

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

Index: mojo/public/cpp/bindings/lib/array_serialization.h
diff --git a/mojo/public/cpp/bindings/lib/array_serialization.h b/mojo/public/cpp/bindings/lib/array_serialization.h
index 9d0e7bb438d0c6137f712c0c984cf733f78fa587..66740cadbbf3a3f3fe6d2b35c627c8d7ba2ebe4e 100644
--- a/mojo/public/cpp/bindings/lib/array_serialization.h
+++ b/mojo/public/cpp/bindings/lib/array_serialization.h
@@ -6,33 +6,30 @@
 #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/map_serialization_forward.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,
-    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);
-
 namespace internal {
 
+// The ArraySerializer template contains static methods for serializing |Array|s
+// of various types.  These methods include:
+//   * size_t GetSerializedSize(..)
+//       Computes the size of the serialized version of the |Array|.
+//   * void SerializeElements(..)
+//       Takes an |Iterator| and a size and serializes it.
+//   * void DeserializeElements(..)
+//       Takes a pointer to an |Array_Data| and deserializes it into a given
+//       |Array|.
 template <typename E,
           typename F,
           bool is_union =
@@ -47,7 +44,9 @@ struct ArraySerializer<E, F, false> {
     return sizeof(Array_Data<F>) + Align(input.size() * sizeof(E));
   }
 
-  static void SerializeElements(Array<E> input,
+  template <typename Iterator>
+  static void SerializeElements(Iterator it,
+                                size_t num_elements,
                                 Buffer* buf,
                                 Array_Data<F>* output,
                                 const ArrayValidateParams* validate_params) {
@@ -55,10 +54,26 @@ struct ArraySerializer<E, F, false> {
         << "Primitive type should be non-nullable";
     MOJO_DCHECK(!validate_params->element_validate_params)
         << "Primitive type should not have array validate params";
+    for (size_t i = 0; i < num_elements; ++i, ++it)
+      output->at(i) = *it;
+  }
 
-    if (input.size())
-      memcpy(output->storage(), &input.storage()[0], input.size() * sizeof(E));
+  // We can optimize serializing PODs by |memcpy|ing directly.
+  // Note that this has precedence over its templated sibling defined above.
+  static void SerializeElements(typename Array<E>::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) {

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

nit: omit {}

[vardhan, 2015/09/28 21:55:44]

Done.

+      memcpy(output->storage(), &(*it), num_elements * sizeof(E));
+    }
   }
+
   static void DeserializeElements(Array_Data<F>* input, Array<E>* output) {
     std::vector<E> result(input->size());
     if (input->size())
@@ -74,7 +89,9 @@ struct ArraySerializer<bool, bool, false> {
     return sizeof(Array_Data<bool>) + Align((input.size() + 7) / 8);
   }
 
-  static void SerializeElements(Array<bool> input,
+  template <typename Iterator>
+  static void SerializeElements(Iterator it,
+                                size_t num_elements,
                                 Buffer* buf,
                                 Array_Data<bool>* output,
                                 const ArrayValidateParams* validate_params) {
@@ -84,9 +101,10 @@ struct ArraySerializer<bool, bool, false> {
         << "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 DeserializeElements(Array_Data<bool>* input,
                                   Array<bool>* output) {
     Array<bool> result(input->size());
@@ -104,23 +122,27 @@ struct ArraySerializer<ScopedHandleBase<H>, H, false> {
     return sizeof(Array_Data<H>) + Align(input.size() * sizeof(H));
   }
 
-  static void SerializeElements(Array<ScopedHandleBase<H>> input,
+  template <typename Iterator>
+  static void SerializeElements(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 DeserializeElements(Array_Data<H>* input,
                                   Array<ScopedHandleBase<H>>* output) {
     Array<ScopedHandleBase<H>> result(input->size());
@@ -130,8 +152,9 @@ struct ArraySerializer<ScopedHandleBase<H>, H, false> {
   }
 };
 
-// This template must only apply to pointer mojo entity (structs and arrays).
-// This is done by ensuring that WrapperTraits<S>::DataType is a pointer.
+// 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,
@@ -144,66 +167,109 @@ struct ArraySerializer<
     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,
+  template <typename Iterator>
+  static void SerializeElements(Iterator it,
+                                size_t num_elements,
                                 Buffer* buf,
                                 Array_Data<S_Data*>* output,
                                 const ArrayValidateParams* validate_params) {
-    for (size_t i = 0; i < input.size(); ++i) {
+    for (size_t i = 0; i < num_elements; ++i, ++it) {
       S_Data* element;
-      SerializeCaller<S>::Run(input[i].Pass(), buf, &element,
-                              validate_params->element_validate_params);
+      SerializeCaller::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 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::Run(input->at(i), &result[i]);
     }
     output->Swap(&result);
   }
 
  private:
-  template <typename T>
+  // SerializeCaller template is used by |ArraySerializer| to dispatch a
+  // serialize

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

nit: please re-wrap

[vardhan, 2015/09/28 21:55:44]

Done.

+  // call on a non-POD type.  This template is defined outside |ArraySerializer|
+  // since you cannot specialize a struct within a class definition.
   struct SerializeCaller {
-    static void Run(T input,
+    // This template needs to be suppressed if |T| is |String|, otherwise it
+    // takes precedence over the |String|-overloaded Run() below.
+    template <typename T,
+              typename = typename EnableIf<!IsSame<T, String>::value, T>::type>
+    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);
+    }
 
-      Serialize_(input.Pass(), buf, output);
+    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,
+    template <typename 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);
+      SerializeArray_(input, buf, output, validate_params);
     }
-  };
 
-  template <typename T, typename U>
-  struct SerializeCaller<Map<T, U>> {
-    static void Run(Map<T, U> input,
+    template <typename Key, typename Value>
+    static void Run(Map<Key, Value>* input,
                     Buffer* buf,
-                    typename Map<T, U>::Data_** output,
+                    typename Map<Key, Value>::Data_** output,
                     const ArrayValidateParams* validate_params) {
-      SerializeMap_(input.Pass(), buf, output, validate_params);
+      SerializeMap_(input, buf, output, validate_params);
+    }
+  };
+
+  struct DeserializeCaller {
+    template <typename T>
+    static void Run(typename WrapperTraits<T>::DataType input, T* output) {
+      Deserialize_(input, 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.
+    template <typename T>
+    static void Run(typename WrapperTraits<StructPtr<T>>::DataType input,
+                    StructPtr<T>* output) {
+      *output = T::New();
+      Deserialize_(input, output->get());
+    }
+
+    template <typename T>
+    static void Run(typename WrapperTraits<InlinedStructPtr<T>>::DataType input,
+                    InlinedStructPtr<T>* output) {
+      *output = T::New();
+      Deserialize_(input, output->get());
     }
   };
 };
@@ -221,68 +287,33 @@ struct ArraySerializer<U, U_Data, true> {
     return size;
   }
 
-  static void SerializeElements(Array<U> input,
+  template <typename Iterator>
+  static void SerializeElements(Iterator it,
+                                size_t num_elements,
                                 Buffer* buf,
                                 Array_Data<U_Data>* output,
                                 const ArrayValidateParams* validate_params) {
-    for (size_t i = 0; i < input.size(); ++i) {
+    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);

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

nit: it->get() instead of (*it).get()?

[vardhan, 2015/09/28 21:55:44]

Done.

       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 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]);
-    }
-    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));
+      auto& elem = input->at(i);
+      if (!elem.is_null()) {
+        using UnwrapedUnionType = typename RemoveStructPtr<U>::type;
+        result[i] = UnwrapedUnionType::New();
+        Deserialize_(&elem, result[i].get());
+      }
     }
-  }
-  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);
   }
 };
@@ -299,24 +330,24 @@ 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) {
-  if (input) {
+  if (input && *input) {

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

Is input ever allowed to be null?

Maybe you should just MOJO_DCHECK(input)?

[vardhan, 2015/09/28 21:55:44]

Doesn't look like it'll ever be null if it goes through the rest of the bindings
code, unless the caller purposely passes in null.  I'll make it a MOJO_DCHECK.

     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);
+          input->begin(), input->size(), buf, result, validate_params);
     }
     *output = result;
   } else {