[net] Add AsyncDNS.TTL histogram.

Counts of TTL obtained in DNS response. Counted when the address is used.



Review URL: https://chromiumcodereview.appspot.com/11065052

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@162329 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 86 insertions, 50 deletions

diff --git a/net/base/host_cache.cc b/net/base/host_cache.cc
index f94b686..9d74646 100644
--- a/net/base/host_cache.cc
+++ b/net/base/host_cache.cc
@@ -11,9 +11,18 @@ namespace net {
 
 //-----------------------------------------------------------------------------
 
+HostCache::Entry::Entry(int error, const AddressList& addrlist,
+                        base::TimeDelta ttl)
+    : error(error),
+      addrlist(addrlist),
+      ttl(ttl) {
+  DCHECK(ttl >= base::TimeDelta());
+}
+
 HostCache::Entry::Entry(int error, const AddressList& addrlist)
     : error(error),
-      addrlist(addrlist) {
+      addrlist(addrlist),
+      ttl(base::TimeDelta::FromSeconds(-1)) {
 }
 
 HostCache::Entry::~Entry() {
@@ -38,15 +47,14 @@ const HostCache::Entry* HostCache::Lookup(const Key& key,
 }
 
 void HostCache::Set(const Key& key,
-                    int error,
-                    const AddressList& addrlist,
+                    const Entry& entry,
                     base::TimeTicks now,
                     base::TimeDelta ttl) {
   DCHECK(CalledOnValidThread());
   if (caching_is_disabled())
     return;
 
-  entries_.Put(key, Entry(error, addrlist), now, now + ttl);
+  entries_.Put(key, entry, now, now + ttl);
 }
 
 void HostCache::clear() {
diff --git a/net/base/host_cache.h b/net/base/host_cache.h
index 7aff49c..2a3aedc 100644
--- a/net/base/host_cache.h
+++ b/net/base/host_cache.h
@@ -22,13 +22,19 @@ namespace net {
 class NET_EXPORT HostCache : NON_EXPORTED_BASE(public base::NonThreadSafe) {
  public:
   // Stores the latest address list that was looked up for a hostname.
-  struct Entry {
+  struct NET_EXPORT Entry {
+    Entry(int error, const AddressList& addrlist, base::TimeDelta ttl);
+    // Use when |ttl| is unknown.
     Entry(int error, const AddressList& addrlist);
     ~Entry();
 
+    bool has_ttl() const { return ttl >= base::TimeDelta(); }
+
     // The resolve results for this entry.
     int error;
     AddressList addrlist;
+    // TTL obtained from the nameserver. Negative if unknown.
+    base::TimeDelta ttl;
   };
 
   struct Key {
@@ -67,11 +73,10 @@ class NET_EXPORT HostCache : NON_EXPORTED_BASE(public base::NonThreadSafe) {
   const Entry* Lookup(const Key& key, base::TimeTicks now);
 
   // Overwrites or creates an entry for |key|.
-  // (|error|, |addrlist|) is the value to set, |now| is the current time
+  // |entry| is the value to set, |now| is the current time
   // |ttl| is the "time to live".
   void Set(const Key& key,
-           int error,
-           const AddressList& addrlist,
+           const Entry& entry,
            base::TimeTicks now,
            base::TimeDelta ttl);
 
diff --git a/net/base/host_cache_unittest.cc b/net/base/host_cache_unittest.cc
index 606aa5e..d71e061 100644
--- a/net/base/host_cache_unittest.cc
+++ b/net/base/host_cache_unittest.cc
@@ -34,13 +34,15 @@ TEST(HostCacheTest, Basic) {
 
   HostCache::Key key1 = Key("foobar.com");
   HostCache::Key key2 = Key("foobar2.com");
+  HostCache::Entry entry = HostCache::Entry(OK, AddressList());
 
   EXPECT_EQ(0U, cache.size());
 
   // Add an entry for "foobar.com" at t=0.
   EXPECT_FALSE(cache.Lookup(key1, now));
-  cache.Set(key1, OK, AddressList(), now, kTTL);
+  cache.Set(key1, entry, now, kTTL);
   EXPECT_TRUE(cache.Lookup(key1, now));
+  EXPECT_TRUE(cache.Lookup(key1, now)->error == entry.error);
 
   EXPECT_EQ(1U, cache.size());
 
@@ -49,7 +51,7 @@ TEST(HostCacheTest, Basic) {
 
   // Add an entry for "foobar2.com" at t=5.
   EXPECT_FALSE(cache.Lookup(key2, now));
-  cache.Set(key2, OK, AddressList(), now, kTTL);
+  cache.Set(key2, entry, now, kTTL);
   EXPECT_TRUE(cache.Lookup(key2, now));
   EXPECT_EQ(2U, cache.size());
 
@@ -68,7 +70,7 @@ TEST(HostCacheTest, Basic) {
   EXPECT_TRUE(cache.Lookup(key2, now));
 
   // Update key1, so it is no longer expired.
-  cache.Set(key1, OK, AddressList(), now, kTTL);
+  cache.Set(key1, entry, now, kTTL);
   EXPECT_TRUE(cache.Lookup(key1, now));
   EXPECT_EQ(2U, cache.size());
 
@@ -85,7 +87,7 @@ TEST(HostCacheTest, Basic) {
 
 // Try caching entries for a failed resolve attempt -- since we set the TTL of
 // such entries to 0 it won't store, but it will kick out the previous result.
-TEST(HostCacheTest, NoCacheNegative) {
+TEST(HostCacheTest, NoCacheZeroTTL) {
   const base::TimeDelta kSuccessEntryTTL = base::TimeDelta::FromSeconds(10);
   const base::TimeDelta kFailureEntryTTL = base::TimeDelta::FromSeconds(0);
 
@@ -96,9 +98,10 @@ TEST(HostCacheTest, NoCacheNegative) {
 
   HostCache::Key key1 = Key("foobar.com");
   HostCache::Key key2 = Key("foobar2.com");
+  HostCache::Entry entry = HostCache::Entry(OK, AddressList());
 
   EXPECT_FALSE(cache.Lookup(key1, now));
-  cache.Set(key1, ERR_NAME_NOT_RESOLVED, AddressList(), now, kFailureEntryTTL);
+  cache.Set(key1, entry, now, kFailureEntryTTL);
   EXPECT_EQ(1U, cache.size());
 
   // We disallow use of negative entries.
@@ -106,9 +109,9 @@ TEST(HostCacheTest, NoCacheNegative) {
 
   // Now overwrite with a valid entry, and then overwrite with negative entry
   // again -- the valid entry should be kicked out.
-  cache.Set(key1, OK, AddressList(), now, kSuccessEntryTTL);
+  cache.Set(key1, entry, now, kSuccessEntryTTL);
   EXPECT_TRUE(cache.Lookup(key1, now));
-  cache.Set(key1, ERR_NAME_NOT_RESOLVED, AddressList(), now, kFailureEntryTTL);
+  cache.Set(key1, entry, now, kFailureEntryTTL);
   EXPECT_FALSE(cache.Lookup(key1, now));
 }
 
@@ -123,13 +126,13 @@ TEST(HostCacheTest, CacheNegativeEntry) {
 
   HostCache::Key key1 = Key("foobar.com");
   HostCache::Key key2 = Key("foobar2.com");
-
+  HostCache::Entry entry = HostCache::Entry(OK, AddressList());
 
   EXPECT_EQ(0U, cache.size());
 
   // Add an entry for "foobar.com" at t=0.
   EXPECT_FALSE(cache.Lookup(key1, now));
-  cache.Set(key1, ERR_NAME_NOT_RESOLVED, AddressList(), now, kFailureEntryTTL);
+  cache.Set(key1, entry, now, kFailureEntryTTL);
   EXPECT_TRUE(cache.Lookup(key1, now));
   EXPECT_EQ(1U, cache.size());
 
@@ -138,7 +141,7 @@ TEST(HostCacheTest, CacheNegativeEntry) {
 
   // Add an entry for "foobar2.com" at t=5.
   EXPECT_FALSE(cache.Lookup(key2, now));
-  cache.Set(key2, ERR_NAME_NOT_RESOLVED, AddressList(), now, kFailureEntryTTL);
+  cache.Set(key2, entry, now, kFailureEntryTTL);
   EXPECT_TRUE(cache.Lookup(key2, now));
   EXPECT_EQ(2U, cache.size());
 
@@ -156,7 +159,7 @@ TEST(HostCacheTest, CacheNegativeEntry) {
   EXPECT_TRUE(cache.Lookup(key2, now));
 
   // Update key1, so it is no longer expired.
-  cache.Set(key1, ERR_NAME_NOT_RESOLVED, AddressList(), now, kFailureEntryTTL);
+  cache.Set(key1, entry, now, kFailureEntryTTL);
   // Re-uses existing entry storage.
   EXPECT_TRUE(cache.Lookup(key1, now));
   EXPECT_EQ(2U, cache.size());
@@ -184,18 +187,19 @@ TEST(HostCacheTest, AddressFamilyIsPartOfKey) {
 
   HostCache::Key key1("foobar.com", ADDRESS_FAMILY_UNSPECIFIED, 0);
   HostCache::Key key2("foobar.com", ADDRESS_FAMILY_IPV4, 0);
+  HostCache::Entry entry = HostCache::Entry(OK, AddressList());
 
   EXPECT_EQ(0U, cache.size());
 
   // Add an entry for ("foobar.com", UNSPECIFIED) at t=0.
   EXPECT_FALSE(cache.Lookup(key1, now));
-  cache.Set(key1, OK, AddressList(), now, kSuccessEntryTTL);
+  cache.Set(key1, entry, now, kSuccessEntryTTL);
   EXPECT_TRUE(cache.Lookup(key1, now));
   EXPECT_EQ(1U, cache.size());
 
   // Add an entry for ("foobar.com", IPV4_ONLY) at t=0.
   EXPECT_FALSE(cache.Lookup(key2, now));
-  cache.Set(key2, OK, AddressList(), now, kSuccessEntryTTL);
+  cache.Set(key2, entry, now, kSuccessEntryTTL);
   EXPECT_TRUE(cache.Lookup(key2, now));
   EXPECT_EQ(2U, cache.size());
 
@@ -219,24 +223,25 @@ TEST(HostCacheTest, HostResolverFlagsArePartOfKey) {
                       HOST_RESOLVER_CANONNAME);
   HostCache::Key key3("foobar.com", ADDRESS_FAMILY_IPV4,
                       HOST_RESOLVER_LOOPBACK_ONLY);
+  HostCache::Entry entry = HostCache::Entry(OK, AddressList());
 
   EXPECT_EQ(0U, cache.size());
 
   // Add an entry for ("foobar.com", IPV4, NONE) at t=0.
   EXPECT_FALSE(cache.Lookup(key1, now));
-  cache.Set(key1, OK, AddressList(), now, kTTL);
+  cache.Set(key1, entry, now, kTTL);
   EXPECT_TRUE(cache.Lookup(key1, now));
   EXPECT_EQ(1U, cache.size());
 
   // Add an entry for ("foobar.com", IPV4, CANONNAME) at t=0.
   EXPECT_FALSE(cache.Lookup(key2, now));
-  cache.Set(key2, OK, AddressList(), now, kTTL);
+  cache.Set(key2, entry, now, kTTL);
   EXPECT_TRUE(cache.Lookup(key2, now));
   EXPECT_EQ(2U, cache.size());
 
   // Add an entry for ("foobar.com", IPV4, LOOPBACK_ONLY) at t=0.
   EXPECT_FALSE(cache.Lookup(key3, now));
-  cache.Set(key3, OK, AddressList(), now, kTTL);
+  cache.Set(key3, entry, now, kTTL);
   EXPECT_TRUE(cache.Lookup(key3, now));
   EXPECT_EQ(3U, cache.size());
 
@@ -257,9 +262,11 @@ TEST(HostCacheTest, NoCache) {
   // Set t=0.
   base::TimeTicks now;
 
+  HostCache::Entry entry = HostCache::Entry(OK, AddressList());
+
   // Lookup and Set should have no effect.
   EXPECT_FALSE(cache.Lookup(Key("foobar.com"),now));
-  cache.Set(Key("foobar.com"), OK, AddressList(), now, kTTL);
+  cache.Set(Key("foobar.com"), entry, now, kTTL);
   EXPECT_FALSE(cache.Lookup(Key("foobar.com"), now));
 
   EXPECT_EQ(0U, cache.size());
@@ -273,12 +280,14 @@ TEST(HostCacheTest, Clear) {
   // Set t=0.
   base::TimeTicks now;
 
+  HostCache::Entry entry = HostCache::Entry(OK, AddressList());
+
   EXPECT_EQ(0u, cache.size());
 
   // Add three entries.
-  cache.Set(Key("foobar1.com"), OK, AddressList(), now, kTTL);
-  cache.Set(Key("foobar2.com"), OK, AddressList(), now, kTTL);
-  cache.Set(Key("foobar3.com"), OK, AddressList(), now, kTTL);
+  cache.Set(Key("foobar1.com"), entry, now, kTTL);
+  cache.Set(Key("foobar2.com"), entry, now, kTTL);
+  cache.Set(Key("foobar3.com"), entry, now, kTTL);
 
   EXPECT_EQ(3u, cache.size());
 
diff --git a/net/base/host_resolver_impl.cc b/net/base/host_resolver_impl.cc
index d9e1aa2..7f785b2 100644
--- a/net/base/host_resolver_impl.cc
+++ b/net/base/host_resolver_impl.cc
@@ -205,6 +205,12 @@ void RecordTotalTime(bool had_dns_config,
   }
 }
 
+void RecordTTL(base::TimeDelta ttl) {
+  UMA_HISTOGRAM_CUSTOM_TIMES("AsyncDNS.TTL", ttl,
+                             base::TimeDelta::FromSeconds(1),
+                             base::TimeDelta::FromDays(1), 100);
+}
+
 //-----------------------------------------------------------------------------
 
 // Wraps call to SystemHostResolverProc as an instance of HostResolverProc.
@@ -1331,7 +1337,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 +1345,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 +1358,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 +1370,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 */);
       return true;
     }
     return false;
@@ -1464,12 +1468,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 +1513,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 +1533,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 +1573,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 +1606,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 +1972,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 +2014,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) {
diff --git a/net/base/host_resolver_impl.h b/net/base/host_resolver_impl.h
index dff20c6..8d0f781 100644
--- a/net/base/host_resolver_impl.h
+++ b/net/base/host_resolver_impl.h
@@ -200,8 +200,7 @@ class NET_EXPORT HostResolverImpl
 
   // Records the result in cache if cache is present.
   void CacheResult(const Key& key,
-                   int net_error,
-                   const AddressList& addr_list,
+                   const HostCache::Entry& entry,
                    base::TimeDelta ttl);
 
   // Removes |job| from |jobs_|, only if it exists.
diff --git a/net/base/mock_host_resolver.cc b/net/base/mock_host_resolver.cc
index efa7ae5..e1ffb7b 100644
--- a/net/base/mock_host_resolver.cc
+++ b/net/base/mock_host_resolver.cc
@@ -171,7 +171,7 @@ int MockHostResolverBase::ResolveProc(size_t id,
     base::TimeDelta ttl;
     if (rv == OK)
       ttl = base::TimeDelta::FromSeconds(kCacheEntryTTLSeconds);
-    cache_->Set(key, rv, addr, base::TimeTicks::Now(), ttl);
+    cache_->Set(key, HostCache::Entry(rv, addr), base::TimeTicks::Now(), ttl);
   }
   if (rv == OK) {
     *addresses = addr;
diff --git a/net/proxy/proxy_resolver_js_bindings.cc b/net/proxy/proxy_resolver_js_bindings.cc
index d849a24..84dcafd 100644
--- a/net/proxy/proxy_resolver_js_bindings.cc
+++ b/net/proxy/proxy_resolver_js_bindings.cc
@@ -230,7 +230,7 @@ class DefaultJSBindings : public ProxyResolverJSBindings {
 
     // Save the result back to the per-request DNS cache.
     if (host_cache) {
-      host_cache->Set(cache_key, result, *address_list,
+      host_cache->Set(cache_key, HostCache::Entry(result, *address_list),
                       base::TimeTicks::Now(),
                       base::TimeDelta::FromSeconds(kCacheEntryTTLSeconds));
     }