[net] HostResolverImpl + DnsTransaction + DnsConfigService = Asynchronous DNS ready for experiments.

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 d9330e213cebb1405002629e2fda5e6042329fac..c7b8c925d947b9b90e7cb835d0b0136f7ac39716 100644
--- a/net/base/host_resolver_impl.cc
+++ b/net/base/host_resolver_impl.cc
@@ -24,12 +24,14 @@
 #include "base/message_loop_proxy.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
+#include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/threading/worker_pool.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
+#include "net/base/address_family.h"
 #include "net/base/address_list.h"
 #include "net/base/address_list_net_log_param.h"
 #include "net/base/dns_reloader.h"
@@ -38,6 +40,12 @@
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
+#include "net/dns/dns_config_service.h"
+#include "net/dns/dns_protocol.h"
+#include "net/dns/dns_response.h"
+#include "net/dns/dns_session.h"
+#include "net/dns/dns_transaction.h"
+#include "net/socket/client_socket_factory.h"
 
 #if defined(OS_WIN)
 #include "net/base/winsock_init.h"
@@ -315,8 +323,8 @@ void LogCancelRequest(const BoundNetLog& source_net_log,
 // Keeps track of the highest priority.
 class PriorityTracker {
  public:
-  PriorityTracker()
-      : highest_priority_(IDLE), total_count_(0) {
+  PriorityTracker(RequestPriority priority)

[cbentzel, 2012/02/14 13:08:26]

explicit

+      : highest_priority_(priority), total_count_(0) {

[cbentzel, 2012/02/14 13:08:26]

This was a little tricky - I expected total_count_ and counts_[priority] to both
be 1. Once I looked at how the AddRequest was made common for the
new-job/existing-job case it became clear why this was needed, but may benefit
from a note.

     memset(counts_, 0, sizeof(counts_));
   }
 
@@ -359,22 +367,66 @@ class PriorityTracker {
 
 //-----------------------------------------------------------------------------
 
+// Convenience wrapper for PrioritizedDispatcher::Handle so that we don't forget
+// to update it when cancelling or changing the priority.
+// TODO(szym): consider making PrioritizedDispatcher::Handle into this.
+class DispatcherHandle {
+ public:
+  DispatcherHandle() : dispatcher_(NULL) {}
+  DispatcherHandle(PrioritizedDispatcher* dispatcher,
+                   const PrioritizedDispatcher::Handle& handle)
+      : dispatcher_(dispatcher), handle_(handle) {
+    if (handle_.is_null())
+      dispatcher_ = NULL;
+  }
+
+  bool is_null() const {
+    return dispatcher_ == NULL;
+  }
+
+  void Reset() {
+    dispatcher_ = NULL;
+    handle_ = PrioritizedDispatcher::Handle();
+  }
+
+  void ChangePriority(RequestPriority priority) {
+    DCHECK(!is_null());
+    handle_ = dispatcher_->ChangePriority(handle_, priority);
+  }
+
+  void Cancel() {
+    DCHECK(!is_null());
+    dispatcher_->Cancel(handle_);
+    Reset();
+  }
+
+ private:
+  PrioritizedDispatcher* dispatcher_;
+  PrioritizedDispatcher::Handle handle_;
+};
+
+//-----------------------------------------------------------------------------
+
 HostResolver* CreateHostResolver(size_t max_concurrent_resolves,
                                  size_t max_retry_attempts,
-                                 bool use_cache,
+                                 HostCache* cache,
+                                 scoped_ptr<DnsConfigService> config_service,
                                  NetLog* net_log) {
   if (max_concurrent_resolves == HostResolver::kDefaultParallelism)
     max_concurrent_resolves = kDefaultMaxProcTasks;
 
   // TODO(szym): Add experiments with reserved slots for higher priority
   // requests.
+  // TODO(szym): Add tighter limits for proc_dispatcher_

[cbentzel, 2012/02/14 13:12:06]

Why do you need this TODO? In this case it will only be using the limits
specified for the regular dispatcher_, and these are the same limits as before.

Also - at one point I thought we had different queues for
speculative/non-speculative DNS queries. Should we do some reserved slots for
non-idle cases? 

[szym, 2012/02/14 15:56:24]

I need this TODO to remember that even if I start using proc_dispatcher_, it
will never queue a job unless the its limits are tighter than those of
dispatcher_. Right now proc_dispatcher_ is not used, and I needed to put some
limits in there. I'll change it to PrioritizedDispatcher::Limits(NUM_PRIORITIES,
0);
 
Even before the big refactor of HostResolverImpl, the is_speculative flag wasn't
used to determine the priority of the Job. There was the JobPool facility, but
with only one instance, there really wasn't any reservation in place.

I think it might be time for an experiment with tiered limits. We'd need to
check the call-sites to Resolve to see if request priority is properly set.

 
   PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, max_concurrent_resolves);
 
   HostResolverImpl* resolver = new HostResolverImpl(
-      use_cache ? HostCache::CreateDefaultCache() : NULL,
+      cache,
+      limits,
       limits,
       HostResolverImpl::ProcTaskParams(NULL, max_retry_attempts),
+      config_service.Pass(),
       net_log);
 
   return resolver;
@@ -389,7 +441,8 @@ HostResolver* CreateSystemHostResolver(size_t max_concurrent_resolves,
                                        NetLog* net_log) {
   return CreateHostResolver(max_concurrent_resolves,
                             max_retry_attempts,
-                            true /* use_cache */,
+                            HostCache::CreateDefaultCache(),
+                            scoped_ptr<DnsConfigService>(NULL),
                             net_log);
 }
 
@@ -398,7 +451,21 @@ HostResolver* CreateNonCachingSystemHostResolver(size_t max_concurrent_resolves,
                                                  NetLog* net_log) {
   return CreateHostResolver(max_concurrent_resolves,
                             max_retry_attempts,
-                            false /* use_cache */,
+                            NULL,
+                            scoped_ptr<DnsConfigService>(NULL),
+                            net_log);
+}
+
+HostResolver* CreateAsyncHostResolver(size_t max_concurrent_resolves,
+                                      size_t max_retry_attempts,
+                                      NetLog* net_log) {
+  scoped_ptr<DnsConfigService> config_service
+      = DnsConfigService::CreateSystemService();

[mmenke, 2012/02/14 18:46:00]

nit:  Put the equals sign on the previous line (Can't find a single instance
where we don't do that in net/, and only one in chrome/)

+  config_service->Watch();
+  return CreateHostResolver(max_concurrent_resolves,
+                            max_retry_attempts,
+                            HostCache::CreateDefaultCache(),
+                            config_service.Pass(),
                             net_log);
 }
 
@@ -505,7 +572,9 @@ class HostResolverImpl::Request {
 class HostResolverImpl::ProcTask
     : public base::RefCountedThreadSafe<HostResolverImpl::ProcTask> {
  public:
-  typedef base::Callback<void(int, int, const AddressList&)> Callback;
+  typedef base::Callback<void(int net_error,
+                              int os_error,
+                              const AddressList& addr_list)> Callback;
 
   ProcTask(const Key& key,
            const ProcTaskParams& params,
@@ -891,6 +960,8 @@ class HostResolverImpl::ProcTask
 //-----------------------------------------------------------------------------
 
 // Represents a request to the worker pool for a "probe for IPv6 support" call.
+//
+// TODO(szym): This could also be replaced with PostTaskAndReply and Callbacks.
 class HostResolverImpl::IPv6ProbeJob
     : public base::RefCountedThreadSafe<HostResolverImpl::IPv6ProbeJob> {
  public:
@@ -958,6 +1029,74 @@ class HostResolverImpl::IPv6ProbeJob
 
 //-----------------------------------------------------------------------------
 
+// Resolves the hostname using DnsTransaction.
+// TODO(szym): This could be moved to separate source file as well.
+class HostResolverImpl::DnsTask {
+ public:
+  typedef base::Callback<void(int net_error,
+                              const AddressList& addr_list,
+                              base::TimeDelta ttl)> Callback;
+
+  DnsTask(DnsTransactionFactory* factory,
+          const Key& key,
+          const Callback& callback,
+          const BoundNetLog& job_net_log)
+      : callback_(callback), net_log_(job_net_log) {
+    DCHECK(factory);
+    DCHECK(!callback.is_null());
+
+    // For now we treat ADDRESS_FAMILY_UNSPEC as if it was IPV4.
+    uint16 qtype = (key.address_family == ADDRESS_FAMILY_IPV6)
+                   ? dns_protocol::kTypeAAAA
+                   : dns_protocol::kTypeA;
+    // TODO(szym): Implement "happy eyeballs".
+    transaction_ = factory->CreateTransaction(
+        key.hostname,
+        qtype,
+        base::Bind(&DnsTask::OnTransactionComplete, base::Unretained(this)),
+        net_log_);
+    DCHECK(transaction_.get());
+  }
+
+  int Start() {
+    net_log_.BeginEvent(
+        NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK,
+        new NetLogSourceParameter("source_dependency",
+                                  transaction_->net_log().source()));
+    return transaction_->Start();
+  }
+
+  void OnTransactionComplete(DnsTransaction* transaction,
+                             int net_error,
+                             const DnsResponse* response) {
+    // TODO(szym): Record performance histograms.
+    if (net_error == OK) {
+      AddressList addr_list;
+      base::TimeDelta ttl;
+      if (response->ParseAddressList(&addr_list, &ttl)) {
+        net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK,
+                          new AddressListNetLogParam(addr_list));
+        callback_.Run(net_error, addr_list, ttl);
+        return;
+      }
+      net_error = ERR_DNS_MALFORMED_RESPONSE;
+    }
+    callback_.Run(net_error, AddressList(), base::TimeDelta());
+    net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK,
+                                      net_error);
+  }
+
+ private:
+  // The listener to the results of this DnsTask.
+  Callback callback_;
+
+  const BoundNetLog net_log_;
+
+  scoped_ptr<DnsTransaction> transaction_;
+};
+
+//-----------------------------------------------------------------------------
+
 // Aggregates all Requests for the same Key. Dispatched via PriorityDispatch.
 // Spawns ProcTask when started.

[cbentzel, 2012/02/14 13:08:26]

The comment "Spawns ProcTask when started" is no longer completely accurate.

 class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
@@ -966,9 +1105,11 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
   // request that spawned it.
   Job(HostResolverImpl* resolver,
       const Key& key,
-      const BoundNetLog& request_net_log)
+      const BoundNetLog& request_net_log,
+      RequestPriority priority)
       : resolver_(resolver->AsWeakPtr()),
         key_(key),
+        priority_tracker_(priority),
         had_non_speculative_request_(false),
         net_log_(BoundNetLog::Make(request_net_log.net_log(),
                                    NetLog::SOURCE_HOST_RESOLVER_IMPL_JOB)),
@@ -1009,10 +1150,6 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
     STLDeleteElements(&requests_);
   }
 
-  HostResolverImpl* resolver() const {
-    return resolver_;
-  }
-
   RequestPriority priority() const {
     return priority_tracker_.highest_priority();
   }
@@ -1030,13 +1167,13 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
     return net_error_;
   }
 
-  // Used by HostResolverImpl with |dispatcher_|.
-  const PrioritizedDispatcher::Handle& handle() const {
-    return handle_;
+  void AddToDispatcher(PrioritizedDispatcher* dispatcher) {
+    DCHECK(handle_.is_null());
+    handle_ = DispatcherHandle(dispatcher, dispatcher->Add(this, priority()));
   }
 
-  void set_handle(const PrioritizedDispatcher::Handle& handle) {
-    handle_ = handle;
+  bool IsWaitingInDispatch() const {
+    return !handle_.is_null();
   }
 
   // The Job will own |req| and destroy it in ~Job.

[cbentzel, 2012/02/14 13:08:26]

Nit: change this to take a scoped_ptr<Request> argument instead?

@@ -1064,6 +1201,9 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
       if (proc_task_)
         proc_task_->set_had_non_speculative_request();
     }
+
+    if (!handle_.is_null())
+      handle_.ChangePriority(priority());
   }
 
   void CancelRequest(Request* req) {
@@ -1078,44 +1218,72 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_REQUEST_DETACH,
         make_scoped_refptr(new JobAttachParameters(
             req->request_net_log().source(), priority())));
+
+    if (!handle_.is_null()) {
+      if (num_active_requests() > 0) {
+        handle_.ChangePriority(priority());
+      } else {
+        handle_.Cancel();
+      }
+    }
   }
 
   // Aborts and destroys the job, completes all requests as aborted.
   void Abort() {
     // Job should only be aborted if it's running.
     DCHECK(is_running());

[cbentzel, 2012/02/14 13:08:26]

Should this remove itself from the PrioritizedDispatcher if !handle_.is_null()

[szym, 2012/02/14 15:56:24]

If is_running() then handle_ should be null. (A running Job cannot be queued.) I
will add a DCHECK for that.

-    proc_task_->Cancel();
-    proc_task_ = NULL;
+    if (proc_task_.get()) {
+      proc_task_->Cancel();
+      proc_task_ = NULL;
+    }
+    dns_task_.reset();
     net_error_ = ERR_ABORTED;
     os_error_ = 0;
-    CompleteRequests(AddressList());
+    CompleteRequests(AddressList(), base::TimeDelta());
   }
 
-  bool is_running() const {
+  bool is_dns_running() const {
+    return dns_task_.get() != NULL;
+  }
+
+  bool is_proc_running() const {
     return proc_task_.get() != NULL;
   }
 
+  bool is_running() const {
+    return is_dns_running() || is_proc_running();
+  }
+
   // Called by HostResolverImpl when this job is evicted due to queue overflow.
   void OnEvicted() {
     // Must not be running.
     DCHECK(!is_running());
-    handle_ = PrioritizedDispatcher::Handle();
+    handle_.Reset();
 
     net_log_.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_EVICTED, NULL);
 
     // This signals to CompleteRequests that this job never ran.
     net_error_ = ERR_HOST_RESOLVER_QUEUE_TOO_LARGE;
     os_error_ = 0;
-    CompleteRequests(AddressList());
+    CompleteRequests(AddressList(), base::TimeDelta());
   }
 
   // PriorityDispatch::Job interface.
   virtual void Start() OVERRIDE {
     DCHECK(!is_running());
-    handle_ = PrioritizedDispatcher::Handle();
+    handle_.Reset();
 
     net_log_.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_STARTED, NULL);
 
+    if (resolver_->dns_transaction_factory_.get()) {
+      StartDnsTask();
+    } else {
+      StartProcTask();
+    }
+  }
+
+ private:
+  void StartProcTask() {
     proc_task_ = new ProcTask(
         key_,
         resolver_->proc_params_,
@@ -1129,33 +1297,70 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
     proc_task_->Start();
   }
 
- private:
+  void StartDnsTask() {
+    dns_task_.reset(new DnsTask(
+        resolver_->dns_transaction_factory_.get(),
+        key_,
+        base::Bind(&Job::OnDnsTaskComplete, base::Unretained(this)),
+        net_log_));
+
+    int rv = dns_task_->Start();
+    if (rv != ERR_IO_PENDING) {
+      DCHECK_NE(OK, rv);
+      // TODO(szym): AddToDispatcher(resolver_->proc_dispatcher_);
+      StartProcTask();
+    }
+  }
+
   // Called by ProcTask when it completes.
   void OnProcTaskComplete(int net_error, int os_error,

[cbentzel, 2012/02/14 13:08:26]

Nit: probably easier to read if this were moved directly under StartProcTask.

-                          const AddressList& addrlist) {
-    DCHECK(is_running());
+                          const AddressList& addr_list) {
+    DCHECK(is_proc_running());
     proc_task_ = NULL;
     net_error_ = net_error;
     os_error_ = os_error;
 
-    // We are the only consumer of |addrlist|, so we can safely change the port
-    // without copy-on-write. This pays off, when job has only one request.
-    AddressList list = addrlist;
-    if (net_error == OK && !requests_.empty())
-      MutableSetPort(requests_.front()->info().port(), &list);
-    CompleteRequests(list);
+    base::TimeDelta ttl = base::TimeDelta::FromSeconds(0);

[cbentzel, 2012/02/14 13:08:26]

Should there be a constant for negative cache TTL? 

+    if (net_error == OK)
+      ttl = base::TimeDelta::FromSeconds(kCacheEntryTTLSeconds);
+    CompleteRequests(addr_list, ttl);
+  }
+
+  // Called by DnsTask when it completes.
+  void OnDnsTaskComplete(int net_error,
+                         const AddressList& addr_list,
+                         base::TimeDelta ttl) {
+    DCHECK(is_dns_running());
+    dns_task_.reset();
+
+    if (net_error != OK) {
+      // TODO(szym): Some net errors indicate lack of connectivity. Starting
+      // ProcTask in that case is a waste of time.
+      StartProcTask();
+      return;
+    }
+
+    net_error_ = OK;
+    os_error_ = 0;
+    CompleteRequests(addr_list, ttl);
   }
 
   // Completes all Requests. Calls OnJobFinished and deletes self.
-  void CompleteRequests(const AddressList& addrlist) {
+  void CompleteRequests(const AddressList& addr_list, base::TimeDelta ttl) {
     CHECK(resolver_);
 
+    // We are the only consumer of |addr_list|, so we can safely change the port
+    // without copy-on-write. This pays off, when job has only one request.
+    AddressList list = addr_list;
+    if (net_error_ == OK && !requests_.empty())
+      MutableSetPort(requests_.front()->info().port(), &list);
+
     // This job must be removed from resolver's |jobs_| now to make room for a
     // new job with the same key in case one of the OnComplete callbacks decides
     // to spawn one. Consequently, the job deletes itself when CompleteRequests
     // is done.
     scoped_ptr<Job> self_deleter(this);
-    resolver_->OnJobFinished(this, addrlist);
+    resolver_->OnJobFinished(this, addr_list, ttl);

[cbentzel, 2012/02/14 13:08:26]

Does this need to be called after deleting self? Will this Job still be in the
PrioritizedDispatcher?

[szym, 2012/02/14 15:56:24]

I'm not clear what you mean. After deleting self I cannot access |resolver_|.
The critical ordering is that the Job is removed from |HostResolverImpl::jobs_|
before running any of the Request callbacks.

On second point: this Job cannot be in the dispatcher. CompleteRequests applies
only if the Job has already been started, which means it is out of the
dispatcher.

[cbentzel, 2012/02/14 18:01:27]

It was the second point I was concerned about. Agree that it's out of the
dispatcher. If it did do after delete self, it would need to copy resolver_ to a
stack variable to prevent the use-after-free. 

 
     // Complete all of the requests that were attached to the job.
     for (RequestsList::const_iterator it = requests_.begin();
@@ -1170,7 +1375,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
       LogFinishRequest(req->source_net_log(), req->request_net_log(),
                        req->info(), net_error_, os_error_);
 
-      req->OnComplete(net_error_, addrlist);
+      req->OnComplete(net_error_, addr_list);
 
       // Check if the resolver was destroyed as a result of running the
       // callback. If it was, we could continue, but we choose to bail.
@@ -1179,7 +1384,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
     }
   }
 
-  // Used to call OnJobFinished and RemoveJob.
+  // Used to call OnJobFinished.
   base::WeakPtr<HostResolverImpl> resolver_;
 
   Key key_;
@@ -1195,14 +1400,17 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
   int net_error_;
   int os_error_;
 
-  // A ProcTask created and started when this Job is dispatched..
+  // Resolves the host using a HostResolverProc.
   scoped_refptr<ProcTask> proc_task_;
 
+  // Resolves the host using a DnsTransaction.
+  scoped_ptr<DnsTask> dns_task_;
+
   // All Requests waiting for the result of this Job. Some can be canceled.
   RequestsList requests_;
 
   // A handle used by HostResolverImpl in |dispatcher_|.
-  PrioritizedDispatcher::Handle handle_;
+  DispatcherHandle handle_;
 };
 
 //-----------------------------------------------------------------------------
@@ -1221,18 +1429,23 @@ HostResolverImpl::ProcTaskParams::~ProcTaskParams() {}
 HostResolverImpl::HostResolverImpl(
     HostCache* cache,
     const PrioritizedDispatcher::Limits& job_limits,
+    const PrioritizedDispatcher::Limits& proc_limits,
     const ProcTaskParams& proc_params,
+    scoped_ptr<DnsConfigService> dns_config_service,
     NetLog* net_log)
     : cache_(cache),
       dispatcher_(job_limits),
+      proc_dispatcher_(proc_limits),

[cbentzel, 2012/02/14 13:08:26]

Is proc_dispatcher_ even used? It looks like we just use dispatcher_ for all
scheduling.

It looks like there is a TODO for it. However, we'd need to add some state to
Job to know whether to do a DNS task or a Proc task. Also, we'd probably want to
go back up to HostResolverImpl if the DNS Job completes so it can be removed
from dispatcher_, and then a new Job added to prioritized_dispatcher_. This may
be a bit ugly because we'd need to transfer the requests over.

[szym, 2012/02/14 15:56:24]

Right now it is not. I'm okay removing it until we determine that we need
tighter limits for ProcTask than for DnsTask (which is expected, but we do not
yet have particular numbers in mind).
Not needed. A Job that has a DnsTask cannot be in any dispatcher. I'll add some
comments to make it more apparent.

[cbentzel, 2012/02/14 18:01:27]

I'd recommend removing it for now, as well as the additional set of limits
passed into HostResolverImpl.

       max_queued_jobs_(job_limits.total_jobs * 100u),
       proc_params_(proc_params),
       default_address_family_(ADDRESS_FAMILY_UNSPECIFIED),
+      dns_config_service_(dns_config_service.Pass()),
       ipv6_probe_monitoring_(false),
       additional_resolver_flags_(0),
       net_log_(net_log) {
 
   DCHECK_GE(dispatcher_.num_priorities(), static_cast<size_t>(NUM_PRIORITIES));
+  DCHECK_EQ(proc_dispatcher_.num_priorities(), dispatcher_.num_priorities());
 
   // Maximum of 4 retry attempts for host resolution.
   static const size_t kDefaultMaxRetryAttempts = 4u;
@@ -1254,6 +1467,9 @@ HostResolverImpl::HostResolverImpl(
 #endif
   NetworkChangeNotifier::AddDNSObserver(this);
 #endif
+
+  if (dns_config_service_.get())
+    dns_config_service_->AddObserver(this);
 }
 
 HostResolverImpl::~HostResolverImpl() {
@@ -1307,8 +1523,8 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
   Job* job;
   if (jobit == jobs_.end()) {
     // Create new Job.
-    job = new Job(this, key, request_net_log);
-    job->set_handle(dispatcher_.Add(job, info.priority()));
+    job = new Job(this, key, request_net_log, info.priority());
+    job->AddToDispatcher(&dispatcher_);
 
     // Check for queue overflow.
     if (dispatcher_.num_queued_jobs() > max_queued_jobs_) {
@@ -1332,8 +1548,6 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
   Request* req = new Request(source_net_log, request_net_log, info, callback,
                              addresses);
   job->AddRequest(req);
-  if (!job->handle().is_null())
-    job->set_handle(dispatcher_.ChangePriority(job->handle(), job->priority()));
   if (out_req)
     *out_req = reinterpret_cast<RequestHandle>(req);
 
@@ -1391,23 +1605,16 @@ void HostResolverImpl::CancelRequest(RequestHandle req_handle) {
 
   job->CancelRequest(req);
 
-  if (!job->handle().is_null()) {
-    // Still in queue.
-    if (job->num_active_requests()) {
-      job->set_handle(dispatcher_.ChangePriority(job->handle(),
-                                                 job->priority()));
+  if (job->num_active_requests() == 0) {
+    if (job->is_running()) {
+      // Job is running (and could be in CompleteRequests right now).
+      // But to be in Request::OnComplete we would have to have a non-canceled
+      // request. So it is safe to Abort it if it has no more active requests.
+      job->Abort();
     } else {
-      dispatcher_.Cancel(job->handle());
       RemoveJob(job);
       delete job;
     }
-  } else {
-    // Job is running (and could be in CompleteRequests right now).
-    // But to be in Request::OnComplete we would have to have a non-canceled
-    // request. So it is safe to Abort it if it has no more active requests.
-    if (!job->num_active_requests()) {
-      job->Abort();
-    }
   }
 }
 
@@ -1482,9 +1689,11 @@ bool HostResolverImpl::ServeFromCache(const Key& key,
   return true;
 }
 
-void HostResolverImpl::OnJobFinished(Job* job, const AddressList& addrlist) {
+void HostResolverImpl::OnJobFinished(Job* job,
+                                     const AddressList& addrlist,
+                                     base::TimeDelta ttl) {
   DCHECK(job);
-  DCHECK(job->handle().is_null());
+  DCHECK(!job->IsWaitingInDispatch());
   RemoveJob(job);
   if (job->net_error() == ERR_HOST_RESOLVER_QUEUE_TOO_LARGE)
     return;
@@ -1495,9 +1704,6 @@ void HostResolverImpl::OnJobFinished(Job* job, const AddressList& addrlist) {
     return;
   // Write result to the cache.
   if (cache_.get()) {
-    base::TimeDelta ttl = base::TimeDelta::FromSeconds(0);
-    if (job->net_error() == OK)
-      ttl = base::TimeDelta::FromSeconds(kCacheEntryTTLSeconds);
     cache_->Set(job->key(), job->net_error(), addrlist,
                 base::TimeTicks::Now(), ttl);
   }
@@ -1557,7 +1763,7 @@ void HostResolverImpl::AbortAllInProgressJobs() {
         return;
     } else {
       // Keep it in |dispatch_|.
-      DCHECK(!job->handle().is_null());
+      DCHECK(job->IsWaitingInDispatch());
     }
   }
 }
@@ -1594,4 +1800,22 @@ void HostResolverImpl::OnDNSChanged() {
   // |this| may be deleted inside AbortAllInProgressJobs().
 }
 
+void HostResolverImpl::OnConfigChanged(const DnsConfig& dns_config) {
+  // We want a new factory in place, before we Abort running Jobs, so that the
+  // newly started jobs use the new factory.
+  bool had_factory = (dns_transaction_factory_.get() != NULL);
+  if (dns_config.IsValid()) {
+    dns_transaction_factory_ = DnsTransactionFactory::CreateFactory(
+        new DnsSession(dns_config,
+                       ClientSocketFactory::GetDefaultFactory(),
+                       base::Bind(&base::RandInt),
+                       net_log_));
+  } else {
+    dns_transaction_factory_.reset();
+  }
+  if (had_factory) {
+    OnDNSChanged();
+  }
+}
+
 }  // namespace net