Reland: Change to Shill's IPConfig objects.

Before a recent Shill change, Static and Saved IP config properties were stored in toplevel properties with prefixed keys like kStaticIPAddressProperty.
This change makes use of Shill now exposing these properties nested under a StaticIPConfig and SavedIPConfig dictionary.

This also makes the translation between a list of nameservers and a comma-separated string obsolete.

Original Review URL: https://codereview.chromium.org/762243002

BUG=411289
TBR=pneubeck@chromium.org

Review URL: https://codereview.chromium.org/778693004

Cr-Commit-Position: refs/heads/master@{#306669}

6 files changed, 55 insertions, 80 deletions

diff --git a/chromeos/network/onc/onc_translation_tables.cc b/chromeos/network/onc/onc_translation_tables.cc
index 5124c02..3e22de3a 100644
--- a/chromeos/network/onc/onc_translation_tables.cc
+++ b/chromeos/network/onc/onc_translation_tables.cc
@@ -207,20 +207,11 @@ const FieldTranslationEntry ipconfig_fields[] = {
       shill::kWebProxyAutoDiscoveryUrlProperty},
     {NULL}};
 
-const FieldTranslationEntry saved_ipconfig_fields[] = {
-    { ::onc::ipconfig::kIPAddress, shill::kSavedIPAddressProperty},
-    { ::onc::ipconfig::kGateway, shill::kSavedIPGatewayProperty},
-    { ::onc::ipconfig::kRoutingPrefix, shill::kSavedIPPrefixlenProperty},
-    // NameServers are converted during translation, see onc_translator_*.
-    // { ::onc::ipconfig::kNameServers, shill::kSavedIPNameServersProperty},
-    {NULL}};
-
-const FieldTranslationEntry static_ipconfig_fields[] = {
-    { ::onc::ipconfig::kIPAddress, shill::kStaticIPAddressProperty},
-    { ::onc::ipconfig::kGateway, shill::kStaticIPGatewayProperty},
-    { ::onc::ipconfig::kRoutingPrefix, shill::kStaticIPPrefixlenProperty},
-    // NameServers are converted during translation, see onc_translator_*.
-    // { ::onc::ipconfig::kNameServers, shill::kStaticIPNameServersProperty},
+const FieldTranslationEntry static_or_saved_ipconfig_fields[] = {
+    { ::onc::ipconfig::kIPAddress, shill::kAddressProperty},
+    { ::onc::ipconfig::kGateway, shill::kGatewayProperty},
+    { ::onc::ipconfig::kRoutingPrefix, shill::kPrefixlenProperty},
+    { ::onc::ipconfig::kNameServers, shill::kNameServersProperty},
     {NULL}};
 
 struct OncValueTranslationEntry {
@@ -249,8 +240,8 @@ const OncValueTranslationEntry onc_value_translation_table[] = {
   { &kNetworkWithStateSignature, network_fields },
   { &kNetworkConfigurationSignature, network_fields },
   { &kIPConfigSignature, ipconfig_fields },
-  { &kSavedIPConfigSignature, saved_ipconfig_fields },
-  { &kStaticIPConfigSignature, static_ipconfig_fields },
+  { &kSavedIPConfigSignature, static_or_saved_ipconfig_fields },
+  { &kStaticIPConfigSignature, static_or_saved_ipconfig_fields },
   { NULL }
 };
 
@@ -260,13 +251,19 @@ struct NestedShillDictionaryEntry {
   const char* const* shill_property_path;
 };
 
-const char* cellular_apn_property_path_entries[] = {
+const char* cellular_apn_path_entries[] = {
   shill::kCellularApnProperty,
   NULL
 };
 
+const char* static_ip_config_path_entries[] = {
+  shill::kStaticIPConfigProperty,
+  NULL
+};
+
 const NestedShillDictionaryEntry nested_shill_dictionaries[] = {
-  { &kCellularApnSignature, cellular_apn_property_path_entries },
+  { &kCellularApnSignature, cellular_apn_path_entries },
+  { &kStaticIPConfigSignature, static_ip_config_path_entries },
   { NULL }
 };
 
diff --git a/chromeos/network/onc/onc_translation_tables.h b/chromeos/network/onc/onc_translation_tables.h
index 96a971d..b737542 100644
--- a/chromeos/network/onc/onc_translation_tables.h
+++ b/chromeos/network/onc/onc_translation_tables.h
@@ -43,6 +43,10 @@ extern const FieldTranslationEntry kCellularDeviceTable[];
 const FieldTranslationEntry* GetFieldTranslationTable(
     const OncValueSignature& onc_signature);
 
+// Returns the path at which the translation of an ONC object will be stored in
+// a Shill dictionary if its signature is |onc_signature|.
+// The default is that values are stored directly in the top level of the Shill
+// dictionary.
 std::vector<std::string> GetPathToNestedShillDictionary(
     const OncValueSignature& onc_signature);
 
diff --git a/chromeos/network/onc/onc_translator_onc_to_shill.cc b/chromeos/network/onc/onc_translator_onc_to_shill.cc
index 5300a9d..8e30af5 100644
--- a/chromeos/network/onc/onc_translator_onc_to_shill.cc
+++ b/chromeos/network/onc/onc_translator_onc_to_shill.cc
@@ -28,17 +28,6 @@ namespace onc {
 
 namespace {
 
-bool ConvertListValueToStringVector(const base::ListValue& string_list,
-                                    std::vector<std::string>* result) {
-  for (size_t i = 0; i < string_list.GetSize(); ++i) {
-    std::string str;
-    if (!string_list.GetString(i, &str))
-      return false;
-    result->push_back(str);
-  }
-  return true;
-}
-
 scoped_ptr<base::StringValue> ConvertValueToString(const base::Value& value) {
   std::string str;
   if (!value.GetAsString(&str))
@@ -71,7 +60,6 @@ class LocalTranslator {
   void TranslateVPN();
   void TranslateWiFi();
   void TranslateEAP();
-  void TranslateStaticIPConfig();
   void TranslateNetworkConfiguration();
 
   // Copies all entries from |onc_object_| to |shill_dictionary_| for which a
@@ -108,8 +96,6 @@ class LocalTranslator {
 void LocalTranslator::TranslateFields() {
   if (onc_signature_ == &kNetworkConfigurationSignature)
     TranslateNetworkConfiguration();
-  else if (onc_signature_ == &kStaticIPConfigSignature)
-    TranslateStaticIPConfig();
   else if (onc_signature_ == &kEthernetSignature)
     TranslateEthernet();
   else if (onc_signature_ == &kVPNSignature)
@@ -141,20 +127,6 @@ void LocalTranslator::TranslateEthernet() {
 }
 
 
-void LocalTranslator::TranslateStaticIPConfig() {
-  const base::ListValue* onc_nameservers = NULL;
-  if (onc_object_->GetListWithoutPathExpansion(::onc::ipconfig::kNameServers,
-                                               &onc_nameservers)) {
-    std::vector<std::string> onc_nameservers_vector;
-    ConvertListValueToStringVector(*onc_nameservers, &onc_nameservers_vector);
-    std::string shill_nameservers = JoinString(onc_nameservers_vector, ',');
-    shill_dictionary_->SetStringWithoutPathExpansion(
-        shill::kStaticIPNameServersProperty, shill_nameservers);
-  }
-
-  CopyFieldsAccordingToSignature();
-}
-
 void LocalTranslator::TranslateOpenVPN() {
   // SaveCredentials needs special handling when translating from Shill -> ONC
   // so handle it explicitly here.
diff --git a/chromeos/network/onc/onc_translator_shill_to_onc.cc b/chromeos/network/onc/onc_translator_shill_to_onc.cc
index 1daf4a7..cfce26a 100644
--- a/chromeos/network/onc/onc_translator_shill_to_onc.cc
+++ b/chromeos/network/onc/onc_translator_shill_to_onc.cc
@@ -84,7 +84,7 @@ class ShillToONCTranslator {
   void TranslateCellularDevice();
   void TranslateNetworkWithState();
   void TranslateIPConfig();
-  void TranslateSavedOrStaticIPConfig(const std::string& nameserver_property);
+  void TranslateSavedOrStaticIPConfig();
   void TranslateSavedIPConfig();
   void TranslateStaticIPConfig();
 
@@ -479,8 +479,19 @@ void ShillToONCTranslator::TranslateNetworkWithState() {
                                  *shill_ipconfigs);
   }
 
-  TranslateAndAddNestedObject(::onc::network_config::kSavedIPConfig);
-  TranslateAndAddNestedObject(::onc::network_config::kStaticIPConfig);
+  const base::DictionaryValue* saved_ipconfig = nullptr;
+  if (shill_dictionary_->GetDictionaryWithoutPathExpansion(
+        shill::kSavedIPConfigProperty, &saved_ipconfig)) {
+    TranslateAndAddNestedObject(::onc::network_config::kSavedIPConfig,
+                                *saved_ipconfig);
+  }
+
+  const base::DictionaryValue* static_ipconfig = nullptr;
+  if (shill_dictionary_->GetDictionaryWithoutPathExpansion(
+        shill::kStaticIPConfigProperty, &static_ipconfig)) {
+    TranslateAndAddNestedObject(::onc::network_config::kStaticIPConfig,
+                                *static_ipconfig);
+  }
 }
 
 void ShillToONCTranslator::TranslateIPConfig() {
@@ -502,24 +513,9 @@ void ShillToONCTranslator::TranslateIPConfig() {
   onc_object_->SetStringWithoutPathExpansion(::onc::ipconfig::kType, type);
 }
 
-void ShillToONCTranslator::TranslateSavedOrStaticIPConfig(
-    const std::string& nameserver_property) {
+void ShillToONCTranslator::TranslateSavedOrStaticIPConfig() {
   CopyPropertiesAccordingToSignature();
-  // Saved/Static IP config nameservers are stored as a comma separated list.
-  std::string shill_nameservers;
-  shill_dictionary_->GetStringWithoutPathExpansion(
-      nameserver_property, &shill_nameservers);
-  std::vector<std::string> onc_nameserver_vector;
-  if (Tokenize(shill_nameservers, ",", &onc_nameserver_vector) > 0) {
-    scoped_ptr<base::ListValue> onc_nameservers(new base::ListValue);
-    for (std::vector<std::string>::iterator iter =
-             onc_nameserver_vector.begin();
-         iter != onc_nameserver_vector.end(); ++iter) {
-      onc_nameservers->AppendString(*iter);
-    }
-    onc_object_->SetWithoutPathExpansion(::onc::ipconfig::kNameServers,
-                                         onc_nameservers.release());
-  }
+
   // Static and Saved IPConfig in Shill are always of type IPv4. Set this type
   // in ONC, but not if the object would be empty except the type.
   if (!onc_object_->empty()) {
@@ -529,11 +525,11 @@ void ShillToONCTranslator::TranslateSavedOrStaticIPConfig(
 }
 
 void ShillToONCTranslator::TranslateSavedIPConfig() {
-  TranslateSavedOrStaticIPConfig(shill::kSavedIPNameServersProperty);
+  TranslateSavedOrStaticIPConfig();
 }
 
 void ShillToONCTranslator::TranslateStaticIPConfig() {
-  TranslateSavedOrStaticIPConfig(shill::kStaticIPNameServersProperty);
+  TranslateSavedOrStaticIPConfig();
 }
 
 void ShillToONCTranslator::TranslateAndAddNestedObject(
diff --git a/chromeos/test/data/network/shill_ethernet.json b/chromeos/test/data/network/shill_ethernet.json
index e342d13..7842203 100644
--- a/chromeos/test/data/network/shill_ethernet.json
+++ b/chromeos/test/data/network/shill_ethernet.json
@@ -1,7 +1,9 @@
 { "GUID": "guid",
   "Type": "ethernet",
-  "StaticIP.Address":"125.125.125.125",
-  "StaticIP.Gateway":"1.1.1.7",
-  "StaticIP.NameServers":"1.1.1.8,125.1.1.3",
-  "StaticIP.Prefixlen":26
+  "StaticIPConfig": {
+    "Address": "125.125.125.125",
+    "Gateway": "1.1.1.7",
+    "NameServers": ["1.1.1.8", "125.1.1.3"],
+    "Prefixlen": 26
+  }
 }
diff --git a/chromeos/test/data/network/shill_ethernet_with_ipconfig.json b/chromeos/test/data/network/shill_ethernet_with_ipconfig.json
index a451a6b..f698b26 100644
--- a/chromeos/test/data/network/shill_ethernet_with_ipconfig.json
+++ b/chromeos/test/data/network/shill_ethernet_with_ipconfig.json
@@ -20,13 +20,17 @@
       "Prefixlen":12
     }
   ],
-  "SavedIP.Address":"124.124.124.124",
-  "SavedIP.Gateway":"1.1.1.4",
-  "SavedIP.NameServers":"1.1.1.5,1.1.1.6",
-  "SavedIP.Prefixlen":25,
-  "StaticIP.Address":"125.125.125.125",
-  "StaticIP.Gateway":"1.1.1.7",
-  "StaticIP.NameServers":"1.1.1.8",
-  "StaticIP.Prefixlen":26,
+  "SavedIPConfig": {
+    "Address":"124.124.124.124",
+    "Gateway":"1.1.1.4",
+    "NameServers":["1.1.1.5","1.1.1.6"],
+    "Prefixlen":25
+  },
+  "StaticIPConfig": {
+    "Address":"125.125.125.125",
+    "Gateway":"1.1.1.7",
+    "NameServers":["1.1.1.8"],
+    "Prefixlen":26
+  },
   "Type":"ethernet"
 }