Reduce frequency of IPv6 probes in HostResolverImpl.

net/dns/host_resolver_impl.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/host_resolver_impl.h"
 
 #if defined(OS_WIN)
 #include <Winsock2.h>
 #elif defined(OS_POSIX)
 #include <netdb.h>
@@ skipping 56 matching lines @@
 const unsigned kCacheEntryTTLSeconds = 60;
 
 // Default TTL for unsuccessful resolutions with ProcTask.
 const unsigned kNegativeCacheEntryTTLSeconds = 0;
 
 // Minimum TTL for successful resolutions with DnsTask.
 const unsigned kMinimumTTLSeconds = kCacheEntryTTLSeconds;
 
 const char kLocalhost[] = "localhost.";
 
+// Time between IPv6 probes, i.e. for how long results of each IPv6 probe are
+// cached.
+const int kIpv6ProbePeriodMs = 1000;

[pauljensen, 2015/05/13 13:00:22]

kIpv6->kIPv6 like kIPv6Address

[Sergey Ulanov, 2015/05/13 18:15:58]

Done, though IMO this is not right. In CamelCase names abbreviations should have
only the first character upper-cased. Style guide is not explicit about it, but
it does give example of DeleteUrl() being a proper function name and not
DeleteURL(). If that rule was followed we wouldn't have unreadable names such as
EnsureNSSSSLInit().

+
 // We use a separate histogram name for each platform to facilitate the
 // display of error codes by their symbolic name (since each platform has
 // different mappings).
 const char kOSErrorsForGetAddrinfoHistogramName[] =
 #if defined(OS_WIN)
     "Net.OSErrorsForGetAddrinfo_Win";
 #elif defined(OS_MACOSX)
     "Net.OSErrorsForGetAddrinfo_Mac";
 #elif defined(OS_LINUX)
     "Net.OSErrorsForGetAddrinfo_Linux";
@@ skipping 292 matching lines @@
   dict->SetString("priority", RequestPriorityToString(priority));
   return dict;
 }
 
 // Creates NetLog parameters for the DNS_CONFIG_CHANGED event.
 base::Value* NetLogDnsConfigCallback(const DnsConfig* config,
                                      NetLogCaptureMode /* capture_mode */) {
   return config->ToValue();
 }
 
+base::Value* NetLogIpv6AvailableCallback(bool ipv6_available,
+                                         bool cached,
+                                         NetLogCaptureMode /* capture_mode */) {
+  base::DictionaryValue* dict = new base::DictionaryValue();
+  dict->SetBoolean("ipv6_available", ipv6_available);
+  dict->SetBoolean("cached", cached);
+  return dict;
+}
+
 // The logging routines are defined here because some requests are resolved
 // without a Request object.
 
 // Logs when a request has just been started.
 void LogStartRequest(const BoundNetLog& source_net_log,
                      const HostResolver::RequestInfo& info) {
   source_net_log.BeginEvent(
       NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST,
       base::Bind(&NetLogRequestInfoCallback, &info));
 }
@@ skipping 1409 matching lines @@
 
 HostResolverImpl::HostResolverImpl(const Options& options, NetLog* net_log)
     : max_queued_jobs_(0),
       proc_params_(NULL, options.max_retry_attempts),
       net_log_(net_log),
       default_address_family_(ADDRESS_FAMILY_UNSPECIFIED),
       received_dns_config_(false),
       num_dns_failures_(0),
       probe_ipv6_support_(true),
       use_local_ipv6_(false),
+      last_ipv6_probe_result_(false),

[pauljensen, 2015/05/13 13:00:22]

nit: Even though this initial value should never be used (unless someone sets
their clock prior to unix epoch) I think defaulting to true represents a safer
initial value.  I believe this would just mean connect() fails instantly on the
IPv6 results.

[Sergey Ulanov, 2015/05/13 18:15:59]

Done.

       resolved_known_ipv6_hostname_(false),
       additional_resolver_flags_(0),
       fallback_to_proctask_(true),
       weak_ptr_factory_(this),
       probe_weak_ptr_factory_(this) {
   if (options.enable_caching)
     cache_ = HostCache::CreateDefaultCache();
 
   PrioritizedDispatcher::Limits job_limits = options.GetDispatcherLimits();
   dispatcher_.reset(new PrioritizedDispatcher(job_limits));
@@ skipping 332 matching lines @@
   if (result) {
     additional_resolver_flags_ |= HOST_RESOLVER_LOOPBACK_ONLY;
   } else {
     additional_resolver_flags_ &= ~HOST_RESOLVER_LOOPBACK_ONLY;
   }
 }
 
 HostResolverImpl::Key HostResolverImpl::GetEffectiveKeyForRequest(
     const RequestInfo& info,
     const IPAddressNumber* ip_number,
-    const BoundNetLog& net_log) const {
+    const BoundNetLog& net_log) {
   HostResolverFlags effective_flags =
       info.host_resolver_flags() | additional_resolver_flags_;
   AddressFamily effective_address_family = info.address_family();
 
   if (info.address_family() == ADDRESS_FAMILY_UNSPECIFIED) {
     if (probe_ipv6_support_ && !use_local_ipv6_ &&
         // When resolving IPv4 literals, there's no need to probe for IPv6.
         // When resolving IPv6 literals, there's no benefit to artificially
         // limiting our resolution based on a probe.  Prior logic ensures
         // that this query is UNSPECIFIED (see info.address_family()
         // check above) and that |default_address_family_| is UNSPECIFIED
         // (|prove_ipv6_support_| is false if |default_address_family_| is
         // set) so the code requesting the resolution should be amenable to
         // receiving a IPv6 resolution.
         ip_number == nullptr) {
-      // Google DNS address.
-      const uint8 kIPv6Address[] =
-          { 0x20, 0x01, 0x48, 0x60, 0x48, 0x60, 0x00, 0x00,
-            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x88, 0x88 };
-      IPAddressNumber address(kIPv6Address,
-                              kIPv6Address + arraysize(kIPv6Address));
-      bool rv6 = IsGloballyReachable(address, net_log);
-      net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_IPV6_REACHABILITY_CHECK,
-                       NetLog::BoolCallback("ipv6_available", rv6));
-      if (!rv6) {
+      bool ipv6_available = IsIpv6Reachable(net_log);
+      if (!ipv6_available) {

[pauljensen, 2015/05/13 13:00:22]

Can we combine these two lines into "if (!IsIpv6Reachable(net_log))"

[Sergey Ulanov, 2015/05/13 18:15:58]

Done.

         effective_address_family = ADDRESS_FAMILY_IPV4;
         effective_flags |= HOST_RESOLVER_DEFAULT_FAMILY_SET_DUE_TO_NO_IPV6;
       }
     } else {
       effective_address_family = default_address_family_;
     }
   }
 
   std::string hostname = info.hostname();
   // Redirect .localhost queries to "localhost." to make sure that they
   // are never sent out on the network, per RFC 6761.
   if (IsLocalhostTLD(info.hostname()))
     hostname = kLocalhost;
 
   return Key(hostname, effective_address_family, effective_flags);
 }
 
+bool HostResolverImpl::IsIpv6Reachable(const BoundNetLog& net_log) {
+  base::TimeTicks now = base::TimeTicks::Now();
+  bool cached = true;
+  if ((now - last_ipv6_probe_time_).InMilliseconds() > kIpv6ProbePeriodMs) {
+    // Google DNS address.
+    const uint8 kIPv6Address[] =
+        { 0x20, 0x01, 0x48, 0x60, 0x48, 0x60, 0x00, 0x00,
+          0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x88, 0x88 };

[pauljensen, 2015/05/13 13:00:22]

Can we either move kIPv6Address up to the beginning of the file with the other
constants, or move kIpv6ProbePeriodMs down to this function?  To be consistent.

[Sergey Ulanov, 2015/05/13 18:15:58]

Done.

+    IPAddressNumber address(kIPv6Address,
+                            kIPv6Address + arraysize(kIPv6Address));
+    last_ipv6_probe_result_ = IsGloballyReachable(address, net_log);
+    last_ipv6_probe_time_ = now;
+    cached = false;
+  }
+  net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_IPV6_REACHABILITY_CHECK,
+                   base::Bind(&NetLogIpv6AvailableCallback,
+                              last_ipv6_probe_result_, cached));
+  return last_ipv6_probe_result_;
+}
+
 void HostResolverImpl::AbortAllInProgressJobs() {
   // In Abort, a Request callback could spawn new Jobs with matching keys, so
   // first collect and remove all running jobs from |jobs_|.
   ScopedVector<Job> jobs_to_abort;
   for (JobMap::iterator it = jobs_.begin(); it != jobs_.end(); ) {
     Job* job = it->second;
     if (job->is_running()) {
       jobs_to_abort.push_back(job);
       jobs_.erase(it++);
     } else {
@@ skipping 49 matching lines @@
   for (JobMap::iterator it = jobs_.begin(); self.get() && it != jobs_.end();) {
     Job* job = it->second;
     ++it;
     // This could remove |job| from |jobs_|, but iterator will remain valid.
     job->ServeFromHosts();
   }
 }
 
 void HostResolverImpl::OnIPAddressChanged() {
   resolved_known_ipv6_hostname_ = false;
+  last_ipv6_probe_time_ = base::TimeTicks();
   // Abandon all ProbeJobs.
   probe_weak_ptr_factory_.InvalidateWeakPtrs();
   if (cache_.get())
     cache_->clear();
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
   new LoopbackProbeJob(probe_weak_ptr_factory_.GetWeakPtr());
 #endif
   AbortAllInProgressJobs();
   // |this| may be deleted inside AbortAllInProgressJobs().
 }
@@ skipping 100 matching lines @@
     dns_client_->SetConfig(dns_config);
     num_dns_failures_ = 0;
     if (dns_client_->GetConfig())
       UMA_HISTOGRAM_BOOLEAN("AsyncDNS.DnsClientEnabled", true);
   }
 
   AbortDnsTasks();
 }
 
 }  // namespace net