DNS: Let requests specify a callback for future cache hits

net/dns/host_resolver_impl.cc

Index: net/dns/host_resolver_impl.cc
diff --git a/net/dns/host_resolver_impl.cc b/net/dns/host_resolver_impl.cc
index 2345dad8c741a17ab5ca385cf92380be2f90ef8c..0368dc9646e8bdd0667b8bf8894aed63bf31b110 100644
--- a/net/dns/host_resolver_impl.cc
+++ b/net/dns/host_resolver_impl.cc
@@ -1782,6 +1782,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
       // Update the net log and notify registered observers.
       LogFinishRequest(req->source_net_log(), req->info(), entry.error());
       if (did_complete) {
+        resolver_->MaybeAddCacheHitCallback(key_, req->info());
         // Record effective total time from creation to completion.
         RecordTotalTime(had_dns_config_, req->info().is_speculative(),
                         base::TimeTicks::Now() - req->request_time());
@@ -1927,6 +1928,7 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
   int rv = ResolveHelper(key, info, ip_address_ptr, addresses, false, nullptr,
                          source_net_log);
   if (rv != ERR_DNS_CACHE_MISS) {
+    MaybeAddCacheHitCallback(key, info);
     LogFinishRequest(source_net_log, info, rv);
     RecordTotalTime(HaveDnsConfig(), info.is_speculative(), base::TimeDelta());
     return rv;
@@ -2055,6 +2057,7 @@ int HostResolverImpl::ResolveHelper(const Key& key,
                      stale_info)) {
     source_net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_CACHE_HIT);
     // |ServeFromCache()| will set |*stale_info| as needed.
+    RunCacheHitCallbacks(key, info);
     return net_error;
   }
   // TODO(szym): Do not do this if nsswitch.conf instructs not to.
@@ -2303,6 +2306,7 @@ void HostResolverImpl::CacheResult(const Key& key,
                                    base::TimeDelta ttl) {
   if (cache_.get())
     cache_->Set(key, entry, base::TimeTicks::Now(), ttl);
+  ClearCacheHitCallbacks(key);

[Charlie Harrison, 2016/07/26 13:41:13]

It is not quite clear why we clear the cache hit callback here. Could you add a
comment? It also might be clear if this call was hoisted into CompleteRequests.

[Julia Tuttle, 2016/07/26 17:51:24]

Done.

 }
 
 void HostResolverImpl::RemoveJob(Job* job) {
@@ -2431,8 +2435,10 @@ void HostResolverImpl::OnIPAddressChanged() {
   last_ipv6_probe_time_ = base::TimeTicks();
   // Abandon all ProbeJobs.
   probe_weak_ptr_factory_.InvalidateWeakPtrs();
-  if (cache_.get())
+  if (cache_.get()) {
     cache_->clear();
+    cache_hit_callbacks_.clear();
+  }
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
   RunLoopbackProbeJob();
 #endif
@@ -2491,8 +2497,10 @@ void HostResolverImpl::UpdateDNSConfig(bool config_changed) {
     // have to drop our internal cache :( Note that OS level DNS caches, such
     // as NSCD's cache should be dropped automatically by the OS when
     // resolv.conf changes so we don't need to do anything to clear that cache.
-    if (cache_.get())
+    if (cache_.get()) {
       cache_->clear();
+      cache_hit_callbacks_.clear();
+    }
 
     // Life check to bail once |this| is deleted.
     base::WeakPtr<HostResolverImpl> self = weak_ptr_factory_.GetWeakPtr();
@@ -2538,6 +2546,31 @@ void HostResolverImpl::OnDnsTaskResolve(int net_error) {
                               std::abs(net_error));
 }
 
+void HostResolverImpl::OnCacheEntryEvicted(const HostCache::Key& key,
+                                           const HostCache::Entry& entry) {
+  ClearCacheHitCallbacks(key);
+}
+
+void HostResolverImpl::ClearCacheHitCallbacks(const HostCache::Key& key) {

[Charlie Harrison, 2016/07/26 13:41:13]

I don't think this method is really necessary.

[Julia Tuttle, 2016/07/26 17:51:24]

Done.

+  cache_hit_callbacks_.erase(key);
+}
+
+void HostResolverImpl::MaybeAddCacheHitCallback(const HostCache::Key& key,
+                                                const RequestInfo& info) {
+  const RequestInfo::CacheHitCallback& callback = info.cache_hit_callback();
+  if (callback.is_null())
+    return;
+  cache_hit_callbacks_[key].push_back(callback);
+}
+
+void HostResolverImpl::RunCacheHitCallbacks(const HostCache::Key& key,
+                                            const RequestInfo& info) {
+  if (cache_hit_callbacks_.count(key) == 0u)

[Charlie Harrison, 2016/07/26 13:41:13]

I think using find(key) is clearer here, and only requires looking up the key in
the map once because you can use the iterator to traverse the callback vector.

[Julia Tuttle, 2016/07/26 17:51:24]

Done.

+    return;
+  for (auto& callback : cache_hit_callbacks_[key])
+    callback.Run(info);
+}
+
 void HostResolverImpl::SetDnsClient(std::unique_ptr<DnsClient> dns_client) {
   // DnsClient and config must be updated before aborting DnsTasks, since doing
   // so may start new jobs.