Validate map keys in C++ Mojo bindings

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

Index: mojo/public/cpp/bindings/lib/validate_params.h
diff --git a/mojo/public/cpp/bindings/lib/validate_params.h b/mojo/public/cpp/bindings/lib/validate_params.h
index 2509b6bb0a6457d227739ff5de14fba3bf0b507f..3a21812af8f9a0fdc7ca3d3a297a3efd22bdea5c 100644
--- a/mojo/public/cpp/bindings/lib/validate_params.h
+++ b/mojo/public/cpp/bindings/lib/validate_params.h
@@ -14,14 +14,36 @@ namespace internal {
 
 using ValidateEnumFunc = bool (*)(int32_t);
 
+// TODO(tibell): Rename to ContainerValidateParams.
 class ArrayValidateParams {
  public:
+  // Validate a map. A map is validated as a pair of arrays, one for the keys

[yzshen1, 2016/06/02 19:52:28]

style nit: Validate -> Validates. Because the sentence's is "[This method]
Validates ..."

And also line 38

[tibell, 2016/06/02 23:55:01]

Done

(Python has the opposite convention.)

+  // and one for the values. Both arguments must be non-null.
+  //
+  // ArrayValidateParams takes ownership of |in_key_validate params| and
+  // |in_element_validate params|.
+  ArrayValidateParams(ArrayValidateParams* in_key_validate_params,
+                      ArrayValidateParams* in_element_validate_params)
+      : expected_num_elements(0),

[yzshen1, 2016/06/02 19:52:28]

Nit: Please consider doing initialization at their definition (i.e., outside of
constructor). So that each constructor only needs to set those fields that are
interesting.

[tibell, 2016/06/02 23:55:02]

Done.

+        element_is_nullable(false),
+        key_validate_params(in_key_validate_params),
+        element_validate_params(in_element_validate_params),
+        validate_enum_func(nullptr) {
+    DCHECK(in_key_validate_params)
+        << "Map validate params require key validate params";
+    DCHECK(in_element_validate_params)
+        << "Map validate params require element validate params";
+  }
+
+  // Validate an array.
+  //
   // ArrayValidateParams takes ownership of |in_element_validate params|.
   ArrayValidateParams(uint32_t in_expected_num_elements,
                       bool in_element_is_nullable,
                       ArrayValidateParams* in_element_validate_params)
       : expected_num_elements(in_expected_num_elements),
         element_is_nullable(in_element_is_nullable),
+        key_validate_params(nullptr),
         element_validate_params(in_element_validate_params),
         validate_enum_func(nullptr) {}
 
@@ -30,12 +52,15 @@ class ArrayValidateParams {
                       ValidateEnumFunc in_validate_enum_func)
       : expected_num_elements(in_expected_num_elements),
         element_is_nullable(false),
+        key_validate_params(nullptr),
         element_validate_params(nullptr),
         validate_enum_func(in_validate_enum_func) {}
 
   ~ArrayValidateParams() {
     if (element_validate_params)
       delete element_validate_params;
+    if (key_validate_params)
+      delete key_validate_params;
   }
 
   // If |expected_num_elements| is not 0, the array is expected to have exactly
@@ -45,6 +70,10 @@ class ArrayValidateParams {
   // Whether the elements are nullable.
   bool element_is_nullable;
 
+  // Validation information for map key. It is either a pointer to another
+  // instance of ArrayValidateParams (if keys are arrays or maps), or nullptr.

[yzshen1, 2016/06/02 19:52:28]

This comment needs to be revised: the param is for the key array itself, it
doesn't matter whether the keys are arrays or maps (actually, keys cannot be
maps. the only type of array allowed as key is string.)

[tibell, 2016/06/02 23:55:01]

Done.

+  ArrayValidateParams* key_validate_params;
+
   // Validation information for elements. It is either a pointer to another

[yzshen1, 2016/06/02 19:59:41]

It seems this comment needs to be updated, too.
In the case of array, this is used to validate each individual element (if
elements are arrays or maps);
In the case of map, this is used to validate the entire value array.

[tibell, 2016/06/02 23:55:01]

Done.

   // instance of ArrayValidateParams (if elements are arrays or maps), or
   // nullptr. In the case of maps, this is used to validate the value array.