Make base::DictionaryValue::Set* return pointers

This change changes the various base::DictionaryValue::Set* methods to
return a pointer to the newly added Value. This is in response to the
discussion on https://codereview.chromium.org/2845113002/.

Furthermore, this is motivated by the fact that various insert methods
of std::map also return a handle to the just added element.

R=brettw@chromium.org
BUG=646113

Review-Url: https://codereview.chromium.org/2850773002
Cr-Commit-Position: refs/heads/master@{#468146}

base/values.cc

@@ -611,7 +611,7 @@ void DictionaryValue::Clear() {
   dict_->clear();
 }
 
-void DictionaryValue::Set(StringPiece path, std::unique_ptr<Value> in_value) {
+Value* DictionaryValue::Set(StringPiece path, std::unique_ptr<Value> in_value) {
   DCHECK(IsStringUTF8(path));
   DCHECK(in_value);
 
@@ -633,67 +633,94 @@ void DictionaryValue::Set(StringPiece path, std::unique_ptr<Value> in_value) {
     current_path = current_path.substr(delimiter_position + 1);
   }
 
-  current_dictionary->SetWithoutPathExpansion(current_path,
-                                              std::move(in_value));
+  return current_dictionary->SetWithoutPathExpansion(current_path,
+                                                     std::move(in_value));
 }
 
-void DictionaryValue::Set(StringPiece path, Value* in_value) {
-  Set(path, WrapUnique(in_value));
+Value* DictionaryValue::Set(StringPiece path, Value* in_value) {
+  return Set(path, WrapUnique(in_value));
 }
 
-void DictionaryValue::SetBoolean(StringPiece path, bool in_value) {
-  Set(path, new Value(in_value));
+Value* DictionaryValue::SetBoolean(StringPiece path, bool in_value) {
+  return Set(path, new Value(in_value));
 }
 
-void DictionaryValue::SetInteger(StringPiece path, int in_value) {
-  Set(path, new Value(in_value));
+Value* DictionaryValue::SetInteger(StringPiece path, int in_value) {
+  return Set(path, new Value(in_value));
 }
 
-void DictionaryValue::SetDouble(StringPiece path, double in_value) {
-  Set(path, new Value(in_value));
+Value* DictionaryValue::SetDouble(StringPiece path, double in_value) {
+  return Set(path, new Value(in_value));
 }
 
-void DictionaryValue::SetString(StringPiece path, StringPiece in_value) {
-  Set(path, new Value(in_value));
+Value* DictionaryValue::SetString(StringPiece path, StringPiece in_value) {
+  return Set(path, new Value(in_value));
 }
 
-void DictionaryValue::SetString(StringPiece path, const string16& in_value) {
-  Set(path, new Value(in_value));
+Value* DictionaryValue::SetString(StringPiece path, const string16& in_value) {
+  return Set(path, new Value(in_value));
 }
 
-void DictionaryValue::SetWithoutPathExpansion(StringPiece key,
-                                              std::unique_ptr<Value> in_value) {
-  (*dict_)[key.as_string()] = std::move(in_value);
+DictionaryValue* DictionaryValue::SetDictionary(
+    StringPiece path,
+    std::unique_ptr<DictionaryValue> in_value) {
+  return static_cast<DictionaryValue*>(Set(path, std::move(in_value)));
 }
 
-void DictionaryValue::SetWithoutPathExpansion(StringPiece key,
-                                              Value* in_value) {
-  SetWithoutPathExpansion(key, WrapUnique(in_value));
+ListValue* DictionaryValue::SetList(StringPiece path,
+                                    std::unique_ptr<ListValue> in_value) {
+  return static_cast<ListValue*>(Set(path, std::move(in_value)));
 }
 
-void DictionaryValue::SetBooleanWithoutPathExpansion(StringPiece path,
-                                                     bool in_value) {
-  SetWithoutPathExpansion(path, base::MakeUnique<base::Value>(in_value));
+Value* DictionaryValue::SetWithoutPathExpansion(
+    StringPiece key,
+    std::unique_ptr<Value> in_value) {
+  return ((*dict_)[key.as_string()] = std::move(in_value)).get();
+}
+
+Value* DictionaryValue::SetWithoutPathExpansion(StringPiece key,
+                                                Value* in_value) {
+  return SetWithoutPathExpansion(key, WrapUnique(in_value));
+}
+
+Value* DictionaryValue::SetBooleanWithoutPathExpansion(StringPiece path,
+                                                       bool in_value) {
+  return SetWithoutPathExpansion(path, base::MakeUnique<base::Value>(in_value));
+}
+
+Value* DictionaryValue::SetIntegerWithoutPathExpansion(StringPiece path,
+                                                       int in_value) {
+  return SetWithoutPathExpansion(path, base::MakeUnique<base::Value>(in_value));
+}
+
+Value* DictionaryValue::SetDoubleWithoutPathExpansion(StringPiece path,
+                                                      double in_value) {
+  return SetWithoutPathExpansion(path, base::MakeUnique<base::Value>(in_value));
 }
 
-void DictionaryValue::SetIntegerWithoutPathExpansion(StringPiece path,
-                                                     int in_value) {
-  SetWithoutPathExpansion(path, base::MakeUnique<base::Value>(in_value));
+Value* DictionaryValue::SetStringWithoutPathExpansion(StringPiece path,
+                                                      StringPiece in_value) {
+  return SetWithoutPathExpansion(path, base::MakeUnique<base::Value>(in_value));
 }
 
-void DictionaryValue::SetDoubleWithoutPathExpansion(StringPiece path,
-                                                    double in_value) {
-  SetWithoutPathExpansion(path, base::MakeUnique<base::Value>(in_value));
+Value* DictionaryValue::SetStringWithoutPathExpansion(
+    StringPiece path,
+    const string16& in_value) {
+  return SetWithoutPathExpansion(path, base::MakeUnique<base::Value>(in_value));
 }
 
-void DictionaryValue::SetStringWithoutPathExpansion(StringPiece path,
-                                                    StringPiece in_value) {
-  SetWithoutPathExpansion(path, base::MakeUnique<base::Value>(in_value));
+DictionaryValue* DictionaryValue::SetDictionaryWithoutPathExpansion(
+    StringPiece path,
+    std::unique_ptr<DictionaryValue> in_value) {
+  return static_cast<DictionaryValue*>(
+      SetWithoutPathExpansion(path, std::move(in_value)));
 }
 
-void DictionaryValue::SetStringWithoutPathExpansion(StringPiece path,
-                                                    const string16& in_value) {
-  SetWithoutPathExpansion(path, base::MakeUnique<base::Value>(in_value));
+ListValue* DictionaryValue::SetListWithoutPathExpansion(
+    StringPiece path,
+    std::unique_ptr<ListValue> in_value) {
+  return static_cast<ListValue*>(
+      SetWithoutPathExpansion(path, std::move(in_value)));
 }
 
 bool DictionaryValue::Get(StringPiece path,

base/values.h

@@ -238,32 +238,41 @@ class BASE_EXPORT DictionaryValue : public Value {
   // 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. |in_value| must be non-null.
-  void Set(StringPiece path, std::unique_ptr<Value> in_value);
+  // Returns a pointer to the inserted value.
+  Value* Set(StringPiece path, std::unique_ptr<Value> in_value);
   // Deprecated version of the above. TODO(estade): remove.
-  void Set(StringPiece path, Value* in_value);
+  Value* Set(StringPiece path, Value* in_value);
 
   // Convenience forms of Set().  These methods will replace any existing
   // value at that path, even if it has a different type.
-  void SetBoolean(StringPiece path, bool in_value);
-  void SetInteger(StringPiece path, int in_value);
-  void SetDouble(StringPiece path, double in_value);
-  void SetString(StringPiece path, StringPiece in_value);
-  void SetString(StringPiece path, const string16& in_value);
+  Value* SetBoolean(StringPiece path, bool in_value);
+  Value* SetInteger(StringPiece path, int in_value);
+  Value* SetDouble(StringPiece path, double in_value);
+  Value* SetString(StringPiece path, StringPiece in_value);
+  Value* SetString(StringPiece path, const string16& in_value);
+  DictionaryValue* SetDictionary(StringPiece path,
+                                 std::unique_ptr<DictionaryValue> in_value);
+  ListValue* SetList(StringPiece path, std::unique_ptr<ListValue> in_value);
 
   // Like Set(), but without special treatment of '.'.  This allows e.g. URLs to
   // be used as paths.
-  void SetWithoutPathExpansion(StringPiece key,
-                               std::unique_ptr<Value> in_value);
+  Value* SetWithoutPathExpansion(StringPiece key,
+                                 std::unique_ptr<Value> in_value);
   // Deprecated version of the above. TODO(estade): remove.
-  void SetWithoutPathExpansion(StringPiece key, Value* in_value);
+  Value* SetWithoutPathExpansion(StringPiece key, Value* in_value);
 
   // Convenience forms of SetWithoutPathExpansion().
-  void SetBooleanWithoutPathExpansion(StringPiece path, bool in_value);
-  void SetIntegerWithoutPathExpansion(StringPiece path, int in_value);
-  void SetDoubleWithoutPathExpansion(StringPiece path, double in_value);
-  void SetStringWithoutPathExpansion(StringPiece path, StringPiece in_value);
-  void SetStringWithoutPathExpansion(StringPiece path,
-                                     const string16& in_value);
+  Value* SetBooleanWithoutPathExpansion(StringPiece path, bool in_value);
+  Value* SetIntegerWithoutPathExpansion(StringPiece path, int in_value);
+  Value* SetDoubleWithoutPathExpansion(StringPiece path, double in_value);
+  Value* SetStringWithoutPathExpansion(StringPiece path, StringPiece in_value);
+  Value* SetStringWithoutPathExpansion(StringPiece path,
+                                       const string16& in_value);
+  DictionaryValue* SetDictionaryWithoutPathExpansion(
+      StringPiece path,
+      std::unique_ptr<DictionaryValue> in_value);
+  ListValue* SetListWithoutPathExpansion(StringPiece path,
+                                         std::unique_ptr<ListValue> in_value);
 
   // Gets the Value associated with the given path starting from this object.
   // A path has the form "<key>" or "<key>.<key>.[...]", where "." indexes

base/values_unittest.cc

@@ -574,6 +574,113 @@ TEST(ValuesTest, DictionaryDeletion) {
   EXPECT_TRUE(dict.empty());
 }
 
+TEST(ValuesTest, DictionarySetReturnsPointer) {
+  {
+    DictionaryValue dict;
+    Value* blank_ptr = dict.Set("foo.bar", base::MakeUnique<base::Value>());
+    EXPECT_EQ(Value::Type::NONE, blank_ptr->type());
+  }
+
+  {
+    DictionaryValue dict;
+    Value* blank_ptr = dict.SetWithoutPathExpansion(
+        "foo.bar", base::MakeUnique<base::Value>());
+    EXPECT_EQ(Value::Type::NONE, blank_ptr->type());
+  }
+
+  {
+    DictionaryValue dict;
+    Value* bool_ptr = dict.SetBooleanWithoutPathExpansion("foo.bar", false);
+    EXPECT_EQ(Value::Type::BOOLEAN, bool_ptr->type());
+    EXPECT_FALSE(bool_ptr->GetBool());
+  }
+
+  {
+    DictionaryValue dict;
+    Value* int_ptr = dict.SetInteger("foo.bar", 42);
+    EXPECT_EQ(Value::Type::INTEGER, int_ptr->type());
+    EXPECT_EQ(42, int_ptr->GetInt());
+  }
+
+  {
+    DictionaryValue dict;
+    Value* int_ptr = dict.SetIntegerWithoutPathExpansion("foo.bar", 123);
+    EXPECT_EQ(Value::Type::INTEGER, int_ptr->type());
+    EXPECT_EQ(123, int_ptr->GetInt());
+  }
+
+  {
+    DictionaryValue dict;
+    Value* double_ptr = dict.SetDouble("foo.bar", 3.142);
+    EXPECT_EQ(Value::Type::DOUBLE, double_ptr->type());
+    EXPECT_EQ(3.142, double_ptr->GetDouble());
+  }
+
+  {
+    DictionaryValue dict;
+    Value* double_ptr = dict.SetDoubleWithoutPathExpansion("foo.bar", 2.718);
+    EXPECT_EQ(Value::Type::DOUBLE, double_ptr->type());
+    EXPECT_EQ(2.718, double_ptr->GetDouble());
+  }
+
+  {
+    DictionaryValue dict;
+    Value* string_ptr = dict.SetString("foo.bar", "foo");
+    EXPECT_EQ(Value::Type::STRING, string_ptr->type());
+    EXPECT_EQ("foo", string_ptr->GetString());
+  }
+
+  {
+    DictionaryValue dict;
+    Value* string_ptr = dict.SetStringWithoutPathExpansion("foo.bar", "bar");
+    EXPECT_EQ(Value::Type::STRING, string_ptr->type());
+    EXPECT_EQ("bar", string_ptr->GetString());
+  }
+
+  {
+    DictionaryValue dict;
+    Value* string16_ptr = dict.SetString("foo.bar", ASCIIToUTF16("baz"));
+    EXPECT_EQ(Value::Type::STRING, string16_ptr->type());
+    EXPECT_EQ("baz", string16_ptr->GetString());
+  }
+
+  {
+    DictionaryValue dict;
+    Value* string16_ptr =
+        dict.SetStringWithoutPathExpansion("foo.bar", ASCIIToUTF16("qux"));
+    EXPECT_EQ(Value::Type::STRING, string16_ptr->type());
+    EXPECT_EQ("qux", string16_ptr->GetString());
+  }
+
+  {
+    DictionaryValue dict;
+    DictionaryValue* dict_ptr = dict.SetDictionary(
+        "foo.bar", base::MakeUnique<base::DictionaryValue>());
+    EXPECT_EQ(Value::Type::DICTIONARY, dict_ptr->type());
+  }
+
+  {
+    DictionaryValue dict;
+    DictionaryValue* dict_ptr = dict.SetDictionaryWithoutPathExpansion(
+        "foo.bar", base::MakeUnique<base::DictionaryValue>());
+    EXPECT_EQ(Value::Type::DICTIONARY, dict_ptr->type());
+  }
+
+  {
+    DictionaryValue dict;
+    ListValue* list_ptr =
+        dict.SetList("foo.bar", base::MakeUnique<base::ListValue>());
+    EXPECT_EQ(Value::Type::LIST, list_ptr->type());
+  }
+
+  {
+    DictionaryValue dict;
+    ListValue* list_ptr = dict.SetListWithoutPathExpansion(
+        "foo.bar", base::MakeUnique<base::ListValue>());
+    EXPECT_EQ(Value::Type::LIST, list_ptr->type());
+  }
+}
+
 TEST(ValuesTest, DictionaryRemoval) {
   std::string key = "test";
   std::unique_ptr<Value> removed_item;