[cros/net] Add DnsConfigService to NetworkChangeNotifierChromeos.

chrome/browser/chromeos/net/network_change_notifier_chromeos.cc

Index: chrome/browser/chromeos/net/network_change_notifier_chromeos.cc
diff --git a/chrome/browser/chromeos/net/network_change_notifier_chromeos.cc b/chrome/browser/chromeos/net/network_change_notifier_chromeos.cc
index 2007857481c882223e8470fd075f7715810fe46d..ddd55a1ecbde022add4d9928890d9f99c1b1879a 100644
--- a/chrome/browser/chromeos/net/network_change_notifier_chromeos.cc
+++ b/chrome/browser/chromeos/net/network_change_notifier_chromeos.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "content/public/browser/browser_thread.h"
+#include "net/dns/dns_config_service_posix.h"
 
 using content::BrowserThread;
 
@@ -26,6 +27,27 @@ bool IsOnline(chromeos::ConnectionState state) {
 
 namespace chromeos {
 
+class NetworkChangeNotifierChromeos::DnsConfigServiceChromeos
+    : public net::DnsConfigServicePosix {
+ public:
+  DnsConfigServiceChromeos() {}
+
+  virtual ~DnsConfigServiceChromeos() {}
+
+  // net::DnsConfigServicePosix:
+  virtual bool StartWatching() OVERRIDE {
+    // Notifications from NetworkLibrary are sent to
+    // NetworkChangeNotifierChromeos.
+    return true;
+  }
+
+  void OnNetworkChange() {
+    InvalidateConfig();
+    InvalidateHosts();
+    ReadNow();
+  }
+};
+
 NetworkChangeNotifierChromeos::NetworkChangeNotifierChromeos()
     : has_active_network_(false),
       connection_state_(chromeos::STATE_UNKNOWN),
@@ -49,12 +71,18 @@ void NetworkChangeNotifierChromeos::Init() {
 
   DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(this);
 
+  dns_config_service_.reset(new DnsConfigServiceChromeos());
+  dns_config_service_->WatchConfig(
+      base::Bind(NetworkChangeNotifier::SetDnsConfig));
+
   UpdateNetworkState(network_library);
 }
 
 void NetworkChangeNotifierChromeos::Shutdown() {
   weak_factory_.InvalidateWeakPtrs();
 
+  dns_config_service_.reset();
+
   if (!chromeos::CrosLibrary::Get())
     return;
 
@@ -112,6 +140,8 @@ void NetworkChangeNotifierChromeos::UpdateNetworkState(
             << ", state= " << network->state();
   }
 
+  dns_config_service_->OnNetworkChange();

[zel, 2012/09/04 17:16:11]

This will fire way too often for what you are trying to detect.

You should probably move this to line 165.  

[szym, 2012/09/04 17:29:31]

How often does this fire?

If I correctly understand the condition in 146-149, it will filter out changes,
unless it's a change of active network or the IP address changes. What if the
user changes the preferred DNS provider? Is it a different network?

[zel, 2012/09/05 19:26:59]

For an active network this will fire when any of it's properties changes (i.e.
signal strength change for WiFi/mobile network), so you you will be dropping DNS
cache all the times.

Yes, DNS settings can also change when the active network didn't change. You
should inspect results of

lib->GetIPConfigs(network->service_path(), ...)

which will return the list of NetworkIPConfig structs that contain name_servers
member.



 

[szym, 2012/09/05 19:48:29]

That is indeed a bit too often, but note that this is not a direct signal to
HostResolverImpl. Instead, on signal DnsConfigService will run res_ninit to
determine if the change actually affects DNS configuration, i.e., if nameservers
or suffix search list changed. Only then NetworkChangeNotifier will broadcast
OnDNSChanged to the observers. So, DnsConfigService serves as a filter for
signals from NetworkLibrary.
 
Why is there more than one NetworkIPConfig per service? Which of the configs
should I be looking at for the current DNS configuration?

+
   // Check if active network was added, removed or changed.
   if ((!network && has_active_network_) ||
       (network && (!has_active_network_ ||