NetworkChangeNotifierChromeos: Handle IPConfig property changes on the default network

chromeos/network/network_change_notifier_chromeos.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/network/network_change_notifier_chromeos.h"
@@ skipping 93 matching lines @@
 }
 
 void NetworkChangeNotifierChromeos::UpdateState(
     const chromeos::NetworkState* default_network,
     bool* connection_type_changed,
     bool* ip_address_changed,
     bool* dns_changed) {
   *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;
   }
 
   // We do have a default network and it is connected.
   net::NetworkChangeNotifier::ConnectionType new_connection_type =
       ConnectionTypeFromShill(default_network->type(),
                               default_network->technology());
   if (new_connection_type != connection_type_) {
     VLOG(1) << "Connection type changed from " << connection_type_ << " -> "
             << new_connection_type;
     *connection_type_changed = true;
+  }
+  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->path() != service_path_ ||
-      default_network->ip_address() != ip_address_) {
-    VLOG(1) << "Service path changed from " << service_path_ << " -> "
-            << default_network->path();
+  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
 net::NetworkChangeNotifier::ConnectionType
 NetworkChangeNotifierChromeos::ConnectionTypeFromShill(
     const std::string& type, const std::string& technology) {
   if (type == flimflam::kTypeEthernet)
     return CONNECTION_ETHERNET;
   else if (type == flimflam::kTypeWifi)
     return CONNECTION_WIFI;
@@ skipping 27 matching lines @@
   params.ip_address_offline_delay_ = base::TimeDelta::FromMilliseconds(4000);
   params.ip_address_online_delay_ = base::TimeDelta::FromMilliseconds(1000);
   params.connection_type_offline_delay_ =
       base::TimeDelta::FromMilliseconds(500);
   params.connection_type_online_delay_ = base::TimeDelta::FromMilliseconds(500);
   return params;
 }
 
 }  // namespace chromeos