ONC: Add IPAddressConfigType and NameServersConfigType

chromeos/network/onc/onc_translator_onc_to_shill.cc

Index: chromeos/network/onc/onc_translator_onc_to_shill.cc
diff --git a/chromeos/network/onc/onc_translator_onc_to_shill.cc b/chromeos/network/onc/onc_translator_onc_to_shill.cc
index 8e30af55446f6889b6c35922c488bf602039aae8..e58f86db3e53eece20e86a81f0169bb640f2adab 100644
--- a/chromeos/network/onc/onc_translator_onc_to_shill.cc
+++ b/chromeos/network/onc/onc_translator_onc_to_shill.cc
@@ -257,6 +257,28 @@ void LocalTranslator::TranslateNetworkConfiguration() {
   if (type == ::onc::network_type::kVPN)
     CopyFieldFromONCToShill(::onc::network_config::kName, shill::kNameProperty);
 
+  std::string ip_address_config_type, name_servers_config_type;
+  onc_object_->GetStringWithoutPathExpansion(
+      ::onc::network_config::kIPAddressConfigType, &ip_address_config_type);
+  onc_object_->GetStringWithoutPathExpansion(
+      ::onc::network_config::kNameServersConfigType, &name_servers_config_type);
+  if ((ip_address_config_type == ::onc::network_config::kIPConfigTypeDHCP) &&

[pneubeck (no reviews), 2015/01/07 09:23:48]

I looked into the current version of network_configuration_handler.cc
and it doesn't handle the NullValue yet. Is this even correct currently?

Why not just dropping this if-clause and keeping only the else part, i.e.

if ((ip_address_config_type == ::onc::network_config::kIPConfigTypeDHCP) ||
 (name_servers_config_type == ::onc::network_config::kIPConfigTypeDHCP)) {
 // If either type is set to DHCP, provide an empty dictionary to
 // ensure that any unset properties are cleared. Note: if either type is
 // specifed, the other type defaults to DHCP if not specified.
 shill_dictionary_->SetWithoutPathExpansion(shill::kStaticIPConfigProperty,
                                            new base::DictionaryValue);
}

[stevenjb, 2015/01/07 17:28:03]

Bah, you're right. I think I need the NULL support for Cellular APN anyway, but
that change is in the follow-up CL that this CL was extracted from. Sigh. This
has dragged out way too long.

As long as we keep the second if(), the tests don't have to be updated since we
don't currently specify DHCP in any of the translation tests. I added a test
that does that.

+      (name_servers_config_type == ::onc::network_config::kIPConfigTypeDHCP)) {
+    // If both IPAddressConfigType and NameServersConfigType are set to DHCP,
+    // provide an empty StaticIPConfig property so that all existing properties
+    // will be cleared.
+    shill_dictionary_->SetWithoutPathExpansion(shill::kStaticIPConfigProperty,
+                                               base::Value::CreateNullValue());
+  } else if (
+      (ip_address_config_type == ::onc::network_config::kIPConfigTypeDHCP) ||
+      (name_servers_config_type == ::onc::network_config::kIPConfigTypeDHCP)) {
+    // Otherwise, if either type is set to DHCP, provide an empty dictionary to
+    // ensure that any unset properties are cleared. Note: if either type is
+    // specifed, the other type defaults to DHCP if not specified.
+    shill_dictionary_->SetWithoutPathExpansion(shill::kStaticIPConfigProperty,
+                                               new base::DictionaryValue);
+  }
+
   CopyFieldsAccordingToSignature();
 }