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

During using this library, I realized that we want to use DISALLOW_COPY_AND_ASSIGN
for the messages and RepeatedMessageConverter is inconsistent with such ones
because it uses a copy constructor as "push_back(NestedType())".  Then
I now think to use ScopedVector.

BUG=109375
TEST=passed locally


Review URL: http://codereview.chromium.org/9187047

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@117786 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 51 insertions, 48 deletions

diff --git a/base/json/json_value_converter.h b/base/json/json_value_converter.h
index ea43dbb..0bb131c 100644
--- a/base/json/json_value_converter.h
+++ b/base/json/json_value_converter.h
@@ -13,6 +13,7 @@
 #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/string_piece.h"
@@ -64,7 +65,7 @@
 //     }
 //   };
 //
-// For repeated field, we just assume std::vector for its container
+// For repeated field, we just assume ScopedVector for its container
 // and you can put RegisterRepeatedInt or some other types.  Use
 // RegisterRepeatedMessage for nested repeated fields.
 //
@@ -240,12 +241,12 @@ class NestedValueConverter : public ValueConverter<NestedType> {
 };
 
 template <typename Element>
-class RepeatedValueConverter : public ValueConverter<std::vector<Element> > {
+class RepeatedValueConverter : public ValueConverter<ScopedVector<Element> > {
  public:
   RepeatedValueConverter() {}
 
   virtual bool Convert(
-      const base::Value& value, std::vector<Element>* field) const OVERRIDE {
+      const base::Value& value, ScopedVector<Element>* field) const OVERRIDE {
     const base::ListValue* list = NULL;
     if (!value.GetAsList(&list)) {
       // The field is not a list.
@@ -258,12 +259,13 @@ class RepeatedValueConverter : public ValueConverter<std::vector<Element> > {
       if (!list->Get(i, &element))
         continue;
 
-      Element e;
-      if (!basic_converter_.Convert(*element, &e)) {
+      Element *e = new Element;
+      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;
   }
@@ -275,12 +277,12 @@ class RepeatedValueConverter : public ValueConverter<std::vector<Element> > {
 
 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;
@@ -291,8 +293,10 @@ class RepeatedMessageConverter
       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;
       }
@@ -314,10 +318,6 @@ class 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>(
@@ -369,49 +369,49 @@ class JSONValueConverter {
   }
 
   void RegisterRepeatedInt(const std::string& field_name,
-                           std::vector<int> StructType::* field) {
+                           ScopedVector<int> StructType::* field) {
     fields_.push_back(
-        new internal::FieldConverter<StructType, std::vector<int> >(
+        new internal::FieldConverter<StructType, ScopedVector<int> >(
             field_name, field, new internal::RepeatedValueConverter<int>));
   }
 
   void RegisterRepeatedString(const std::string& field_name,
-                              std::vector<std::string> StructType::* field) {
+                              ScopedVector<std::string> StructType::* field) {
     fields_.push_back(
-        new internal::FieldConverter<StructType, std::vector<std::string> >(
+        new internal::FieldConverter<StructType, ScopedVector<std::string> >(
             field_name,
             field,
             new internal::RepeatedValueConverter<std::string>));
   }
 
   void RegisterRepeatedString(const std::string& field_name,
-                              std::vector<string16> StructType::* field) {
+                              ScopedVector<string16> StructType::* field) {
     fields_.push_back(
-        new internal::FieldConverter<StructType, std::vector<string16> >(
+        new internal::FieldConverter<StructType, ScopedVector<string16> >(
             field_name,
             field,
             new internal::RepeatedValueConverter<string16>));
   }
 
   void RegisterRepeatedDouble(const std::string& field_name,
-                              std::vector<double> StructType::* field) {
+                              ScopedVector<double> StructType::* field) {
     fields_.push_back(
-        new internal::FieldConverter<StructType, std::vector<double> >(
+        new internal::FieldConverter<StructType, ScopedVector<double> >(
             field_name, field, new internal::RepeatedValueConverter<double>));
   }
 
   void RegisterRepeatedBool(const std::string& field_name,
-                            std::vector<bool> StructType::* field) {
+                            ScopedVector<bool> StructType::* field) {
     fields_.push_back(
-        new internal::FieldConverter<StructType, std::vector<bool> >(
+        new internal::FieldConverter<StructType, ScopedVector<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>));
@@ -422,12 +422,12 @@ class JSONValueConverter {
     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;
         }
       }
@@ -436,7 +436,7 @@ class JSONValueConverter {
   }
 
  private:
-  std::vector<internal::FieldConverterBase*> fields_;
+  ScopedVector<internal::FieldConverterBase> fields_;
 
   DISALLOW_COPY_AND_ASSIGN(JSONValueConverter);
 };
diff --git a/base/json/json_value_converter_unittest.cc b/base/json/json_value_converter_unittest.cc
index b91dd6f..d23a1e9 100644
--- a/base/json/json_value_converter_unittest.cc
+++ b/base/json/json_value_converter_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/values.h"
 #include "base/json/json_reader.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
 #include "base/string_piece.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -25,7 +26,7 @@ struct SimpleMessage {
   std::string bar;
   bool baz;
   SimpleEnum simple_enum;
-  std::vector<int> ints;
+  ScopedVector<int> ints;
   SimpleMessage() : foo(0), baz(false) {}
 
   static bool ParseSimpleEnum(const StringPiece& value, SimpleEnum* field) {
@@ -54,7 +55,7 @@ struct SimpleMessage {
 struct NestedMessage {
   double foo;
   SimpleMessage child;
-  std::vector<SimpleMessage> children;
+  ScopedVector<SimpleMessage> children;
 
   NestedMessage() : foo(0) {}
 
@@ -88,8 +89,8 @@ TEST(JSONValueConverterTest, ParseSimpleMessage) {
   EXPECT_TRUE(message.baz);
   EXPECT_EQ(SimpleMessage::FOO, message.simple_enum);
   EXPECT_EQ(2, static_cast<int>(message.ints.size()));
-  EXPECT_EQ(1, message.ints[0]);
-  EXPECT_EQ(2, message.ints[1]);
+  EXPECT_EQ(1, *(message.ints[0]));
+  EXPECT_EQ(2, *(message.ints[1]));
 }
 
 TEST(JSONValueConverterTest, ParseNestedMessage) {
@@ -124,15 +125,17 @@ TEST(JSONValueConverterTest, ParseNestedMessage) {
   EXPECT_TRUE(message.child.baz);
 
   EXPECT_EQ(2, static_cast<int>(message.children.size()));
-  const SimpleMessage& first_child = message.children[0];
-  EXPECT_EQ(2, first_child.foo);
-  EXPECT_EQ("foobar", first_child.bar);
-  EXPECT_TRUE(first_child.baz);
-
-  const SimpleMessage& second_child = message.children[1];
-  EXPECT_EQ(3, second_child.foo);
-  EXPECT_EQ("barbaz", second_child.bar);
-  EXPECT_FALSE(second_child.baz);
+  const SimpleMessage* first_child = message.children[0];
+  ASSERT_TRUE(first_child);
+  EXPECT_EQ(2, first_child->foo);
+  EXPECT_EQ("foobar", first_child->bar);
+  EXPECT_TRUE(first_child->baz);
+
+  const SimpleMessage* second_child = message.children[1];
+  ASSERT_TRUE(second_child);
+  EXPECT_EQ(3, second_child->foo);
+  EXPECT_EQ("barbaz", second_child->bar);
+  EXPECT_FALSE(second_child->baz);
 }
 
 TEST(JSONValueConverterTest, ParseFailures) {
@@ -169,8 +172,8 @@ TEST(JSONValueConverterTest, ParseWithMissingFields) {
   EXPECT_EQ(1, message.foo);
   EXPECT_TRUE(message.baz);
   EXPECT_EQ(2, static_cast<int>(message.ints.size()));
-  EXPECT_EQ(1, message.ints[0]);
-  EXPECT_EQ(2, message.ints[1]);
+  EXPECT_EQ(1, *(message.ints[0]));
+  EXPECT_EQ(2, *(message.ints[1]));
 }
 
 TEST(JSONValueConverterTest, EnumParserFails) {