Add Shill to ONC translation of IPConfig.

chromeos/network/onc/onc_translator_shill_to_onc.cc

Index: chromeos/network/onc/onc_translator_shill_to_onc.cc
diff --git a/chromeos/network/onc/onc_translator_shill_to_onc.cc b/chromeos/network/onc/onc_translator_shill_to_onc.cc
index 11f59f45e84c478f84412b9bb1825bb2535c7320..1f0185077d27c280c3999e11e626485a9012f624 100644
--- a/chromeos/network/onc/onc_translator_shill_to_onc.cc
+++ b/chromeos/network/onc/onc_translator_shill_to_onc.cc
@@ -66,6 +66,7 @@ class ShillToONCTranslator {
   void TranslateWiFiWithState();
   void TranslateCellularWithState();
   void TranslateNetworkWithState();
+  void TranslateIPConfig();
 
   // Creates an ONC object from |dictionary| according to the signature
   // associated to |onc_field_name| and adds it to |onc_object_| at
@@ -131,6 +132,8 @@ ShillToONCTranslator::CreateTranslatedONCObject() {
     TranslateWiFiWithState();
   } else if (onc_signature_ == &kCellularWithStateSignature) {
     TranslateCellularWithState();
+  } else if (onc_signature_ == &kIPConfigSignature) {
+    TranslateIPConfig();
   } else {
     CopyPropertiesAccordingToSignature();
   }
@@ -254,10 +257,10 @@ void ShillToONCTranslator::TranslateCellularWithState() {
         shill::kCellularApnProperty, &dictionary)) {
     TranslateAndAddNestedObject(::onc::cellular::kAPN, *dictionary);
   }
-  const base::ListValue* list = NULL;
+  const base::ListValue* shill_apns = NULL;
   if (shill_dictionary_->GetListWithoutPathExpansion(
-          shill::kCellularApnListProperty, &list)) {
-    TranslateAndAddListOfObjects(::onc::cellular::kAPNList, *list);
+          shill::kCellularApnListProperty, &shill_apns)) {
+    TranslateAndAddListOfObjects(::onc::cellular::kAPNList, *shill_apns);
   }
 }
 
@@ -299,6 +302,37 @@ void ShillToONCTranslator::TranslateNetworkWithState() {
     onc_object_->SetStringWithoutPathExpansion(
         ::onc::network_config::kConnectionState, onc_state);
   }
+
+  const base::DictionaryValue* shill_ipconfig = NULL;
+  if (shill_dictionary_->GetDictionaryWithoutPathExpansion(
+          shill::kIPConfigProperty, &shill_ipconfig)) {
+    // ONC has a list of IPConfigs and Shill only a single IPConfig object.
+    // Create a list with a single entry.

[stevenjb, 2014/04/23 19:43:42]

So, I think we should fudge this. In Shill, Service.IPConfig is an Object, but
Device.IPConfigs is an array{object}. In order to support ipv6 we will need an
array, so I think Chrome should always use "IPConfigs" and if we fetch these
properties from Service we will put them into a single entry array (ListValue),
but eventually we will need to always fetch them from Device.

[pneubeck (no reviews), 2014/04/25 10:07:34]

Done.
You'll be able to use this translation
 - if an IPConfigS is embedded in a Service
 - or on a standalone IPConfig dictionary (just use the exposed
kIPConfigSignature)
 - but not yet for a stand-alone array of IPConfig objects.

For the latter, we could add a TranslateShillArrayToONCArray function. You can
also repeatedly translate the individual IPConfig dictionaries.

An IPConfig (without 's') _field_ embedded within a Service will not be handled
by the translator.

+    ShillToONCTranslator nested_translator(*shill_ipconfig, kIPConfigSignature);
+    scoped_ptr<base::DictionaryValue> onc_ipconfig =
+        nested_translator.CreateTranslatedONCObject();
+    // If the nested object couldn't be parsed, simply omit it.
+    if (!onc_ipconfig->empty()) {

[armansito, 2014/04/23 18:46:36]

Early return here would be better, i.e if (empty) return;

[pneubeck (no reviews), 2014/04/25 10:07:34]

I kept the pattern in this function, which doesn't returns in order to translate
as much as possible, not early returning on errors.

I'd rather like to keep it this way.
(I adapted TranslateIPConfig)

+      scoped_ptr<base::ListValue> onc_ipconfigs(new base::ListValue());
+      onc_ipconfigs->Append(onc_ipconfig.release());
+      onc_object_->SetWithoutPathExpansion(::onc::network_config::kIPConfigs,
+                                           onc_ipconfigs.release());
+    }
+  }
+}
+
+void ShillToONCTranslator::TranslateIPConfig() {
+  CopyPropertiesAccordingToSignature();
+  std::string ip_address;
+  if (shill_dictionary_->GetStringWithoutPathExpansion(shill::kAddressProperty,
+                                                        &ip_address)) {

[armansito, 2014/04/23 18:46:36]

nit: align "&ip_address" with "shill::kAddressProperty" (looks unaligned in my
browser).

[pneubeck (no reviews), 2014/04/25 10:07:34]

Done.

+    std::string type = ::onc::ipconfig::kIPv4;
+    if (ip_address.find(':') != std::string::npos) {

[stevenjb, 2014/04/23 19:43:42]

Hmm, I guess wwe can use this for identifying ipv6, but I think we're better off
tracking 'Method'.

[pneubeck (no reviews), 2014/04/25 10:07:34]

Missed that one. Thanks.

+      // ip_address is an IPv6 address and not IPv4.
+      type = ::onc::ipconfig::kIPv6;
+    }
+    onc_object_->SetStringWithoutPathExpansion(::onc::ipconfig::kType, type);
+  }
 }
 
 void ShillToONCTranslator::TranslateAndAddNestedObject(
@@ -345,14 +379,13 @@ void ShillToONCTranslator::TranslateAndAddListOfObjects(
         *field_signature->value_signature->onc_array_entry_signature);
     scoped_ptr<base::DictionaryValue> nested_object =
         nested_translator.CreateTranslatedONCObject();
+    // If the nested object couldn't be parsed, simply omit it.
     if (nested_object->empty())
-      // The nested object couldn't be parsed, so simply omit it.
       continue;
     result->Append(nested_object.release());
   }
+  // If there are no entries in the list, there is no need to expose this field.
   if (result->empty())
-    // There are no entries in the list, so there is no need to expose this
-    // field.
     return;
   onc_object_->SetWithoutPathExpansion(onc_field_name, result.release());
 }