[net] Add AsyncDNS.TTL histogram.

net/base/host_resolver_impl.cc

Index: net/base/host_resolver_impl.cc
diff --git a/net/base/host_resolver_impl.cc b/net/base/host_resolver_impl.cc
index d9e1aa2b7e79fa2e2eafe49c6e54f2ec36fee623..ac7ac7b1c0b8bf95d849a7cfbc53ad2ff6ceb53a 100644
--- a/net/base/host_resolver_impl.cc
+++ b/net/base/host_resolver_impl.cc
@@ -205,6 +205,10 @@ void RecordTotalTime(bool had_dns_config,
   }
 }
 
+void RecordTTL(base::TimeDelta ttl) {
+  UMA_HISTOGRAM_CUSTOM_COUNTS("AsyncDNS.TTL", ttl.InSeconds(), 1, 86400, 100);

[cbentzel, 2012/10/13 10:50:45]

Should be UMA_HISTOGRAM_CUSTOM_TIMES?

[szym, 2012/10/15 19:45:36]

I initially used _TIMES, but then I looked at the histogram results and didn't
like the non-round number of milliseconds on the bucket boundaries. I don't
think it makes any difference so I'll make it _TIMES of TimeDelta.

+}
+
 //-----------------------------------------------------------------------------
 
 // Wraps call to SystemHostResolverProc as an instance of HostResolverProc.
@@ -1331,7 +1335,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
       // If we were called from a Request's callback within CompleteRequests,
       // that Request could not have been cancelled, so num_active_requests()
       // could not be 0. Therefore, we are not in CompleteRequests().
-      CompleteRequests(OK, AddressList(), base::TimeDelta());
+      CompleteRequestsWithError(OK /* cancelled */);
     }
   }
 
@@ -1339,7 +1343,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
   // and destroys the job.
   void Abort() {
     DCHECK(is_running());
-    CompleteRequests(ERR_ABORTED, AddressList(), base::TimeDelta());
+    CompleteRequestsWithError(ERR_ABORTED);
   }
 
   // Called by HostResolverImpl when this job is evicted due to queue overflow.
@@ -1352,9 +1356,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
     net_log_.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_EVICTED);
 
     // This signals to CompleteRequests that this job never ran.
-    CompleteRequests(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE,
-                     AddressList(),
-                     base::TimeDelta());
+    CompleteRequestsWithError(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE);
   }
 
   // Attempts to serve the job from HOSTS. Returns true if succeeded and
@@ -1366,7 +1368,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
                                   requests_.front()->info(),
                                   &addr_list)) {
       // This will destroy the Job.
-      CompleteRequests(OK, addr_list, base::TimeDelta());
+      CompleteRequests(OK, addr_list, base::TimeDelta(), false /* true_ttl */);

[cbentzel, 2012/10/13 10:50:45]

Would it make sense to change to CompleteRequests(addr_list, base::TimeDelta())
and have a CompleteRequestsHelper which both that and CompleteRequestsWithError
call?

[szym, 2012/10/15 19:45:36]

I don't see the benefit. Wouldn't this be the only place where it is used? The
other places need to pass a custom net_error or custom true_ttl.

       return true;
     }
     return false;
@@ -1464,12 +1466,12 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
       }
     }
 
-    base::TimeDelta ttl = base::TimeDelta::FromSeconds(
-        kNegativeCacheEntryTTLSeconds);
+    base::TimeDelta ttl =
+        base::TimeDelta::FromSeconds(kNegativeCacheEntryTTLSeconds);
     if (net_error == OK)
       ttl = base::TimeDelta::FromSeconds(kCacheEntryTTLSeconds);
 
-    CompleteRequests(net_error, addr_list, ttl);
+    CompleteRequests(net_error, addr_list, ttl, false /* true_ttl */);
   }
 
   void StartDnsTask() {
@@ -1509,14 +1511,16 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
     }
 
     UmaAsyncDnsResolveStatus(RESOLVE_STATUS_DNS_SUCCESS);
+    RecordTTL(ttl);
 
-    CompleteRequests(net_error, addr_list, ttl);
+    CompleteRequests(net_error, addr_list, ttl, true /* true_ttl */);
   }
 
   // Performs Job's last rites. Completes all Requests. Deletes this.
   void CompleteRequests(int net_error,
                         const AddressList& addr_list,
-                        base::TimeDelta ttl) {
+                        base::TimeDelta ttl,
+                        bool true_ttl) {
     CHECK(resolver_);
 
     // This job must be removed from resolver's |jobs_| now to make room for a
@@ -1527,8 +1531,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
 
     resolver_->RemoveJob(this);
 
-    // |addr_list| will be destroyed once we destroy |proc_task_| and
-    // |dns_task_|.
+    // |addr_list| will be destroyed with |proc_task_| and |dns_task_|.
     AddressList list = addr_list;
 
     if (is_running()) {
@@ -1568,8 +1571,12 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
 
     bool did_complete = (net_error != ERR_ABORTED) &&
                         (net_error != ERR_HOST_RESOLVER_QUEUE_TOO_LARGE);
-    if (did_complete)
-      resolver_->CacheResult(key_, net_error, list, ttl);
+    if (did_complete) {
+      HostCache::Entry entry = true_ttl ?
+          HostCache::Entry(net_error, list, ttl) :
+          HostCache::Entry(net_error, list);
+      resolver_->CacheResult(key_, entry, ttl);
+    }
 
     // Complete all of the requests that were attached to the job.
     for (RequestsList::const_iterator it = requests_.begin();
@@ -1597,6 +1604,11 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
     }
   }
 
+  // Convenience wrapper for CompleteRequests in case of failure.
+  void CompleteRequestsWithError(int net_error) {
+    CompleteRequests(net_error, AddressList(), base::TimeDelta(), false);
+  }
+
   RequestPriority priority() const {
     return priority_tracker_.highest_priority();
   }
@@ -1958,6 +1970,8 @@ bool HostResolverImpl::ServeFromCache(const Key& key,
 
   *net_error = cache_entry->error;
   if (*net_error == OK) {
+    if (cache_entry->has_ttl())
+      RecordTTL(cache_entry->ttl);
     *addresses = cache_entry->addrlist;
     EnsurePortOnAddressList(info.port(), addresses);
   }
@@ -1998,11 +2012,10 @@ bool HostResolverImpl::ServeFromHosts(const Key& key,
 }
 
 void HostResolverImpl::CacheResult(const Key& key,
-                                   int net_error,
-                                   const AddressList& addr_list,
+                                   const HostCache::Entry& entry,
                                    base::TimeDelta ttl) {
   if (cache_.get())
-    cache_->Set(key, net_error, addr_list, base::TimeTicks::Now(), ttl);
+    cache_->Set(key, entry, base::TimeTicks::Now(), ttl);
 }
 
 void HostResolverImpl::RemoveJob(Job* job) {