DNS: Let requests specify a callback for future cache hits

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 1764 matching lines @@
 
     // Complete all of the requests that were attached to the job.
     for (const std::unique_ptr<Request>& req : requests_) {
       if (req->was_canceled())
         continue;
 
       DCHECK_EQ(this, req->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());
       }
       req->OnComplete(entry.error(), entry.addresses());
 
       // Check if the resolver was destroyed as a result of running the
       // callback. If it was, we could continue, but we choose to bail.
       if (!resolver_.get())
         return;
@@ skipping 125 matching lines @@
   if (ip_address.AssignFromIPLiteral(info.hostname()))
     ip_address_ptr = &ip_address;
 
   // Build a key that identifies the request in the cache and in the
   // outstanding jobs map.
   Key key = GetEffectiveKeyForRequest(info, ip_address_ptr, source_net_log);
 
   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;
   }
 
   // Next we need to attach our request to a "job". This job is responsible for
   // calling "getaddrinfo(hostname)" on a worker thread.
 
   JobMap::iterator jobit = jobs_.find(key);
   Job* job;
@@ skipping 108 matching lines @@
 
   int net_error = ERR_UNEXPECTED;
   if (ResolveAsIP(key, info, ip_address, &net_error, addresses)) {
     MakeNotStale(stale_info);
     return net_error;
   }
   if (ServeFromCache(key, info, &net_error, addresses, allow_stale,
                      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.
   // http://crbug.com/117655
   if (ServeFromHosts(key, info, addresses)) {
     source_net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_HOSTS_HIT);
     MakeNotStale(stale_info);
     return OK;
   }
 
@@ skipping 228 matching lines @@
   }
 
   return true;
 }
 
 void HostResolverImpl::CacheResult(const Key& key,
                                    const HostCache::Entry& entry,
                                    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) {
   DCHECK(job);
   JobMap::iterator it = jobs_.find(job->key());
   if (it != jobs_.end() && it->second == job)
     jobs_.erase(it);
 }
 
 HostResolverImpl::Key HostResolverImpl::GetEffectiveKeyForRequest(
@@ skipping 108 matching lines @@
     // 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())
+  if (cache_.get()) {
     cache_->clear();
+    cache_hit_callbacks_.clear();
+  }
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
   RunLoopbackProbeJob();
 #endif
   AbortAllInProgressJobs();
   // |this| may be deleted inside AbortAllInProgressJobs().
 }
 
 void HostResolverImpl::OnConnectionTypeChanged(
     NetworkChangeNotifier::ConnectionType type) {
   proc_params_.unresponsive_delay =
@@ skipping 38 matching lines @@
       // TODO(pauljensen): Is this necessary?
       config_changed = true;
     }
   }
 
   if (config_changed) {
     // If the DNS server has changed, existing cached info could be wrong so we
     // 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();
 
     // Existing jobs will have been sent to the original server so they need to
     // be aborted.
     AbortAllInProgressJobs();
 
     // |this| may be deleted inside AbortAllInProgressJobs().
     if (self.get())
@@ skipping 25 matching lines @@
   dns_client_->SetConfig(DnsConfig());
 
   // Switch jobs with active DnsTasks over to using ProcTasks.
   AbortDnsTasks();
 
   UMA_HISTOGRAM_BOOLEAN("AsyncDNS.DnsClientEnabled", false);
   UMA_HISTOGRAM_SPARSE_SLOWLY("AsyncDNS.DnsClientDisabledReason",
                               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.
   dns_client_ = std::move(dns_client);
   if (dns_client_ && !dns_client_->GetConfig() &&
       num_dns_failures_ < kMaximumDnsFailures) {
     DnsConfig dns_config;
     NetworkChangeNotifier::GetDnsConfig(&dns_config);
     dns_client_->SetConfig(dns_config);
     num_dns_failures_ = 0;
     if (dns_client_->GetConfig())
       UMA_HISTOGRAM_BOOLEAN("AsyncDNS.DnsClientEnabled", true);
   }
 
   AbortDnsTasks();
 }
 
 }  // namespace net