Add DictionaryValue::Set and ::SetWithoutPathExpansion functions

that take scoped_ptr.

Mark the non-scoped_ptr versions as deprecated. Update tests. Update one client.

BUG=446238

Review URL: https://codereview.chromium.org/829333004

Cr-Commit-Position: refs/heads/master@{#310121}

3 files changed, 64 insertions, 25 deletions

diff --git a/base/values.cc b/base/values.cc
index 5d45ec3..b478b62 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -371,7 +371,7 @@ void DictionaryValue::Clear() {
   dictionary_.clear();
 }
 
-void DictionaryValue::Set(const std::string& path, Value* in_value) {
+void DictionaryValue::Set(const std::string& path, scoped_ptr<Value> in_value) {
   DCHECK(IsStringUTF8(path));
   DCHECK(in_value);
 
@@ -392,7 +392,11 @@ void DictionaryValue::Set(const std::string& path, Value* in_value) {
     current_path.erase(0, delimiter_position + 1);
   }
 
-  current_dictionary->SetWithoutPathExpansion(current_path, in_value);
+  current_dictionary->SetWithoutPathExpansion(current_path, in_value.Pass());
+}
+
+void DictionaryValue::Set(const std::string& path, Value* in_value) {
+  Set(path, make_scoped_ptr(in_value));
 }
 
 void DictionaryValue::SetBoolean(const std::string& path, bool in_value) {
@@ -418,18 +422,24 @@ void DictionaryValue::SetString(const std::string& path,
 }
 
 void DictionaryValue::SetWithoutPathExpansion(const std::string& key,
-                                              Value* in_value) {
+                                              scoped_ptr<Value> in_value) {
+  Value* bare_ptr = in_value.release();
   // If there's an existing value here, we need to delete it, because
   // we own all our children.
   std::pair<ValueMap::iterator, bool> ins_res =
-      dictionary_.insert(std::make_pair(key, in_value));
+      dictionary_.insert(std::make_pair(key, bare_ptr));
   if (!ins_res.second) {
-    DCHECK_NE(ins_res.first->second, in_value);  // This would be bogus
+    DCHECK_NE(ins_res.first->second, bare_ptr);  // This would be bogus
     delete ins_res.first->second;
-    ins_res.first->second = in_value;
+    ins_res.first->second = bare_ptr;
   }
 }
 
+void DictionaryValue::SetWithoutPathExpansion(const std::string& key,
+                                              Value* in_value) {
+  SetWithoutPathExpansion(key, make_scoped_ptr(in_value));
+}
+
 void DictionaryValue::SetBooleanWithoutPathExpansion(
     const std::string& path, bool in_value) {
   SetWithoutPathExpansion(path, new FundamentalValue(in_value));
diff --git a/base/values.h b/base/values.h
index 04b2d26..4c22f3d 100644
--- a/base/values.h
+++ b/base/values.h
@@ -228,9 +228,9 @@ class BASE_EXPORT DictionaryValue : public Value {
   // within a key, but there are no other restrictions on keys.
   // If the key at any step of the way doesn't exist, or exists but isn't
   // a DictionaryValue, a new DictionaryValue will be created and attached
-  // to the path in that location.
-  // Note that the dictionary takes ownership of the value referenced by
-  // |in_value|, and therefore |in_value| must be non-NULL.
+  // to the path in that location. |in_value| must be non-null.
+  void Set(const std::string& path, scoped_ptr<Value> in_value);
+  // Deprecated version of the above. TODO(estade): remove.
   void Set(const std::string& path, Value* in_value);
 
   // Convenience forms of Set().  These methods will replace any existing
@@ -243,6 +243,9 @@ class BASE_EXPORT DictionaryValue : public Value {
 
   // Like Set(), but without special treatment of '.'.  This allows e.g. URLs to
   // be used as paths.
+  void SetWithoutPathExpansion(const std::string& key,
+                               scoped_ptr<Value> in_value);
+  // Deprecated version of the above. TODO(estade): remove.
   void SetWithoutPathExpansion(const std::string& key, Value* in_value);
 
   // Convenience forms of SetWithoutPathExpansion().
diff --git a/base/values_unittest.cc b/base/values_unittest.cc
index cbb07f3..296880a 100644
--- a/base/values_unittest.cc
+++ b/base/values_unittest.cc
@@ -312,6 +312,31 @@ TEST(ValuesTest, DictionaryRemoval) {
 
 TEST(ValuesTest, DictionaryWithoutPathExpansion) {
   DictionaryValue dict;
+  dict.Set("this.is.expanded", make_scoped_ptr(Value::CreateNullValue()));
+  dict.SetWithoutPathExpansion("this.isnt.expanded",
+                               make_scoped_ptr(Value::CreateNullValue()));
+
+  EXPECT_FALSE(dict.HasKey("this.is.expanded"));
+  EXPECT_TRUE(dict.HasKey("this"));
+  Value* value1;
+  EXPECT_TRUE(dict.Get("this", &value1));
+  DictionaryValue* value2;
+  ASSERT_TRUE(dict.GetDictionaryWithoutPathExpansion("this", &value2));
+  EXPECT_EQ(value1, value2);
+  EXPECT_EQ(1U, value2->size());
+
+  EXPECT_TRUE(dict.HasKey("this.isnt.expanded"));
+  Value* value3;
+  EXPECT_FALSE(dict.Get("this.isnt.expanded", &value3));
+  Value* value4;
+  ASSERT_TRUE(dict.GetWithoutPathExpansion("this.isnt.expanded", &value4));
+  EXPECT_EQ(Value::TYPE_NULL, value4->GetType());
+}
+
+// Tests the deprecated version of SetWithoutPathExpansion.
+// TODO(estade): remove.
+TEST(ValuesTest, DictionaryWithoutPathExpansionDeprecated) {
+  DictionaryValue dict;
   dict.Set("this.is.expanded", Value::CreateNullValue());
   dict.SetWithoutPathExpansion("this.isnt.expanded", Value::CreateNullValue());
 
@@ -334,8 +359,8 @@ TEST(ValuesTest, DictionaryWithoutPathExpansion) {
 
 TEST(ValuesTest, DictionaryRemovePath) {
   DictionaryValue dict;
-  dict.Set("a.long.way.down", new FundamentalValue(1));
-  dict.Set("a.long.key.path", new FundamentalValue(true));
+  dict.SetInteger("a.long.way.down", 1);
+  dict.SetBoolean("a.long.key.path", true);
 
   scoped_ptr<Value> removed_item;
   EXPECT_TRUE(dict.RemovePath("a.long.way.down", &removed_item));
@@ -360,17 +385,17 @@ TEST(ValuesTest, DictionaryRemovePath) {
 TEST(ValuesTest, DeepCopy) {
   DictionaryValue original_dict;
   Value* original_null = Value::CreateNullValue();
-  original_dict.Set("null", original_null);
+  original_dict.Set("null", make_scoped_ptr(original_null));
   FundamentalValue* original_bool = new FundamentalValue(true);
-  original_dict.Set("bool", original_bool);
+  original_dict.Set("bool", make_scoped_ptr(original_bool));
   FundamentalValue* original_int = new FundamentalValue(42);
-  original_dict.Set("int", original_int);
+  original_dict.Set("int", make_scoped_ptr(original_int));
   FundamentalValue* original_double = new FundamentalValue(3.14);
-  original_dict.Set("double", original_double);
+  original_dict.Set("double", make_scoped_ptr(original_double));
   StringValue* original_string = new StringValue("hello");
-  original_dict.Set("string", original_string);
+  original_dict.Set("string", make_scoped_ptr(original_string));
   StringValue* original_string16 = new StringValue(ASCIIToUTF16("hello16"));
-  original_dict.Set("string16", original_string16);
+  original_dict.Set("string16", make_scoped_ptr(original_string16));
 
   scoped_ptr<char[]> original_buffer(new char[42]);
   memset(original_buffer.get(), '!', 42);
@@ -382,11 +407,11 @@ TEST(ValuesTest, DeepCopy) {
   original_list->Append(original_list_element_0);
   FundamentalValue* original_list_element_1 = new FundamentalValue(1);
   original_list->Append(original_list_element_1);
-  original_dict.Set("list", original_list);
+  original_dict.Set("list", make_scoped_ptr(original_list));
 
   DictionaryValue* original_nested_dictionary = new DictionaryValue();
-  original_nested_dictionary->Set("key", new StringValue("value"));
-  original_dict.Set("dictionary", original_nested_dictionary);
+  original_nested_dictionary->SetString("key", "value");
+  original_dict.Set("dictionary", make_scoped_ptr(original_nested_dictionary));
 
   scoped_ptr<DictionaryValue> copy_dict(original_dict.DeepCopy());
   ASSERT_TRUE(copy_dict.get());
@@ -515,7 +540,7 @@ TEST(ValuesTest, Equals) {
   dv.SetDouble("c", 2.5);
   dv.SetString("d1", "string");
   dv.SetString("d2", ASCIIToUTF16("http://google.com"));
-  dv.Set("e", Value::CreateNullValue());
+  dv.Set("e", make_scoped_ptr(Value::CreateNullValue()));
 
   scoped_ptr<DictionaryValue> copy;
   copy.reset(dv.DeepCopy());
@@ -524,7 +549,7 @@ TEST(ValuesTest, Equals) {
   ListValue* list = new ListValue;
   list->Append(Value::CreateNullValue());
   list->Append(new DictionaryValue);
-  dv.Set("f", list);
+  dv.Set("f", make_scoped_ptr(list));
 
   EXPECT_FALSE(dv.Equals(copy.get()));
   copy->Set("f", list->DeepCopy());
@@ -622,9 +647,10 @@ TEST(ValuesTest, DeepCopyCovariantReturnTypes) {
 TEST(ValuesTest, RemoveEmptyChildren) {
   scoped_ptr<DictionaryValue> root(new DictionaryValue);
   // Remove empty lists and dictionaries.
-  root->Set("empty_dict", new DictionaryValue);
-  root->Set("empty_list", new ListValue);
-  root->SetWithoutPathExpansion("a.b.c.d.e", new DictionaryValue);
+  root->Set("empty_dict", make_scoped_ptr(new DictionaryValue));
+  root->Set("empty_list", make_scoped_ptr(new ListValue));
+  root->SetWithoutPathExpansion("a.b.c.d.e",
+                                make_scoped_ptr(new DictionaryValue));
   root.reset(root->DeepCopyWithoutEmptyChildren());
   EXPECT_TRUE(root->empty());