Validate map keys in C++ Mojo bindings

mojo/public/cpp/bindings/lib/validate_params.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_VALIDATE_PARAMS_H_
 #define MOJO_PUBLIC_CPP_BINDINGS_LIB_VALIDATE_PARAMS_H_
 
 #include <stdint.h>
 
 #include "base/macros.h"
 
 namespace mojo {
 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) {}
 
   // Validates an array of enums.
   ArrayValidateParams(uint32_t in_expected_num_elements,
                       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
   // that number of elements.
   uint32_t expected_num_elements;
 
   // 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.
   ArrayValidateParams* element_validate_params;
 
   // Validation function for enum elements.
   ValidateEnumFunc validate_enum_func;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ArrayValidateParams);
 };
 
 }  // namespace internal
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_LIB_VALIDATE_PARAMS_H_