More base::Values-related bare pointer -> scoped_ptr conversions

base/values_unittest.cc

 // Copyright (c) 2012 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 <limits>
 
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
@@ skipping 300 matching lines @@
     EXPECT_FALSE(deletion_flag);
     EXPECT_TRUE(dict.HasKey(key));
     EXPECT_TRUE(dict.Remove(key, NULL));
     EXPECT_TRUE(deletion_flag);
     EXPECT_FALSE(dict.HasKey(key));
   }
 }
 
 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()));
+  dict.Set("this.is.expanded", Value::CreateNullValue());
+  dict.SetWithoutPathExpansion("this.isnt.expanded", 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());
 
@@ skipping 49 matching lines @@
 
   removed_item.reset();
   EXPECT_TRUE(dict.RemovePath("a.long.key.path", &removed_item));
   ASSERT_TRUE(removed_item);
   EXPECT_TRUE(removed_item->IsType(base::Value::TYPE_BOOLEAN));
   EXPECT_TRUE(dict.empty());
 }
 
 TEST(ValuesTest, DeepCopy) {
   DictionaryValue original_dict;
-  Value* original_null = Value::CreateNullValue();
+  Value* original_null = Value::CreateNullValue().release();

[danakj, 2015/05/08 18:07:26]

can you make original_null a scoped_ptr?

[Evan Stade, 2015/05/08 18:32:09]

not really --- it's referenced below; this test needs to maintain a weak ref.

[danakj, 2015/05/08 18:33:42]

Can you put the weak ref in a separate variable so we don't express ownership
with a raw pointer?

[Evan Stade, 2015/05/08 18:38:27]

I don't think expressing ownership with a raw pointer is that bad if

a) it's a test file
b) ownership is handed off on literally the next line of code

Notice how every other function in this test uses make_scoped_ptr for the same
reason.

[danakj, 2015/05/08 18:48:28]

This test was written before scoped_ptr existed from what I can see in blame.

[Evan Stade, 2015/05/08 18:54:12]

All the lines with "make_scoped_ptr" were not written prior to the introduction
of scoped_ptr.

   original_dict.Set("null", make_scoped_ptr(original_null));
   FundamentalValue* original_bool = new FundamentalValue(true);
   original_dict.Set("bool", make_scoped_ptr(original_bool));
   FundamentalValue* original_int = new FundamentalValue(42);
   original_dict.Set("int", make_scoped_ptr(original_int));
   FundamentalValue* original_double = new FundamentalValue(3.14);
   original_dict.Set("double", make_scoped_ptr(original_double));
   StringValue* original_string = new StringValue("hello");
   original_dict.Set("string", make_scoped_ptr(original_string));
   StringValue* original_string16 = new StringValue(ASCIIToUTF16("hello16"));
@@ skipping 118 matching lines @@
   ASSERT_TRUE(copy_value);
   ASSERT_NE(copy_value, original_nested_dictionary);
   ASSERT_TRUE(copy_value->IsType(Value::TYPE_DICTIONARY));
   DictionaryValue* copy_nested_dictionary = NULL;
   ASSERT_TRUE(copy_value->GetAsDictionary(&copy_nested_dictionary));
   ASSERT_TRUE(copy_nested_dictionary);
   EXPECT_TRUE(copy_nested_dictionary->HasKey("key"));
 }
 
 TEST(ValuesTest, Equals) {
-  Value* null1 = Value::CreateNullValue();
-  Value* null2 = Value::CreateNullValue();
-  EXPECT_NE(null1, null2);
-  EXPECT_TRUE(null1->Equals(null2));
+  scoped_ptr<Value> null1(Value::CreateNullValue());
+  scoped_ptr<Value> null2(Value::CreateNullValue());
+  EXPECT_NE(null1.get(), null2.get());
+  EXPECT_TRUE(null1->Equals(null2.get()));
 
-  Value* boolean = new FundamentalValue(false);
-  EXPECT_FALSE(null1->Equals(boolean));
-  delete null1;
-  delete null2;
-  delete boolean;
+  FundamentalValue boolean(false);
+  EXPECT_FALSE(null1->Equals(&boolean));
 
   DictionaryValue dv;
   dv.SetBoolean("a", false);
   dv.SetInteger("b", 2);
   dv.SetDouble("c", 2.5);
   dv.SetString("d1", "string");
   dv.SetString("d2", ASCIIToUTF16("http://google.com"));
-  dv.Set("e", make_scoped_ptr(Value::CreateNullValue()));
+  dv.Set("e", Value::CreateNullValue());
 
   scoped_ptr<DictionaryValue> copy = dv.CreateDeepCopy();
   EXPECT_TRUE(dv.Equals(copy.get()));
 
   ListValue* list = new ListValue;
   list->Append(Value::CreateNullValue());
   list->Append(new DictionaryValue);
   dv.Set("f", make_scoped_ptr(list));
 
   EXPECT_FALSE(dv.Equals(copy.get()));
@@ skipping 29 matching lines @@
 
   // NULL and Value::CreateNullValue() are intentionally different: We need
   // support for NULL as a return value for "undefined" without caring for
   // ownership of the pointer.
   EXPECT_FALSE(Value::Equals(null1.get(), NULL));
   EXPECT_FALSE(Value::Equals(NULL, null1.get()));
 }
 
 TEST(ValuesTest, DeepCopyCovariantReturnTypes) {
   DictionaryValue original_dict;
-  Value* original_null = Value::CreateNullValue();
+  Value* original_null = Value::CreateNullValue().release();

[danakj, 2015/05/08 18:07:26]

can you make original_null a scoped_ptr too?

[Evan Stade, 2015/05/08 18:32:09]

Done.

   original_dict.Set("null", make_scoped_ptr(original_null));
   FundamentalValue* original_bool = new FundamentalValue(true);
   original_dict.Set("bool", make_scoped_ptr(original_bool));
   FundamentalValue* original_int = new FundamentalValue(42);
   original_dict.Set("int", make_scoped_ptr(original_int));
   FundamentalValue* original_double = new FundamentalValue(3.14);
   original_dict.Set("double", make_scoped_ptr(original_double));
   StringValue* original_string = new StringValue("hello");
   original_dict.Set("string", make_scoped_ptr(original_string));
   StringValue* original_string16 = new StringValue(ASCIIToUTF16("hello16"));
@@ skipping 510 matching lines @@
   EXPECT_FALSE(main_list.GetList(1, NULL));
   EXPECT_FALSE(main_list.GetList(2, NULL));
   EXPECT_FALSE(main_list.GetList(3, NULL));
   EXPECT_FALSE(main_list.GetList(4, NULL));
   EXPECT_FALSE(main_list.GetList(5, NULL));
   EXPECT_TRUE(main_list.GetList(6, NULL));
   EXPECT_FALSE(main_list.GetList(7, NULL));
 }
 
 }  // namespace base