NetworkChangeNotifierChromeos: Handle IPConfig property changes on the default network

BUG=164501
TEST=verify that dns refreshes while in connected state result in a dns change.


Review URL: https://chromiumcodereview.appspot.com/12634019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@191929 0039d316-1c4b-4281-b951-d872f2087c98

19 files changed, 397 insertions, 157 deletions

diff --git a/chromeos/dbus/shill_ipconfig_client_stub.cc b/chromeos/dbus/shill_ipconfig_client_stub.cc
index c78605d..cac80b4 100644
--- a/chromeos/dbus/shill_ipconfig_client_stub.cc
+++ b/chromeos/dbus/shill_ipconfig_client_stub.cc
@@ -43,10 +43,15 @@ void ShillIPConfigClientStub::GetProperties(
     const DictionaryValueCallback& callback) {
   if (callback.is_null())
     return;
+  const base::DictionaryValue* dict = NULL;
+  if (!ipconfigs_.GetDictionaryWithoutPathExpansion(ipconfig_path.value(),
+                                                    &dict))
+    return;
   MessageLoop::current()->PostTask(
-      FROM_HERE, base::Bind(&ShillIPConfigClientStub::PassProperties,
-                            weak_ptr_factory_.GetWeakPtr(),
-                            callback));
+        FROM_HERE, base::Bind(&ShillIPConfigClientStub::PassProperties,
+                              weak_ptr_factory_.GetWeakPtr(),
+                              dict,
+                              callback));
 }
 
 base::DictionaryValue* ShillIPConfigClientStub::CallGetPropertiesAndBlock(
@@ -61,6 +66,18 @@ void ShillIPConfigClientStub::SetProperty(
     const VoidDBusMethodCallback& callback) {
   if (callback.is_null())
     return;
+  base::DictionaryValue* dict = NULL;
+  if (ipconfigs_.GetDictionaryWithoutPathExpansion(ipconfig_path.value(),
+                                                   &dict)) {
+    // Update existing ip config stub object's properties.
+    dict->SetWithoutPathExpansion(name, value.DeepCopy());
+  } else {
+    // Create a new stub ipconfig object, and update its properties.
+    DictionaryValue* dvalue = new DictionaryValue;
+    dvalue->SetWithoutPathExpansion(name, value.DeepCopy());
+    ipconfigs_.SetWithoutPathExpansion(ipconfig_path.value(),
+                                       dvalue);
+  }
   MessageLoop::current()->PostTask(
       FROM_HERE, base::Bind(callback, DBUS_METHOD_CALL_SUCCESS));
 }
@@ -84,8 +101,9 @@ void ShillIPConfigClientStub::Remove(const dbus::ObjectPath& ipconfig_path,
 }
 
 void ShillIPConfigClientStub::PassProperties(
+    const base::DictionaryValue* values,
     const DictionaryValueCallback& callback) const {
-  callback.Run(DBUS_METHOD_CALL_SUCCESS, properties_);
+  callback.Run(DBUS_METHOD_CALL_SUCCESS, *values);
 }
 
 }  // namespace chromeos
diff --git a/chromeos/dbus/shill_ipconfig_client_stub.h b/chromeos/dbus/shill_ipconfig_client_stub.h
index 1325bba..bc77486 100644
--- a/chromeos/dbus/shill_ipconfig_client_stub.h
+++ b/chromeos/dbus/shill_ipconfig_client_stub.h
@@ -42,10 +42,12 @@ class ShillIPConfigClientStub : public ShillIPConfigClient {
                       const VoidDBusMethodCallback& callback) OVERRIDE;
 
  private:
-  // Runs callback with |properties_|.
-  void PassProperties(const DictionaryValueCallback& callback) const;
+  // Runs callback with |values|.
+  void PassProperties(const base::DictionaryValue* values,
+                      const DictionaryValueCallback& callback) const;
 
-  base::DictionaryValue properties_;
+  // Dictionary of <ipconfig_path, property dictionaries>
+  base::DictionaryValue ipconfigs_;
 
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
diff --git a/chromeos/dbus/shill_manager_client_stub.cc b/chromeos/dbus/shill_manager_client_stub.cc
index 468c727b..f19c5a5 100644
--- a/chromeos/dbus/shill_manager_client_stub.cc
+++ b/chromeos/dbus/shill_manager_client_stub.cc
@@ -176,8 +176,12 @@ void ShillManagerClientStub::ConfigureService(
     return;
   }
 
+  std::string ipconfig_path;
+  properties.GetString(shill::kIPConfigProperty, &ipconfig_path);
+
   // Add the service to the service client stub if not already there.
-  service_client->AddService(guid, guid, type, flimflam::kStateIdle, true);
+  service_client->AddServiceWithIPConfig(guid, guid, type, flimflam::kStateIdle,
+                                         ipconfig_path, true);
 
   // Merge the new properties with existing properties, if any.
   scoped_ptr<base::DictionaryValue> merged_properties;
diff --git a/chromeos/dbus/shill_manager_client_stub.h b/chromeos/dbus/shill_manager_client_stub.h
index 1eb666d..eebea36 100644
--- a/chromeos/dbus/shill_manager_client_stub.h
+++ b/chromeos/dbus/shill_manager_client_stub.h
@@ -98,9 +98,9 @@ class ShillManagerClientStub : public ShillManagerClient,
   virtual void RemoveService(const std::string& service_path) OVERRIDE;
   virtual void AddTechnology(const std::string& type, bool enabled) OVERRIDE;
   virtual void RemoveTechnology(const std::string& type) OVERRIDE;
-  virtual void ClearProperties() OVERRIDE;
   virtual void AddGeoNetwork(const std::string& technology,
                              const base::DictionaryValue& network) OVERRIDE;
+  virtual void ClearProperties() OVERRIDE;
 
  private:
   void AddServiceToWatchList(const std::string& service_path);
diff --git a/chromeos/dbus/shill_service_client.h b/chromeos/dbus/shill_service_client.h
index 7f9e9b1..05713be 100644
--- a/chromeos/dbus/shill_service_client.h
+++ b/chromeos/dbus/shill_service_client.h
@@ -49,6 +49,12 @@ class CHROMEOS_EXPORT ShillServiceClient {
                             const std::string& type,
                             const std::string& state,
                             bool add_to_watch_list) = 0;
+    virtual void AddServiceWithIPConfig(const std::string& service_path,
+                                        const std::string& name,
+                                        const std::string& type,
+                                        const std::string& state,
+                                        const std::string& ipconfig_path,
+                                        bool add_to_watch_list) = 0;
     virtual void RemoveService(const std::string& service_path) = 0;
     virtual void SetServiceProperty(const std::string& service_path,
                                     const std::string& property,
diff --git a/chromeos/dbus/shill_service_client_stub.cc b/chromeos/dbus/shill_service_client_stub.cc
index 0a380bc..3b8f1de 100644
--- a/chromeos/dbus/shill_service_client_stub.cc
+++ b/chromeos/dbus/shill_service_client_stub.cc
@@ -254,6 +254,17 @@ void ShillServiceClientStub::AddService(const std::string& service_path,
                                         const std::string& type,
                                         const std::string& state,
                                         bool add_to_watch_list) {
+  AddServiceWithIPConfig(service_path, name, type, state, "",
+                         add_to_watch_list);
+}
+
+void ShillServiceClientStub::AddServiceWithIPConfig(
+    const std::string& service_path,
+    const std::string& name,
+    const std::string& type,
+    const std::string& state,
+    const std::string& ipconfig_path,
+    bool add_to_watch_list) {
   DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()->
       AddService(service_path, add_to_watch_list);
 
@@ -271,6 +282,10 @@ void ShillServiceClientStub::AddService(const std::string& service_path,
   properties->SetWithoutPathExpansion(
       flimflam::kStateProperty,
       base::Value::CreateStringValue(state));
+  if (!ipconfig_path.empty())
+    properties->SetWithoutPathExpansion(
+        shill::kIPConfigProperty,
+        base::Value::CreateStringValue(ipconfig_path));
 }
 
 void ShillServiceClientStub::RemoveService(const std::string& service_path) {
diff --git a/chromeos/dbus/shill_service_client_stub.h b/chromeos/dbus/shill_service_client_stub.h
index d248494..507f95f 100644
--- a/chromeos/dbus/shill_service_client_stub.h
+++ b/chromeos/dbus/shill_service_client_stub.h
@@ -73,6 +73,12 @@ class ShillServiceClientStub : public ShillServiceClient,
                           const std::string& type,
                           const std::string& state,
                           bool add_to_watch_list) OVERRIDE;
+  virtual void AddServiceWithIPConfig(const std::string& service_path,
+                                      const std::string& name,
+                                      const std::string& type,
+                                      const std::string& state,
+                                      const std::string& ipconfig_path,
+                                      bool add_to_watch_list) OVERRIDE;
   virtual void RemoveService(const std::string& service_path) OVERRIDE;
   virtual void SetServiceProperty(const std::string& service_path,
                                   const std::string& property,
diff --git a/chromeos/network/network_change_notifier_chromeos.cc b/chromeos/network/network_change_notifier_chromeos.cc
index 2510735..224930c 100644
--- a/chromeos/network/network_change_notifier_chromeos.cc
+++ b/chromeos/network/network_change_notifier_chromeos.cc
@@ -6,6 +6,7 @@
 
 #include "base/basictypes.h"
 #include "base/bind.h"
+#include "base/string_util.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/network/network_change_notifier_chromeos.h"
 #include "chromeos/network/network_state.h"
@@ -111,21 +112,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 +137,33 @@ 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_) {
+    VLOG(1) << "DNS servers changed.\n"
+            << "Old DNS servers were: "
+            << JoinString(dns_servers_, ",") << "\n"
+            << "New DNS servers are: "
+            << JoinString(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
diff --git a/chromeos/network/network_change_notifier_chromeos.h b/chromeos/network/network_change_notifier_chromeos.h
index 6bd8edeb6..aabfddc 100644
--- a/chromeos/network/network_change_notifier_chromeos.h
+++ b/chromeos/network/network_change_notifier_chromeos.h
@@ -73,6 +73,8 @@ class CHROMEOS_EXPORT NetworkChangeNotifierChromeos
   NetworkChangeNotifier::ConnectionType connection_type_;
   // IP address for the current default network.
   std::string ip_address_;
+  // DNS servers for the current default network.
+  std::vector<std::string> dns_servers_;
   // Service path for the current default network.
   std::string service_path_;
 
diff --git a/chromeos/network/network_change_notifier_chromeos_unittest.cc b/chromeos/network/network_change_notifier_chromeos_unittest.cc
index 1771322..371d6f6 100644
--- a/chromeos/network/network_change_notifier_chromeos_unittest.cc
+++ b/chromeos/network/network_change_notifier_chromeos_unittest.cc
@@ -7,6 +7,7 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/strings/string_split.h"
 #include "chromeos/network/network_change_notifier_factory_chromeos.h"
 #include "chromeos/network/network_state.h"
 #include "net/base/network_change_notifier.h"
@@ -15,6 +16,44 @@
 
 namespace chromeos {
 
+namespace {
+
+const char kDnsServers1[] = "192.168.0.1,192.168.0.2";
+const char kDnsServers2[] = "192.168.3.1,192.168.3.2";
+const char kIpAddress1[] = "192.168.1.1";
+const char kIpAddress2[] = "192.168.1.2";
+const char kService1[] = "/service/1";
+const char kService2[] = "/service/2";
+const char kService3[] = "/service/3";
+
+struct NotifierState {
+  net::NetworkChangeNotifier::ConnectionType type;
+  const char* service_path;
+  const char* ip_address;
+  const char* dns_servers;
+};
+
+struct DefaultNetworkState {
+  bool is_connected;
+  const char* type;
+  const char* technology;
+  const char* service_path;
+  const char* ip_address;
+  const char* dns_servers;
+};
+
+struct NotifierUpdateTestCase {
+  const char* test_description;
+  NotifierState initial_state;
+  DefaultNetworkState default_network_state;
+  NotifierState expected_state;
+  bool expected_type_changed;
+  bool expected_ip_changed;
+  bool expected_dns_changed;
+};
+
+} // namespace
+
 using net::NetworkChangeNotifier;
 
 TEST(NetworkChangeNotifierChromeosTest, ConnectionTypeFromShill) {
@@ -67,42 +106,44 @@ class NetworkChangeNotifierChromeosUpdateTest : public testing::Test {
   }
   virtual ~NetworkChangeNotifierChromeosUpdateTest() {}
 
-  void SetNotifierState(NetworkChangeNotifier::ConnectionType type,
-                        std::string service_path,
-                        std::string ip_address) {
-    notifier_.ip_address_ = ip_address;
-    notifier_.service_path_ = service_path;
-    notifier_.connection_type_ = type;
+  void SetNotifierState(const NotifierState& notifier_state) {
+    notifier_.connection_type_ = notifier_state.type;
+    notifier_.service_path_ = notifier_state.service_path;
+    notifier_.ip_address_ = notifier_state.ip_address;
+    std::vector<std::string> dns_servers;
+    base::SplitString(notifier_state.dns_servers, ',', &dns_servers);
+    notifier_.dns_servers_ = dns_servers;
   }
 
-  void VerifyNotifierState(NetworkChangeNotifier::ConnectionType expected_type,
-                           std::string expected_service_path,
-                           std::string expected_ip_address) {
-    EXPECT_EQ(expected_type, notifier_.connection_type_);
-    EXPECT_EQ(expected_ip_address, notifier_.ip_address_);
-    EXPECT_EQ(expected_service_path, notifier_.service_path_);
+  void VerifyNotifierState(const NotifierState& notifier_state) {
+    EXPECT_EQ(notifier_state.type, notifier_.connection_type_);
+    EXPECT_EQ(notifier_state.service_path, notifier_.service_path_);
+    EXPECT_EQ(notifier_state.ip_address, notifier_.ip_address_);
+    std::vector<std::string> dns_servers;
+    base::SplitString(notifier_state.dns_servers, ',', &dns_servers);
+    EXPECT_EQ(dns_servers, notifier_.dns_servers_);
   }
 
   // Sets the default network state used for notifier updates.
-  void SetDefaultNetworkState(bool is_connected,
-                             std::string type,
-                             std::string technology,
-                             std::string service_path,
-                             std::string ip_address) {
-    if (is_connected)
+  void SetDefaultNetworkState(
+      const DefaultNetworkState& default_network_state) {
+    if (default_network_state.is_connected)
       default_network_.connection_state_ = flimflam::kStateOnline;
     else
       default_network_.connection_state_ = flimflam::kStateConfiguration;
-    default_network_.type_ = type;
-    default_network_.technology_ = technology;
-    default_network_.path_ = service_path;
-    default_network_.ip_address_ = ip_address;
+    default_network_.type_ = default_network_state.type;
+    default_network_.technology_ = default_network_state.technology;
+    default_network_.path_ = default_network_state.service_path;
+    default_network_.set_ip_address(default_network_state.ip_address);
+    std::vector<std::string> dns_servers;
+    base::SplitString(default_network_state.dns_servers, ',', &dns_servers);
+    default_network_.set_dns_servers(dns_servers);
   }
 
   // Process an default network update based on the state of |default_network_|.
   void ProcessDefaultNetworkUpdate(bool* type_changed,
-                                  bool* ip_changed,
-                                  bool* dns_changed) {
+                                   bool* ip_changed,
+                                   bool* dns_changed) {
     notifier_.UpdateState(&default_network_, type_changed, ip_changed,
                           dns_changed);
   }
@@ -112,74 +153,74 @@ class NetworkChangeNotifierChromeosUpdateTest : public testing::Test {
   NetworkChangeNotifierChromeos notifier_;
 };
 
-TEST_F(NetworkChangeNotifierChromeosUpdateTest, UpdateDefaultNetworkOffline) {
-  // Test that Online to Offline transitions are correctly handled.
-  SetNotifierState(NetworkChangeNotifier::CONNECTION_ETHERNET, "/service/1",
-                   "192.168.1.1");
-  SetDefaultNetworkState(false,  // offline.
-                        flimflam::kTypeEthernet, "", "/service/1", "");
-  bool type_changed = false, ip_changed = false, dns_changed = false;
-  ProcessDefaultNetworkUpdate(&type_changed, &ip_changed, &dns_changed);
-  VerifyNotifierState(NetworkChangeNotifier::CONNECTION_NONE, "", "");
-  EXPECT_TRUE(type_changed);
-  EXPECT_TRUE(ip_changed);
-  EXPECT_TRUE(dns_changed);
-}
-
-TEST_F(NetworkChangeNotifierChromeosUpdateTest, UpdateDefaultNetworkOnline) {
-  // Test that Offline to Online transitions are correctly handled.
-  SetNotifierState(NetworkChangeNotifier::CONNECTION_NONE, "", "");
-
-  SetDefaultNetworkState(false,  // offline.
-                        flimflam::kTypeEthernet, "",
-                        "192.168.0.1", "/service/1");
-  bool type_changed = false, ip_changed = false, dns_changed = false;
-  ProcessDefaultNetworkUpdate(&type_changed, &ip_changed, &dns_changed);
-  // If the new default network is still offline, nothing should have changed.
-  VerifyNotifierState(NetworkChangeNotifier::CONNECTION_NONE, "", "");
-  EXPECT_FALSE(type_changed);
-  EXPECT_FALSE(ip_changed);
-  EXPECT_FALSE(dns_changed);
-
-  SetDefaultNetworkState(true,  // online.
-                        flimflam::kTypeEthernet, "", "/service/1",
-                        "192.168.0.1");
-  ProcessDefaultNetworkUpdate(&type_changed, &ip_changed, &dns_changed);
-  // Now the new default network is online, so this should trigger a notifier
-  // state change.
-  VerifyNotifierState(NetworkChangeNotifier::CONNECTION_ETHERNET, "/service/1",
-                      "192.168.0.1");
-  EXPECT_TRUE(type_changed);
-  EXPECT_TRUE(ip_changed);
-  EXPECT_TRUE(dns_changed);
-}
+NotifierUpdateTestCase test_cases[] = {
+  { "Online -> Offline",
+    { NetworkChangeNotifier::CONNECTION_ETHERNET, kService1, kIpAddress1,
+      kDnsServers1 },
+    { false, flimflam::kTypeEthernet, "", kService1, "", "" },
+    { NetworkChangeNotifier::CONNECTION_NONE, "", "", "" },
+    true, true, true
+  },
+  { "Offline -> Offline",
+    { NetworkChangeNotifier::CONNECTION_NONE, "", "", "" },
+    { false, flimflam::kTypeEthernet, "", kService1, kIpAddress1,
+      kDnsServers1 },
+    { NetworkChangeNotifier::CONNECTION_NONE, "", "", "" },
+    false, false, false
+  },
+  { "Offline -> Online",
+    { NetworkChangeNotifier::CONNECTION_NONE, "", "", "" },
+    { true, flimflam::kTypeEthernet, "", kService1, kIpAddress1, kDnsServers1 },
+    { NetworkChangeNotifier::CONNECTION_ETHERNET, kService1, kIpAddress1,
+      kDnsServers1 },
+    true, true, true
+  },
+  { "Online -> Online (new default service, different connection type)",
+    { NetworkChangeNotifier::CONNECTION_ETHERNET, kService1, kIpAddress1,
+      kDnsServers1 },
+    { true, flimflam::kTypeWifi, "", kService2, kIpAddress1, kDnsServers1 },
+    { NetworkChangeNotifier::CONNECTION_WIFI, kService2, kIpAddress1,
+      kDnsServers1 },
+    true, true, true
+  },
+  { "Online -> Online (new default service, same connection type)",
+    { NetworkChangeNotifier::CONNECTION_WIFI, kService2, kIpAddress1,
+      kDnsServers1 },
+    { true, flimflam::kTypeWifi, "", kService3, kIpAddress1, kDnsServers1 },
+    { NetworkChangeNotifier::CONNECTION_WIFI, kService3, kIpAddress1,
+      kDnsServers1 },
+    false, true, true
+  },
+  { "Online -> Online (same default service, new IP address, same DNS)",
+    { NetworkChangeNotifier::CONNECTION_WIFI, kService3, kIpAddress1,
+      kDnsServers1 },
+    { true, flimflam::kTypeWifi, "", kService3, kIpAddress2, kDnsServers1 },
+    { NetworkChangeNotifier::CONNECTION_WIFI, kService3, kIpAddress2,
+      kDnsServers1 },
+    false, true, false
+  },
+  { "Online -> Online (same default service, same IP address, new DNS)",
+    { NetworkChangeNotifier::CONNECTION_WIFI, kService3, kIpAddress2,
+      kDnsServers1 },
+    { true, flimflam::kTypeWifi, "", kService3, kIpAddress2, kDnsServers2 },
+    { NetworkChangeNotifier::CONNECTION_WIFI, kService3, kIpAddress2,
+      kDnsServers2 },
+    false, false, true
+  }
+};
 
-TEST_F(NetworkChangeNotifierChromeosUpdateTest, UpdateDefaultNetworkChanged) {
-  // Test that Online to Online transitions (default network changes) are
-  // correctly handled.
-  SetNotifierState(NetworkChangeNotifier::CONNECTION_ETHERNET, "/service/1",
-                   "192.168.1.1");
-
-  SetDefaultNetworkState(true,  // online.
-                        flimflam::kTypeWifi, "", "/service/2", "192.168.1.2");
-  bool type_changed = false, ip_changed = false, dns_changed = false;
-  ProcessDefaultNetworkUpdate(&type_changed, &ip_changed, &dns_changed);
-  VerifyNotifierState(NetworkChangeNotifier::CONNECTION_WIFI, "/service/2",
-                      "192.168.1.2" );
-  EXPECT_TRUE(type_changed);
-  EXPECT_TRUE(ip_changed);
-  EXPECT_TRUE(dns_changed);
-
-  SetDefaultNetworkState(true,  // online.
-                        flimflam::kTypeWifi, "", "/service/3", "192.168.1.2");
-  ProcessDefaultNetworkUpdate(&type_changed, &ip_changed, &dns_changed);
-  VerifyNotifierState(NetworkChangeNotifier::CONNECTION_WIFI, "/service/3",
-                      "192.168.1.2" );
-  EXPECT_FALSE(type_changed);
-  // A service path change (even with a corresponding IP change) should still
-  // trigger an IP address update to observers.
-  EXPECT_TRUE(ip_changed);
-  EXPECT_TRUE(dns_changed);
+TEST_F(NetworkChangeNotifierChromeosUpdateTest, UpdateDefaultNetwork) {
+  for (size_t i = 0; i < arraysize(test_cases); ++i) {
+    SCOPED_TRACE(test_cases[i].test_description);
+    SetNotifierState(test_cases[i].initial_state);
+    SetDefaultNetworkState(test_cases[i].default_network_state);
+    bool type_changed = false, ip_changed = false, dns_changed = false;
+    ProcessDefaultNetworkUpdate(&type_changed, &ip_changed, &dns_changed);
+    VerifyNotifierState(test_cases[i].expected_state);
+    EXPECT_TRUE(type_changed == test_cases[i].expected_type_changed);
+    EXPECT_TRUE(ip_changed == test_cases[i].expected_ip_changed);
+    EXPECT_TRUE(dns_changed == test_cases[i].expected_dns_changed);
+  }
 }
 
 }  // namespace chromeos
diff --git a/chromeos/network/network_state.cc b/chromeos/network/network_state.cc
index bac4c60..8355028 100644
--- a/chromeos/network/network_state.cc
+++ b/chromeos/network/network_state.cc
@@ -4,9 +4,25 @@
 
 #include "chromeos/network/network_state.h"
 
+#include "base/stringprintf.h"
 #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)
@@ -33,6 +49,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 == IPConfigProperty(flimflam::kAddressProperty)) {
+    return GetStringValue(key, value, &ip_address_);
+  } else if (key == IPConfigProperty(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) {
@@ -69,6 +93,16 @@ void NetworkState::GetProperties(base::DictionaryValue* dictionary) const {
                                             connection_state());
   dictionary->SetStringWithoutPathExpansion(flimflam::kErrorProperty,
                                             error());
+  base::DictionaryValue* ipconfig_properties = new DictionaryValue;
+  ipconfig_properties->SetStringWithoutPathExpansion(flimflam::kAddressProperty,
+                                                     ip_address());
+  base::ListValue* name_servers = new ListValue;
+  name_servers->AppendStrings(dns_servers());
+  ipconfig_properties->SetWithoutPathExpansion(flimflam::kNameServersProperty,
+                                               name_servers);
+  dictionary->SetWithoutPathExpansion(shill::kIPConfigProperty,
+                                      ipconfig_properties);
+
   dictionary->SetStringWithoutPathExpansion(flimflam::kActivationStateProperty,
                                             activation_state());
   dictionary->SetStringWithoutPathExpansion(flimflam::kRoamingStateProperty,
@@ -116,4 +150,9 @@ bool NetworkState::StateIsConnecting(const std::string& connection_state) {
           connection_state == flimflam::kStateCarrier);
 }
 
+// static
+std::string NetworkState::IPConfigProperty(const char* key) {
+  return base::StringPrintf("%s.%s", shill::kIPConfigProperty, key);
+}
+
 }  // namespace chromeos
diff --git a/chromeos/network/network_state.h b/chromeos/network/network_state.h
index 9959824..d43def6 100644
--- a/chromeos/network/network_state.h
+++ b/chromeos/network/network_state.h
@@ -5,6 +5,9 @@
 #ifndef CHROMEOS_NETWORK_NETWORK_STATE_H_
 #define CHROMEOS_NETWORK_NETWORK_STATE_H_
 
+#include <string>
+#include <vector>
+
 #include "chromeos/network/managed_state.h"
 
 namespace base {
@@ -36,6 +39,7 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
   // Accessors
   const std::string& security() const { return security_; }
   const std::string& ip_address() const { return ip_address_; }
+  const std::vector<std::string>& dns_servers() const { return dns_servers_; }
   const std::string& device_path() const { return device_path_; }
   const std::string& guid() const { return guid_; }
   const std::string& connection_state() const { return connection_state_; }
@@ -61,6 +65,10 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
   static bool StateIsConnected(const std::string& connection_state);
   static bool StateIsConnecting(const std::string& connection_state);
 
+  // Helper to return a full prefixed version of an IPConfig property
+  // key.
+  static std::string IPConfigProperty(const char* key);
+
  private:
   friend class NetworkStateHandler;
   friend class NetworkChangeNotifierChromeosUpdateTest;
@@ -69,17 +77,24 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
   void set_ip_address(const std::string& ip_address) {
     ip_address_ = ip_address;
   }
+  void set_dns_servers(const std::vector<std::string>& dns_servers) {
+    dns_servers_ = dns_servers;
+  }
 
   // Common Network Service properties
   std::string security_;
   std::string device_path_;
   std::string guid_;
-  std::string ip_address_;
   std::string connection_state_;
   std::string error_;
   bool auto_connect_;
   bool favorite_;
   int priority_;
+  // IPConfig properties.
+  // Note: These do not correspond to actual Shill.Service properties
+  // but are derived from the service's corresponding IPConfig object.
+  std::string ip_address_;
+  std::vector<std::string> dns_servers_;
   // Wireless properties
   int signal_strength_;
   // Cellular properties
diff --git a/chromeos/network/network_state_handler.cc b/chromeos/network/network_state_handler.cc
index be8696e..e623b0e 100644
--- a/chromeos/network/network_state_handler.cc
+++ b/chromeos/network/network_state_handler.cc
@@ -433,20 +433,16 @@ void NetworkStateHandler::UpdateNetworkServiceProperty(
     detail += " = " + vstr;
   network_event_log::AddEntry(kLogModule, "NetworkPropertyUpdated", detail);
 
-  if (network->connection_state() != prev_connection_state)
+  if (network->connection_state() != prev_connection_state) {
     OnNetworkConnectionStateChanged(network);
-  NetworkPropertiesUpdated(network);
-}
+  }
+  else if (network->path() == default_network_path_ &&
+           key != flimflam::kSignalStrengthProperty) {
+    // WiFi signal strength updates are too noisy, so don't
+    // trigger default network updates for those changes.
+    OnDefaultNetworkChanged();
+  }
 
-void NetworkStateHandler::UpdateNetworkServiceIPAddress(
-    const std::string& service_path,
-    const std::string& ip_address) {
-  NetworkState* network = GetModifiableNetworkState(service_path);
-  if (!network)
-    return;
-  std::string detail = network->name() + ".IPAddress = " + ip_address;
-  network_event_log::AddEntry(kLogModule, "NetworkIPChanged", detail);
-  network->set_ip_address(ip_address);
   NetworkPropertiesUpdated(network);
 }
 
diff --git a/chromeos/network/network_state_handler.h b/chromeos/network/network_state_handler.h
index ce65e0d..534b936 100644
--- a/chromeos/network/network_state_handler.h
+++ b/chromeos/network/network_state_handler.h
@@ -186,11 +186,6 @@ class CHROMEOS_EXPORT NetworkStateHandler
       const std::string& key,
       const base::Value& value) OVERRIDE;
 
-  // Sets the IP Address for the network associated with |service_path|.
-  virtual void UpdateNetworkServiceIPAddress(
-      const std::string& service_path,
-      const std::string& ip_address) OVERRIDE;
-
   // Called by ShillPropertyHandler when a watched device property changes.
   virtual void UpdateDeviceProperty(
       const std::string& device_path,
diff --git a/chromeos/network/network_state_handler_observer.h b/chromeos/network/network_state_handler_observer.h
index a23076c..d5f134b 100644
--- a/chromeos/network/network_state_handler_observer.h
+++ b/chromeos/network/network_state_handler_observer.h
@@ -33,8 +33,11 @@ class CHROMEOS_EXPORT NetworkStateHandlerObserver {
   // The list of devices changed, or a property changed (e.g. scanning).
   virtual void DeviceListChanged();
 
-  // The default network changed (includes VPNs) or its connection state
-  // changed. |network| will be NULL if there is no longer a default network.
+  // The default network changed (includes VPNs) or one of its properties
+  // changed. This won't be called if the WiFi signal strength property
+  // changes. If interested in those events, use NetworkPropertiesUpdated()
+  // below.
+  // |network| will be NULL if there is no longer a default network.
   virtual void DefaultNetworkChanged(const NetworkState* network);
 
   // The connection state of |network| changed.
diff --git a/chromeos/network/network_state_handler_unittest.cc b/chromeos/network/network_state_handler_unittest.cc
index dc54c31..685d913 100644
--- a/chromeos/network/network_state_handler_unittest.cc
+++ b/chromeos/network/network_state_handler_unittest.cc
@@ -42,7 +42,8 @@ class TestObserver : public chromeos::NetworkStateHandlerObserver {
   explicit TestObserver(NetworkStateHandler* handler)
       : handler_(handler),
         manager_changed_count_(0),
-        network_count_(0) {
+        network_count_(0),
+        default_network_change_count_(0) {
   }
 
   virtual ~TestObserver() {
@@ -63,6 +64,7 @@ class TestObserver : public chromeos::NetworkStateHandlerObserver {
   }
 
   virtual void DefaultNetworkChanged(const NetworkState* network) OVERRIDE {
+    ++default_network_change_count_;
     default_network_ = network ? network->path() : "";
     default_network_connection_state_ =
         network ?  network->connection_state() : "";
@@ -81,6 +83,9 @@ class TestObserver : public chromeos::NetworkStateHandlerObserver {
 
   size_t manager_changed_count() { return manager_changed_count_; }
   size_t network_count() { return network_count_; }
+  size_t default_network_change_count() {
+    return default_network_change_count_;
+  }
   std::string default_network() { return default_network_; }
   std::string default_network_connection_state() {
     return default_network_connection_state_;
@@ -103,6 +108,7 @@ class TestObserver : public chromeos::NetworkStateHandlerObserver {
   NetworkStateHandler* handler_;
   size_t manager_changed_count_;
   size_t network_count_;
+  size_t default_network_change_count_;
   std::string default_network_;
   std::string default_network_connection_state_;
   std::map<std::string, int> property_updates_;
@@ -282,6 +288,26 @@ TEST_F(NetworkStateHandlerTest, DefaultServiceChanged) {
   EXPECT_EQ(wifi1, test_observer_->default_network());
   EXPECT_EQ(flimflam::kStateOnline,
             test_observer_->default_network_connection_state());
+  // We should have seen 2 default network updates - for the default
+  // service change, and for the state change.
+  EXPECT_EQ(2u, test_observer_->default_network_change_count());
+
+  // Updating a property on the default network should trigger
+  // a default network change.
+  DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
+      dbus::ObjectPath(wifi1),
+      flimflam::kSecurityProperty, base::StringValue("TestSecurity"),
+      base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(3u, test_observer_->default_network_change_count());
+
+  // No default network updates for signal strength changes.
+  DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
+      dbus::ObjectPath(wifi1),
+      flimflam::kSignalStrengthProperty, base::FundamentalValue(32),
+      base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(3u, test_observer_->default_network_change_count());
 }
 
 }  // namespace chromeos
diff --git a/chromeos/network/shill_property_handler.cc b/chromeos/network/shill_property_handler.cc
index 2041139..6749518 100644
--- a/chromeos/network/shill_property_handler.cc
+++ b/chromeos/network/shill_property_handler.cc
@@ -16,6 +16,7 @@
 #include "chromeos/dbus/shill_manager_client.h"
 #include "chromeos/dbus/shill_service_client.h"
 #include "chromeos/network/network_event_log.h"
+#include "chromeos/network/network_state.h"
 #include "dbus/object_path.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
@@ -415,7 +416,7 @@ void ShillPropertyHandler::NetworkServicePropertyChangedCallback(
     const std::string& key,
     const base::Value& value) {
   if (key == shill::kIPConfigProperty) {
-    // Handle IPConfig here and call listener_->UpdateNetworkServiceIPAddress
+    // Request the IPConfig for the network and update network properties
     // when the request completes.
     std::string ip_config_path;
     value.GetAsString(&ip_config_path);
@@ -434,16 +435,30 @@ void ShillPropertyHandler::GetIPConfigCallback(
     DBusMethodCallStatus call_status,
     const base::DictionaryValue& properties)  {
   if (call_status != DBUS_METHOD_CALL_SUCCESS) {
-    LOG(ERROR) << "Failed to get IP properties for: " << service_path;
+    LOG(ERROR) << "Failed to get IP Config properties for: " << service_path;
     return;
   }
-  std::string ip_address;
-  if (!properties.GetStringWithoutPathExpansion(flimflam::kAddressProperty,
-                                                &ip_address)) {
+  const base::Value* ip_address;
+  if (!properties.GetWithoutPathExpansion(flimflam::kAddressProperty,
+                                          &ip_address)) {
     LOG(ERROR) << "Failed to get IP Address property for: " << service_path;
     return;
   }
-  listener_->UpdateNetworkServiceIPAddress(service_path, ip_address);
+  listener_->UpdateNetworkServiceProperty(
+      service_path,
+      NetworkState::IPConfigProperty(flimflam::kAddressProperty),
+      *ip_address);
+
+  const base::Value* dns_servers = NULL;
+  if (!properties.GetWithoutPathExpansion(
+          flimflam::kNameServersProperty, &dns_servers)) {
+    LOG(ERROR) << "Failed to get Name servers property for: " << service_path;
+    return;
+  }
+  listener_->UpdateNetworkServiceProperty(
+      service_path,
+      NetworkState::IPConfigProperty(flimflam::kNameServersProperty),
+      *dns_servers);
 }
 
 void ShillPropertyHandler::NetworkDevicePropertyChangedCallback(
diff --git a/chromeos/network/shill_property_handler.h b/chromeos/network/shill_property_handler.h
index 154a656..ff0ea1d 100644
--- a/chromeos/network/shill_property_handler.h
+++ b/chromeos/network/shill_property_handler.h
@@ -72,12 +72,6 @@ class CHROMEOS_EXPORT ShillPropertyHandler
     // changes.
     virtual void ManagerPropertyChanged() = 0;
 
-    // Called whent the IP address of a service has been updated. Occurs after
-    // UpdateManagedStateProperties is called for the service.
-    virtual void UpdateNetworkServiceIPAddress(
-        const std::string& service_path,
-        const std::string& ip_address) = 0;
-
     // Called when a managed state list has changed, after properties for any
     // new entries in the list have been received and
     // UpdateManagedStateProperties has been called for each new entry.
diff --git a/chromeos/network/shill_property_handler_unittest.cc b/chromeos/network/shill_property_handler_unittest.cc
index e151105..01fb282 100644
--- a/chromeos/network/shill_property_handler_unittest.cc
+++ b/chromeos/network/shill_property_handler_unittest.cc
@@ -14,6 +14,7 @@
 #include "base/values.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_device_client.h"
+#include "chromeos/dbus/shill_ipconfig_client.h"
 #include "chromeos/dbus/shill_manager_client.h"
 #include "chromeos/dbus/shill_service_client.h"
 #include "dbus/object_path.h"
@@ -24,6 +25,9 @@ namespace chromeos {
 
 namespace {
 
+void DoNothingWithCallStatus(DBusMethodCallStatus call_status) {
+}
+
 void ErrorCallbackFunction(const std::string& error_name,
                            const std::string& error_message) {
   LOG(ERROR) << "Shill Error: " << error_name << " : " << error_message;
@@ -64,12 +68,6 @@ class TestListener : public internal::ShillPropertyHandler::Listener {
     ++manager_updates_;
   }
 
-  virtual void UpdateNetworkServiceIPAddress(
-      const std::string& service_path,
-      const std::string& ip_address) OVERRIDE {
-    AddPropertyUpdate(flimflam::kServicesProperty, service_path);
-  }
-
   virtual void ManagedStateListChanged(
       ManagedState::ManagedType type) OVERRIDE {
     AddStateListUpdate(GetTypeString(type));
@@ -157,6 +155,8 @@ class ShillPropertyHandlerTest : public testing::Test {
     service_test_ =
         DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
     ASSERT_TRUE(service_test_);
+    SetupShillPropertyHandler();
+    message_loop_.RunUntilIdle();
   }
 
   virtual void TearDown() OVERRIDE {
@@ -179,7 +179,18 @@ class ShillPropertyHandlerTest : public testing::Test {
                   const std::string& state,
                   bool add_to_watch_list) {
     ASSERT_TRUE(IsValidType(type));
-    service_test_->AddService(id, id, type, state, add_to_watch_list);
+    service_test_->AddService(id, id, type, state,
+                              add_to_watch_list);
+  }
+
+  void AddServiceWithIPConfig(const std::string& type,
+                              const std::string& id,
+                              const std::string& state,
+                              const std::string& ipconfig_path,
+                              bool add_to_watch_list) {
+    ASSERT_TRUE(IsValidType(type));
+    service_test_->AddServiceWithIPConfig(id, id, type, state,
+                                          ipconfig_path, add_to_watch_list);
   }
 
   void RemoveService(const std::string& id) {
@@ -234,8 +245,6 @@ class ShillPropertyHandlerTest : public testing::Test {
 };
 
 TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerStub) {
-  SetupShillPropertyHandler();
-  message_loop_.RunUntilIdle();
   EXPECT_EQ(1, listener_->manager_updates());
   EXPECT_TRUE(shill_property_handler_->TechnologyAvailable(
       flimflam::kTypeWifi));
@@ -252,8 +261,6 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerStub) {
 }
 
 TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerTechnologyChanged) {
-  SetupShillPropertyHandler();
-  message_loop_.RunUntilIdle();
   EXPECT_EQ(1, listener_->manager_updates());
   // Add a disabled technology.
   manager_test_->AddTechnology(flimflam::kTypeWimax, false);
@@ -277,8 +284,6 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerTechnologyChanged) {
 }
 
 TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerDevicePropertyChanged) {
-  SetupShillPropertyHandler();
-  message_loop_.RunUntilIdle();
   EXPECT_EQ(1, listener_->manager_updates());
   EXPECT_EQ(1, listener_->list_updates(flimflam::kDevicesProperty));
   const size_t kNumShillManagerClientStubImplDevices = 2;
@@ -304,8 +309,6 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerDevicePropertyChanged) {
 }
 
 TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServicePropertyChanged) {
-  SetupShillPropertyHandler();
-  message_loop_.RunUntilIdle();
   EXPECT_EQ(1, listener_->manager_updates());
   EXPECT_EQ(1, listener_->list_updates(flimflam::kServicesProperty));
   const size_t kNumShillManagerClientStubImplServices = 4;
@@ -368,6 +371,54 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServicePropertyChanged) {
   EXPECT_EQ(0, listener_->errors());
 }
 
-// TODO(stevenjb): Test IP Configs.
+TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerIPConfigPropertyChanged) {
+  // Set the properties for an IP Config object.
+  const std::string kTestIPConfigPath("test_ip_config_path");
+  base::StringValue ip_address("192.168.1.1");
+  DBusThreadManager::Get()->GetShillIPConfigClient()->SetProperty(
+      dbus::ObjectPath(kTestIPConfigPath),
+      flimflam::kAddressProperty,
+      ip_address,
+      base::Bind(&DoNothingWithCallStatus));
+  base::ListValue dns_servers;
+  dns_servers.Append(base::Value::CreateStringValue("192.168.1.100"));
+  dns_servers.Append(base::Value::CreateStringValue("192.168.1.101"));
+  DBusThreadManager::Get()->GetShillIPConfigClient()->SetProperty(
+      dbus::ObjectPath(kTestIPConfigPath),
+      flimflam::kNameServersProperty,
+      dns_servers,
+      base::Bind(&DoNothingWithCallStatus));
+  message_loop_.RunUntilIdle();
+
+  // Add a service with an empty ipconfig and then update
+  // its ipconfig property.
+  const std::string kTestServicePath1("test_wifi_service1");
+  AddService(flimflam::kTypeWifi, kTestServicePath1,
+             flimflam::kStateIdle, true);
+  message_loop_.RunUntilIdle();
+  // This is the initial property update.
+  EXPECT_EQ(1, listener_->
+            property_updates(flimflam::kServicesProperty)[kTestServicePath1]);
+  DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
+      dbus::ObjectPath(kTestServicePath1),
+      shill::kIPConfigProperty,
+      base::StringValue(kTestIPConfigPath),
+      base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
+  message_loop_.RunUntilIdle();
+  // IPConfig property change on the service should trigger property updates for
+  // IP Address and DNS.
+  EXPECT_EQ(3, listener_->
+            property_updates(flimflam::kServicesProperty)[kTestServicePath1]);
+
+  // Now, Add a new watched service with the IPConfig already set.
+  const std::string kTestServicePath2("test_wifi_service2");
+  AddServiceWithIPConfig(flimflam::kTypeWifi, kTestServicePath2,
+                         flimflam::kStateIdle, kTestIPConfigPath, true);
+  message_loop_.RunUntilIdle();
+  // A watched service with the IPConfig property already set must
+  // trigger property updates for IP Address and DNS when added.
+  EXPECT_EQ(3, listener_->
+            property_updates(flimflam::kServicesProperty)[kTestServicePath2]);
+}
 
 }  // namespace chromeos