Use std::unique_ptr to transfer base::Value ownership in extensions::ValueStoreChange

extensions/browser/value_store/value_store_unittest.cc

 // Copyright 2014 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.
 
 #include "extensions/browser/value_store/value_store_unittest.h"
 
+#include <utility>
+
 #include "base/json/json_writer.h"
 #include "base/memory/linked_ptr.h"
 #include "base/values.h"
 
 using content::BrowserThread;
 
 namespace {
 
 // To save typing ValueStore::DEFAULTS everywhere.
 const ValueStore::WriteOptions DEFAULTS = ValueStore::DEFAULTS;
@@ skipping 132 matching lines @@
   list123_.push_back(key2_);
   list123_.push_back(key3_);
 
   set1_.insert(list1_.begin(), list1_.end());
   set2_.insert(list2_.begin(), list2_.end());
   set3_.insert(list3_.begin(), list3_.end());
   set12_.insert(list12_.begin(), list12_.end());
   set13_.insert(list13_.begin(), list13_.end());
   set123_.insert(list123_.begin(), list123_.end());
 
-  dict1_->Set(key1_, val1_->DeepCopy());
-  dict3_->Set(key3_, val3_->DeepCopy());
-  dict12_->Set(key1_, val1_->DeepCopy());
-  dict12_->Set(key2_, val2_->DeepCopy());
-  dict123_->Set(key1_, val1_->DeepCopy());
-  dict123_->Set(key2_, val2_->DeepCopy());
-  dict123_->Set(key3_, val3_->DeepCopy());
+  dict1_->Set(key1_, val1_->CreateDeepCopy());
+  dict3_->Set(key3_, val3_->CreateDeepCopy());
+  dict12_->Set(key1_, val1_->CreateDeepCopy());
+  dict12_->Set(key2_, val2_->CreateDeepCopy());
+  dict123_->Set(key1_, val1_->CreateDeepCopy());
+  dict123_->Set(key2_, val2_->CreateDeepCopy());
+  dict123_->Set(key3_, val3_->CreateDeepCopy());
 }
 
 ValueStoreTest::~ValueStoreTest() {}
 
 void ValueStoreTest::SetUp() {
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
   storage_.reset((GetParam())(temp_dir_.path().AppendASCII("dbName")));
   ASSERT_TRUE(storage_.get());
 }
 
 void ValueStoreTest::TearDown() {
   storage_.reset();
 }
 
 TEST_P(ValueStoreTest, GetWhenEmpty) {
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(empty_list_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list123_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
 TEST_P(ValueStoreTest, GetWithSingleValue) {
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key1_, NULL, val1_->DeepCopy()));
+    changes.push_back(
+        ValueStoreChange(key1_, nullptr, val1_->CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq,
         changes, storage_->Set(DEFAULTS, key1_, *val1_));
   }
 
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key2_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key3_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(empty_list_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get(list123_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get());
 }
 
 TEST_P(ValueStoreTest, GetWithMultipleValues) {
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key1_, NULL, val1_->DeepCopy()));
-    changes.push_back(ValueStoreChange(key2_, NULL, val2_->DeepCopy()));
+    changes.push_back(
+        ValueStoreChange(key1_, nullptr, val1_->CreateDeepCopy()));
+    changes.push_back(
+        ValueStoreChange(key2_, nullptr, val2_->CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Set(DEFAULTS, *dict12_));
   }
 
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key3_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(empty_list_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict12_, storage_->Get(list123_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict12_, storage_->Get());
 }
 
 TEST_P(ValueStoreTest, RemoveWhenEmpty) {
   EXPECT_PRED_FORMAT2(ChangesEq, ValueStoreChangeList(),
                       storage_->Remove(key1_));
 
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
 TEST_P(ValueStoreTest, RemoveWithSingleValue) {
   storage_->Set(DEFAULTS, *dict1_);
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key1_, val1_->DeepCopy(), NULL));
+    changes.push_back(
+        ValueStoreChange(key1_, val1_->CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Remove(key1_));
   }
 
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key2_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list12_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
 TEST_P(ValueStoreTest, RemoveWithMultipleValues) {
   storage_->Set(DEFAULTS, *dict123_);
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key3_, val3_->DeepCopy(), NULL));
+    changes.push_back(
+        ValueStoreChange(key3_, val3_->CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Remove(key3_));
   }
 
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key3_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(empty_list_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get(list1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict12_, storage_->Get(list12_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get(list13_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict12_, storage_->Get(list123_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict12_, storage_->Get());
 
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key1_, val1_->DeepCopy(), NULL));
-    changes.push_back(ValueStoreChange(key2_, val2_->DeepCopy(), NULL));
+    changes.push_back(
+        ValueStoreChange(key1_, val1_->CreateDeepCopy(), nullptr));
+    changes.push_back(
+        ValueStoreChange(key2_, val2_->CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Remove(list12_));
   }
 
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key3_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(empty_list_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list12_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list13_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list123_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
 TEST_P(ValueStoreTest, SetWhenOverwriting) {
   storage_->Set(DEFAULTS, key1_, *val2_);
   {
     ValueStoreChangeList changes;
+    changes.push_back(ValueStoreChange(key1_, val2_->CreateDeepCopy(),
+                                       val1_->CreateDeepCopy()));
     changes.push_back(
-        ValueStoreChange(key1_, val2_->DeepCopy(), val1_->DeepCopy()));
-    changes.push_back(ValueStoreChange(key2_, NULL, val2_->DeepCopy()));
+        ValueStoreChange(key2_, nullptr, val2_->CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Set(DEFAULTS, *dict12_));
   }
 
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key3_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(empty_list_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get(list1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict12_, storage_->Get(list12_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get(list13_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict12_, storage_->Get(list123_));
   EXPECT_PRED_FORMAT2(SettingsEq, *dict12_, storage_->Get());
 }
 
 TEST_P(ValueStoreTest, ClearWhenEmpty) {
   EXPECT_PRED_FORMAT2(ChangesEq, ValueStoreChangeList(), storage_->Clear());
 
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(empty_list_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list123_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
 TEST_P(ValueStoreTest, ClearWhenNotEmpty) {
   storage_->Set(DEFAULTS, *dict12_);
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key1_, val1_->DeepCopy(), NULL));
-    changes.push_back(ValueStoreChange(key2_, val2_->DeepCopy(), NULL));
+    changes.push_back(
+        ValueStoreChange(key1_, val1_->CreateDeepCopy(), nullptr));
+    changes.push_back(
+        ValueStoreChange(key2_, val2_->CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Clear());
   }
 
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(empty_list_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list123_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
 // Dots should be allowed in key names; they shouldn't be interpreted as
 // indexing into a dictionary.
 TEST_P(ValueStoreTest, DotsInKeyNames) {
   std::string dot_key("foo.bar");
   base::StringValue dot_value("baz.qux");
   std::vector<std::string> dot_list;
   dot_list.push_back(dot_key);
   base::DictionaryValue dot_dict;
-  dot_dict.SetWithoutPathExpansion(dot_key, dot_value.DeepCopy());
+  dot_dict.SetWithoutPathExpansion(dot_key, dot_value.CreateDeepCopy());
 
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(dot_key));
 
   {
     ValueStoreChangeList changes;
     changes.push_back(
-        ValueStoreChange(dot_key, NULL, dot_value.DeepCopy()));
+        ValueStoreChange(dot_key, nullptr, dot_value.CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq,
         changes, storage_->Set(DEFAULTS, dot_key, dot_value));
   }
   EXPECT_PRED_FORMAT2(ChangesEq,
       ValueStoreChangeList(), storage_->Set(DEFAULTS, dot_key, dot_value));
 
   EXPECT_PRED_FORMAT2(SettingsEq, dot_dict, storage_->Get(dot_key));
 
   {
     ValueStoreChangeList changes;
     changes.push_back(
-        ValueStoreChange(dot_key, dot_value.DeepCopy(), NULL));
+        ValueStoreChange(dot_key, dot_value.CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Remove(dot_key));
   }
   EXPECT_PRED_FORMAT2(ChangesEq,
       ValueStoreChangeList(), storage_->Remove(dot_key));
   {
     ValueStoreChangeList changes;
     changes.push_back(
-        ValueStoreChange(dot_key, NULL, dot_value.DeepCopy()));
+        ValueStoreChange(dot_key, nullptr, dot_value.CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Set(DEFAULTS, dot_dict));
   }
 
   EXPECT_PRED_FORMAT2(SettingsEq, dot_dict, storage_->Get(dot_list));
   EXPECT_PRED_FORMAT2(SettingsEq, dot_dict, storage_->Get());
 
   {
     ValueStoreChangeList changes;
     changes.push_back(
-        ValueStoreChange(dot_key, dot_value.DeepCopy(), NULL));
+        ValueStoreChange(dot_key, dot_value.CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Remove(dot_list));
   }
 
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(dot_key));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
 TEST_P(ValueStoreTest, DotsInKeyNamesWithDicts) {
   base::DictionaryValue outer_dict;
-  base::DictionaryValue* inner_dict = new base::DictionaryValue();
-  outer_dict.Set("foo", inner_dict);
+  std::unique_ptr<base::DictionaryValue> inner_dict(
+      new base::DictionaryValue());
+  outer_dict.Set("foo", std::move(inner_dict));
   inner_dict->SetString("bar", "baz");

[Devlin, 2016/05/03 16:55:05]

UAF (was already a conceptual problem, but now this will crash).

[dcheng, 2016/05/03 17:16:26]

Done. This was "safe" before but is now unsafe.

 
   {
     ValueStoreChangeList changes;
     changes.push_back(
-        ValueStoreChange("foo", NULL, inner_dict->DeepCopy()));
+        ValueStoreChange("foo", nullptr, inner_dict->CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq,
         changes, storage_->Set(DEFAULTS, outer_dict));
   }
 
   EXPECT_PRED_FORMAT2(SettingsEq, outer_dict, storage_->Get("foo"));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get("foo.bar"));
 }
 
 TEST_P(ValueStoreTest, ComplexChangedKeysScenarios) {
   // Test:
   //   - Setting over missing/changed/same keys, combinations.
   //   - Removing over missing and present keys, combinations.
   //   - Clearing.
   std::vector<std::string> complex_list;
   base::DictionaryValue complex_changed_dict;
 
   storage_->Set(DEFAULTS, key1_, *val1_);
   EXPECT_PRED_FORMAT2(ChangesEq,
       ValueStoreChangeList(), storage_->Set(DEFAULTS, key1_, *val1_));
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(
-        key1_, val1_->DeepCopy(), val2_->DeepCopy()));
+    changes.push_back(ValueStoreChange(key1_, val1_->CreateDeepCopy(),
+                                       val2_->CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq,
         changes, storage_->Set(DEFAULTS, key1_, *val2_));
   }
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key1_, val2_->DeepCopy(), NULL));
+    changes.push_back(
+        ValueStoreChange(key1_, val2_->CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Remove(key1_));
     EXPECT_PRED_FORMAT2(ChangesEq,
         ValueStoreChangeList(), storage_->Remove(key1_));
   }
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key1_, NULL, val1_->DeepCopy()));
+    changes.push_back(
+        ValueStoreChange(key1_, nullptr, val1_->CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq,
         changes, storage_->Set(DEFAULTS, key1_, *val1_));
   }
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key1_, val1_->DeepCopy(), NULL));
+    changes.push_back(
+        ValueStoreChange(key1_, val1_->CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Clear());
     EXPECT_PRED_FORMAT2(ChangesEq, ValueStoreChangeList(), storage_->Clear());
   }
 
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key1_, NULL, val1_->DeepCopy()));
-    changes.push_back(ValueStoreChange(key2_, NULL, val2_->DeepCopy()));
+    changes.push_back(
+        ValueStoreChange(key1_, nullptr, val1_->CreateDeepCopy()));
+    changes.push_back(
+        ValueStoreChange(key2_, nullptr, val2_->CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Set(DEFAULTS, *dict12_));
     EXPECT_PRED_FORMAT2(ChangesEq,
         ValueStoreChangeList(), storage_->Set(DEFAULTS, *dict12_));
   }
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key3_, NULL, val3_->DeepCopy()));
+    changes.push_back(
+        ValueStoreChange(key3_, nullptr, val3_->CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Set(DEFAULTS, *dict123_));
   }
   {
     base::DictionaryValue to_set;
-    to_set.Set(key1_, val2_->DeepCopy());
-    to_set.Set(key2_, val2_->DeepCopy());
-    to_set.Set("asdf", val1_->DeepCopy());
-    to_set.Set("qwerty", val3_->DeepCopy());
+    to_set.Set(key1_, val2_->CreateDeepCopy());
+    to_set.Set(key2_, val2_->CreateDeepCopy());
+    to_set.Set("asdf", val1_->CreateDeepCopy());
+    to_set.Set("qwerty", val3_->CreateDeepCopy());
 
     ValueStoreChangeList changes;
+    changes.push_back(ValueStoreChange(key1_, val1_->CreateDeepCopy(),
+                                       val2_->CreateDeepCopy()));
     changes.push_back(
-        ValueStoreChange(key1_, val1_->DeepCopy(), val2_->DeepCopy()));
-    changes.push_back(ValueStoreChange("asdf", NULL, val1_->DeepCopy()));
+        ValueStoreChange("asdf", nullptr, val1_->CreateDeepCopy()));
     changes.push_back(
-        ValueStoreChange("qwerty", NULL, val3_->DeepCopy()));
+        ValueStoreChange("qwerty", nullptr, val3_->CreateDeepCopy()));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Set(DEFAULTS, to_set));
   }
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key1_, val2_->DeepCopy(), NULL));
-    changes.push_back(ValueStoreChange(key2_, val2_->DeepCopy(), NULL));
+    changes.push_back(
+        ValueStoreChange(key1_, val2_->CreateDeepCopy(), nullptr));
+    changes.push_back(
+        ValueStoreChange(key2_, val2_->CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Remove(list12_));
   }
   {
     std::vector<std::string> to_remove;
     to_remove.push_back(key1_);
     to_remove.push_back("asdf");
 
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange("asdf", val1_->DeepCopy(), NULL));
+    changes.push_back(
+        ValueStoreChange("asdf", val1_->CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Remove(to_remove));
   }
   {
     ValueStoreChangeList changes;
-    changes.push_back(ValueStoreChange(key3_, val3_->DeepCopy(), NULL));
     changes.push_back(
-        ValueStoreChange("qwerty", val3_->DeepCopy(), NULL));
+        ValueStoreChange(key3_, val3_->CreateDeepCopy(), nullptr));
+    changes.push_back(
+        ValueStoreChange("qwerty", val3_->CreateDeepCopy(), nullptr));
     EXPECT_PRED_FORMAT2(ChangesEq, changes, storage_->Clear());
     EXPECT_PRED_FORMAT2(ChangesEq, ValueStoreChangeList(), storage_->Clear());
   }
 }