chromeos/dbus/shill: Elim DictionaryValue from unit tests

Bug: 1137487
Change-Id: I42be555e181abb32053492f130fa735dec45d705
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2468342
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Azeem Arshad <azeemarshad@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818626}

chromeos/dbus/shill/shill_client_unittest_base.cc

@@ -30,22 +30,22 @@ namespace chromeos {
 namespace {
 
 // Pops a string-to-string dictionary from the reader.
-base::DictionaryValue* PopStringToStringDictionary(
+std::unique_ptr<base::Value> PopStringToStringDictionary(
     dbus::MessageReader* reader) {
-  dbus::MessageReader array_reader(NULL);
+  dbus::MessageReader array_reader(nullptr);
   if (!reader->PopArray(&array_reader))
-    return NULL;
-  std::unique_ptr<base::DictionaryValue> result(new base::DictionaryValue);
+    return nullptr;
+  auto result = std::make_unique<base::Value>(base::Value::Type::DICTIONARY);
   while (array_reader.HasMoreData()) {
-    dbus::MessageReader entry_reader(NULL);
+    dbus::MessageReader entry_reader(nullptr);
     std::string key;
     std::string value;
     if (!array_reader.PopDictEntry(&entry_reader) ||
         !entry_reader.PopString(&key) || !entry_reader.PopString(&value))
-      return NULL;
+      return nullptr;
     result->SetKey(key, base::Value(value));
   }
-  return result.release();
+  return result;
 }
 
 }  // namespace
@@ -85,7 +85,7 @@ ShillClientUnittestBase::ShillClientUnittestBase(
     const dbus::ObjectPath& object_path)
     : interface_name_(interface_name),
       object_path_(object_path),
-      response_(NULL) {}
+      response_(nullptr) {}
 
 ShillClientUnittestBase::~ShillClientUnittestBase() = default;
 
@@ -233,16 +233,6 @@ void ShillClientUnittestBase::ExpectArrayOfStringsArgument(
   EXPECT_FALSE(reader->HasMoreData());
 }
 
-// static
-void ShillClientUnittestBase::ExpectValueArgument(
-    const base::Value* expected_value,
-    dbus::MessageReader* reader) {
-  std::unique_ptr<base::Value> value(dbus::PopDataAsValue(reader));
-  ASSERT_TRUE(value.get());
-  EXPECT_TRUE(value->Equals(expected_value));
-  EXPECT_FALSE(reader->HasMoreData());
-}
-
 // static
 void ShillClientUnittestBase::ExpectStringAndValueArguments(
     const std::string& expected_string,
@@ -258,34 +248,35 @@ void ShillClientUnittestBase::ExpectStringAndValueArguments(
 }
 
 // static
-void ShillClientUnittestBase::ExpectDictionaryValueArgument(
-    const base::DictionaryValue* expected_dictionary,
+void ShillClientUnittestBase::ExpectValueDictionaryArgument(
+    const base::Value* expected_dictionary,
     bool string_valued,
     dbus::MessageReader* reader) {
-  dbus::MessageReader array_reader(NULL);
+  ASSERT_TRUE(expected_dictionary->is_dict());
+  dbus::MessageReader array_reader(nullptr);
   ASSERT_TRUE(reader->PopArray(&array_reader));
   while (array_reader.HasMoreData()) {
-    dbus::MessageReader entry_reader(NULL);
+    dbus::MessageReader entry_reader(nullptr);
     ASSERT_TRUE(array_reader.PopDictEntry(&entry_reader));
     std::string key;
     ASSERT_TRUE(entry_reader.PopString(&key));
     if (string_valued) {
       std::string value;
-      std::string expected_value;
       ASSERT_TRUE(entry_reader.PopString(&value));
-      EXPECT_TRUE(expected_dictionary->GetStringWithoutPathExpansion(
-          key, &expected_value));
-      EXPECT_EQ(expected_value, value);
+      const std::string* expected_value =
+          expected_dictionary->FindStringKey(key);
+      ASSERT_TRUE(expected_value);
+      EXPECT_EQ(*expected_value, value);
       continue;
     }
-    dbus::MessageReader variant_reader(NULL);
+    dbus::MessageReader variant_reader(nullptr);
     ASSERT_TRUE(entry_reader.PopVariant(&variant_reader));
     std::unique_ptr<base::Value> value;
     // Variants in the dictionary can be basic types or string-to-string
     // dictinoary.
     switch (variant_reader.GetDataType()) {
       case dbus::Message::ARRAY:
-        value.reset(PopStringToStringDictionary(&variant_reader));
+        value = PopStringToStringDictionary(&variant_reader);
         break;
       case dbus::Message::BOOL:
       case dbus::Message::INT32:
@@ -296,26 +287,24 @@ void ShillClientUnittestBase::ExpectDictionaryValueArgument(
         NOTREACHED();
     }
     ASSERT_TRUE(value.get());
-    const base::Value* expected_value = NULL;
-    EXPECT_TRUE(
-        expected_dictionary->GetWithoutPathExpansion(key, &expected_value));
+    const base::Value* expected_value = expected_dictionary->FindKey(key);
+    ASSERT_TRUE(expected_value);
     EXPECT_TRUE(value->Equals(expected_value));
   }
 }
 
 // static
-base::DictionaryValue*
-ShillClientUnittestBase::CreateExampleServiceProperties() {
-  base::DictionaryValue* properties = new base::DictionaryValue;
-  properties->SetKey(shill::kGuidProperty,
-                     base::Value("00000000-0000-0000-0000-000000000000"));
-  properties->SetKey(shill::kModeProperty, base::Value(shill::kModeManaged));
-  properties->SetKey(shill::kTypeProperty, base::Value(shill::kTypeWifi));
+base::Value ShillClientUnittestBase::CreateExampleServiceProperties() {
+  base::Value properties(base::Value::Type::DICTIONARY);
+  properties.SetKey(shill::kGuidProperty,
+                    base::Value("00000000-0000-0000-0000-000000000000"));
+  properties.SetKey(shill::kModeProperty, base::Value(shill::kModeManaged));
+  properties.SetKey(shill::kTypeProperty, base::Value(shill::kTypeWifi));
   const std::string ssid = "testssid";
-  properties->SetKey(shill::kWifiHexSsid,
-                     base::Value(base::HexEncode(ssid.c_str(), ssid.size())));
-  properties->SetKey(shill::kSecurityClassProperty,
-                     base::Value(shill::kSecurityPsk));
+  properties.SetKey(shill::kWifiHexSsid,
+                    base::Value(base::HexEncode(ssid.c_str(), ssid.size())));
+  properties.SetKey(shill::kSecurityClassProperty,
+                    base::Value(shill::kSecurityPsk));
   return properties;
 }
 
@@ -346,8 +335,8 @@ void ShillClientUnittestBase::ExpectStringResultWithoutStatus(
 }
 
 // static
-void ShillClientUnittestBase::ExpectDictionaryValueResultWithoutStatus(
-    const base::DictionaryValue* expected_result,
+void ShillClientUnittestBase::ExpectValueResultWithoutStatus(
+    const base::Value* expected_result,
     base::Value result) {
   std::string expected_result_string;
   base::JSONWriter::Write(*expected_result, &expected_result_string);
@@ -357,12 +346,12 @@ void ShillClientUnittestBase::ExpectDictionaryValueResultWithoutStatus(
 }
 
 // static
-void ShillClientUnittestBase::ExpectDictionaryValueResult(
-    const base::DictionaryValue* expected_result,
+void ShillClientUnittestBase::ExpectValueResult(
+    const base::Value* expected_result,
     base::Optional<base::Value> result) {
   EXPECT_TRUE(result);
-  ExpectDictionaryValueResultWithoutStatus(
-      expected_result, std::move(result).value_or(base::Value()));
+  ExpectValueResultWithoutStatus(expected_result,
+                                 std::move(result).value_or(base::Value()));
 }
 
 void ShillClientUnittestBase::OnConnectToPlatformMessage(

chromeos/dbus/shill/shill_client_unittest_base.h

@@ -29,11 +29,8 @@ using ::testing::MatcherInterface;
 using ::testing::MatchResultListener;
 
 namespace base {
-
 class Value;
-class DictionaryValue;
-
-}  // namespace base
+}
 
 namespace dbus {
 
@@ -125,24 +122,19 @@ class ShillClientUnittestBase : public testing::Test {
       const std::vector<std::string>& expected_strings,
       dbus::MessageReader* reader);
 
-  // Expects the reader to have a Value.
-  static void ExpectValueArgument(const base::Value* expected_value,
-                                  dbus::MessageReader* reader);
-
   // Expects the reader to have a string and a Value.
   static void ExpectStringAndValueArguments(const std::string& expected_string,
                                             const base::Value* expected_value,
                                             dbus::MessageReader* reader);
 
   // Expects the reader to have a string-to-variant dictionary.
-  static void ExpectDictionaryValueArgument(
-      const base::DictionaryValue* expected_dictionary,
+  static void ExpectValueDictionaryArgument(
+      const base::Value* expected_dictionary,
       bool string_valued,
       dbus::MessageReader* reader);
 
-  // Creates a DictionaryValue with example Service properties. The caller owns
-  // the result.
-  static base::DictionaryValue* CreateExampleServiceProperties();
+  // Creates a dictionary Value with example Service properties.
+  static base::Value CreateExampleServiceProperties();
 
   // Expects the call status to be SUCCESS.
   static void ExpectNoResultValue(bool result);
@@ -158,14 +150,12 @@ class ShillClientUnittestBase : public testing::Test {
       const std::string& result);
 
   // Checks the result and expects the call status to be SUCCESS.
-  static void ExpectDictionaryValueResult(
-      const base::DictionaryValue* expected_result,
-      base::Optional<base::Value> result);
+  static void ExpectValueResult(const base::Value* expected_result,
+                                base::Optional<base::Value> result);
 
   // Expects the |expected_result| to match the |result|.
-  static void ExpectDictionaryValueResultWithoutStatus(
-      const base::DictionaryValue* expected_result,
-      base::Value result);
+  static void ExpectValueResultWithoutStatus(const base::Value* expected_result,
+                                             base::Value result);
 
   // A message loop to emulate asynchronous behavior.
   base::test::SingleThreadTaskEnvironment task_environment_;

chromeos/dbus/shill/shill_device_client_unittest.cc

@@ -127,13 +127,13 @@ TEST_F(ShillDeviceClientTest, GetProperties) {
   writer.CloseContainer(&array_writer);
 
   // Set expectations.
-  base::DictionaryValue value;
+  base::Value value(base::Value::Type::DICTIONARY);
   value.SetKey(shill::kCellularAllowRoamingProperty, base::Value(kValue));
   PrepareForMethodCall(shill::kGetPropertiesFunction,
                        base::BindRepeating(&ExpectNoArgument), response.get());
   // Call method.
   client_->GetProperties(dbus::ObjectPath(kExampleDevicePath),
-                         base::BindOnce(&ExpectDictionaryValueResult, &value));
+                         base::BindOnce(&ExpectValueResult, &value));
   // Run the message loop.
   base::RunLoop().RunUntilIdle();
 }

chromeos/dbus/shill/shill_ipconfig_client_unittest.cc

@@ -106,7 +106,7 @@ TEST_F(ShillIPConfigClientTest, GetProperties) {
   writer.CloseContainer(&array_writer);
 
   // Create the expected value.
-  base::DictionaryValue value;
+  base::Value value(base::Value::Type::DICTIONARY);
   value.SetKey(shill::kAddressProperty, base::Value(kAddress));
   value.SetKey(shill::kMtuProperty, base::Value(kMtu));
 
@@ -115,7 +115,7 @@ TEST_F(ShillIPConfigClientTest, GetProperties) {
                        base::BindRepeating(&ExpectNoArgument), response.get());
   // Call method.
   client_->GetProperties(dbus::ObjectPath(kExampleIPConfigPath),
-                         base::BindOnce(&ExpectDictionaryValueResult, &value));
+                         base::BindOnce(&ExpectValueResult, &value));
   // Run the message loop.
   base::RunLoop().RunUntilIdle();
 }

chromeos/dbus/shill/shill_manager_client_unittest.cc

@@ -114,13 +114,13 @@ TEST_F(ShillManagerClientTest, GetProperties) {
   writer.CloseContainer(&array_writer);
 
   // Create the expected value.
-  base::DictionaryValue value;
+  base::Value value(base::Value::Type::DICTIONARY);
   value.SetKey(shill::kArpGatewayProperty, base::Value(true));
   // Set expectations.
   PrepareForMethodCall(shill::kGetPropertiesFunction,
                        base::BindRepeating(&ExpectNoArgument), response.get());
   // Call method.
-  client_->GetProperties(base::BindOnce(&ExpectDictionaryValueResult, &value));
+  client_->GetProperties(base::BindOnce(&ExpectValueResult, &value));
   // Run the message loop.
   base::RunLoop().RunUntilIdle();
 }
@@ -153,20 +153,20 @@ TEST_F(ShillManagerClientTest, GetNetworksForGeolocation) {
   writer.CloseContainer(&type_dict_writer);
 
   // Create the expected value.
-  auto property_dict_value = std::make_unique<base::DictionaryValue>();
-  property_dict_value->SetKey(shill::kGeoMacAddressProperty,
-                              base::Value("01:23:45:67:89:AB"));
-  auto type_entry_value = std::make_unique<base::ListValue>();
-  type_entry_value->Append(std::move(property_dict_value));
-  base::DictionaryValue type_dict_value;
-  type_dict_value.SetWithoutPathExpansion("wifi", std::move(type_entry_value));
+  base::Value property_dict_value(base::Value::Type::DICTIONARY);
+  property_dict_value.SetKey(shill::kGeoMacAddressProperty,
+                             base::Value("01:23:45:67:89:AB"));
+  base::Value type_entry_value(base::Value::Type::LIST);
+  type_entry_value.Append(std::move(property_dict_value));
+  base::Value type_dict_value(base::Value::Type::DICTIONARY);
+  type_dict_value.SetKey("wifi", std::move(type_entry_value));
 
   // Set expectations.
   PrepareForMethodCall(shill::kGetNetworksForGeolocation,
                        base::BindRepeating(&ExpectNoArgument), response.get());
   // Call method.
   client_->GetNetworksForGeolocation(
-      base::BindOnce(&ExpectDictionaryValueResult, &type_dict_value));
+      base::BindOnce(&ExpectValueResult, &type_dict_value));
 
   // Run the message loop.
   base::RunLoop().RunUntilIdle();
@@ -285,18 +285,18 @@ TEST_F(ShillManagerClientTest, ConfigureService) {
   dbus::MessageWriter writer(response.get());
   writer.AppendObjectPath(object_path);
   // Create the argument dictionary.
-  std::unique_ptr<base::DictionaryValue> arg(CreateExampleServiceProperties());
+  base::Value arg = CreateExampleServiceProperties();
   // Use a variant valued dictionary rather than a string valued one.
   const bool string_valued = false;
   // Set expectations.
-  PrepareForMethodCall(shill::kConfigureServiceFunction,
-                       base::BindRepeating(&ExpectDictionaryValueArgument,
-                                           arg.get(), string_valued),
-                       response.get());
+  PrepareForMethodCall(
+      shill::kConfigureServiceFunction,
+      base::BindRepeating(&ExpectValueDictionaryArgument, &arg, string_valued),
+      response.get());
   // Call method.
   base::MockCallback<ShillManagerClient::ErrorCallback> mock_error_callback;
   client_->ConfigureService(
-      *arg, base::BindOnce(&ExpectObjectPathResultWithoutStatus, object_path),
+      arg, base::BindOnce(&ExpectObjectPathResultWithoutStatus, object_path),
       mock_error_callback.Get());
   EXPECT_CALL(mock_error_callback, Run(_, _)).Times(0);
 
@@ -311,18 +311,18 @@ TEST_F(ShillManagerClientTest, GetService) {
   dbus::MessageWriter writer(response.get());
   writer.AppendObjectPath(object_path);
   // Create the argument dictionary.
-  std::unique_ptr<base::DictionaryValue> arg(CreateExampleServiceProperties());
+  base::Value arg = CreateExampleServiceProperties();
   // Use a variant valued dictionary rather than a string valued one.
   const bool string_valued = false;
   // Set expectations.
-  PrepareForMethodCall(shill::kGetServiceFunction,
-                       base::BindRepeating(&ExpectDictionaryValueArgument,
-                                           arg.get(), string_valued),
-                       response.get());
+  PrepareForMethodCall(
+      shill::kGetServiceFunction,
+      base::BindRepeating(&ExpectValueDictionaryArgument, &arg, string_valued),
+      response.get());
   // Call method.
   base::MockCallback<ShillManagerClient::ErrorCallback> mock_error_callback;
   client_->GetService(
-      *arg, base::BindOnce(&ExpectObjectPathResultWithoutStatus, object_path),
+      arg, base::BindOnce(&ExpectObjectPathResultWithoutStatus, object_path),
       mock_error_callback.Get());
   EXPECT_CALL(mock_error_callback, Run(_, _)).Times(0);
 

chromeos/dbus/shill/shill_profile_client_unittest.cc

@@ -109,10 +109,10 @@ TEST_F(ShillProfileClientTest, GetProperties) {
   writer.CloseContainer(&array_writer);
 
   // Create the expected value.
-  auto entries = std::make_unique<base::ListValue>();
-  entries->AppendString(kExampleEntryPath);
-  base::DictionaryValue value;
-  value.SetWithoutPathExpansion(shill::kEntriesProperty, std::move(entries));
+  base::Value entries(base::Value::Type::LIST);
+  entries.Append(kExampleEntryPath);
+  base::Value value(base::Value::Type::DICTIONARY);
+  value.SetKey(shill::kEntriesProperty, std::move(entries));
   // Set expectations.
   PrepareForMethodCall(shill::kGetPropertiesFunction,
                        base::BindRepeating(&ExpectNoArgument), response.get());
@@ -120,7 +120,7 @@ TEST_F(ShillProfileClientTest, GetProperties) {
   base::MockCallback<ShillProfileClient::ErrorCallback> error_callback;
   client_->GetProperties(
       dbus::ObjectPath(kDefaultProfilePath),
-      base::BindOnce(&ExpectDictionaryValueResultWithoutStatus, &value),
+      base::BindOnce(&ExpectValueResultWithoutStatus, &value),
       error_callback.Get());
   EXPECT_CALL(error_callback, Run(_, _)).Times(0);
 
@@ -142,7 +142,7 @@ TEST_F(ShillProfileClientTest, GetEntry) {
   writer.CloseContainer(&array_writer);
 
   // Create the expected value.
-  base::DictionaryValue value;
+  base::Value value(base::Value::Type::DICTIONARY);
   value.SetKey(shill::kTypeProperty, base::Value(shill::kTypeWifi));
   // Set expectations.
   PrepareForMethodCall(
@@ -151,10 +151,9 @@ TEST_F(ShillProfileClientTest, GetEntry) {
       response.get());
   // Call method.
   base::MockCallback<ShillProfileClient::ErrorCallback> error_callback;
-  client_->GetEntry(
-      dbus::ObjectPath(kDefaultProfilePath), kExampleEntryPath,
-      base::BindOnce(&ExpectDictionaryValueResultWithoutStatus, &value),
-      error_callback.Get());
+  client_->GetEntry(dbus::ObjectPath(kDefaultProfilePath), kExampleEntryPath,
+                    base::BindOnce(&ExpectValueResultWithoutStatus, &value),
+                    error_callback.Get());
   EXPECT_CALL(error_callback, Run(_, _)).Times(0);
   // Run the message loop.
   base::RunLoop().RunUntilIdle();
@@ -165,7 +164,7 @@ TEST_F(ShillProfileClientTest, DeleteEntry) {
   std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
 
   // Create the expected value.
-  base::DictionaryValue value;
+  base::Value value(base::Value::Type::DICTIONARY);
   value.SetKey(shill::kArpGatewayProperty, base::Value(true));
   // Set expectations.
   PrepareForMethodCall(

chromeos/dbus/shill/shill_service_client_unittest.cc

@@ -99,13 +99,13 @@ TEST_F(ShillServiceClientTest, GetProperties) {
   writer.CloseContainer(&array_writer);
 
   // Set expectations.
-  base::DictionaryValue value;
+  base::Value value(base::Value::Type::DICTIONARY);
   value.SetKey(shill::kSignalStrengthProperty, base::Value(kValue));
   PrepareForMethodCall(shill::kGetPropertiesFunction,
                        base::BindRepeating(&ExpectNoArgument), response.get());
   // Call method.
   client_->GetProperties(dbus::ObjectPath(kExampleServicePath),
-                         base::BindOnce(&ExpectDictionaryValueResult, &value));
+                         base::BindOnce(&ExpectValueResult, &value));
   // Run the message loop.
   base::RunLoop().RunUntilIdle();
 }
@@ -139,18 +139,18 @@ TEST_F(ShillServiceClientTest, SetProperties) {
   std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
 
   // Set expectations.
-  std::unique_ptr<base::DictionaryValue> arg(CreateExampleServiceProperties());
+  base::Value arg = CreateExampleServiceProperties();
   // Use a variant valued dictionary rather than a string valued one.
   const bool string_valued = false;
-  PrepareForMethodCall(shill::kSetPropertiesFunction,
-                       base::BindRepeating(&ExpectDictionaryValueArgument,
-                                           arg.get(), string_valued),
-                       response.get());
+  PrepareForMethodCall(
+      shill::kSetPropertiesFunction,
+      base::BindRepeating(&ExpectValueDictionaryArgument, &arg, string_valued),
+      response.get());
 
   // Call method.
   base::MockCallback<base::OnceClosure> mock_closure;
   base::MockCallback<ShillServiceClient::ErrorCallback> mock_error_callback;
-  client_->SetProperties(dbus::ObjectPath(kExampleServicePath), *arg,
+  client_->SetProperties(dbus::ObjectPath(kExampleServicePath), arg,
                          mock_closure.Get(), mock_error_callback.Get());
   EXPECT_CALL(mock_closure, Run()).Times(1);
   EXPECT_CALL(mock_error_callback, Run(_, _)).Times(0);

chromeos/dbus/shill/shill_third_party_vpn_driver_client_unittest.cc

@@ -136,7 +136,7 @@ TEST_F(ShillThirdPartyVpnDriverClientTest, SetParameters) {
   dbus::MessageWriter writer(response.get());
   writer.AppendString(std::string("deadbeef"));
 
-  base::DictionaryValue parameters;
+  base::Value parameters(base::Value::Type::DICTIONARY);
   const std::string kAddress("1.1.1.1");
   parameters.SetKey(shill::kAddressParameterThirdPartyVpn,
                     base::Value(kAddress));
@@ -145,7 +145,7 @@ TEST_F(ShillThirdPartyVpnDriverClientTest, SetParameters) {
 
   PrepareForMethodCall(
       shill::kSetParametersFunction,
-      base::BindRepeating(&ExpectDictionaryValueArgument, &parameters, true),
+      base::BindRepeating(&ExpectValueDictionaryArgument, &parameters, true),
       response.get());
 
   client_->SetParameters(