NetworkChangeNotifierChromeos: Handle IPConfig property changes on the default network

chromeos/network/network_change_notifier_chromeos.cc

Index: chromeos/network/network_change_notifier_chromeos.cc
diff --git a/chromeos/network/network_change_notifier_chromeos.cc b/chromeos/network/network_change_notifier_chromeos.cc
index 2510735562d5b098570b174192672c9fb259553d..859714f1d2a51fb8cdd6f58a26eec776d62ef203 100644
--- a/chromeos/network/network_change_notifier_chromeos.cc
+++ b/chromeos/network/network_change_notifier_chromeos.cc
@@ -111,21 +111,19 @@ void NetworkChangeNotifierChromeos::UpdateState(
   *connection_type_changed = false;
   *ip_address_changed = false;
   *dns_changed = false;
-  // TODO(gauravsh): DNS changes will be detected once ip config
-  // support is hooked into NetworkStateHandler. For now,
-  // we report a DNS change on changes to the default network (including
-  // loss).
   if (!default_network || !default_network->IsConnectedState()) {
     // If we lost a default network, we must update our state and notify
-    // observers, otherwise we have nothing do. (Under normal circumstances,
+    // observers, otherwise we have nothing to do. (Under normal circumstances,
     // we should never get duplicate no default network notifications).
     if (connection_type_ != CONNECTION_NONE) {
+      VLOG(1) << "Lost default network!";
       *ip_address_changed = true;
       *dns_changed = true;
       *connection_type_changed = true;
       connection_type_ = CONNECTION_NONE;
       service_path_.clear();
       ip_address_.clear();
+      dns_servers_.clear();
     }
     return;
   }
@@ -138,20 +136,30 @@ void NetworkChangeNotifierChromeos::UpdateState(
     VLOG(1) << "Connection type changed from " << connection_type_ << " -> "
             << new_connection_type;
     *connection_type_changed = true;
-    *dns_changed = true;
   }
-  if (default_network->path() != service_path_ ||
-      default_network->ip_address() != ip_address_) {
+  if (default_network->path() != service_path_) {
     VLOG(1) << "Service path changed from " << service_path_ << " -> "
             << default_network->path();
+    // If we had a default network service change, network resources
+    // must always be invalidated.
+    *ip_address_changed = true;
+    *dns_changed = true;
+  }
+  if (default_network->ip_address() != ip_address_) {
     VLOG(1) << "IP Address changed from " << ip_address_ << " -> "
             << default_network->ip_address();
     *ip_address_changed = true;
+  }
+  if (default_network->dns_servers() != dns_servers_) {

[pneubeck (no reviews), 2013/03/28 10:37:54]

Not for this CL, but maybe for future changes:
If NetworkState would report which properties have changed, we wouldn't need the
caching of ip_address and dns_servers in this class.

[gauravsh, 2013/03/28 18:48:54]

Perhaps. That'd be fine with the current implementation - but I anticipate that
refinements to the NCN will require us to know the actual values for certain
transitions.

[stevenjb, 2013/03/28 19:27:13]

I would prefer to avoid the bookkeeping overhead of tracking changed values in
NetworkStateHandler; doing that robustly is tricky at best and tends to have a
lot of edge cases. I think it is reasonable and simpler to do that sort of
bookkeeping in the UI code.

+    VLOG(1) <<"DNSservers changed from " << dns_servers_ << " -> "
+            << default_network->dns_servers();
     *dns_changed = true;
   }
+
   connection_type_ = new_connection_type;
   service_path_ = default_network->path();
   ip_address_ = default_network->ip_address();
+  dns_servers_ = default_network->dns_servers();
 }
 
 // static