Move Storage for ListValue and DictValue in Union

base/values_unittest.cc

Index: base/values_unittest.cc
diff --git a/base/values_unittest.cc b/base/values_unittest.cc
index e62b6e579b134f5bd92291b1b796618c689f239a..37759d0bed3e335b011f67867e9d0df641d64ded 100644
--- a/base/values_unittest.cc
+++ b/base/values_unittest.cc
@@ -89,6 +89,16 @@ TEST(ValuesTest, ConstructBinary) {
   EXPECT_EQ(std::vector<char>({0xF, 0x0, 0x0, 0xB, 0xA, 0x2}), value.GetBlob());
 }
 
+TEST(ValuesTest, ConstructDict) {
+  DictionaryValue value;
+  EXPECT_EQ(Value::Type::DICTIONARY, value.type());
+}
+
+TEST(ValuesTest, ConstructList) {
+  ListValue value;
+  EXPECT_EQ(Value::Type::LIST, value.type());
+}
+
 // Group of tests for the copy constructors and copy-assigmnent. For equality
 // checks comparisons of the interesting fields are done instead of relying on
 // Equals being correct.
@@ -166,6 +176,50 @@ TEST(ValuesTest, CopyBinary) {
   EXPECT_EQ(value.GetBlob(), blank.GetBlob());
 }
 
+TEST(ValuesTest, CopyDictionary) {
+  // TODO(crbug.com/646113): Clean this up once DictionaryValue switched to
+  // value semantics.
+  int copy;
+  DictionaryValue value;
+  value.SetInteger("Int", 123);
+
+  DictionaryValue copied_value(value);
+  copied_value.GetInteger("Int", &copy);
+
+  EXPECT_EQ(value.type(), copied_value.type());
+  EXPECT_EQ(123, copy);
+
+  auto blank = MakeUnique<Value>();
+
+  *blank = value;
+  EXPECT_EQ(Value::Type::DICTIONARY, blank->type());
+
+  static_cast<DictionaryValue*>(blank.get())->GetInteger("Int", &copy);
+  EXPECT_EQ(123, copy);
+}
+
+TEST(ValuesTest, CopyList) {
+  // TODO(crbug.com/646113): Clean this up once ListValue switched to
+  // value semantics.
+  int copy;
+  ListValue value;
+  value.AppendInteger(123);
+
+  ListValue copied_value(value);
+  copied_value.GetInteger(0, &copy);
+
+  EXPECT_EQ(value.type(), copied_value.type());
+  EXPECT_EQ(123, copy);
+
+  auto blank = MakeUnique<Value>();
+
+  *blank = value;
+  EXPECT_EQ(Value::Type::LIST, blank->type());
+
+  static_cast<ListValue*>(blank.get())->GetInteger(0, &copy);
+  EXPECT_EQ(123, copy);
+}
+
 // Group of tests for the move constructors and move-assigmnent.
 TEST(ValuesTest, MoveBool) {
   FundamentalValue true_value(true);
@@ -242,6 +296,44 @@ TEST(ValuesTest, MoveBinary) {
   EXPECT_EQ(buffer, blank.GetBlob());
 }
 
+TEST(ValuesTest, MoveDictionary) {
+  // TODO(crbug.com/646113): Clean this up once DictionaryValue switched to
+  // value semantics.
+  int move;
+  DictionaryValue value;
+  value.SetInteger("Int", 123);
+
+  DictionaryValue moved_value(std::move(value));
+  moved_value.GetInteger("Int", &move);
+
+  EXPECT_EQ(Value::Type::DICTIONARY, moved_value.type());
+  EXPECT_EQ(123, move);
+
+  Value blank;
+
+  blank = DictionaryValue();
+  EXPECT_EQ(Value::Type::DICTIONARY, blank.type());
+}
+
+TEST(ValuesTest, MoveList) {
+  // TODO(crbug.com/646113): Clean this up once ListValue switched to
+  // value semantics.
+  int move;
+  ListValue value;
+  value.AppendInteger(123);
+
+  ListValue moved_value(std::move(value));
+  moved_value.GetInteger(0, &move);
+
+  EXPECT_EQ(Value::Type::LIST, moved_value.type());
+  EXPECT_EQ(123, move);
+
+  Value blank;
+
+  blank = ListValue();
+  EXPECT_EQ(Value::Type::LIST, blank.type());
+}
+
 TEST(ValuesTest, Basic) {
   // Test basic dictionary getting/setting
   DictionaryValue settings;
@@ -399,61 +491,20 @@ TEST(ValuesTest, StringValue) {
                   static_cast<const StringValue**>(NULL)));
 }
 
-// This is a Value object that allows us to tell if it's been
-// properly deleted by modifying the value of external flag on destruction.
-class DeletionTestValue : public Value {
- public:
-  explicit DeletionTestValue(bool* deletion_flag) : Value(Type::NONE) {
-    Init(deletion_flag);  // Separate function so that we can use ASSERT_*
-  }
-
-  void Init(bool* deletion_flag) {
-    ASSERT_TRUE(deletion_flag);
-    deletion_flag_ = deletion_flag;
-    *deletion_flag_ = false;
-  }
-
-  ~DeletionTestValue() override { *deletion_flag_ = true; }
-
- private:
-  bool* deletion_flag_;
-};
-
 TEST(ValuesTest, ListDeletion) {
-  bool deletion_flag = true;
-
-  {
-    ListValue list;
-    list.Append(MakeUnique<DeletionTestValue>(&deletion_flag));
-    EXPECT_FALSE(deletion_flag);
-  }
-  EXPECT_TRUE(deletion_flag);
-
-  {
-    ListValue list;
-    list.Append(MakeUnique<DeletionTestValue>(&deletion_flag));
-    EXPECT_FALSE(deletion_flag);
-    list.Clear();
-    EXPECT_TRUE(deletion_flag);
-  }
-
-  {
-    ListValue list;
-    list.Append(MakeUnique<DeletionTestValue>(&deletion_flag));
-    EXPECT_FALSE(deletion_flag);
-    EXPECT_TRUE(list.Set(0, Value::CreateNullValue()));
-    EXPECT_TRUE(deletion_flag);
-  }
+  ListValue list;
+  list.Append(MakeUnique<Value>());
+  EXPECT_FALSE(list.empty());
+  list.Clear();
+  EXPECT_TRUE(list.empty());
 }
 
 TEST(ValuesTest, ListRemoval) {
-  bool deletion_flag = true;
   std::unique_ptr<Value> removed_item;
 
   {
     ListValue list;
-    list.Append(MakeUnique<DeletionTestValue>(&deletion_flag));
-    EXPECT_FALSE(deletion_flag);
+    list.Append(MakeUnique<Value>());
     EXPECT_EQ(1U, list.GetSize());
     EXPECT_FALSE(list.Remove(std::numeric_limits<size_t>::max(),
                              &removed_item));
@@ -462,88 +513,55 @@ TEST(ValuesTest, ListRemoval) {
     ASSERT_TRUE(removed_item);
     EXPECT_EQ(0U, list.GetSize());
   }
-  EXPECT_FALSE(deletion_flag);
   removed_item.reset();
-  EXPECT_TRUE(deletion_flag);
 
   {
     ListValue list;
-    list.Append(MakeUnique<DeletionTestValue>(&deletion_flag));
-    EXPECT_FALSE(deletion_flag);
+    list.Append(MakeUnique<Value>());
     EXPECT_TRUE(list.Remove(0, NULL));
-    EXPECT_TRUE(deletion_flag);
     EXPECT_EQ(0U, list.GetSize());
   }
 
   {
     ListValue list;
-    std::unique_ptr<DeletionTestValue> value(
-        new DeletionTestValue(&deletion_flag));
-    DeletionTestValue* original_value = value.get();
+    auto value = MakeUnique<Value>();
+    Value* original_value = value.get();
     list.Append(std::move(value));
-    EXPECT_FALSE(deletion_flag);
     size_t index = 0;
     list.Remove(*original_value, &index);
     EXPECT_EQ(0U, index);
-    EXPECT_TRUE(deletion_flag);
     EXPECT_EQ(0U, list.GetSize());
   }
 }
 
 TEST(ValuesTest, DictionaryDeletion) {
   std::string key = "test";
-  bool deletion_flag = true;
-
-  {
-    DictionaryValue dict;
-    dict.Set(key, MakeUnique<DeletionTestValue>(&deletion_flag));
-    EXPECT_FALSE(deletion_flag);
-  }
-  EXPECT_TRUE(deletion_flag);
-
-  {
-    DictionaryValue dict;
-    dict.Set(key, MakeUnique<DeletionTestValue>(&deletion_flag));
-    EXPECT_FALSE(deletion_flag);
-    dict.Clear();
-    EXPECT_TRUE(deletion_flag);
-  }
-
-  {
-    DictionaryValue dict;
-    dict.Set(key, MakeUnique<DeletionTestValue>(&deletion_flag));
-    EXPECT_FALSE(deletion_flag);
-    dict.Set(key, Value::CreateNullValue());
-    EXPECT_TRUE(deletion_flag);
-  }
+  DictionaryValue dict;
+  dict.Set(key, MakeUnique<Value>());
+  EXPECT_FALSE(dict.empty());
+  dict.Clear();
+  EXPECT_TRUE(dict.empty());
 }
 
 TEST(ValuesTest, DictionaryRemoval) {
   std::string key = "test";
-  bool deletion_flag = true;
   std::unique_ptr<Value> removed_item;
 
   {
     DictionaryValue dict;
-    dict.Set(key, MakeUnique<DeletionTestValue>(&deletion_flag));
-    EXPECT_FALSE(deletion_flag);
+    dict.Set(key, MakeUnique<Value>());
     EXPECT_TRUE(dict.HasKey(key));
     EXPECT_FALSE(dict.Remove("absent key", &removed_item));
     EXPECT_TRUE(dict.Remove(key, &removed_item));
     EXPECT_FALSE(dict.HasKey(key));
     ASSERT_TRUE(removed_item);
   }
-  EXPECT_FALSE(deletion_flag);
-  removed_item.reset();
-  EXPECT_TRUE(deletion_flag);
 
   {
     DictionaryValue dict;
-    dict.Set(key, MakeUnique<DeletionTestValue>(&deletion_flag));
-    EXPECT_FALSE(deletion_flag);
+    dict.Set(key, MakeUnique<Value>());
     EXPECT_TRUE(dict.HasKey(key));
     EXPECT_TRUE(dict.Remove(key, NULL));
-    EXPECT_TRUE(deletion_flag);
     EXPECT_FALSE(dict.HasKey(key));
   }
 }