Use ScopedVector instead of std::vector in case of repeated messages.

base/json/json_value_converter.h

 // Copyright (c) 2012 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 BASE_JSON_JSON_VALUE_CONVERTER_H_
 #define BASE_JSON_JSON_VALUE_CONVERTER_H_
 #pragma once
 
 #include <string>
 #include <vector>
 
 #include "base/base_export.h"
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
 #include "base/stl_util.h"
 #include "base/string16.h"
 #include "base/values.h"
 
 // JSONValueConverter converts a JSON value into a C++ struct in a
 // lightweight way.
 //
 // Usage:
 // For real examples, you may want to refer to _unittest.cc file.
 //
@@ skipping 30 matching lines @@
 // method.  Then, just use RegisterNestedField() from the containing struct's
 // RegisterJSONConverter method.
 //   struct Nested {
 //     Message foo;
 //     static void RegisterJSONConverter(...) {
 //       ...
 //       converter->RegisterNestedField("foo", &Nested::foo);
 //     }
 //   };
 //
-// For repeated field, we just assume std::vector for its container
-// and you can put RegisterRepeatedInt or some other types.  Use
-// RegisterRepeatedMessage for nested repeated fields.
+// For repeated field, we just assume ScopedVector for its container
+// and you can use RegisterRepeatedMessage (not std::vector to avoid
+// copy constructors).  For repeated fields of basic types such like
+// int, we assume std::vector for its container and you can use
+// RegisterRepeatedInt and so on.
 //
 
 namespace base {
 
 template <typename StructType>
 class JSONValueConverter;
 
 namespace internal {
 
 class FieldConverterBase {
@@ skipping 122 matching lines @@
   JSONValueConverter<NestedType> converter_;
   DISALLOW_COPY_AND_ASSIGN(NestedValueConverter);
 };
 
 template <typename Element>
 class RepeatedValueConverter : public ValueConverter<std::vector<Element> > {
  public:
   RepeatedValueConverter() {}
 
   virtual bool Convert(
       const base::Value& value, std::vector<Element>* field) const OVERRIDE {

[awong, 2012/01/13 00:27:55]

Why do we still use a std::vector<> here?

[Jun Mukai, 2012/01/13 01:09:21]

Either is fine but I don't have a strong motivation to rewrite here because this
is for basic types such like int, double, bool, and strings.  Also
ScopedVector<int> will return int* which seems not handy to use.

The problem here might be that I haven't yet met a real case scenario to use
this.  It could be okay to keep this now and remove this in another CL.

Any is fine to me.  I'll try rewriting to ScopedVector and see how it's going to
be.

[Jun Mukai, 2012/01/13 02:10:37]

Done.

[satorux1, 2012/01/13 02:13:51]

nice. ints and doubles are cheap to copy but strings aren't. making it
ScopedVector would also be good for consistency. 

     const base::ListValue* list = NULL;
     if (!value.GetAsList(&list)) {
       // The field is not a list.
       return false;
     }
 
     field->reserve(list->GetSize());
     for (size_t i = 0; i < list->GetSize(); ++i) {
       base::Value* element = NULL;
       if (!list->Get(i, &element))
         continue;
 
       Element e;
-      if (!basic_converter_.Convert(*element, &e)) {
+      if (basic_converter_.Convert(*element, &e)) {
+        field->push_back(e);
+      } else {
         DVLOG(1) << "failure at " << i << "-th element";
         return false;
       }
-      field->push_back(e);
     }
     return true;
   }
 
  private:
   BasicValueConverter<Element> basic_converter_;
   DISALLOW_COPY_AND_ASSIGN(RepeatedValueConverter);
 };
 
 template <typename NestedType>
 class RepeatedMessageConverter
-    : public ValueConverter<std::vector<NestedType> > {
+    : public ValueConverter<ScopedVector<NestedType> > {
  public:
   RepeatedMessageConverter() {}
 
-  virtual bool Convert(
-      const base::Value& value, std::vector<NestedType>* field) const OVERRIDE {
+  virtual bool Convert(const base::Value& value,
+                       ScopedVector<NestedType>* field) const OVERRIDE {
     const base::ListValue* list = NULL;
     if (!value.GetAsList(&list))
       return false;
 
     field->reserve(list->GetSize());
     for (size_t i = 0; i < list->GetSize(); ++i) {
       base::Value* element = NULL;
       if (!list->Get(i, &element))
         continue;
 
-      field->push_back(NestedType());
-      if (!converter_.Convert(*element, &field->back())) {
+      NestedType* nested = new NestedType();
+      if (converter_.Convert(*element, nested)) {
+        field->push_back(nested);
+      } else {
         DVLOG(1) << "failure at " << i << "-th element";
         return false;
       }
     }
     return true;
   }
 
  private:
   JSONValueConverter<NestedType> converter_;
   DISALLOW_COPY_AND_ASSIGN(RepeatedMessageConverter);
 };
 
 }  // namespace internal
 
 template <class StructType>
 class JSONValueConverter {
  public:
   JSONValueConverter() {
     StructType::RegisterJSONConverter(this);
   }
 
-  ~JSONValueConverter() {
-    STLDeleteContainerPointers(fields_.begin(), fields_.end());
-  }
-
   void RegisterIntField(const std::string& field_name,
                         int StructType::* field) {
     fields_.push_back(new internal::FieldConverter<StructType, int>(
         field_name, field, new internal::BasicValueConverter<int>));
   }
 
   void RegisterStringField(const std::string& field_name,
                            std::string StructType::* field) {
     fields_.push_back(new internal::FieldConverter<StructType, std::string>(
         field_name, field, new internal::BasicValueConverter<std::string>));
@@ skipping 60 matching lines @@
 
   void RegisterRepeatedBool(const std::string& field_name,
                             std::vector<bool> StructType::* field) {
     fields_.push_back(
         new internal::FieldConverter<StructType, std::vector<bool> >(
             field_name, field, new internal::RepeatedValueConverter<bool>));
   }
 
   template <class NestedType>
   void RegisterRepeatedMessage(const std::string& field_name,
-                               std::vector<NestedType> StructType::* field) {
+                               ScopedVector<NestedType> StructType::* field) {
     fields_.push_back(
-        new internal::FieldConverter<StructType, std::vector<NestedType> >(
+        new internal::FieldConverter<StructType, ScopedVector<NestedType> >(
             field_name,
             field,
             new internal::RepeatedMessageConverter<NestedType>));
   }
 
   bool Convert(const base::Value& value, StructType* output) const {
     const DictionaryValue* dictionary_value = NULL;
     if (!value.GetAsDictionary(&dictionary_value))
       return false;
 
-    for(std::vector<internal::FieldConverterBase*>::const_iterator it =
-            fields_.begin(); it != fields_.end(); ++it) {
+    for(size_t i = 0; i < fields_.size(); ++i) {
+      const internal::FieldConverterBase* field_converter = fields_[i];
       base::Value* field = NULL;
-      if (dictionary_value->Get((*it)->field_path(), &field)) {
-        if (!(*it)->ConvertField(*field, output)) {
-          DVLOG(1) << "failure at field " << (*it)->field_path();
+      if (dictionary_value->Get(field_converter->field_path(), &field)) {
+        if (!field_converter->ConvertField(*field, output)) {
+          DVLOG(1) << "failure at field " << field_converter->field_path();
           return false;
         }
       }
     }
     return true;
   }
 
  private:
-  std::vector<internal::FieldConverterBase*> fields_;
+  ScopedVector<internal::FieldConverterBase> fields_;
 
   DISALLOW_COPY_AND_ASSIGN(JSONValueConverter);
 };
 
 }  // namespace base
 
 #endif  // BASE_JSON_JSON_VALUE_CONVERTER_H_