Use Managed properties for Preferred and Provider.

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 dc01b2a53dd51335958cc529ae46db46780fe5c8..cd22cad38a6ab87ced9dee18ef2f75aeff357926 100644
--- a/chromeos/network/onc/onc_translator_shill_to_onc.cc
+++ b/chromeos/network/onc/onc_translator_shill_to_onc.cc
@@ -89,6 +89,12 @@ class ShillToONCTranslator {
   // |onc_field_name|.
   void TranslateAndAddNestedObject(const std::string& onc_field_name);
 
+  // Sets |onc_field_name| in dictionary |onc_dictionary_name| in |onc_object_|
+  // to |value| if the dictionary exists.
+  void SetNestedOncValue(const std::string& onc_dictionary_name,
+                         const std::string& onc_field_name,
+                         const base::Value& value);
+
   // Translates a list of nested objects and adds the list to |onc_object_| at
   // |onc_field_name|. If there are errors while parsing individual objects or
   // if the resulting list contains no entries, the result will not be added to
@@ -182,8 +188,7 @@ void ShillToONCTranslator::TranslateOpenVPN() {
   for (const OncFieldSignature* field_signature = onc_signature_->fields;
        field_signature->onc_field_name != NULL; ++field_signature) {
     const std::string& onc_field_name = field_signature->onc_field_name;
-    if (onc_field_name == ::onc::vpn::kSaveCredentials ||
-        onc_field_name == ::onc::openvpn::kRemoteCertKU ||
+    if (onc_field_name == ::onc::openvpn::kRemoteCertKU ||

[pneubeck (no reviews), 2014/08/19 19:26:38]

Why?
These exceptions are for OpenVPN properties that Shill returns not as strings
but as the actual types.

[stevenjb, 2014/08/19 23:20:18]

SaveCredentials is a Service property, not a Service.OpenVPN property, so this
doesn't belong here for the Shill -> ONC translation. I fixed up the unit tests
to deal with the asymetric nature of Provider properties and added an explicit
check for the correct SaveCredentials type in TranslateVPN().

         onc_field_name == ::onc::openvpn::kServerCAPEMs) {
       CopyProperty(field_signature);
       continue;
@@ -233,18 +238,47 @@ void ShillToONCTranslator::TranslateIPsec() {
 }
 
 void ShillToONCTranslator::TranslateVPN() {

[pneubeck (no reviews), 2014/08/19 19:26:38]

I'll take a more thorough look at this part tomorrow.

[stevenjb, 2014/08/19 23:20:18]

OK, hopefully the need for these changes will be more clear with the updated
tests. (I had been testing with actual Shill results up to now).

-  TranslateWithTableAndSet(
-      shill::kProviderTypeProperty, kVPNTypeTable, ::onc::vpn::kType);
   CopyPropertiesAccordingToSignature();
 
-  std::string vpn_type;
-  if (onc_object_->GetStringWithoutPathExpansion(::onc::vpn::kType,
-                                                 &vpn_type)) {
-    if (vpn_type == ::onc::vpn::kTypeL2TP_IPsec) {
-      TranslateAndAddNestedObject(::onc::vpn::kIPsec);
-      TranslateAndAddNestedObject(::onc::vpn::kL2TP);
-    } else {
-      TranslateAndAddNestedObject(vpn_type);
+  // Parse Shill Provider dictionary.
+  const base::DictionaryValue* provider = NULL;
+  if (!shill_dictionary_->GetDictionaryWithoutPathExpansion(
+          shill::kProviderProperty, &provider)) {
+    LOG(ERROR) << "Shill VPN object with no Provider dictionary";
+    return;
+  }
+  std::string shill_provider_type, onc_provider_type;
+  if (!provider->GetStringWithoutPathExpansion(
+          shill::kTypeProperty, &shill_provider_type)) {
+    LOG(ERROR) << "Shill VPN object with no Provider.Type";
+    return;
+  }
+  TranslateStringToONC(kVPNTypeTable, shill_provider_type, &onc_provider_type);
+  onc_object_->SetStringWithoutPathExpansion(
+      ::onc::vpn::kType, onc_provider_type);
+  std::string provider_host;
+  provider->GetStringWithoutPathExpansion(shill::kHostProperty, &provider_host);
+  onc_object_->SetStringWithoutPathExpansion(::onc::vpn::kHost, provider_host);
+
+  const base::Value* save_credentials = NULL;
+  shill_dictionary_->GetWithoutPathExpansion(
+      shill::kSaveCredentialsProperty, &save_credentials);
+
+  // Translate the nested dictionary.
+  if (onc_provider_type == ::onc::vpn::kTypeL2TP_IPsec) {
+    TranslateAndAddNestedObject(::onc::vpn::kIPsec, *provider);
+    TranslateAndAddNestedObject(::onc::vpn::kL2TP, *provider);
+    if (save_credentials) {
+      SetNestedOncValue(
+          ::onc::vpn::kIPsec, ::onc::vpn::kSaveCredentials, *save_credentials);
+      SetNestedOncValue(
+          ::onc::vpn::kL2TP, ::onc::vpn::kSaveCredentials, *save_credentials);
+    }
+  } else {
+    TranslateAndAddNestedObject(onc_provider_type, *provider);
+    if (save_credentials) {
+      SetNestedOncValue(
+          onc_provider_type, ::onc::vpn::kSaveCredentials, *save_credentials);
     }
   }
 }
@@ -404,11 +438,23 @@ void ShillToONCTranslator::TranslateAndAddNestedObject(
                                          *field_signature->value_signature);
   scoped_ptr<base::DictionaryValue> nested_object =
       nested_translator.CreateTranslatedONCObject();
-  if (nested_object->empty())
-    return;
+  // Note: this may create an empty nested object which is necessary in case
+  // properties are added later with SetNestedOncValue.
   onc_object_->SetWithoutPathExpansion(onc_field_name, nested_object.release());
 }
 
+void ShillToONCTranslator::SetNestedOncValue(
+    const std::string& onc_dictionary_name,
+    const std::string& onc_field_name,
+    const base::Value& value) {
+  base::DictionaryValue* nested;
+  if (!onc_object_->GetDictionaryWithoutPathExpansion(
+          onc_dictionary_name, &nested)) {
+    NOTREACHED();
+  }
+  nested->SetWithoutPathExpansion(onc_field_name, value.DeepCopy());
+}
+
 void ShillToONCTranslator::TranslateAndAddListOfObjects(
     const std::string& onc_field_name,
     const base::ListValue& list) {