[cros/net] Add DnsConfigService to NetworkChangeNotifierChromeos.

chrome/browser/chromeos/net/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 "chrome/browser/chromeos/net/network_change_notifier_chromeos.h"
 
 #include "base/bind.h"
 #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;
 
 namespace {
 
 // Delay for online change notification reporting.
 const int kOnlineNotificationDelayMS = 500;
 const int kInitialNotificationCheckDelayMS = 1000;
 
 bool IsOnline(chromeos::ConnectionState state) {
   return state == chromeos::STATE_ONLINE ||
          state == chromeos::STATE_PORTAL;
 }
 
 }
 
 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),
       connection_type_(CONNECTION_NONE),
       is_online_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   BrowserThread::PostDelayedTask(
          BrowserThread::UI, FROM_HERE,
          base::Bind(
              &NetworkChangeNotifierChromeos::UpdateInitialState, this),
          base::TimeDelta::FromMilliseconds(kInitialNotificationCheckDelayMS));
 }
 
 NetworkChangeNotifierChromeos::~NetworkChangeNotifierChromeos() {
 }
 
 void NetworkChangeNotifierChromeos::Init() {
   chromeos::NetworkLibrary* network_library =
       chromeos::CrosLibrary::Get()->GetNetworkLibrary();
   network_library->AddNetworkManagerObserver(this);
 
   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;
 
   chromeos::NetworkLibrary* lib =
       chromeos::CrosLibrary::Get()->GetNetworkLibrary();
   lib->RemoveNetworkManagerObserver(this);
   lib->RemoveObserverForAllNetworks(this);
 
   DBusThreadManager::Get()->GetPowerManagerClient()->RemoveObserver(this);
 }
@@ skipping 37 matching lines @@
     chromeos::NetworkLibrary* lib) {
   const chromeos::Network* network = lib->active_network();
 
   if (network) {
     VLOG(1) << "UpdateNetworkState: " << network->name()
             << ", type= " << network->type()
             << ", device= " << network->device_path()
             << ", 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_ ||
                    network->service_path() != service_path_ ||
                    network->ip_address() != ip_address_))) {
     if (has_active_network_)
       lib->RemoveObserverForAllNetworks(this);
     if (!network) {
       has_active_network_ = false;
       service_path_.clear();
@@ skipping 136 matching lines @@
       }
     case chromeos::TYPE_BLUETOOTH:
     case chromeos::TYPE_VPN:
     case chromeos::TYPE_UNKNOWN:
       break;
   }
   return net::NetworkChangeNotifier::CONNECTION_UNKNOWN;
 }
 
 }  // namespace chromeos