NetworkChangeNotifierChromeos: Handle IPConfig property changes on the default network

chromeos/network/network_state.cc

Index: chromeos/network/network_state.cc
diff --git a/chromeos/network/network_state.cc b/chromeos/network/network_state.cc
index 761a9859e78cf197b45d2152435f0486b2a22dc2..97aab3994e659e1c25709c3f8ec1b054e92c5100 100644
--- a/chromeos/network/network_state.cc
+++ b/chromeos/network/network_state.cc
@@ -7,6 +7,21 @@
 #include "base/values.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
+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;
+}
+
+}  // namespace
+
 namespace chromeos {
 
 NetworkState::NetworkState(const std::string& path)
@@ -28,6 +43,14 @@ bool NetworkState::PropertyChanged(const std::string& key,
     return GetStringValue(key, value, &connection_state_);
   } else if (key == flimflam::kErrorProperty) {
     return GetStringValue(key, value, &error_);
+  } else if (key == flimflam::kAddressProperty) {
+    return GetStringValue(key, value, &ip_address_);
+  } else if (key == flimflam::kNameServersProperty) {
+    dns_servers_.clear();
+    const base::ListValue* dns_servers;
+    if (value.GetAsList(&dns_servers) &&
+        ConvertListValueToStringVector(*dns_servers, &dns_servers_))
+      return true;
   } else if (key == flimflam::kActivationStateProperty) {
     return GetStringValue(key, value, &activation_state_);
   } else if (key == flimflam::kRoamingStateProperty) {
@@ -58,6 +81,12 @@ void NetworkState::GetProperties(base::DictionaryValue* dictionary) const {
                                             connection_state());
   dictionary->SetStringWithoutPathExpansion(flimflam::kErrorProperty,
                                             error());
+  dictionary->SetStringWithoutPathExpansion(flimflam::kAddressProperty,

[pneubeck (no reviews), 2013/03/29 19:16:53]

My veto here:

The only usage for this function is to obtain a correct subset of a Shill
dictionary which can be translated to "extended" ONC. The result is then
returned by GetState/GetVisibleNetworks of the extension API.
I will not accept to add yet another conditional translation for these new keys.

I see two alternatives which I'm fine with:

- Either remove these two dictionary->Set..  and adapt the comment (that this
function returns only a subset of the properties accepted by Property changed).
This is ok, because we don't have any user of these properties through the
extension API, yet.

- Or make it a correct Shill (nested) dictionary. That was my 1st proposal: A
shill service has a property IPConfig and that in turn has Address and
NameServers.

[stevenjb, 2013/03/29 19:41:25]

OK, now I understand the concern, and agree that this makes sense, lets go with
Philipp's second suggestion.

[gauravsh, 2013/04/01 20:02:06]

Done.

+                                            ip_address());
+  base::ListValue* value = new ListValue;
+  value->AppendStrings(dns_servers());
+  dictionary->SetWithoutPathExpansion(flimflam::kNameServersProperty,
+                                      value);

[stevenjb, 2013/03/29 00:52:10]

nit: one line?

[gauravsh, 2013/04/01 20:02:06]

Done.

   dictionary->SetStringWithoutPathExpansion(flimflam::kActivationStateProperty,
                                             activation_state());
   dictionary->SetStringWithoutPathExpansion(flimflam::kRoamingStateProperty,