net: add caching and inflight merging to DnsRRResolver

(This also removes support for DNSSEC lookups. This is a temporary measure
while I work on refactoring it.)

BUG=none
TEST=net_unittests

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

4 files changed, 548 insertions, 122 deletions

diff --git a/net/base/dns_util.h b/net/base/dns_util.h
index c91587d..60bfc3f 100644
--- a/net/base/dns_util.h
+++ b/net/base/dns_util.h
@@ -36,6 +36,7 @@ static const uint16 kDNS_DS = 43;
 static const uint16 kDNS_RRSIG = 46;
 static const uint16 kDNS_DNSKEY = 48;
 static const uint16 kDNS_ANY = 0xff;
+static const uint16 kDNS_TESTING = 0xfffe;  // in private use area.
 
 // http://www.iana.org/assignments/dns-sec-alg-numbers/dns-sec-alg-numbers.xhtml
 static const uint8 kDNSSEC_RSA_SHA1 = 5;
diff --git a/net/base/dnsrr_resolver.cc b/net/base/dnsrr_resolver.cc
index dbbb0af..9401658 100644
--- a/net/base/dnsrr_resolver.cc
+++ b/net/base/dnsrr_resolver.cc
@@ -8,8 +8,11 @@
 #include <resolv.h>
 #endif
 
+#include "base/lock.h"
 #include "base/message_loop.h"
 #include "base/scoped_ptr.h"
+#include "base/singleton.h"
+#include "base/stl_util-inl.h"
 #include "base/string_piece.h"
 #include "base/task.h"
 #include "base/worker_pool.h"
@@ -17,81 +20,152 @@
 #include "net/base/dns_util.h"
 #include "net/base/net_errors.h"
 
+DISABLE_RUNNABLE_METHOD_REFCOUNT(net::RRResolverWorker);
+DISABLE_RUNNABLE_METHOD_REFCOUNT(net::RRResolverHandle);
+
+// Life of a query:
+//
+// DnsRRResolver RRResolverJob RRResolverWorker       ...         Handle
+//      |                       (origin loop)    (worker loop)
+//      |
+//   Resolve()
+//      |---->----<creates>
+//      |
+//      |---->-------------------<creates>
+//      |
+//      |---->---------------------------------------------------<creates>
+//      |
+//      |---->--------------------Start
+//      |                           |
+//      |                        PostTask
+//      |
+//      |                                     <starts resolving>
+//      |---->-----AddHandle                          |
+//                                                    |
+//                                                    |
+//                                                    |
+//                                                  Finish
+//                                                    |
+//                                                 PostTask
+//
+//                                   |
+//                                DoReply
+//      |----<-----------------------|
+//  HandleResult
+//      |
+//      |---->-----HandleResult
+//                      |
+//                      |------>-----------------------------------Post
+//
+//
+//
+// A cache hit:
+//
+// DnsRRResolver CacheHitCallbackTask  Handle
+//      |
+//   Resolve()
+//      |---->----<creates>
+//      |
+//      |---->------------------------<creates>
+//      |
+//      |
+//   PostTask
+//
+// (MessageLoop cycles)
+//
+//                   Run
+//                    |
+//                    |----->-----------Post
+
+
+
 namespace net {
 
 static const uint16 kClassIN = 1;
+// kMaxCacheEntries is the number of RRResponse object that we'll cache.
+static const unsigned kMaxCacheEntries = 32;
+// kNegativeTTLSecs is the number of seconds for which we'll cache a negative
+// cache entry.
+static const unsigned kNegativeTTLSecs = 60;
+
+RRResponse::RRResponse()
+    : ttl(0), dnssec(false), negative(false) {
+}
 
-namespace {
-
-class CompletionCallbackTask : public Task,
-                               public MessageLoop::DestructionObserver {
+class RRResolverHandle {
  public:
-  explicit CompletionCallbackTask(CompletionCallback* callback,
-                                  MessageLoop* ml)
+  RRResolverHandle(CompletionCallback* callback, RRResponse* response)
       : callback_(callback),
-        rv_(OK),
-        posted_(false),
-        message_loop_(ml) {
-    ml->AddDestructionObserver(this);
+        response_(response) {
   }
 
-  void set_rv(int rv) {
-    rv_ = rv;
+  // Cancel ensures that the result callback will never be made.
+  void Cancel() {
+    callback_ = NULL;
   }
 
-  void Post() {
-    AutoLock locked(lock_);
-    DCHECK(!posted_);
+  // Post copies the contents of |response| to the caller's RRResponse and
+  // calls the callback.
+  void Post(int rv, const RRResponse* response) {
+    if (!callback_)
+      return;  // we were canceled.
 
-    if (!message_loop_) {
-      // MessageLoop got deleted, nothing to do.
-      delete this;
-      return;
-    }
-    posted_ = true;
-    message_loop_->PostTask(FROM_HERE, this);
-  }
-
-  // Task interface
-  void Run() {
-    message_loop_->RemoveDestructionObserver(this);
-    callback_->Run(rv_);
-    // We will be deleted by the message loop.
-  }
-
-  // DestructionObserver interface
-  virtual void WillDestroyCurrentMessageLoop() {
-    AutoLock locked(lock_);
-    message_loop_ = NULL;
+    if (response_ && response)
+      *response_ = *response;
+    callback_->Run(rv);
   }
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(CompletionCallbackTask);
+  friend class RRResolverWorker;
+  friend class DnsRRResolver;
 
   CompletionCallback* callback_;
-  int rv_;
-  bool posted_;
-
-  Lock lock_;  // covers |message_loop_|
-  MessageLoop* message_loop_;
+  RRResponse* const response_;
 };
 
-#if defined(OS_POSIX)
-class ResolveTask : public Task {
+
+// RRResolverWorker runs on a worker thread and takes care of the blocking
+// process of performing the DNS resolution.
+class RRResolverWorker {
  public:
-  ResolveTask(const std::string& name, uint16 rrtype,
-              uint16 flags, CompletionCallback* callback,
-              RRResponse* response, MessageLoop* ml)
+  RRResolverWorker(const std::string& name, uint16 rrtype, uint16 flags,
+                   DnsRRResolver* dnsrr_resolver)
       : name_(name),
         rrtype_(rrtype),
         flags_(flags),
-        subtask_(new CompletionCallbackTask(callback, ml)),
-        response_(response) {
+        origin_loop_(MessageLoop::current()),
+        dnsrr_resolver_(dnsrr_resolver),
+        canceled_(false) {
+  }
+
+  bool Start() {
+    DCHECK_EQ(MessageLoop::current(), origin_loop_);
+
+    return WorkerPool::PostTask(
+               FROM_HERE, NewRunnableMethod(this, &RRResolverWorker::Run),
+               true /* task is slow */);
   }
 
+  // Cancel is called from the origin loop when the DnsRRResolver is getting
+  // deleted.
+  void Cancel() {
+    DCHECK_EQ(MessageLoop::current(), origin_loop_);
+    AutoLock locked(lock_);
+    canceled_ = true;
+  }
+
+ private:
+
+#if defined(OS_POSIX)
+
   virtual void Run() {
     // Runs on a worker thread.
 
+    if (HandleTestCases()) {
+      Finish();
+      return;
+    }
+
     bool r = true;
     if ((_res.options & RES_INIT) == 0) {
       if (res_ninit(&_res) != 0)
@@ -111,10 +185,18 @@ class ResolveTask : public Task {
 #endif
       _res.options = saved_options;
     }
-    int error = r ? OK : ERR_NAME_NOT_RESOLVED;
 
-    subtask_->set_rv(error);
-    subtask_.release()->Post();
+    response_.fetch_time = base::Time::Now();
+
+    if (r) {
+      result_ = OK;
+    } else {
+      result_ = ERR_NAME_NOT_RESOLVED;
+      response_.negative = true;
+      response_.ttl = kNegativeTTLSecs;
+    }
+
+    Finish();
   }
 
   bool Do() {
@@ -142,38 +224,97 @@ class ResolveTask : public Task {
     if (len == -1)
       return false;
 
-    return response_->ParseFromResponse(answer, len, rrtype_);
+    return response_.ParseFromResponse(answer, len, rrtype_);
   }
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(ResolveTask);
+#else  // OS_WIN
+
+  virtual void Run() {
+    if (HandleTestCases()) {
+      Finish();
+      return;
+    }
+
+    response_.fetch_time = base::Time::Now();
+    response_.negative = true;
+    result_ = ERR_NAME_NOT_RESOLVED;
+    Finish();
+  }
+
+#endif // OS_WIN
+
+  // HandleTestCases stuffs in magic test values in the event that the query is
+  // from a unittest.
+  bool HandleTestCases() {
+    if (rrtype_ == kDNS_TESTING) {
+      response_.fetch_time = base::Time::Now();
+
+      if (name_ == "www.testing.notatld") {
+        response_.ttl = 86400;
+        response_.negative = false;
+        response_.rrdatas.push_back("goats!");
+        result_ = OK;
+        return true;
+      } else if (name_ == "nx.testing.notatld") {
+        response_.negative = true;
+        result_ = ERR_NAME_NOT_RESOLVED;
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  // DoReply runs on the origin thread.
+  void DoReply() {
+    DCHECK_EQ(MessageLoop::current(), origin_loop_);
+    // No locking here because, since the worker thread part of the lookup is
+    // complete, only one thread can access this object now.
+    if (!canceled_)
+      dnsrr_resolver_->HandleResult(name_, rrtype_, result_, response_);
+    delete this;
+  }
+
+  void Finish() {
+    // Runs on the worker thread.
+    // We assume that the origin loop outlives the DnsRRResolver. If the
+    // DnsRRResolver is deleted, it will call Cancel on us. If it does so
+    // before the Acquire, we'll delete ourselves and return. If it's trying to
+    // do so concurrently, then it'll block on the lock and we'll call PostTask
+    // while the DnsRRResolver (and therefore the MessageLoop) is still alive.
+    // If it does so after this function, we assume that the MessageLoop will
+    // process pending tasks. In which case we'll notice the |canceled_| flag
+    // in DoReply.
+
+    bool canceled;
+    {
+      AutoLock locked(lock_);
+      canceled = canceled_;
+      if (!canceled) {
+        origin_loop_->PostTask(
+            FROM_HERE, NewRunnableMethod(this, &RRResolverWorker::DoReply));
+      }
+    }
+
+    if (canceled)
+      delete this;
+  }
 
   const std::string name_;
   const uint16 rrtype_;
   const uint16 flags_;
-  scoped_ptr<CompletionCallbackTask> subtask_;
-  RRResponse* const response_;
-};
-#else  // OS_POSIX
-// On non-Linux platforms we fail everything for now.
-class ResolveTask : public Task {
- public:
-  ResolveTask(const std::string& name, uint16 rrtype,
-              uint16 flags, CompletionCallback* callback,
-              RRResponse* response, MessageLoop* ml)
-      : subtask_(new CompletionCallbackTask(callback, ml)) {
-  }
+  MessageLoop* const origin_loop_;
+  DnsRRResolver* const dnsrr_resolver_;
 
-  virtual void Run() {
-    subtask_->set_rv(ERR_NAME_NOT_RESOLVED);
-    subtask_->Post();
-  }
+  Lock lock_;
+  bool canceled_;
 
- private:
-  CompletionCallbackTask* const subtask_;
-  DISALLOW_COPY_AND_ASSIGN(ResolveTask);
+  int result_;
+  RRResponse response_;
+
+  DISALLOW_COPY_AND_ASSIGN(RRResolverWorker);
 };
-#endif
+
 
 // A Buffer is used for walking over a DNS packet.
 class Buffer {
@@ -309,7 +450,11 @@ class Buffer {
   const unsigned packet_len_;
 };
 
-}  // anonymous namespace
+bool RRResponse::HasExpired(const base::Time current_time) const {
+  const base::TimeDelta delta(base::TimeDelta::FromSeconds(ttl));
+  const base::Time expiry = fetch_time + delta;
+  return current_time >= expiry;
+}
 
 bool RRResponse::ParseFromResponse(const uint8* p, unsigned len,
                                    uint16 rrtype_requested) {
@@ -317,6 +462,7 @@ bool RRResponse::ParseFromResponse(const uint8* p, unsigned len,
   name.clear();
   ttl = 0;
   dnssec = false;
+  negative = false;
   rrdatas.clear();
   signatures.clear();
 
@@ -397,21 +543,197 @@ bool RRResponse::ParseFromResponse(const uint8* p, unsigned len,
 }
 
 
-// static
-bool DnsRRResolver::Resolve(const std::string& name, uint16 rrtype,
-                            uint16 flags, CompletionCallback* callback,
-                            RRResponse* response) {
+// An RRResolverJob is a one-to-one counterpart of an RRResolverWorker. It
+// lives only on the DnsRRResolver's origin message loop.
+class RRResolverJob {
+ public:
+  RRResolverJob(RRResolverWorker* worker)
+      : worker_(worker) {
+  }
+
+  ~RRResolverJob() {
+    Cancel(ERR_NAME_NOT_RESOLVED);
+  }
+
+  void AddHandle(RRResolverHandle* handle) {
+    handles_.push_back(handle);
+  }
+
+  void HandleResult(int result, const RRResponse& response) {
+    worker_ = NULL;
+    PostAll(result, &response);
+  }
+
+  void Cancel(int error) {
+    if (worker_) {
+      worker_->Cancel();
+      worker_ = NULL;
+    }
+
+    PostAll(error, NULL);
+  }
+
+ private:
+  void PostAll(int result, const RRResponse* response) {
+    std::vector<RRResolverHandle*> handles;
+    handles_.swap(handles);
+
+    for (std::vector<RRResolverHandle*>::iterator
+         i = handles.begin(); i != handles.end(); i++) {
+      (*i)->Post(result, response);
+      delete *i;
+    }
+  }
+
+  std::vector<RRResolverHandle*> handles_;
+  RRResolverWorker* worker_;
+};
+
+
+DnsRRResolver::DnsRRResolver()
+    : requests_(0),
+      cache_hits_(0),
+      inflight_joins_(0),
+      in_destructor_(false) {
+}
+
+DnsRRResolver::~DnsRRResolver() {
+  DCHECK(!in_destructor_);
+  in_destructor_ = true;
+  STLDeleteValues(&inflight_);
+}
+
+intptr_t DnsRRResolver::Resolve(const std::string& name, uint16 rrtype,
+                                uint16 flags, CompletionCallback* callback,
+                                RRResponse* response,
+                                int priority /* ignored */,
+                                const BoundNetLog& netlog /* ignored */) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(!in_destructor_);
+
   if (!callback || !response || name.empty())
-    return false;
+    return kInvalidHandle;
 
   // Don't allow queries of type ANY
   if (rrtype == kDNS_ANY)
-    return false;
+    return kInvalidHandle;
+
+  requests_++;
+
+  const std::pair<std::string, uint16> key(make_pair(name, rrtype));
+  // First check the cache.
+  std::map<std::pair<std::string, uint16>, RRResponse>::iterator i;
+  i = cache_.find(key);
+  if (i != cache_.end()) {
+    if (!i->second.HasExpired(base::Time::Now())) {
+      int error;
+      if (i->second.negative) {
+        error = ERR_NAME_NOT_RESOLVED;
+      } else {
+        error = OK;
+        *response = i->second;
+      }
+      RRResolverHandle* handle = new RRResolverHandle(
+          callback, NULL /* no response pointer because we've already filled */
+                         /* it in */);
+      cache_hits_++;
+      // We need a typed NULL pointer in order to make the templates work out.
+      static const RRResponse* kNoResponse = NULL;
+      MessageLoop::current()->PostTask(
+          FROM_HERE, NewRunnableMethod(handle, &RRResolverHandle::Post, error,
+                                       kNoResponse));
+      return reinterpret_cast<intptr_t>(handle);
+    } else {
+      // entry has expired.
+      cache_.erase(i);
+    }
+  }
+
+  // No cache hit. See if a request is currently in flight.
+  RRResolverJob* job;
+  std::map<std::pair<std::string, uint16>, RRResolverJob*>::const_iterator j;
+  j = inflight_.find(key);
+  if (j != inflight_.end()) {
+    // The request is in flight already. We'll just attach our callback.
+    inflight_joins_++;
+    job = j->second;
+  } else {
+    // Need to make a new request.
+    RRResolverWorker* worker = new RRResolverWorker(name, rrtype, flags, this);
+    job = new RRResolverJob(worker);
+    inflight_.insert(make_pair(key, job));
+    if (!worker->Start()) {
+      delete job;
+      delete worker;
+      return kInvalidHandle;
+    }
+  }
+
+  RRResolverHandle* handle = new RRResolverHandle(callback, response);
+  job->AddHandle(handle);
+  return reinterpret_cast<intptr_t>(handle);
+}
 
-  ResolveTask* task = new ResolveTask(name, rrtype, flags, callback, response,
-                                      MessageLoop::current());
+void DnsRRResolver::CancelResolve(intptr_t h) {
+  DCHECK(CalledOnValidThread());
+  RRResolverHandle* handle = reinterpret_cast<RRResolverHandle*>(h);
+  handle->Cancel();
+}
+
+void DnsRRResolver::OnIPAddressChanged() {
+  DCHECK(CalledOnValidThread());
+  DCHECK(!in_destructor_);
+
+  std::map<std::pair<std::string, uint16>, RRResolverJob*> inflight;
+  inflight.swap(inflight_);
+  cache_.clear();
+
+  for (std::map<std::pair<std::string, uint16>, RRResolverJob*>::iterator
+       i = inflight.begin(); i != inflight.end(); i++) {
+    i->second->Cancel(ERR_ABORTED);
+    delete i->second;
+  }
+}
+
+// HandleResult is called on the origin message loop.
+void DnsRRResolver::HandleResult(const std::string& name, uint16 rrtype,
+                                 int result, const RRResponse& response) {
+  DCHECK(CalledOnValidThread());
+
+  const std::pair<std::string, uint16> key(std::make_pair(name, rrtype));
+
+  DCHECK_GE(kMaxCacheEntries, 1u);
+  DCHECK_LE(cache_.size(), kMaxCacheEntries);
+  if (cache_.size() == kMaxCacheEntries) {
+    // need to remove an element of the cache.
+    const base::Time current_time(base::Time::Now());
+    for (std::map<std::pair<std::string, uint16>, RRResponse>::iterator
+         i = cache_.begin(); i != cache_.end(); ++i) {
+      if (i->second.HasExpired(current_time)) {
+        cache_.erase(i);
+        break;
+      }
+    }
+  }
+  if (cache_.size() == kMaxCacheEntries) {
+    // if we didn't clear out any expired entries, we just remove the first
+    // element. Crummy but simple.
+    cache_.erase(cache_.begin());
+  }
+
+  cache_.insert(std::make_pair(key, response));
+
+  std::map<std::pair<std::string, uint16>, RRResolverJob*>::iterator j;
+  j = inflight_.find(key);
+  if (j == inflight_.end()) {
+    NOTREACHED();
+    return;
+  }
+  RRResolverJob* job = j->second;
+  inflight_.erase(j);
 
-  return WorkerPool::PostTask(FROM_HERE, task, true /* task is slow */);
+  job->HandleResult(result, response);
+  delete job;
 }
 
 }  // namespace net
diff --git a/net/base/dnsrr_resolver.h b/net/base/dnsrr_resolver.h
index 28bceaa..b82b298 100644
--- a/net/base/dnsrr_resolver.h
+++ b/net/base/dnsrr_resolver.h
@@ -6,17 +6,27 @@
 #define NET_BASE_DNSRR_RESOLVER_H_
 #pragma once
 
+#include <map>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/non_thread_safe.h"
+#include "base/ref_counted.h"
+#include "base/time.h"
 #include "build/build_config.h"
 #include "net/base/completion_callback.h"
+#include "net/base/network_change_notifier.h"
+
+class MessageLoop;
 
 namespace net {
 
 // RRResponse contains the result of a successful request for a resource record.
 struct RRResponse {
+  RRResponse();
+
   // name contains the canonical name of the resulting domain. If the queried
   // name was a CNAME then this can differ.
   std::string name;
@@ -27,12 +37,27 @@ struct RRResponse {
   std::vector<std::string> rrdatas;
   // sigs contains the RRSIG records returned.
   std::vector<std::string> signatures;
+  // fetch_time is the time at which the response was received from the
+  // network.
+  base::Time fetch_time;
+  // negative is true if this is a negative cache entry, i.e. is a placeholder
+  // to remember that a given RR doesn't exist.
+  bool negative;
+
+  // HasExpired returns true if |fetch_time| + |ttl| is less than
+  // |current_time|.
+  bool HasExpired(base::Time current_time) const;
 
   // For testing only
   bool ParseFromResponse(const uint8* data, unsigned len,
                          uint16 rrtype_requested);
 };
 
+class BoundNetLog;
+class RRResolverWorker;
+class RRResolverJob;
+class RRResolverHandle;
+
 // DnsRRResolver resolves arbitary DNS resource record types. It should not be
 // confused with HostResolver and should not be used to resolve A/AAAA records.
 //
@@ -41,23 +66,70 @@ struct RRResponse {
 //
 // DnsRRResolver should only be used when the data is specifically DNS data and
 // the name is a fully qualified DNS domain.
-class DnsRRResolver {
+//
+// A DnsRRResolver must be used from the MessageLoop which created it.
+class DnsRRResolver : public NonThreadSafe,
+                      public NetworkChangeNotifier::Observer {
  public:
   enum {
+    kInvalidHandle = 0,
+  };
+
+  enum {
     // Try harder to get a DNSSEC signed response. This doesn't mean that the
     // RRResponse will always have the dnssec bit set.
     FLAG_WANT_DNSSEC = 1,
   };
 
+  typedef intptr_t Handle;
+
+  DnsRRResolver();
+  ~DnsRRResolver();
+
+  uint64 requests() const { return requests_; }
+  uint64 cache_hits() const { return cache_hits_; }
+  uint64 inflight_joins() const { return inflight_joins_; }
+
   // Resolve starts the resolution process. When complete, |callback| is called
   // with a result. If the result is |OK| then |response| is filled with the
-  // result of the resolution. Note the |callback| is called on the current
+  // result of the resolution. Note that |callback| is called via the current
   // MessageLoop.
-  static bool Resolve(const std::string& name, uint16 rrtype,
-                      uint16 flags, CompletionCallback* callback,
-                      RRResponse* response);
+  //
+  // This returns a handle value which can be passed to |CancelResolve|. If
+  // this function returns kInvalidHandle then the resolution failed
+  // immediately because it was improperly formed.
+  Handle Resolve(const std::string& name, uint16 rrtype,
+                 uint16 flags, CompletionCallback* callback,
+                 RRResponse* response, int priority,
+                 const BoundNetLog& netlog);
+
+  // CancelResolve cancels an inflight lookup. The callback for this lookup
+  // must not have already been called.
+  void CancelResolve(Handle handle);
+
+  // Implementation of NetworkChangeNotifier::Observer
+  virtual void OnIPAddressChanged();
 
  private:
+  friend class RRResolverWorker;
+
+  void HandleResult(const std::string& name, uint16 rrtype, int result,
+                    const RRResponse& response);
+
+  // cache maps from a request to a cached response. The cached answer may have
+  // expired and the size of |cache| must be <= kMaxCacheEntries.
+  //                < name      , rrtype>
+  std::map<std::pair<std::string, uint16>, RRResponse> cache_;
+  // inflight maps from a request to an active resolution which is taking
+  // place.
+  std::map<std::pair<std::string, uint16>, RRResolverJob*> inflight_;
+
+  uint64 requests_;
+  uint64 cache_hits_;
+  uint64 inflight_joins_;
+
+  bool in_destructor_;
+
   DISALLOW_COPY_AND_ASSIGN(DnsRRResolver);
 };
 
diff --git a/net/base/dnsrr_resolver_unittest.cc b/net/base/dnsrr_resolver_unittest.cc
index 23a0b40..f5b545b 100644
--- a/net/base/dnsrr_resolver_unittest.cc
+++ b/net/base/dnsrr_resolver_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/lock.h"
 #include "net/base/dns_util.h"
 #include "net/base/net_errors.h"
+#include "net/base/net_log.h"
 #include "net/base/test_completion_callback.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -19,49 +20,79 @@ class DnsRRResolverTest : public testing::Test {
 
 #if defined(OS_LINUX)
 
-class Rendezvous : public CallbackRunner<Tuple1<int> > {
+class ExplodingCallback : public CallbackRunner<Tuple1<int> > {
  public:
-  Rendezvous()
-      : have_result_(false),
-        cv_(&lock_) {
-  }
-
-  int WaitForResult() {
-    lock_.Acquire();
-    while (!have_result_)
-      cv_.Wait();
-    lock_.Release();
-    return result_;
-  }
-
   virtual void RunWithParams(const Tuple1<int>& params) {
-    lock_.Acquire();
-    result_ = params.a;
-    have_result_ = true;
-    lock_.Release();
-    cv_.Broadcast();
+    FAIL();
   }
-
- private:
-  bool have_result_;
-  int result_;
-  Lock lock_;
-  ConditionVariable cv_;
 };
 
 // This test is disabled because it depends on the external network to pass.
 // However, it may be useful when chaging the code.
-TEST_F(DnsRRResolverTest, DISABLED_NetworkResolve) {
+TEST_F(DnsRRResolverTest, Resolve) {
   RRResponse response;
-  Rendezvous callback;
-  ASSERT_TRUE(DnsRRResolver::Resolve(
-        "agl._pka.imperialviolet.org", kDNS_TXT, 0, &callback, &response));
+  TestCompletionCallback callback;
+  DnsRRResolver resolver;
+  DnsRRResolver::Handle handle;
+
+  handle = resolver.Resolve("www.testing.notatld", kDNS_TESTING, 0,
+                            &callback, &response, 0, BoundNetLog());
+  ASSERT_TRUE(handle != DnsRRResolver::kInvalidHandle);
   ASSERT_EQ(OK, callback.WaitForResult());
   ASSERT_EQ(1u, response.rrdatas.size());
-  ASSERT_EQ(1u, response.signatures.size());
-  ASSERT_STREQ("]v=pka1;fpr=2AF0032B48E856CE06157A1AD43C670DE04AAA74;"
-               "uri=http://www.imperialviolet.org/key.asc",
-               response.rrdatas[0].c_str());
+  ASSERT_STREQ("goats!", response.rrdatas[0].c_str());
+  ASSERT_EQ(1u, resolver.requests());
+  ASSERT_EQ(0u, resolver.cache_hits());
+  ASSERT_EQ(0u, resolver.inflight_joins());
+
+  // Test a cache hit.
+  handle = resolver.Resolve("www.testing.notatld", kDNS_TESTING, 0,
+                            &callback, &response, 0, BoundNetLog());
+  ASSERT_TRUE(handle != DnsRRResolver::kInvalidHandle);
+  ASSERT_EQ(OK, callback.WaitForResult());
+  ASSERT_EQ(1u, response.rrdatas.size());
+  ASSERT_STREQ("goats!", response.rrdatas[0].c_str());
+  ASSERT_EQ(2u, resolver.requests());
+  ASSERT_EQ(1u, resolver.cache_hits());
+  ASSERT_EQ(0u, resolver.inflight_joins());
+
+  // Test that a callback is never made. This depends on there before another
+  // test after this one which will pump the MessageLoop.
+  ExplodingCallback callback3;
+  handle = resolver.Resolve("www.testing.notatld", kDNS_TESTING, 0,
+                            &callback3, &response, 0, BoundNetLog());
+  ASSERT_TRUE(handle != DnsRRResolver::kInvalidHandle);
+  resolver.CancelResolve(handle);
+  ASSERT_EQ(3u, resolver.requests());
+  ASSERT_EQ(2u, resolver.cache_hits());
+  ASSERT_EQ(0u, resolver.inflight_joins());
+
+  // Test what happens in the event of a network config change.
+  handle = resolver.Resolve("nx.testing.notatld", kDNS_TESTING, 0,
+                            &callback, &response, 0, BoundNetLog());
+  ASSERT_TRUE(handle != DnsRRResolver::kInvalidHandle);
+  resolver.OnIPAddressChanged();
+  ASSERT_TRUE(callback.have_result());
+  ASSERT_EQ(ERR_ABORTED, callback.WaitForResult());
+  ASSERT_EQ(4u, resolver.requests());
+  ASSERT_EQ(2u, resolver.cache_hits());
+  ASSERT_EQ(0u, resolver.inflight_joins());
+
+  // Test an inflight join. (Note that this depends on the cache being flushed
+  // by OnIPAddressChanged.)
+  TestCompletionCallback callback2;
+  DnsRRResolver::Handle handle2;
+  handle = resolver.Resolve("nx.testing.notatld", kDNS_TESTING, 0,
+                            &callback, &response, 0, BoundNetLog());
+  ASSERT_TRUE(handle != DnsRRResolver::kInvalidHandle);
+  handle2 = resolver.Resolve("nx.testing.notatld", kDNS_TESTING, 0,
+                             &callback2, &response, 0, BoundNetLog());
+  ASSERT_TRUE(handle2 != DnsRRResolver::kInvalidHandle);
+  ASSERT_EQ(ERR_NAME_NOT_RESOLVED, callback.WaitForResult());
+  ASSERT_EQ(ERR_NAME_NOT_RESOLVED, callback2.WaitForResult());
+  ASSERT_EQ(6u, resolver.requests());
+  ASSERT_EQ(2u, resolver.cache_hits());
+  ASSERT_EQ(1u, resolver.inflight_joins());
 }
 
 // This is a DNS packet resulting from querying a recursive resolver for a TXT