[base] Construct DictValue from flat_map<string, Value>

This change allows the construction of a dictionary value from
flat_map<string, Value>, while keeping unique_ptr<Value> as the mapped
type of the underlying storage. This is in line of reducing heap
allocations for Value, and simplifies its usage.

It is still planned to eventually remove unique_ptr<Value> from the
underlying storage as well.

Bug: 646113
Change-Id: I3ebe11f11faef77e19a4610aba84c541e5f5b3ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2505980
Reviewed-by: Christoph Schwering <schwering@google.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823277}

base/json/json_parser.cc

@@ -422,8 +422,7 @@ Optional<Value> JSONParser::ConsumeDictionary() {
       return nullopt;
     }
 
-    dict_storage.emplace_back(key.DestructiveAsString(),
-                              std::make_unique<Value>(std::move(*value)));
+    dict_storage.emplace_back(key.DestructiveAsString(), std::move(*value));
 
     token = GetNextToken();
     if (token == T_LIST_SEPARATOR) {

base/value_iterators_unittest.cc

@@ -33,11 +33,6 @@ bool are_equal(InputIterator1 first1,
 
 }  // namespace
 
-TEST(ValueIteratorsTest, SameDictStorage) {
-  static_assert(std::is_same<Value::DictStorage, DictStorage>::value,
-                "DictStorage differs between Value and Value Iterators.");
-}
-
 TEST(ValueIteratorsTest, IsAssignable) {
   static_assert(
       !std::is_assignable<dict_iterator::reference::first_type, std::string>(),

base/values.cc

@@ -169,7 +169,7 @@ Value::Value(Type type) {
       data_.emplace<BlobStorage>();
       return;
     case Type::DICTIONARY:
-      data_.emplace<DictStorage>();
+      data_.emplace<LegacyDictStorage>();
       return;
     case Type::LIST:
       data_.emplace<ListStorage>();
@@ -221,14 +221,22 @@ Value::Value(base::span<const uint8_t> in_blob)
 Value::Value(BlobStorage&& in_blob) noexcept : data_(std::move(in_blob)) {}
 
 Value::Value(const DictStorage& in_dict)
-    : data_(absl::in_place_type_t<DictStorage>()) {
+    : data_(absl::in_place_type_t<LegacyDictStorage>()) {
+  dict().reserve(in_dict.size());
   for (const auto& it : in_dict) {
     dict().try_emplace(dict().end(), it.first,
-                       std::make_unique<Value>(it.second->Clone()));
+                       std::make_unique<Value>(it.second.Clone()));
   }
 }
 
-Value::Value(DictStorage&& in_dict) noexcept : data_(std::move(in_dict)) {}
+Value::Value(DictStorage&& in_dict) noexcept
+    : data_(absl::in_place_type_t<LegacyDictStorage>()) {
+  dict().reserve(in_dict.size());
+  for (auto& it : in_dict) {
+    dict().try_emplace(dict().end(), std::move(it.first),
+                       std::make_unique<Value>(std::move(it.second)));
+  }
+}
 
 Value::Value(span<const Value> in_list)
     : data_(absl::in_place_type_t<ListStorage>()) {
@@ -241,6 +249,18 @@ Value::Value(ListStorage&& in_list) noexcept : data_(std::move(in_list)) {}
 
 Value& Value::operator=(Value&& that) noexcept = default;
 
+Value::Value(const LegacyDictStorage& storage)
+    : data_(absl::in_place_type_t<LegacyDictStorage>()) {
+  dict().reserve(storage.size());
+  for (const auto& it : storage) {
+    dict().try_emplace(dict().end(), it.first,
+                       std::make_unique<Value>(it.second->Clone()));
+  }
+}
+
+Value::Value(LegacyDictStorage&& storage) noexcept
+    : data_(std::move(storage)) {}
+
 Value::Value(absl::monostate) {}
 
 Value::Value(DoubleStorage storage) : data_(std::move(storage)) {}
@@ -719,6 +739,18 @@ Value::const_dict_iterator_proxy Value::DictItems() const {
   return const_dict_iterator_proxy(&dict());
 }
 
+Value::DictStorage Value::TakeDict() {
+  DictStorage storage;
+  storage.reserve(dict().size());
+  for (auto& pair : dict()) {
+    storage.try_emplace(storage.end(), std::move(pair.first),
+                        std::move(*pair.second));
+  }
+
+  dict().clear();
+  return storage;
+}
+
 size_t Value::DictSize() const {
   return dict().size();
 }
@@ -913,8 +945,8 @@ bool operator<(const Value& lhs, const Value& rhs) {
       return std::lexicographical_compare(
           std::begin(lhs.dict()), std::end(lhs.dict()), std::begin(rhs.dict()),
           std::end(rhs.dict()),
-          [](const Value::DictStorage::value_type& u,
-             const Value::DictStorage::value_type& v) {
+          [](const Value::LegacyDictStorage::value_type& u,
+             const Value::LegacyDictStorage::value_type& v) {
             return std::tie(u.first, *u.second) < std::tie(v.first, *v.second);
           });
     case Value::Type::LIST:
@@ -1022,9 +1054,12 @@ std::unique_ptr<DictionaryValue> DictionaryValue::From(
 }
 
 DictionaryValue::DictionaryValue() : Value(Type::DICTIONARY) {}
-DictionaryValue::DictionaryValue(const DictStorage& in_dict) : Value(in_dict) {}
-DictionaryValue::DictionaryValue(DictStorage&& in_dict) noexcept
-    : Value(std::move(in_dict)) {}
+
+DictionaryValue::DictionaryValue(const LegacyDictStorage& storage)
+    : Value(storage) {}
+
+DictionaryValue::DictionaryValue(LegacyDictStorage&& storage) noexcept
+    : Value(std::move(storage)) {}
 
 bool DictionaryValue::HasKey(StringPiece key) const {
   DCHECK(IsStringUTF8AllowingNoncharacters(key));

base/values.h

@@ -92,8 +92,13 @@ class Value;
 class BASE_EXPORT Value {
  public:
   using BlobStorage = std::vector<uint8_t>;
-  using DictStorage = flat_map<std::string, std::unique_ptr<Value>>;
   using ListStorage = std::vector<Value>;
+  using DictStorage = flat_map<std::string, Value>;
+
+  // Like `DictStorage`, but with std::unique_ptr in the mapped type. This is
+  // due to legacy reasons, and should be removed once no caller relies on
+  // stability of pointers anymore.
+  using LegacyDictStorage = flat_map<std::string, std::unique_ptr<Value>>;
 
   using ListView = CheckedContiguousRange<ListStorage>;
   using ConstListView = CheckedContiguousConstRange<ListStorage>;
@@ -463,6 +468,11 @@ class BASE_EXPORT Value {
   dict_iterator_proxy DictItems();
   const_dict_iterator_proxy DictItems() const;
 
+  // Transfers ownership of the underlying dict to the caller. Subsequent
+  // calls to DictItems() will return an empty dict.
+  // Note: This requires that type() is Type::DICTIONARY.
+  DictStorage TakeDict();
+
   // Returns the size of the dictionary, if the dictionary is empty, and clears
   // the dictionary. Note: These CHECK that type() is Type::DICTIONARY.
   size_t DictSize() const;
@@ -533,11 +543,17 @@ class BASE_EXPORT Value {
 
  protected:
   // Checked convenience accessors for dict and list.
-  const DictStorage& dict() const { return absl::get<DictStorage>(data_); }
-  DictStorage& dict() { return absl::get<DictStorage>(data_); }
+  const LegacyDictStorage& dict() const {
+    return absl::get<LegacyDictStorage>(data_);
+  }
+  LegacyDictStorage& dict() { return absl::get<LegacyDictStorage>(data_); }
   const ListStorage& list() const { return absl::get<ListStorage>(data_); }
   ListStorage& list() { return absl::get<ListStorage>(data_); }
 
+  // Internal constructors, allowing the simplify the implementation of Clone().
+  explicit Value(const LegacyDictStorage& storage);
+  explicit Value(LegacyDictStorage&& storage) noexcept;
+
  private:
   // Special case for doubles, which are aligned to 8 bytes on some
   // 32-bit architectures. In this case, a simple declaration as a
@@ -567,7 +583,7 @@ class BASE_EXPORT Value {
                 DoubleStorage,
                 std::string,
                 BlobStorage,
-                DictStorage,
+                LegacyDictStorage,
                 ListStorage>
       data_;
 };
@@ -577,15 +593,15 @@ class BASE_EXPORT Value {
 // are |std::string|s and should be UTF-8 encoded.
 class BASE_EXPORT DictionaryValue : public Value {
  public:
-  using const_iterator = DictStorage::const_iterator;
-  using iterator = DictStorage::iterator;
+  using const_iterator = LegacyDictStorage::const_iterator;
+  using iterator = LegacyDictStorage::iterator;
 
   // Returns |value| if it is a dictionary, nullptr otherwise.
   static std::unique_ptr<DictionaryValue> From(std::unique_ptr<Value> value);
 
   DictionaryValue();
-  explicit DictionaryValue(const DictStorage& in_dict);
-  explicit DictionaryValue(DictStorage&& in_dict) noexcept;
+  explicit DictionaryValue(const LegacyDictStorage& in_dict);
+  explicit DictionaryValue(LegacyDictStorage&& in_dict) noexcept;
 
   // Returns true if the current dictionary has a value for the given key.
   // DEPRECATED, use Value::FindKey(key) instead.
@@ -759,7 +775,7 @@ class BASE_EXPORT DictionaryValue : public Value {
 
    private:
     const DictionaryValue& target_;
-    DictStorage::const_iterator it_;
+    LegacyDictStorage::const_iterator it_;
   };
 
   // Iteration.

base/values_unittest.cc

@@ -146,17 +146,17 @@ TEST(ValuesTest, ConstructDict) {
 
 TEST(ValuesTest, ConstructDictFromStorage) {
   Value::DictStorage storage;
-  storage.emplace("foo", std::make_unique<Value>("bar"));
+  storage.emplace("foo", "bar");
   {
-    DictionaryValue value(storage);
+    Value value(storage);
     EXPECT_EQ(Value::Type::DICTIONARY, value.type());
     EXPECT_EQ(Value::Type::STRING, value.FindKey("foo")->type());
     EXPECT_EQ("bar", value.FindKey("foo")->GetString());
   }
 
-  *storage["foo"] = base::Value("baz");
+  storage["foo"] = base::Value("baz");
   {
-    DictionaryValue value(std::move(storage));
+    Value value(std::move(storage));
     EXPECT_EQ(Value::Type::DICTIONARY, value.type());
     EXPECT_EQ(Value::Type::STRING, value.FindKey("foo")->type());
     EXPECT_EQ("baz", value.FindKey("foo")->GetString());
@@ -280,7 +280,7 @@ TEST(ValuesTest, CopyBinary) {
 
 TEST(ValuesTest, CopyDictionary) {
   Value::DictStorage storage;
-  storage.emplace("Int", std::make_unique<Value>(123));
+  storage.emplace("Int", 123);
   Value value(std::move(storage));
 
   Value copied_value(value.Clone());
@@ -382,7 +382,7 @@ TEST(ValuesTest, MoveBinary) {
 
 TEST(ValuesTest, MoveConstructDictionary) {
   Value::DictStorage storage;
-  storage.emplace("Int", std::make_unique<Value>(123));
+  storage.emplace("Int", 123);
 
   Value value(std::move(storage));
   Value moved_value(std::move(value));
@@ -392,7 +392,7 @@ TEST(ValuesTest, MoveConstructDictionary) {
 
 TEST(ValuesTest, MoveAssignDictionary) {
   Value::DictStorage storage;
-  storage.emplace("Int", std::make_unique<Value>(123));
+  storage.emplace("Int", 123);
 
   Value blank;
   blank = Value(std::move(storage));
@@ -400,6 +400,35 @@ TEST(ValuesTest, MoveAssignDictionary) {
   EXPECT_EQ(123, blank.FindKey("Int")->GetInt());
 }
 
+TEST(ValuesTest, TakeDict) {
+  // Prepare a dict with a value of each type.
+  Value::DictStorage storage;
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
+  Value value(std::move(storage));
+
+  // Take ownership of the dict and make sure its contents are what we expect.
+  auto dict = value.TakeDict();
+  EXPECT_EQ(8u, dict.size());
+  EXPECT_TRUE(dict["null"].is_none());
+  EXPECT_TRUE(dict["bool"].is_bool());
+  EXPECT_TRUE(dict["int"].is_int());
+  EXPECT_TRUE(dict["double"].is_double());
+  EXPECT_TRUE(dict["string"].is_string());
+  EXPECT_TRUE(dict["blob"].is_blob());
+  EXPECT_TRUE(dict["list"].is_list());
+  EXPECT_TRUE(dict["dict"].is_dict());
+
+  // Validate that |value| no longer contains values.
+  EXPECT_TRUE(value.DictEmpty());
+}
+
 TEST(ValuesTest, MoveList) {
   Value::ListStorage storage;
   storage.emplace_back(123);
@@ -572,7 +601,7 @@ TEST(ValuesTest, ClearList) {
 
 TEST(ValuesTest, FindKey) {
   Value::DictStorage storage;
-  storage.emplace("foo", std::make_unique<Value>("bar"));
+  storage.emplace("foo", "bar");
   Value dict(std::move(storage));
   EXPECT_NE(nullptr, dict.FindKey("foo"));
   EXPECT_EQ(nullptr, dict.FindKey("baz"));
@@ -584,7 +613,7 @@ TEST(ValuesTest, FindKey) {
 
 TEST(ValuesTest, FindKeyChangeValue) {
   Value::DictStorage storage;
-  storage.emplace("foo", std::make_unique<Value>("bar"));
+  storage.emplace("foo", "bar");
   Value dict(std::move(storage));
   Value* found = dict.FindKey("foo");
   EXPECT_NE(nullptr, found);
@@ -596,7 +625,7 @@ TEST(ValuesTest, FindKeyChangeValue) {
 
 TEST(ValuesTest, FindKeyConst) {
   Value::DictStorage storage;
-  storage.emplace("foo", std::make_unique<Value>("bar"));
+  storage.emplace("foo", "bar");
   const Value dict(std::move(storage));
   EXPECT_NE(nullptr, dict.FindKey("foo"));
   EXPECT_EQ(nullptr, dict.FindKey("baz"));
@@ -604,14 +633,14 @@ TEST(ValuesTest, FindKeyConst) {
 
 TEST(ValuesTest, FindKeyOfType) {
   Value::DictStorage storage;
-  storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
-  storage.emplace("bool", std::make_unique<Value>(Value::Type::BOOLEAN));
-  storage.emplace("int", std::make_unique<Value>(Value::Type::INTEGER));
-  storage.emplace("double", std::make_unique<Value>(Value::Type::DOUBLE));
-  storage.emplace("string", std::make_unique<Value>(Value::Type::STRING));
-  storage.emplace("blob", std::make_unique<Value>(Value::Type::BINARY));
-  storage.emplace("list", std::make_unique<Value>(Value::Type::LIST));
-  storage.emplace("dict", std::make_unique<Value>(Value::Type::DICTIONARY));
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
 
   Value dict(std::move(storage));
   EXPECT_NE(nullptr, dict.FindKeyOfType("null", Value::Type::NONE));
@@ -689,14 +718,14 @@ TEST(ValuesTest, FindKeyOfType) {
 
 TEST(ValuesTest, FindKeyOfTypeConst) {
   Value::DictStorage storage;
-  storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
-  storage.emplace("bool", std::make_unique<Value>(Value::Type::BOOLEAN));
-  storage.emplace("int", std::make_unique<Value>(Value::Type::INTEGER));
-  storage.emplace("double", std::make_unique<Value>(Value::Type::DOUBLE));
-  storage.emplace("string", std::make_unique<Value>(Value::Type::STRING));
-  storage.emplace("blob", std::make_unique<Value>(Value::Type::BINARY));
-  storage.emplace("list", std::make_unique<Value>(Value::Type::LIST));
-  storage.emplace("dict", std::make_unique<Value>(Value::Type::DICTIONARY));
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
 
   const Value dict(std::move(storage));
   EXPECT_NE(nullptr, dict.FindKeyOfType("null", Value::Type::NONE));
@@ -774,14 +803,14 @@ TEST(ValuesTest, FindKeyOfTypeConst) {
 
 TEST(ValuesTest, FindBoolKey) {
   Value::DictStorage storage;
-  storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
-  storage.emplace("bool", std::make_unique<Value>(Value::Type::BOOLEAN));
-  storage.emplace("int", std::make_unique<Value>(Value::Type::INTEGER));
-  storage.emplace("double", std::make_unique<Value>(Value::Type::DOUBLE));
-  storage.emplace("string", std::make_unique<Value>(Value::Type::STRING));
-  storage.emplace("blob", std::make_unique<Value>(Value::Type::BINARY));
-  storage.emplace("list", std::make_unique<Value>(Value::Type::LIST));
-  storage.emplace("dict", std::make_unique<Value>(Value::Type::DICTIONARY));
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
 
   const Value dict(std::move(storage));
   EXPECT_EQ(base::nullopt, dict.FindBoolKey("null"));
@@ -796,14 +825,14 @@ TEST(ValuesTest, FindBoolKey) {
 
 TEST(ValuesTest, FindIntKey) {
   Value::DictStorage storage;
-  storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
-  storage.emplace("bool", std::make_unique<Value>(Value::Type::BOOLEAN));
-  storage.emplace("int", std::make_unique<Value>(Value::Type::INTEGER));
-  storage.emplace("double", std::make_unique<Value>(Value::Type::DOUBLE));
-  storage.emplace("string", std::make_unique<Value>(Value::Type::STRING));
-  storage.emplace("blob", std::make_unique<Value>(Value::Type::BINARY));
-  storage.emplace("list", std::make_unique<Value>(Value::Type::LIST));
-  storage.emplace("dict", std::make_unique<Value>(Value::Type::DICTIONARY));
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
 
   const Value dict(std::move(storage));
   EXPECT_EQ(base::nullopt, dict.FindIntKey("null"));
@@ -818,14 +847,14 @@ TEST(ValuesTest, FindIntKey) {
 
 TEST(ValuesTest, FindDoubleKey) {
   Value::DictStorage storage;
-  storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
-  storage.emplace("bool", std::make_unique<Value>(Value::Type::BOOLEAN));
-  storage.emplace("int", std::make_unique<Value>(Value::Type::INTEGER));
-  storage.emplace("double", std::make_unique<Value>(Value::Type::DOUBLE));
-  storage.emplace("string", std::make_unique<Value>(Value::Type::STRING));
-  storage.emplace("blob", std::make_unique<Value>(Value::Type::BINARY));
-  storage.emplace("list", std::make_unique<Value>(Value::Type::LIST));
-  storage.emplace("dict", std::make_unique<Value>(Value::Type::DICTIONARY));
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
 
   const Value dict(std::move(storage));
   EXPECT_EQ(base::nullopt, dict.FindDoubleKey("null"));
@@ -840,14 +869,14 @@ TEST(ValuesTest, FindDoubleKey) {
 
 TEST(ValuesTest, FindStringKey) {
   Value::DictStorage storage;
-  storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
-  storage.emplace("bool", std::make_unique<Value>(Value::Type::BOOLEAN));
-  storage.emplace("int", std::make_unique<Value>(Value::Type::INTEGER));
-  storage.emplace("double", std::make_unique<Value>(Value::Type::DOUBLE));
-  storage.emplace("string", std::make_unique<Value>(Value::Type::STRING));
-  storage.emplace("blob", std::make_unique<Value>(Value::Type::BINARY));
-  storage.emplace("list", std::make_unique<Value>(Value::Type::LIST));
-  storage.emplace("dict", std::make_unique<Value>(Value::Type::DICTIONARY));
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
 
   const Value dict(std::move(storage));
   EXPECT_EQ(nullptr, dict.FindStringKey("null"));
@@ -862,13 +891,13 @@ TEST(ValuesTest, FindStringKey) {
 
 TEST(ValuesTest, MutableFindStringKey) {
   Value::DictStorage storage;
-  storage.emplace("string", std::make_unique<Value>("foo"));
+  storage.emplace("string", "foo");
   Value dict(std::move(storage));
 
   *(dict.FindStringKey("string")) = "bar";
 
   Value::DictStorage expected_storage;
-  expected_storage.emplace("string", std::make_unique<Value>("bar"));
+  expected_storage.emplace("string", "bar");
   Value expected_dict(std::move(expected_storage));
 
   EXPECT_EQ(expected_dict, dict);
@@ -876,14 +905,14 @@ TEST(ValuesTest, MutableFindStringKey) {
 
 TEST(ValuesTest, FindDictKey) {
   Value::DictStorage storage;
-  storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
-  storage.emplace("bool", std::make_unique<Value>(Value::Type::BOOLEAN));
-  storage.emplace("int", std::make_unique<Value>(Value::Type::INTEGER));
-  storage.emplace("double", std::make_unique<Value>(Value::Type::DOUBLE));
-  storage.emplace("string", std::make_unique<Value>(Value::Type::STRING));
-  storage.emplace("blob", std::make_unique<Value>(Value::Type::BINARY));
-  storage.emplace("list", std::make_unique<Value>(Value::Type::LIST));
-  storage.emplace("dict", std::make_unique<Value>(Value::Type::DICTIONARY));
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
 
   const Value dict(std::move(storage));
   EXPECT_EQ(nullptr, dict.FindDictKey("null"));
@@ -898,14 +927,14 @@ TEST(ValuesTest, FindDictKey) {
 
 TEST(ValuesTest, FindListKey) {
   Value::DictStorage storage;
-  storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
-  storage.emplace("bool", std::make_unique<Value>(Value::Type::BOOLEAN));
-  storage.emplace("int", std::make_unique<Value>(Value::Type::INTEGER));
-  storage.emplace("double", std::make_unique<Value>(Value::Type::DOUBLE));
-  storage.emplace("string", std::make_unique<Value>(Value::Type::STRING));
-  storage.emplace("blob", std::make_unique<Value>(Value::Type::BINARY));
-  storage.emplace("list", std::make_unique<Value>(Value::Type::LIST));
-  storage.emplace("dict", std::make_unique<Value>(Value::Type::DICTIONARY));
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
 
   const Value dict(std::move(storage));
   EXPECT_EQ(nullptr, dict.FindListKey("null"));
@@ -920,14 +949,14 @@ TEST(ValuesTest, FindListKey) {
 
 TEST(ValuesTest, FindBlobKey) {
   Value::DictStorage storage;
-  storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
-  storage.emplace("bool", std::make_unique<Value>(Value::Type::BOOLEAN));
-  storage.emplace("int", std::make_unique<Value>(Value::Type::INTEGER));
-  storage.emplace("double", std::make_unique<Value>(Value::Type::DOUBLE));
-  storage.emplace("string", std::make_unique<Value>(Value::Type::STRING));
-  storage.emplace("blob", std::make_unique<Value>(Value::Type::BINARY));
-  storage.emplace("list", std::make_unique<Value>(Value::Type::LIST));
-  storage.emplace("dict", std::make_unique<Value>(Value::Type::DICTIONARY));
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
 
   const Value dict(std::move(storage));
   EXPECT_EQ(nullptr, dict.FindBlobKey("null"));
@@ -942,14 +971,14 @@ TEST(ValuesTest, FindBlobKey) {
 
 TEST(ValuesTest, SetKey) {
   Value::DictStorage storage;
-  storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
-  storage.emplace("bool", std::make_unique<Value>(Value::Type::BOOLEAN));
-  storage.emplace("int", std::make_unique<Value>(Value::Type::INTEGER));
-  storage.emplace("double", std::make_unique<Value>(Value::Type::DOUBLE));
-  storage.emplace("string", std::make_unique<Value>(Value::Type::STRING));
-  storage.emplace("blob", std::make_unique<Value>(Value::Type::BINARY));
-  storage.emplace("list", std::make_unique<Value>(Value::Type::LIST));
-  storage.emplace("dict", std::make_unique<Value>(Value::Type::DICTIONARY));
+  storage.emplace("null", Value::Type::NONE);
+  storage.emplace("bool", Value::Type::BOOLEAN);
+  storage.emplace("int", Value::Type::INTEGER);
+  storage.emplace("double", Value::Type::DOUBLE);
+  storage.emplace("string", Value::Type::STRING);
+  storage.emplace("blob", Value::Type::BINARY);
+  storage.emplace("list", Value::Type::LIST);
+  storage.emplace("dict", Value::Type::DICTIONARY);
 
   Value dict(Value::Type::DICTIONARY);
   dict.SetKey(StringPiece("null"), Value(Value::Type::NONE));

chrome/browser/autofill/automated_tests/cache_replayer_unittest.cc

@@ -186,7 +186,6 @@ bool WriteJSON(const base::FilePath& file_path,
   // Make json list node that contains all query requests.
   base::Value::DictStorage urls_dict;
   for (const auto& request_response_pair : request_response_pairs) {
-    Value::DictStorage request_response_node;
     std::string serialized_request;
     std::string url;
     if (!MakeSerializedRequest(request_response_pair.first, request_type,
@@ -194,23 +193,25 @@ bool WriteJSON(const base::FilePath& file_path,
       return false;
     }
 
-    request_response_node["SerializedRequest"] =
-        std::make_unique<Value>(std::move(serialized_request));
-    request_response_node["SerializedResponse"] = std::make_unique<Value>(
+    Value::DictStorage request_response_node;
+    request_response_node.emplace("SerializedRequest",
+                                  std::move(serialized_request));
+    request_response_node.emplace(
+        "SerializedResponse",
         MakeSerializedResponse(request_response_pair.second));
     // Populate json dict node that contains Autofill Server requests per URL.
-    if (urls_dict.find(url) == urls_dict.end())
-      urls_dict[url] = std::make_unique<Value>(Value::ListStorage());
-    urls_dict[url]->Append(Value(std::move(request_response_node)));
+    // This will construct an empty list for `url` if it didn't exist already.
+    auto& url_list = urls_dict.emplace(url, Value::Type::LIST).first->second;
+    url_list.Append(Value(std::move(request_response_node)));
   }
 
   // Make json dict node that contains requests per domain.
   base::Value::DictStorage domains_dict;
-  domains_dict[kHostname] = std::make_unique<Value>(std::move(urls_dict));
+  domains_dict.emplace(kHostname, std::move(urls_dict));
 
   // Make json root dict.
   base::Value::DictStorage root_dict;
-  root_dict["Requests"] = std::make_unique<Value>(std::move(domains_dict));
+  root_dict.emplace("Requests", std::move(domains_dict));
 
   // Write content to JSON file.
   return WriteJSONNode(file_path, Value(std::move(root_dict)));
@@ -292,27 +293,27 @@ TEST_P(
   // Put some textual content for HTTP request. Content does not matter because
   // the Query content will be parsed from the URL that corresponds to the
   // dictionary key.
-  request_response_node["SerializedRequest"] =
-      std::make_unique<Value>(base::StrCat(
-          {"GET ", CreateQueryUrl("1234").c_str(), " HTTP/1.1\r\n\r\n"}));
-  request_response_node["SerializedResponse"] =
-      std::make_unique<Value>(MakeSerializedResponse(AutofillQueryResponse()));
+  request_response_node.emplace(
+      "SerializedRequest", base::StrCat({"GET ", CreateQueryUrl("1234").c_str(),
+                                         " HTTP/1.1\r\n\r\n"}));
+  request_response_node.emplace(
+      "SerializedResponse", MakeSerializedResponse(AutofillQueryResponse()));
+
+  base::Value::ListStorage url_list;
+  url_list.emplace_back(std::move(request_response_node));
+
   // Populate json dict node that contains Autofill Server requests per URL.
   base::Value::DictStorage urls_dict;
   // The query parameter in the URL cannot be parsed to a proto because
   // parameter value is in invalid format.
-  std::string invalid_request_url = CreateQueryUrl(GetParam());
-  urls_dict[invalid_request_url] =
-      std::make_unique<Value>(Value::ListStorage());
-  urls_dict[invalid_request_url]->Append(
-      Value(std::move(request_response_node)));
+  urls_dict.emplace(CreateQueryUrl(GetParam()), std::move(url_list));
 
   // Make json dict node that contains requests per domain.
   base::Value::DictStorage domains_dict;
-  domains_dict[kHostname] = std::make_unique<Value>(std::move(urls_dict));
+  domains_dict.emplace(kHostname, std::move(urls_dict));
   // Make json root dict.
   base::Value::DictStorage root_dict;
-  root_dict["Requests"] = std::make_unique<Value>(std::move(domains_dict));
+  root_dict.emplace("Requests", std::move(domains_dict));
   // Write content to JSON file.
   ASSERT_TRUE(WriteJSONNode(file_path, Value(std::move(root_dict))));
 

chrome/browser/extensions/api/cryptotoken_private/cryptotoken_private_api_unittest.cc

@@ -256,9 +256,9 @@ class CryptoTokenPermissionTest : public ExtensionApiUnittest {
     function->set_has_callback(true);
 
     base::Value::DictStorage dict;
-    dict.emplace("appId", std::make_unique<base::Value>(app_id));
-    dict.emplace("tabId", std::make_unique<base::Value>(tab_id_));
-    dict.emplace("origin", std::make_unique<base::Value>(app_id));
+    dict.emplace("appId", app_id);
+    dict.emplace("tabId", tab_id_);
+    dict.emplace("origin", app_id);
     auto args = std::make_unique<base::Value>(base::Value::Type::LIST);
     args->Append(base::Value(std::move(dict)));
     auto args_list = base::ListValue::From(std::move(args));

components/autofill/core/common/logging/log_buffer.cc

@@ -76,7 +76,7 @@ bool TryCoalesceString(std::vector<base::Value>* buffer,
 
 base::Value CreateEmptyFragment() {
   base::Value::DictStorage storage;
-  storage.try_emplace("type", std::make_unique<base::Value>("fragment"));
+  storage.try_emplace("type", "fragment");
   return base::Value(storage);
 }
 
@@ -115,9 +115,8 @@ LogBuffer& operator<<(LogBuffer& buf, Tag&& tag) {
     return buf;
 
   base::Value::DictStorage storage;
-  storage.try_emplace("type", std::make_unique<base::Value>("element"));
-  storage.try_emplace("value",
-                      std::make_unique<base::Value>(std::move(tag.name)));
+  storage.try_emplace("type", "element");
+  storage.try_emplace("value", std::move(tag.name));
   buf.buffer_.emplace_back(std::move(storage));
   return buf;
 }
@@ -148,8 +147,7 @@ LogBuffer& operator<<(LogBuffer& buf, Attrib&& attrib) {
                        base::Value(std::move(attrib.value)));
   } else {
     base::Value::DictStorage dict;
-    dict.try_emplace(std::move(attrib.name),
-                     std::make_unique<base::Value>(std::move(attrib.value)));
+    dict.try_emplace(std::move(attrib.name), std::move(attrib.value));
     node.SetKey("attributes", base::Value(std::move(dict)));
   }
 
@@ -173,10 +171,10 @@ LogBuffer& operator<<(LogBuffer& buf, base::StringPiece text) {
     return buf;
 
   base::Value::DictStorage storage;
-  storage.try_emplace("type", std::make_unique<base::Value>("text"));
+  storage.try_emplace("type", "text");
   // This text is not HTML escaped because the rest of the frame work takes care
   // of that and it must not be escaped twice.
-  storage.try_emplace("value", std::make_unique<base::Value>(text));
+  storage.try_emplace("value", text);
   base::Value node_to_add(std::move(storage));
   AppendChildToLastNode(&buf.buffer_, std::move(node_to_add));
   return buf;

ipc/ipc_message_utils.cc

@@ -167,7 +167,7 @@ bool ReadDictionaryValue(const base::Pickle* m,
   if (!ReadParam(m, iter, &size))
     return false;
 
-  std::vector<std::pair<std::string, std::unique_ptr<base::Value>>> entries;
+  std::vector<base::Value::LegacyDictStorage::value_type> entries;
   entries.resize(size);
   for (auto& entry : entries) {
     entry.second = std::make_unique<base::Value>();
@@ -176,7 +176,8 @@ bool ReadDictionaryValue(const base::Pickle* m,
       return false;
   }
 
-  *value = base::DictionaryValue(base::Value::DictStorage(std::move(entries)));
+  *value =
+      base::DictionaryValue(base::Value::LegacyDictStorage(std::move(entries)));
   return true;
 }
 

mojo/public/cpp/base/values_mojom_traits.cc

@@ -20,8 +20,8 @@ bool StructTraits<mojo_base::mojom::DictionaryValueDataView, base::Value>::Read(
   dict_storage.reserve(view.size());
   for (size_t i = 0; i < view.size(); ++i) {
     base::StringPiece key;
-    auto value = std::make_unique<base::Value>();
-    if (!view.keys().Read(i, &key) || !view.values().Read(i, value.get()))
+    base::Value value;
+    if (!view.keys().Read(i, &key) || !view.values().Read(i, &value))
       return false;
     dict_storage.emplace_back(key.as_string(), std::move(value));
   }

mojo/public/cpp/base/values_unittest.cc

@@ -85,17 +85,14 @@ TEST(ValuesStructTraitsTest, DictionaryValue) {
   // move-only types and initializer lists don't mix. Initializer lists can't be
   // modified: thus it's not possible to move.
   std::vector<base::Value::DictStorage::value_type> storage;
-  storage.emplace_back("null", std::make_unique<base::Value>());
-  storage.emplace_back("bool", std::make_unique<base::Value>(false));
-  storage.emplace_back("int", std::make_unique<base::Value>(0));
-  storage.emplace_back("double", std::make_unique<base::Value>(0.0));
-  storage.emplace_back("string", std::make_unique<base::Value>("0"));
-  storage.emplace_back(
-      "binary", std::make_unique<base::Value>(base::Value::BlobStorage({0})));
-  storage.emplace_back(
-      "dictionary", std::make_unique<base::Value>(base::Value::DictStorage()));
-  storage.emplace_back(
-      "list", std::make_unique<base::Value>(base::Value::ListStorage()));
+  storage.emplace_back("null", base::Value());
+  storage.emplace_back("bool", false);
+  storage.emplace_back("int", 0);
+  storage.emplace_back("double", 0.0);
+  storage.emplace_back("string", "0");
+  storage.emplace_back("binary", base::Value::BlobStorage({0}));
+  storage.emplace_back("dictionary", base::Value::DictStorage());
+  storage.emplace_back("list", base::Value::ListStorage());
 
   base::Value in(base::Value::DictStorage(std::move(storage)));
   base::Value out;

ui/base/test/skia_gold_pixel_diff.cc

@@ -84,9 +84,8 @@ void FillInSystemEnvironment(base::Value::DictStorage& ds) {
   LOG(WARNING) << "Unknown Processor.";
 #endif
 
-  ds["system"] =
-      std::make_unique<base::Value>(SkiaGoldPixelDiff::GetPlatform());
-  ds["processor"] = std::make_unique<base::Value>(processor);
+  ds["system"] = base::Value(SkiaGoldPixelDiff::GetPlatform());
+  ds["processor"] = base::Value(processor);
 }
 
 // Returns whether image comparison failure should result in Gerrit comments.