Display DNS probe results.

chrome/browser/net/dns_probe_service.cc

Index: chrome/browser/net/dns_probe_service.cc
diff --git a/chrome/browser/net/dns_probe_service.cc b/chrome/browser/net/dns_probe_service.cc
index 8a0e9f073e973e6581e7c11a1924c5d5760cf34c..a9eb03a74e1205a4ea0056539e7754ed2bbc768d 100644
--- a/chrome/browser/net/dns_probe_service.cc
+++ b/chrome/browser/net/dns_probe_service.cc
@@ -11,13 +11,14 @@
 #include "chrome/common/net/net_error_info.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_util.h"
+#include "net/base/network_change_notifier.h"
 #include "net/dns/dns_client.h"
 #include "net/dns/dns_config_service.h"
 #include "net/dns/dns_protocol.h"
 
 using base::FieldTrialList;
 using base::StringToInt;
-using chrome_common_net::DnsProbeResult;
+using chrome_common_net::DnsProbeStatus;
 using net::DnsClient;
 using net::DnsConfig;
 using net::IPAddressNumber;
@@ -72,24 +73,109 @@ const int kNameserverCountMax = 10;
 
 }  // namespace
 
-DnsProbeService::DnsProbeService()
-    : system_result_(DnsProbeJob::SERVERS_UNKNOWN),
+class DnsProbeServiceImpl
+    : public DnsProbeService,
+      net::NetworkChangeNotifier::IPAddressObserver {
+ public:
+  DnsProbeServiceImpl(scoped_ptr<DnsProbeService::JobFactory> job_factory);
+  virtual ~DnsProbeServiceImpl();
+
+  virtual void ProbeDns(const CallbackType& callback) OVERRIDE;
+
+  virtual void ExpireResultForTesting() OVERRIDE { ExpireResult(); }
+
+  // NetworkChangeNotifier::IPAddressObserver implementation:
+  virtual void OnIPAddressChanged() OVERRIDE;
+
+ protected:
+  void ExpireResult();
+
+ private:
+  enum State {
+    STATE_NO_RESULTS,
+    STATE_PROBE_RUNNING,
+    STATE_RESULTS_CACHED,
+  };
+
+  void StartProbes();
+  void OnProbesComplete();
+  void CallCallbacks();
+
+  void OnProbeJobComplete(DnsProbeJob* job, DnsProbeJob::Result result);
+  chrome_common_net::DnsProbeStatus EvaluateResults() const;
+  void HistogramProbes() const;
+
+  bool ResultsExpired();
+
+  scoped_ptr<DnsProbeService::JobFactory> job_factory_;
+  scoped_ptr<DnsProbeJob> system_job_;
+  scoped_ptr<DnsProbeJob> public_job_;
+  DnsProbeJob::Result system_result_;
+  DnsProbeJob::Result public_result_;
+  std::vector<CallbackType> callbacks_;
+  State state_;
+  chrome_common_net::DnsProbeStatus result_;
+  base::Time probe_start_time_;
+
+  DISALLOW_COPY_AND_ASSIGN(DnsProbeServiceImpl);
+};
+
+class DefaultDnsProbeJobFactory : public DnsProbeService::JobFactory {
+ public:
+  DefaultDnsProbeJobFactory();
+
+  virtual scoped_ptr<DnsProbeJob> CreateSystemJob(
+      const DnsProbeJob::CallbackType& job_callback) OVERRIDE;
+  virtual scoped_ptr<DnsProbeJob> CreatePublicJob(
+      const DnsProbeJob::CallbackType& job_callback) OVERRIDE;
+
+ private:
+  void GetSystemDnsConfig(net::DnsConfig* config);
+  void GetPublicDnsConfig(net::DnsConfig* config);
+
+  scoped_ptr<DnsProbeJob> CreateProbeJob(
+      const net::DnsConfig& dns_config,
+      const DnsProbeJob::CallbackType& job_callback);
+
+  // How many DNS request attempts the probe jobs will make before giving up
+  // (Overrides the attempts field in the system DnsConfig.)
+  const int dns_attempts_;
+
+  DISALLOW_COPY_AND_ASSIGN(DefaultDnsProbeJobFactory);
+};
+
+DnsProbeService* DnsProbeService::CreateDefault() {
+  return new DnsProbeServiceImpl(
+      scoped_ptr<DnsProbeService::JobFactory>(
+          new DefaultDnsProbeJobFactory()));
+}
+
+DnsProbeService* DnsProbeService::CreateDefaultWithJobFactory(
+    scoped_ptr<DnsProbeService::JobFactory> job_factory) {
+  return new DnsProbeServiceImpl(job_factory.Pass());
+}
+
+DnsProbeServiceImpl::DnsProbeServiceImpl(
+    scoped_ptr<DnsProbeService::JobFactory> job_factory)
+    : DnsProbeService(),
+      job_factory_(job_factory.Pass()),
+      system_result_(DnsProbeJob::SERVERS_UNKNOWN),
       public_result_(DnsProbeJob::SERVERS_UNKNOWN),
       state_(STATE_NO_RESULTS),
-      result_(chrome_common_net::DNS_PROBE_UNKNOWN),
-      dns_attempts_(GetAttemptsFromFieldTrial()) {
+      result_(chrome_common_net::DNS_PROBE_MAX) {
   NetworkChangeNotifier::AddIPAddressObserver(this);
 }
 
-DnsProbeService::~DnsProbeService() {
+DnsProbeServiceImpl::~DnsProbeServiceImpl() {
   NetworkChangeNotifier::RemoveIPAddressObserver(this);
 }
 
-void DnsProbeService::ProbeDns(const DnsProbeService::CallbackType& callback) {
+void DnsProbeServiceImpl::ProbeDns(
+    const DnsProbeService::CallbackType& callback) {
   callbacks_.push_back(callback);
 
   if (state_ == STATE_RESULTS_CACHED && ResultsExpired())
-    ExpireResults();
+    ExpireResult();
 
   switch (state_) {
   case STATE_NO_RESULTS:
@@ -104,39 +190,25 @@ void DnsProbeService::ProbeDns(const DnsProbeService::CallbackType& callback) {
   }
 }
 
-scoped_ptr<DnsProbeJob> DnsProbeService::CreateSystemProbeJob(
-    const DnsProbeJob::CallbackType& job_callback) {
-  DnsConfig system_config;
-  GetSystemDnsConfig(&system_config);
-  return CreateProbeJob(system_config, job_callback);
-}
-
-scoped_ptr<DnsProbeJob> DnsProbeService::CreatePublicProbeJob(
-    const DnsProbeJob::CallbackType& job_callback) {
-  DnsConfig public_config;
-  GetPublicDnsConfig(&public_config);
-  return CreateProbeJob(public_config, job_callback);
-}
-
-void DnsProbeService::OnIPAddressChanged() {
+void DnsProbeServiceImpl::OnIPAddressChanged() {
   if (state_ == STATE_RESULTS_CACHED)
-    ExpireResults();
+    ExpireResult();
 }
 
-void DnsProbeService::ExpireResults() {
+void DnsProbeServiceImpl::ExpireResult() {
   DCHECK_EQ(STATE_RESULTS_CACHED, state_);
 
   state_ = STATE_NO_RESULTS;
-  result_ = chrome_common_net::DNS_PROBE_UNKNOWN;
+  result_ = chrome_common_net::DNS_PROBE_MAX;
 }
 
-void DnsProbeService::StartProbes() {
+void DnsProbeServiceImpl::StartProbes() {
   DCHECK_NE(STATE_PROBE_RUNNING, state_);
   DCHECK(!system_job_.get());
   DCHECK(!public_job_.get());
 
   DnsProbeJob::CallbackType job_callback =
-      base::Bind(&DnsProbeService::OnProbeJobComplete,
+      base::Bind(&DnsProbeServiceImpl::OnProbeJobComplete,
                  base::Unretained(this));
 
   // TODO(ttuttle): Do we want to keep explicit flags for "job done"?
@@ -144,8 +216,8 @@ void DnsProbeService::StartProbes() {
   system_result_ = DnsProbeJob::SERVERS_UNKNOWN;
   public_result_ = DnsProbeJob::SERVERS_UNKNOWN;
 
-  system_job_ = CreateSystemProbeJob(job_callback);
-  public_job_ = CreatePublicProbeJob(job_callback);
+  system_job_ = job_factory_->CreateSystemJob(job_callback);
+  public_job_ = job_factory_->CreatePublicJob(job_callback);
 
   // If we can't create one or both jobs, fail the probe immediately.
   if (!system_job_.get() || !public_job_.get()) {
@@ -154,7 +226,7 @@ void DnsProbeService::StartProbes() {
     state_ = STATE_RESULTS_CACHED;
     // TODO(ttuttle): Should this be BAD_CONFIG?  Currently I think it only
     // happens when the system DnsConfig has no servers.
-    result_ = chrome_common_net::DNS_PROBE_UNKNOWN;
+    result_ = chrome_common_net::DNS_PROBE_FINISHED_UNKNOWN;
     CallCallbacks();
     return;
   }
@@ -163,7 +235,7 @@ void DnsProbeService::StartProbes() {
   probe_start_time_ = base::Time::Now();
 }
 
-void DnsProbeService::OnProbesComplete() {
+void DnsProbeServiceImpl::OnProbesComplete() {
   DCHECK_EQ(STATE_PROBE_RUNNING, state_);
 
   state_ = STATE_RESULTS_CACHED;
@@ -174,92 +246,85 @@ void DnsProbeService::OnProbesComplete() {
   CallCallbacks();
 }
 
-void DnsProbeService::HistogramProbes() const {
-  const DnsProbeResult kMaxResult = chrome_common_net::DNS_PROBE_MAX;
+void DnsProbeServiceImpl::HistogramProbes() const {
+  const DnsProbeStatus kMaxStatus = chrome_common_net::DNS_PROBE_MAX;
 
   DCHECK_EQ(STATE_RESULTS_CACHED, state_);
-  DCHECK_NE(kMaxResult, result_);
+  DCHECK(chrome_common_net::DnsProbeStatusIsFinished(result_));
 
   base::TimeDelta elapsed = base::Time::Now() - probe_start_time_;
 
-  UMA_HISTOGRAM_ENUMERATION("DnsProbe.Probe.Result", result_, kMaxResult);
-  UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Probe.Elapsed", elapsed);
+  UMA_HISTOGRAM_ENUMERATION("DnsProbe.Status", result_, kMaxStatus);
+  UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Elapsed", elapsed);
 
   if (NetworkChangeNotifier::IsOffline()) {
-    UMA_HISTOGRAM_ENUMERATION("DnsProbe.Probe.NcnOffline.Result",
-                              result_, kMaxResult);
-    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Probe.NcnOffline.Elapsed", elapsed);
+    UMA_HISTOGRAM_ENUMERATION("DnsProbe.Status_NcnOffline",
+                              result_, kMaxStatus);
+    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Elapsed_NcnOffline", elapsed);
   } else {
-    UMA_HISTOGRAM_ENUMERATION("DnsProbe.Probe.NcnOnline.Result",
-                              result_, kMaxResult);
-    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Probe.NcnOnline.Elapsed", elapsed);
+    UMA_HISTOGRAM_ENUMERATION("DnsProbe.Status_NcnOnline",
+                              result_, kMaxStatus);
+    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Elapsed_NcnOnline", elapsed);
   }
 
   switch (result_) {
-  case chrome_common_net::DNS_PROBE_UNKNOWN:
-    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Probe.ResultUnknown.Elapsed",
+  case chrome_common_net::DNS_PROBE_FINISHED_UNKNOWN:
+    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Elapsed_Unknown",
                                elapsed);
     break;
-  case chrome_common_net::DNS_PROBE_NO_INTERNET:
-    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Probe.ResultNoInternet.Elapsed",
+  case chrome_common_net::DNS_PROBE_FINISHED_NO_INTERNET:
+    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Elapsed_NoInternet",
                                elapsed);
     break;
-  case chrome_common_net::DNS_PROBE_BAD_CONFIG:
-    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Probe.ResultBadConfig.Elapsed",
+  case chrome_common_net::DNS_PROBE_FINISHED_BAD_CONFIG:
+    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Elapsed_BadConfig",
                                elapsed);
-
-    // Histogram some extra data to see why BAD_CONFIG is happening.
-    UMA_HISTOGRAM_ENUMERATION(
-        "DnsProbe.Probe.ResultBadConfig.SystemJobResult",
-        system_result_,
-        DnsProbeJob::MAX_RESULT);
-    UMA_HISTOGRAM_CUSTOM_COUNTS(
-        "DnsProbe.Probe.ResultBadConfig.SystemNameserverCount",
-        system_nameserver_count_,
-        0, kNameserverCountMax, kNameserverCountMax + 1);
-    UMA_HISTOGRAM_BOOLEAN(
-        "DnsProbe.Probe.ResultBadConfig.SystemIsLocalhost",
-        system_is_localhost_);
     break;
-  case chrome_common_net::DNS_PROBE_NXDOMAIN:
-    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Probe.ResultNxdomain.Elapsed",
+  case chrome_common_net::DNS_PROBE_FINISHED_NXDOMAIN:
+    UMA_HISTOGRAM_MEDIUM_TIMES("DnsProbe.Elapsed_Nxdomain",
                                elapsed);
     break;
+
+  // These aren't actually results.
+  case chrome_common_net::DNS_PROBE_POSSIBLE:
+  case chrome_common_net::DNS_PROBE_NOT_RUN:
+  case chrome_common_net::DNS_PROBE_STARTED:
   case chrome_common_net::DNS_PROBE_MAX:
     NOTREACHED();
     break;
   }
 }
 
-DnsProbeResult DnsProbeService::EvaluateResults() const {
+DnsProbeStatus DnsProbeServiceImpl::EvaluateResults() const {
   DCHECK_NE(DnsProbeJob::SERVERS_UNKNOWN, system_result_);
   DCHECK_NE(DnsProbeJob::SERVERS_UNKNOWN, public_result_);
 
   // If the system DNS is working, assume the domain doesn't exist.
   if (system_result_ == DnsProbeJob::SERVERS_CORRECT)
-    return chrome_common_net::DNS_PROBE_NXDOMAIN;
+    return chrome_common_net::DNS_PROBE_FINISHED_NXDOMAIN;
 
   // If the system DNS is not working but another public server is, assume the
   // DNS config is bad (or perhaps the DNS servers are down or broken).
   if (public_result_ == DnsProbeJob::SERVERS_CORRECT)
-    return chrome_common_net::DNS_PROBE_BAD_CONFIG;
+    return chrome_common_net::DNS_PROBE_FINISHED_BAD_CONFIG;
 
   // If the system DNS is not working and another public server is unreachable,
   // assume the internet connection is down (note that system DNS may be a
   // router on the LAN, so it may be reachable but returning errors.)
   if (public_result_ == DnsProbeJob::SERVERS_UNREACHABLE)
-    return chrome_common_net::DNS_PROBE_NO_INTERNET;
+    return chrome_common_net::DNS_PROBE_FINISHED_NO_INTERNET;
 
   // Otherwise: the system DNS is not working and another public server is
   // responding but with errors or incorrect results.  This is an awkward case;
   // an invasive captive portal or a restrictive firewall may be intercepting
   // or rewriting DNS traffic, or the public server may itself be failing or
   // down.
-  return chrome_common_net::DNS_PROBE_UNKNOWN;
+  return chrome_common_net::DNS_PROBE_FINISHED_UNKNOWN;
 }
 
-void DnsProbeService::CallCallbacks() {
+void DnsProbeServiceImpl::CallCallbacks() {
   DCHECK_EQ(STATE_RESULTS_CACHED, state_);
+  DCHECK(chrome_common_net::DnsProbeStatusIsFinished(result_));
   DCHECK(!callbacks_.empty());
 
   std::vector<CallbackType> callbacks = callbacks_;
@@ -271,19 +336,8 @@ void DnsProbeService::CallCallbacks() {
   }
 }
 
-scoped_ptr<DnsProbeJob> DnsProbeService::CreateProbeJob(
-    const DnsConfig& dns_config,
-    const DnsProbeJob::CallbackType& job_callback) {
-  if (!dns_config.IsValid())
-    return scoped_ptr<DnsProbeJob>(NULL);
-
-  scoped_ptr<DnsClient> dns_client(DnsClient::CreateClient(NULL));
-  dns_client->SetConfig(dns_config);
-  return DnsProbeJob::CreateJob(dns_client.Pass(), job_callback, NULL);
-}
-
-void DnsProbeService::OnProbeJobComplete(DnsProbeJob* job,
-                                         DnsProbeJob::Result result) {
+void DnsProbeServiceImpl::OnProbeJobComplete(DnsProbeJob* job,
+                                             DnsProbeJob::Result result) {
   DCHECK_EQ(STATE_PROBE_RUNNING, state_);
 
   if (job == system_job_.get()) {
@@ -303,7 +357,31 @@ void DnsProbeService::OnProbeJobComplete(DnsProbeJob* job,
   }
 }
 
-void DnsProbeService::GetSystemDnsConfig(DnsConfig* config) {
+bool DnsProbeServiceImpl::ResultsExpired() {
+  const base::TimeDelta kMaxResultAge =
+      base::TimeDelta::FromMilliseconds(kMaxResultAgeMs);
+  return base::Time::Now() - probe_start_time_ > kMaxResultAge;
+}
+
+DefaultDnsProbeJobFactory::DefaultDnsProbeJobFactory()
+    : dns_attempts_(GetAttemptsFromFieldTrial()) {
+}
+
+scoped_ptr<DnsProbeJob> DefaultDnsProbeJobFactory::CreateSystemJob(
+    const DnsProbeJob::CallbackType& job_callback) {
+  DnsConfig system_config;
+  GetSystemDnsConfig(&system_config);
+  return CreateProbeJob(system_config, job_callback);
+}
+
+scoped_ptr<DnsProbeJob> DefaultDnsProbeJobFactory::CreatePublicJob(
+    const DnsProbeJob::CallbackType& job_callback) {
+  DnsConfig public_config;
+  GetPublicDnsConfig(&public_config);
+  return CreateProbeJob(public_config, job_callback);
+}
+
+void DefaultDnsProbeJobFactory::GetSystemDnsConfig(DnsConfig* config) {
   NetworkChangeNotifier::GetDnsConfig(config);
 
   // DNS probes don't need or want the suffix search list populated
@@ -312,17 +390,11 @@ void DnsProbeService::GetSystemDnsConfig(DnsConfig* config) {
   if (dns_attempts_ != kAttemptsUseDefault)
     config->attempts = dns_attempts_;
 
-  // Take notes in case the config turns out to be bad, so we can histogram
-  // some useful data.
-  system_nameserver_count_ = config->nameservers.size();
-  system_is_localhost_ = (system_nameserver_count_ == 1)
-                         && IsLocalhost(config->nameservers[0].address());
-
   // Disable port randomization.
   config->randomize_ports = false;
 }
 
-void DnsProbeService::GetPublicDnsConfig(DnsConfig* config) {
+void DefaultDnsProbeJobFactory::GetPublicDnsConfig(DnsConfig* config) {
   *config = DnsConfig();
 
   config->nameservers.push_back(MakeDnsEndPoint(kPublicDnsPrimary));
@@ -335,10 +407,15 @@ void DnsProbeService::GetPublicDnsConfig(DnsConfig* config) {
   config->randomize_ports = false;
 }
 
-bool DnsProbeService::ResultsExpired() {
-  const base::TimeDelta kMaxResultAge =
-      base::TimeDelta::FromMilliseconds(kMaxResultAgeMs);
-  return base::Time::Now() - probe_start_time_ > kMaxResultAge;
+scoped_ptr<DnsProbeJob> DefaultDnsProbeJobFactory::CreateProbeJob(
+    const DnsConfig& dns_config,
+    const DnsProbeJob::CallbackType& job_callback) {
+  if (!dns_config.IsValid())
+    return scoped_ptr<DnsProbeJob>(NULL);
+
+  scoped_ptr<DnsClient> dns_client(DnsClient::CreateClient(NULL));
+  dns_client->SetConfig(dns_config);
+  return DnsProbeJob::CreateJob(dns_client.Pass(), job_callback, NULL);
 }
 
 } // namespace chrome_browser_net