[net] Move DnsConfigService to NetworkChangeNotifier.

net/dns/dns_config_service.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 "net/dns/dns_config_service.h"
 
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/values.h"
 #include "net/base/ip_endpoint.h"
@@ skipping 57 matching lines @@
   dict->SetInteger("attempts", attempts);
   dict->SetBoolean("rotate", rotate);
   dict->SetBoolean("edns0", edns0);
   dict->SetInteger("num_hosts", hosts.size());
 
   return dict;
 }
 
 
 DnsConfigService::DnsConfigService()
-    : have_config_(false),
+    : watch_failed_(false),
+      have_config_(false),
       have_hosts_(false),
       need_update_(false),
       last_sent_empty_(true) {}
 
 DnsConfigService::~DnsConfigService() {
-  // Must always clean up.
-  NetworkChangeNotifier::RemoveDNSObserver(this);
 }
 
-void DnsConfigService::Read(const CallbackType& callback) {
+void DnsConfigService::ReadConfig(const CallbackType& callback) {
   DCHECK(CalledOnValidThread());
   DCHECK(!callback.is_null());
   DCHECK(callback_.is_null());
   callback_ = callback;
-  OnDNSChanged(NetworkChangeNotifier::CHANGE_DNS_WATCH_STARTED);
+  ReadNow();
 }
 
-void DnsConfigService::Watch(const CallbackType& callback) {
+void DnsConfigService::WatchConfig(const CallbackType& callback) {
   DCHECK(CalledOnValidThread());
   DCHECK(!callback.is_null());
   DCHECK(callback_.is_null());
-  NetworkChangeNotifier::AddDNSObserver(this);
   callback_ = callback;
-  if (NetworkChangeNotifier::IsWatchingDNS())
-    OnDNSChanged(NetworkChangeNotifier::CHANGE_DNS_WATCH_STARTED);
-  // else: Wait until signal before reading.
+  watch_failed_ = !StartWatching();
+  ReadNow();
 }
 
 void DnsConfigService::InvalidateConfig() {
   DCHECK(CalledOnValidThread());
   base::TimeTicks now = base::TimeTicks::Now();
   if (!last_invalidate_config_time_.is_null()) {
     UMA_HISTOGRAM_LONG_TIMES("AsyncDNS.ConfigNotifyInterval",
                              now - last_invalidate_config_time_);
   }
   last_invalidate_config_time_ = now;
@@ skipping 53 matching lines @@
   }
   UMA_HISTOGRAM_BOOLEAN("AsyncDNS.HostsChange", changed);
 
   have_hosts_ = true;
   if (have_config_)
     OnCompleteConfig();
 }
 
 void DnsConfigService::StartTimer() {
   DCHECK(CalledOnValidThread());
-  if (last_sent_empty_) {
+  if (last_sent_empty_ && !watch_failed_) {
     DCHECK(!timer_.IsRunning());
     return;  // No need to withdraw again.
   }
   timer_.Stop();
 
   // Give it a short timeout to come up with a valid config. Otherwise withdraw
   // the config from the receiver. The goal is to avoid perceivable network
   // outage (when using the wrong config) but at the same time avoid
   // unnecessary Job aborts in HostResolverImpl. The signals come from multiple
   // sources so it might receive multiple events during a config change.
 
   // DHCP and user-induced changes are on the order of seconds, so 100ms should
   // not add perceivable delay. On the other hand, config readers should finish
   // within 100ms with the rare exception of I/O block or extra large HOSTS.
   const base::TimeDelta kTimeout = base::TimeDelta::FromMilliseconds(100);
 
   timer_.Start(FROM_HERE,
                kTimeout,
                this,
                &DnsConfigService::OnTimeout);
 }
 
 void DnsConfigService::OnTimeout() {
   DCHECK(CalledOnValidThread());
-  DCHECK(!last_sent_empty_);
+  DCHECK(!last_sent_empty_ || watch_failed_);
   // Indicate that even if there is no change in On*Read, we will need to
   // update the receiver when the config becomes complete.
   need_update_ = true;
   // Empty config is considered invalid.
   last_sent_empty_ = true;
   last_sent_empty_time_ = base::TimeTicks::Now();
   callback_.Run(DnsConfig());
 }
 
 void DnsConfigService::OnCompleteConfig() {
+  if (watch_failed_)
+    return;  // Let it time out.

[mmenke, 2012/08/31 15:05:59]

Should we stop the timer if |needs_update_| is false?

[szym, 2012/08/31 15:55:57]

I see. The idea is to not signal anything if we have not detected a real change.

Sticking with the OnTimeout approach would also require that we don't set
|need_update_ = true| in OnTimeout if watch_failed_, and that we reset
need_update_ to false at some point.

I think it's probably less confusing to just |callback_.Run(DnsConfig())| if
|watch_failed_| is true.

I updated the tests to expect immediate notifications rather than time outs.

[mmenke, 2012/08/31 16:04:03]

I agree - think that this is the less confusing solution.

   timer_.Stop();
   if (!need_update_)
     return;
   need_update_ = false;
   last_sent_empty_ = false;
   callback_.Run(dns_config_);
 }
 
 }  // namespace net