Set onc_source UI data parameter when importing ONC.

chrome/browser/chromeos/cros/onc_network_parser.cc

Index: chrome/browser/chromeos/cros/onc_network_parser.cc
diff --git a/chrome/browser/chromeos/cros/onc_network_parser.cc b/chrome/browser/chromeos/cros/onc_network_parser.cc
index fa7ef6c5fb2571541e6e91a1da32f3bf3fb409f7..d1380ae00dfbc07a9540c2db917a4db1ec3bad83 100644
--- a/chrome/browser/chromeos/cros/onc_network_parser.cc
+++ b/chrome/browser/chromeos/cros/onc_network_parser.cc
@@ -6,7 +6,7 @@
 
 #include "base/base64.h"
 #include "base/json/json_value_serializer.h"
-#include "base/json/json_writer.h"  // for debug output only.
+#include "base/json/json_writer.h"
 #include "base/stringprintf.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/cros/native_network_constants.h"
@@ -43,14 +43,17 @@ OncValueSignature network_configuration_signature[] = {
   { "Remove", PROPERTY_INDEX_ONC_REMOVE, TYPE_BOOLEAN },
   { "Type", PROPERTY_INDEX_TYPE, TYPE_STRING },
   { "WiFi", PROPERTY_INDEX_ONC_WIFI, TYPE_DICTIONARY },
-  { "VPN", PROPERTY_INDEX_ONC_VPN, TYPE_DICTIONARY }
+  { "VPN", PROPERTY_INDEX_ONC_VPN, TYPE_DICTIONARY },
+  { NULL }

[kmixter1, 2011/12/06 12:52:37]

Ouch, nice catch.

 };
 
 OncValueSignature wifi_signature[] = {
-  { "AutoConnect", PROPERTY_INDEX_AUTO_CONNECT, TYPE_BOOLEAN },
+  { "AutoConnect", PROPERTY_INDEX_AUTO_CONNECT, TYPE_BOOLEAN,
+    NetworkUIData::kPropertyAutoConnect },
   { "EAP", PROPERTY_INDEX_EAP, TYPE_DICTIONARY },
   { "HiddenSSID", PROPERTY_INDEX_HIDDEN_SSID, TYPE_BOOLEAN },
-  { "Passphrase", PROPERTY_INDEX_PASSPHRASE, TYPE_STRING },
+  { "Passphrase", PROPERTY_INDEX_PASSPHRASE, TYPE_STRING,
+    NetworkUIData::kPropertyPassphrase },
   { "ProxyURL", PROPERTY_INDEX_PROXY_CONFIG, TYPE_STRING },
   { "Security", PROPERTY_INDEX_SECURITY, TYPE_STRING },
   { "SSID", PROPERTY_INDEX_SSID, TYPE_STRING },
@@ -58,17 +61,24 @@ OncValueSignature wifi_signature[] = {
 };
 
 OncValueSignature eap_signature[] = {
-  { "AnonymousIdentity", PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY, TYPE_STRING },
+  { "AnonymousIdentity", PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY, TYPE_STRING,
+    NetworkUIData::kPropertyEAPAnonymousIdentity },

[kmixter1, 2011/12/06 12:52:37]

why can't these use the same name as in ONC?

[Mattias Nissler (ping if slow), 2011/12/06 13:37:59]

See other comment.

   { "ClientCertPattern", PROPERTY_INDEX_ONC_CLIENT_CERT_PATTERN,
     TYPE_DICTIONARY },
   { "ClientCertRef", PROPERTY_INDEX_ONC_CLIENT_CERT_REF, TYPE_STRING },
   { "ClientCertType", PROPERTY_INDEX_ONC_CLIENT_CERT_TYPE, TYPE_STRING },
-  { "Identity", PROPERTY_INDEX_EAP_IDENTITY, TYPE_STRING },
-  { "Inner", PROPERTY_INDEX_EAP_PHASE_2_AUTH, TYPE_STRING },
-  { "Outer", PROPERTY_INDEX_EAP_METHOD, TYPE_STRING },
-  { "Password", PROPERTY_INDEX_EAP_PASSWORD, TYPE_STRING },
-  { "ServerCARef", PROPERTY_INDEX_EAP_CA_CERT, TYPE_STRING },
-  { "UseSystemCAs", PROPERTY_INDEX_EAP_USE_SYSTEM_CAS, TYPE_BOOLEAN },
+  { "Identity", PROPERTY_INDEX_EAP_IDENTITY, TYPE_STRING,
+    NetworkUIData::kPropertyEAPIdentity },
+  { "Inner", PROPERTY_INDEX_EAP_PHASE_2_AUTH, TYPE_STRING,
+    NetworkUIData::kPropertyEAPPhase2Auth },
+  { "Outer", PROPERTY_INDEX_EAP_METHOD, TYPE_STRING,
+    NetworkUIData::kPropertyEAPMethod },
+  { "Password", PROPERTY_INDEX_EAP_PASSWORD, TYPE_STRING,
+    NetworkUIData::kPropertyEAPPassphrase },
+  { "ServerCARef", PROPERTY_INDEX_EAP_CA_CERT, TYPE_STRING,
+    NetworkUIData::kPropertyEAPServerCaCertNssNickname },
+  { "UseSystemCAs", PROPERTY_INDEX_EAP_USE_SYSTEM_CAS, TYPE_BOOLEAN,
+    NetworkUIData::kPropertyEAPUseSystemCAs },
   { NULL }
 };
 
@@ -161,8 +171,10 @@ ConnectionType ParseNetworkType(const std::string& type) {
 
 // -------------------- OncNetworkParser --------------------
 
-OncNetworkParser::OncNetworkParser(const std::string& onc_blob)
+OncNetworkParser::OncNetworkParser(const std::string& onc_blob,
+                                   NetworkUIData::ONCSource onc_source)
     : NetworkParser(get_onc_mapper()),
+      onc_source_(onc_source),
       network_configs_(NULL),
       certificates_(NULL) {
   VLOG(2) << __func__ << ": OncNetworkParser called on " << onc_blob;
@@ -209,6 +221,23 @@ int OncNetworkParser::GetCertificatesSize() const {
   return certificates_ ? certificates_->GetSize() : 0;
 }
 
+Network* OncNetworkParser::ParseNetwork(int n) {
+  if (!network_configs_)
+    return NULL;
+  DictionaryValue* info = NULL;
+  if (!network_configs_->GetDictionary(n, &info))
+    return NULL;
+  if (VLOG_IS_ON(2)) {
+    std::string network_json;
+    base::JSONWriter::Write(static_cast<base::Value*>(info),
+                            true, &network_json);
+    VLOG(2) << "Parsing network at index " << n
+            << ": " << network_json;
+  }
+
+  return CreateNetworkFromInfo(std::string(), *info);
+}
+
 bool OncNetworkParser::ParseCertificate(int cert_index) {
   CHECK(certificates_);
   CHECK(static_cast<size_t>(cert_index) < certificates_->GetSize());
@@ -256,23 +285,6 @@ bool OncNetworkParser::ParseCertificate(int cert_index) {
   return false;
 }
 
-Network* OncNetworkParser::ParseNetwork(int n) {
-  if (!network_configs_)
-    return NULL;
-  DictionaryValue* info = NULL;
-  if (!network_configs_->GetDictionary(n, &info))
-    return NULL;
-  if (VLOG_IS_ON(2)) {
-    std::string network_json;
-    base::JSONWriter::Write(static_cast<base::Value*>(info),
-                            true, &network_json);
-    VLOG(2) << "Parsing network at index " << n
-            << ": " << network_json;
-  }
-
-  return CreateNetworkFromInfo(std::string(), *info);
-}
-
 Network* OncNetworkParser::CreateNetworkFromInfo(
     const std::string& service_path,
     const DictionaryValue& info) {
@@ -280,6 +292,13 @@ Network* OncNetworkParser::CreateNetworkFromInfo(
   if (type == TYPE_UNKNOWN)  // Return NULL if cannot parse network type.
     return NULL;
   scoped_ptr<Network> network(CreateNewNetwork(type, service_path));
+
+  // Initialize UI data.
+  NetworkUIData ui_data;
+  ui_data.set_onc_source(onc_source_);
+  ui_data.FillDictionary(network->ui_data());
+
+  // Parse all properties recursively.
   if (!ParseNestedObject(network.get(),
                          "NetworkConfiguration",
                          static_cast<const base::Value&>(info),
@@ -288,11 +307,19 @@ Network* OncNetworkParser::CreateNetworkFromInfo(
     LOG(WARNING) << "Network " << network->name() << " had problems.";
     return NULL;
   }
+
+  // Update the UI data property.
+  std::string ui_data_json;
+  base::JSONWriter::Write(network->ui_data(), false, &ui_data_json);
+  base::StringValue ui_data_string_value(ui_data_json);
+  network->UpdatePropertyMap(PROPERTY_INDEX_UI_DATA, ui_data_string_value);
+
   if (VLOG_IS_ON(2)) {
     VLOG(2) << "Created Network '" << network->name()
             << "' from info. Path:" << service_path
             << " Type:" << ConnectionTypeToString(type);
   }
+
   return network.release();
 }
 
@@ -487,6 +514,7 @@ bool OncNetworkParser::ParseNestedObject(Network* network,
       any_errors = true;
       continue;
     }
+
     if (VLOG_IS_ON(2)) {
       std::string value_json;
       base::JSONWriter::Write(inner_value, true, &value_json);
@@ -494,6 +522,9 @@ bool OncNetworkParser::ParseNestedObject(Network* network,
               << "(" << index << ")] = " << value_json;
     }
   }
+
+  ParseUIData(network, dict, signature);
+
   return !any_errors;
 }
 
@@ -590,6 +621,56 @@ bool OncNetworkParser::ParseNetworkConfigurationValue(
   return false;
 }
 
+void OncNetworkParser::ParseUIData(Network* network,
+                                   const base::DictionaryValue* dict,
+                                   OncValueSignature* signature) const {

[stevenjb, 2011/12/08 02:26:35]

|signature| is a list of signatures, yes, not a pointer to a single signature?
We should name it appropriately, e.g. signatures, or signature_list.

+  // Collect the set of recommended properties.
+  std::set<std::string> recommended;
+  bool all_recommended = false;
+  base::ListValue* recommended_list = NULL;
+  if (dict->GetList("Recommended", &recommended_list)) {
+    for (base::ListValue::const_iterator entry(recommended_list->begin());
+         entry != recommended_list->end(); ++entry) {
+      std::string entry_string;
+      if ((*entry)->GetAsString(&entry_string)) {
+        if (entry_string == ".") {
+          all_recommended = true;
+          break;
+        } else {
+          recommended.insert(entry_string);
+        }
+      } else {
+        LOG(WARNING) << "Entries in the recommended list should be strings!";
+      }
+    }
+  }
+
+  // Do a pass over |signature|, filling in UI data where applicable.
+  NetworkUIData ui_data;
+  for (int i = 0; signature[i].field != NULL; ++i) {
+    if (!signature[i].ui_data_key)
+      continue;
+
+    bool controlled_by_policy =
+        (onc_source_ == NetworkUIData::ONC_SOURCE_DEVICE_POLICY ||
+         onc_source_ == NetworkUIData::ONC_SOURCE_USER_POLICY) &&
+        !all_recommended &&

[kmixter1, 2011/12/06 12:52:37]

I was thinking "." meant something a little different - not so much that every
field was recommended (as a shorthand) but that the entity itself was
recommended.  So a recommended certificate can be removed and a recommended
network can be forgotten.  But properties within either can still be mandatory. 
Does that make sense?

[Mattias Nissler (ping if slow), 2011/12/06 13:37:59]

Ah, so I didn't understand what your intention with "." was. Does having
recommended certs and networks actually make sense? For networks, I guess it
doesn't, since the user can still select which network to use anyway. Then, the
only reason for recommended networks would be for the user to be able to remove
network entries, but do we really want to support this use case? Also how, to
bring recommended networks back once you've removed them? Regarding certs, I
cannot see a use case for recommended certs at all. Is there one?

[kmixter1, 2011/12/07 23:08:29]

I added "." just for completeness in the specification.  With the way we were
specifying the Recommended field, we could express a value of any scalar field
as being recommended but we had no way to make things in arrays be recommended. 
Conveniently most things in arrays that could be made recommended were objects,
and so adding "." closed that gap.  Whether or not anyone would ever want it, I
don't know.  I'm quite happy to punt on that capability indefinitely.

+        recommended.find(signature[i].ui_data_key) == recommended.end();

[stevenjb, 2011/12/08 02:26:35]

I'm confused by this. Above, we appear to be filling in recommended<> from
dict["Recommended"], which is ONC data. Why are we searching for ui_data_key
here?

+    const Value* default_value = NULL;
+    network->GetProperty(signature[i].index, &default_value);
+    NetworkPropertyUIData property_ui_data(
+        controlled_by_policy ? NetworkPropertyUIData::CONTROLLER_POLICY :
+                               NetworkPropertyUIData::CONTROLLER_USER,
+        default_value ? default_value->DeepCopy() : NULL);
+    ui_data.SetProperty(signature[i].ui_data_key, property_ui_data);

[stevenjb, 2011/12/08 02:26:35]

I can see why it is convenient to save this off, and if we're not going to parse
ONC data on every login, then it makes sense to store it in flimflam, but I
don't understand why we aren't just using signature[i].field.

+  }
+
+  // Merge the result into the network-level |ui_data| dictionary.
+  DictionaryValue ui_data_dict;
+  ui_data.FillDictionary(&ui_data_dict);
+  network->ui_data()->MergeDictionary(&ui_data_dict);
+}
+
 // -------------------- OncWirelessNetworkParser --------------------
 
 OncWirelessNetworkParser::OncWirelessNetworkParser() {}