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

net/base/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/base/host_resolver_impl.h"
 
 #if defined(OS_WIN)
 #include <Winsock2.h>
 #elif defined(OS_POSIX)
 #include <netdb.h>
 #endif
 
 #include <cmath>
 #include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/debug/debugger.h"
 #include "base/debug/stack_trace.h"
 #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"
 #include "net/base/host_port_pair.h"
 #include "net/base/host_resolver_proc.h"
 #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"
 #endif
 
 namespace net {
 
 namespace {
 
 // Limit the size of hostnames that will be resolved to combat issues in
 // some platform's resolvers.
 const size_t kMaxHostLength = 4096;
 
 // Default TTL for successful resolutions with ProcTask.
 const unsigned kCacheEntryTTLSeconds = 60;
 
+// Default TTL for unsuccessful resolutions with ProcTask.
+const unsigned kNegativeCacheEntryTTLSeconds = 0;
+
 // Maximum of 8 concurrent resolver threads (excluding retries).
 // Some routers (or resolvers) appear to start to provide host-not-found if
 // too many simultaneous resolutions are pending.  This number needs to be
 // further optimized, but 8 is what FF currently does.
 static const size_t kDefaultMaxProcTasks = 8u;
 
 // Helper to mutate the linked list contained by AddressList to the given
 // port. Note that in general this is dangerous since the AddressList's
 // data might be shared (and you should use AddressList::SetPort).
 //
 // However since we allocated the AddressList ourselves we can safely
 // do this optimization and avoid reallocating the list.
-void MutableSetPort(int port, AddressList* addrlist) {
+void MutableSetPort(int port, AddressList* addr_list) {
   struct addrinfo* mutable_head =
-      const_cast<struct addrinfo*>(addrlist->head());
+      const_cast<struct addrinfo*>(addr_list->head());
   SetPortForAllAddrinfos(mutable_head, port);
 }
 
 // We use a separate histogram name for each platform to facilitate the
 // display of error codes by their symbolic name (since each platform has
 // different mappings).
 const char kOSErrorsForGetAddrinfoHistogramName[] =
 #if defined(OS_WIN)
     "Net.OSErrorsForGetAddrinfo_Win";
 #elif defined(OS_MACOSX)
@@ skipping 55 matching lines @@
 }
 
 // Wraps call to SystemHostResolverProc as an instance of HostResolverProc.
 // TODO(szym): This should probably be declared in host_resolver_proc.h.
 class CallSystemHostResolverProc : public HostResolverProc {
  public:
   CallSystemHostResolverProc() : HostResolverProc(NULL) {}
   virtual int Resolve(const std::string& hostname,
                       AddressFamily address_family,
                       HostResolverFlags host_resolver_flags,
-                      AddressList* addrlist,
+                      AddressList* addr_list,
                       int* os_error) OVERRIDE {
     return SystemHostResolverProc(hostname,
                                   address_family,
                                   host_resolver_flags,
-                                  addrlist,
+                                  addr_list,
                                   os_error);
   }
 };
 
 // Extra parameters to attach to the NetLog when the resolve failed.
-class HostResolveFailedParams : public NetLog::EventParameters {
+class ProcTaskFailedParams : public NetLog::EventParameters {
  public:
-  HostResolveFailedParams(uint32 attempt_number,
-                          int net_error,
-                          int os_error)
+  ProcTaskFailedParams(uint32 attempt_number, int net_error, int os_error)
       : attempt_number_(attempt_number),
         net_error_(net_error),
         os_error_(os_error) {
   }
 
   virtual Value* ToValue() const OVERRIDE {
     DictionaryValue* dict = new DictionaryValue();
     if (attempt_number_)
       dict->SetInteger("attempt_number", attempt_number_);
 
@@ skipping 21 matching lines @@
 
     return dict;
   }
 
  private:
   const uint32 attempt_number_;
   const int net_error_;
   const int os_error_;
 };
 
+// Extra parameters to attach to the NetLog when the DnsTask failed.
+class DnsTaskFailedParams : public NetLog::EventParameters {
+ public:
+  DnsTaskFailedParams(int net_error, int dns_error)
+      : net_error_(net_error), dns_error_(dns_error) {
+  }
+
+  virtual Value* ToValue() const OVERRIDE {
+    DictionaryValue* dict = new DictionaryValue();
+    dict->SetInteger("net_error", net_error_);
+    if (dns_error_)
+      dict->SetInteger("dns_error", dns_error_);
+    return dict;
+  }
+
+ private:
+  const int net_error_;
+  const int dns_error_;
+};
+
 // Parameters representing the information in a RequestInfo object, along with
 // the associated NetLog::Source.
 class RequestInfoParameters : public NetLog::EventParameters {
  public:
   RequestInfoParameters(const HostResolver::RequestInfo& info,
                         const NetLog::Source& source)
       : info_(info), source_(source) {}
 
   virtual Value* ToValue() const OVERRIDE {
     DictionaryValue* dict = new DictionaryValue();
     dict->SetString("host", info_.host_port_pair().ToString());
     dict->SetInteger("address_family",
                      static_cast<int>(info_.address_family()));
     dict->SetBoolean("allow_cached_response", info_.allow_cached_response());
     dict->SetBoolean("is_speculative", info_.is_speculative());
     dict->SetInteger("priority", info_.priority());
 
     if (source_.is_valid())
       dict->Set("source_dependency", source_.ToValue());
 
     return dict;
   }
 
  private:
   const HostResolver::RequestInfo info_;
   const NetLog::Source source_;
 };
 
-// Parameters associated with the creation of a HostResolverImpl::Job
-// or a HostResolverImpl::ProcTask.
+// Parameters associated with the creation of a HostResolverImpl::Job.
 class JobCreationParameters : public NetLog::EventParameters {
  public:
   JobCreationParameters(const std::string& host,
                         const NetLog::Source& source)
       : host_(host), source_(source) {}
 
   virtual Value* ToValue() const OVERRIDE {
     DictionaryValue* dict = new DictionaryValue();
     dict->SetString("host", host_);
     dict->Set("source_dependency", source_.ToValue());
@@ skipping 39 matching lines @@
   request_net_log.BeginEvent(
       NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST,
       make_scoped_refptr(new RequestInfoParameters(
           info, source_net_log.source())));
 }
 
 // Logs when a request has just completed (before its callback is run).
 void LogFinishRequest(const BoundNetLog& source_net_log,
                       const BoundNetLog& request_net_log,
                       const HostResolver::RequestInfo& info,
-                      int net_error,
-                      int os_error) {
-  scoped_refptr<NetLog::EventParameters> params;
-  if (net_error != OK) {
-    params = new HostResolveFailedParams(0, net_error, os_error);
-  }
-
-  request_net_log.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST, params);
+                      int net_error) {
+  request_net_log.EndEventWithNetErrorCode(
+      NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST, net_error);
   source_net_log.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL, NULL);
 }
 
 // Logs when a request has been cancelled.
 void LogCancelRequest(const BoundNetLog& source_net_log,
                       const BoundNetLog& request_net_log,
                       const HostResolverImpl::RequestInfo& info) {
   request_net_log.AddEvent(NetLog::TYPE_CANCELLED, NULL);
   request_net_log.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST, NULL);
   source_net_log.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL, NULL);
 }
 
 //-----------------------------------------------------------------------------
 
 // Keeps track of the highest priority.
 class PriorityTracker {
  public:
-  PriorityTracker()
-      : highest_priority_(IDLE), total_count_(0) {
+  explicit PriorityTracker(RequestPriority priority)
+      : highest_priority_(priority), total_count_(0) {
     memset(counts_, 0, sizeof(counts_));
   }
 
   RequestPriority highest_priority() const {
     return highest_priority_;
   }
 
   size_t total_count() const {
     return total_count_;
   }
@@ skipping 24 matching lines @@
  private:
   RequestPriority highest_priority_;
   size_t total_count_;
   size_t counts_[NUM_PRIORITIES];
 };
 
 //-----------------------------------------------------------------------------
 
 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.
 
   PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, max_concurrent_resolves);
 
   HostResolverImpl* resolver = new HostResolverImpl(
-      use_cache ? HostCache::CreateDefaultCache() : NULL,
+      cache,
       limits,
       HostResolverImpl::ProcTaskParams(NULL, max_retry_attempts),
+      config_service.Pass(),
       net_log);
 
   return resolver;
 }
 
 }  // anonymous namespace
 
 //-----------------------------------------------------------------------------
 
 HostResolver* CreateSystemHostResolver(size_t max_concurrent_resolves,
                                        size_t max_retry_attempts,
                                        NetLog* net_log) {
   return CreateHostResolver(max_concurrent_resolves,
                             max_retry_attempts,
-                            true /* use_cache */,
+                            HostCache::CreateDefaultCache(),
+                            scoped_ptr<DnsConfigService>(NULL),
                             net_log);
 }
 
 HostResolver* CreateNonCachingSystemHostResolver(size_t max_concurrent_resolves,
                                                  size_t max_retry_attempts,
                                                  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();
+  config_service->Watch();

[mmenke, 2012/02/21 16:34:24]

This was going to be moved to HostResolverImpl, right?

[szym, 2012/02/21 17:18:20]

The order of AddObserver and Watch doesn't matter. It's also not obvious that
the service passed to HostResolverImpl ctor is not already watching in which
case Watch() would be a no-op call. I think it makes more sense if the service
is told to Watch at the time it is created.

+  return CreateHostResolver(max_concurrent_resolves,
+                            max_retry_attempts,
+                            HostCache::CreateDefaultCache(),
+                            config_service.Pass(),
+                            net_log);
+}
+
 //-----------------------------------------------------------------------------
 
 // Holds the data for a request that could not be completed synchronously.
 // It is owned by a Job. Canceled Requests are only marked as canceled rather
 // than removed from the Job's |requests_| list.
 class HostResolverImpl::Request {
  public:
   Request(const BoundNetLog& source_net_log,
           const BoundNetLog& request_net_log,
           const RequestInfo& info,
@@ skipping 18 matching lines @@
     return callback_.is_null();
   }
 
   void set_job(Job* job) {
     DCHECK(job);
     // Identify which job the request is waiting on.
     job_ = job;
   }
 
   // Prepare final AddressList and call completion callback.
-  void OnComplete(int error, const AddressList& addrlist) {
+  void OnComplete(int error, const AddressList& addr_list) {
     if (error == OK)
-      *addresses_ = CreateAddressListUsingPort(addrlist, info_.port());
+      *addresses_ = CreateAddressListUsingPort(addr_list, info_.port());
     CompletionCallback callback = callback_;
     MarkAsCanceled();
     callback.Run(error);
   }
 
   Job* job() const {
     return job_;
   }
 
   // NetLog for the source, passed in HostResolver::Resolve.
@@ skipping 42 matching lines @@
 // resolution (OnLookupComplete) is completed or not. If the original attempt
 // hasn't completed, then we start another attempt for host resolution. We take
 // the results from the first attempt that finishes and ignore the results from
 // all other attempts.
 //
 // TODO(szym): Move to separate source file for testing and mocking.
 //
 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,
+                              const AddressList& addr_list)> Callback;
 
   ProcTask(const Key& key,
            const ProcTaskParams& params,
            const Callback& callback,
            const BoundNetLog& job_net_log)
       : key_(key),
         params_(params),
         callback_(callback),
         origin_loop_(base::MessageLoopProxy::current()),
         attempt_number_(0),
         completed_attempt_number_(0),
         completed_attempt_error_(ERR_UNEXPECTED),
         had_non_speculative_request_(false),
-        net_log_(BoundNetLog::Make(
-            job_net_log.net_log(),
-            NetLog::SOURCE_HOST_RESOLVER_IMPL_PROC_TASK)) {
+        net_log_(job_net_log) {
     if (!params_.resolver_proc)
       params_.resolver_proc = HostResolverProc::GetDefault();
     // If default is unset, use the system proc.
     if (!params_.resolver_proc)
       params_.resolver_proc = new CallSystemHostResolverProc();
 
-    job_net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_CREATE_PROC_TASK,
-                         new NetLogSourceParameter("source_dependency",
-                                                   net_log_.source()));
-
-    net_log_.BeginEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK,
-                        new JobCreationParameters(key_.hostname,
-                                                  job_net_log.source()));
+    net_log_.BeginEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, NULL);
   }
 
   void Start() {
     DCHECK(origin_loop_->BelongsToCurrentThread());
     StartLookupAttempt();
   }
 
   // Cancels this ProcTask. It will be orphaned. Any outstanding resolve
   // attempts running on worker threads will continue running. Only once all the
   // attempts complete will the final reference to this ProcTask be released.
   void Cancel() {
     DCHECK(origin_loop_->BelongsToCurrentThread());
 
     if (was_canceled())
       return;
 
-    net_log_.AddEvent(NetLog::TYPE_CANCELLED, NULL);
-
     callback_.Reset();
 
     net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, NULL);
   }
 
   void set_had_non_speculative_request() {
     DCHECK(origin_loop_->BelongsToCurrentThread());
     had_non_speculative_request_ = true;
   }
 
@@ skipping 48 matching lines @@
   // WARNING: This code runs inside a worker pool. The shutdown code cannot
   // wait for it to finish, so we must be very careful here about using other
   // objects (like MessageLoops, Singletons, etc). During shutdown these objects
   // may no longer exist. Multiple DoLookups() could be running in parallel, so
   // any state inside of |this| must not mutate .
   void DoLookup(const base::TimeTicks& start_time,
                 const uint32 attempt_number) {
     AddressList results;
     int os_error = 0;
     // Running on the worker thread
-
     int error = params_.resolver_proc->Resolve(key_.hostname,
                                                key_.address_family,
                                                key_.host_resolver_flags,
                                                &results,
                                                &os_error);
 
     origin_loop_->PostTask(
         FROM_HERE,
         base::Bind(&ProcTask::OnLookupComplete, this, results, start_time,
                    attempt_number, error, os_error));
@@ skipping 15 matching lines @@
                         const base::TimeTicks& start_time,
                         const uint32 attempt_number,
                         int error,
                         const int os_error) {
     DCHECK(origin_loop_->BelongsToCurrentThread());
     DCHECK(error || results.head());
 
     bool was_retry_attempt = attempt_number > 1;
 
     // Ideally the following code would be part of host_resolver_proc.cc,
-    // however it isn't safe to call NetworkChangeNotifier from worker
-    // threads. So we do it here on the IO thread instead.
+    // however it isn't safe to call NetworkChangeNotifier from worker threads.
+    // So we do it here on the IO thread instead.
     if (error != OK && NetworkChangeNotifier::IsOffline())
       error = ERR_INTERNET_DISCONNECTED;
 
-    // If this is the first attempt that is finishing later, then record
-    // data for the first attempt. Won't contaminate with retry attempt's
-    // data.
+    // If this is the first attempt that is finishing later, then record data
+    // for the first attempt. Won't contaminate with retry attempt's data.
     if (!was_retry_attempt)
       RecordPerformanceHistograms(start_time, error, os_error);
 
     RecordAttemptHistograms(start_time, attempt_number, error, os_error);
 
     if (was_canceled())
       return;
 
     scoped_refptr<NetLog::EventParameters> params;
     if (error != OK) {
-      params = new HostResolveFailedParams(attempt_number, error, os_error);
+      params = new ProcTaskFailedParams(attempt_number, error, os_error);
     } else {
       params = new NetLogIntegerParameter("attempt_number", attempt_number);
     }
     net_log_.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_ATTEMPT_FINISHED, params);
 
     if (was_completed())
       return;
 
     // Copy the results from the first worker thread that resolves the host.
     results_ = results;
     completed_attempt_number_ = attempt_number;
     completed_attempt_error_ = error;
 
     if (was_retry_attempt) {
       // If retry attempt finishes before 1st attempt, then get stats on how
       // much time is saved by having spawned an extra attempt.
       retry_attempt_finished_time_ = base::TimeTicks::Now();
     }
 
     if (error != OK) {
-      params = new HostResolveFailedParams(0, error, os_error);
+      params = new ProcTaskFailedParams(0, error, os_error);
     } else {
       params = new AddressListNetLogParam(results_);
     }
     net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, params);
 
-    callback_.Run(error, os_error, results_);
+    callback_.Run(error, results_);
   }
 
   void RecordPerformanceHistograms(const base::TimeTicks& start_time,
                                    const int error,
                                    const int os_error) const {
     DCHECK(origin_loop_->BelongsToCurrentThread());
     enum Category {  // Used in HISTOGRAM_ENUMERATION.
       RESOLVE_SUCCESS,
       RESOLVE_FAIL,
       RESOLVE_SPECULATIVE_SUCCESS,
@@ skipping 168 matching lines @@
   AddressList results_;
 
   BoundNetLog net_log_;
 
   DISALLOW_COPY_AND_ASSIGN(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:
   explicit IPv6ProbeJob(HostResolverImpl* resolver)
       : resolver_(resolver),
         origin_loop_(base::MessageLoopProxy::current()) {
     DCHECK(resolver);
   }
 
   void Start() {
@@ skipping 47 matching lines @@
   HostResolverImpl* resolver_;
 
   // Used to post ourselves onto the origin thread.
   scoped_refptr<base::MessageLoopProxy> origin_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(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, NULL);
+    return transaction_->Start();
+  }
+
+  void OnTransactionComplete(DnsTransaction* transaction,
+                             int net_error,
+                             const DnsResponse* response) {
+    // TODO(szym): Record performance histograms.
+    // Run |callback_| last since the owning Job will then delete this DnsTask.
+    DnsResponse::Result result = DnsResponse::DNS_SUCCESS;
+    if (net_error == OK) {
+      AddressList addr_list;
+      base::TimeDelta ttl;
+      result = response->ParseToAddressList(&addr_list, &ttl);
+      if (result == DnsResponse::DNS_SUCCESS) {
+        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;
+    }
+    net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK,
+                      new DnsTaskFailedParams(net_error, result));
+    callback_.Run(net_error, AddressList(), base::TimeDelta());
+  }
+
+ 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.
 class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
  public:
   // Creates new job for |key| where |request_net_log| is bound to the
-  // request that spawned it.
+  // request that spawned it and |priority| is the initial priority.
   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)),
-        net_error_(ERR_IO_PENDING),
-        os_error_(0) {
+                                   NetLog::SOURCE_HOST_RESOLVER_IMPL_JOB)) {
     request_net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_CREATE_JOB, NULL);
 
     net_log_.BeginEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB,
         make_scoped_refptr(new JobCreationParameters(
             key_.hostname, request_net_log.source())));
+
+    handle_ = resolver_->dispatcher_.Add(this, priority);
   }
 
   virtual ~Job() {
-    if (net_error_ == ERR_IO_PENDING) {
-      if (is_running()) {
-        DCHECK_EQ(ERR_IO_PENDING, net_error_);
+    if (is_running()) {
+      // |resolver_| was destroyed with this Job still in flight.
+      // Clean-up, record in the log, but don't run any callbacks.
+      if (is_proc_running()) {
         proc_task_->Cancel();
         proc_task_ = NULL;
-        net_error_ = ERR_ABORTED;
-      } else {
-        net_log_.AddEvent(NetLog::TYPE_CANCELLED, NULL);
-        net_error_ = OK;  // For NetLog.
       }
+      net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB,
+                                        ERR_ABORTED);
+    } else if (is_queued()) {
+      // This Job was cancelled without running.
+      // TODO(szym): is there's any benefit in having this distinction?

[mmenke, 2012/02/21 16:34:24]

I'd say no, but think it's fine as-is for now.

+      net_log_.AddEvent(NetLog::TYPE_CANCELLED, NULL);
+      net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB, NULL);
+    }
+    // else CompleteRequests logged EndEvent.
 
-      for (RequestsList::const_iterator it = requests_.begin();
-           it != requests_.end(); ++it) {
-        Request* req = *it;
-        if (req->was_canceled())
-          continue;
-        DCHECK_EQ(this, req->job());
-        LogCancelRequest(req->source_net_log(), req->request_net_log(),
-                         req->info());
-      }
+    // Log any remaining Requests as cancelled.
+    for (RequestsList::const_iterator it = requests_.begin();
+         it != requests_.end(); ++it) {
+      Request* req = *it;
+      if (req->was_canceled())
+        continue;
+      DCHECK_EQ(this, req->job());
+      LogCancelRequest(req->source_net_log(), req->request_net_log(),
+                       req->info());
     }
-    net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB,
-                                      net_error_);
     STLDeleteElements(&requests_);
   }
 
-  HostResolverImpl* resolver() const {
-    return resolver_;
-  }
-
   RequestPriority priority() const {
     return priority_tracker_.highest_priority();
   }
 
   // Number of non-canceled requests in |requests_|.
   size_t num_active_requests() const {
     return priority_tracker_.total_count();
   }
 
   const Key& key() const {
     return key_;
   }
 
-  int net_error() const {
-    return net_error_;
+  bool is_queued() const {
+    return !handle_.is_null();
   }
 
-  // Used by HostResolverImpl with |dispatcher_|.
-  const PrioritizedDispatcher::Handle& handle() const {
-    return handle_;
-  }
-
-  void set_handle(const PrioritizedDispatcher::Handle& handle) {
-    handle_ = handle;
-  }
-
-  // The Job will own |req| and destroy it in ~Job.
-  void AddRequest(Request* req) {
+  void AddRequest(scoped_ptr<Request> req) {
     DCHECK_EQ(key_.hostname, req->info().hostname());
 
     req->set_job(this);
-    requests_.push_back(req);
-
     priority_tracker_.Add(req->info().priority());
 
     req->request_net_log().AddEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_ATTACH,
         make_scoped_refptr(new NetLogSourceParameter(
             "source_dependency", net_log_.source())));
 
     net_log_.AddEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_REQUEST_ATTACH,
         make_scoped_refptr(new JobAttachParameters(
             req->request_net_log().source(), priority())));
 
     // TODO(szym): Check if this is still needed.
     if (!req->info().is_speculative()) {
       had_non_speculative_request_ = true;
       if (proc_task_)
         proc_task_->set_had_non_speculative_request();
     }
+
+    requests_.push_back(req.release());
+
+    if (!handle_.is_null())
+      handle_ = resolver_->dispatcher_.ChangePriority(handle_, priority());
   }
 
   void CancelRequest(Request* req) {
     DCHECK_EQ(key_.hostname, req->info().hostname());
     DCHECK(!req->was_canceled());
     // Don't remove it from |requests_| just mark it canceled.
     req->MarkAsCanceled();
     LogCancelRequest(req->source_net_log(), req->request_net_log(),
                      req->info());
     priority_tracker_.Remove(req->info().priority());
     net_log_.AddEvent(
         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_ = resolver_->dispatcher_.ChangePriority(handle_, priority());
+      } else {
+        resolver_->dispatcher_.Cancel(handle_);
+        handle_.Reset();
+      }
+    }
   }
 
   // Aborts and destroys the job, completes all requests as aborted.
+  // The caller should clean up.
   void Abort() {
     // Job should only be aborted if it's running.
     DCHECK(is_running());
-    proc_task_->Cancel();
-    proc_task_ = NULL;
-    net_error_ = ERR_ABORTED;
-    os_error_ = 0;
-    CompleteRequests(AddressList());
+    if (is_proc_running()) {
+      proc_task_->Cancel();
+      proc_task_ = NULL;
+    }
+    dns_task_.reset();
+    CompleteRequests(ERR_ABORTED, AddressList(), base::TimeDelta());
+  }
+
+  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 proc_task_.get() != NULL;
+    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);
 
+    scoped_ptr<Job> self_deleter(this);
+    resolver_->RemoveJob(this);
+
     // This signals to CompleteRequests that this job never ran.
-    net_error_ = ERR_HOST_RESOLVER_QUEUE_TOO_LARGE;
-    os_error_ = 0;
-    CompleteRequests(AddressList());
+    CompleteRequests(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE,
+                     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:
+  // TODO(szym): Since DnsTransaction does not consume threads, we can increase
+  // the limits on |dispatcher_|. But in order to keep the number of WorkerPool
+  // threads low, we will need to use an "inner" PrioritizedDispatcher with
+  // tighter limits.
+  void StartProcTask() {
+    DCHECK(!dns_task_.get());
     proc_task_ = new ProcTask(
         key_,
         resolver_->proc_params_,
         base::Bind(&Job::OnProcTaskComplete, base::Unretained(this)),
         net_log_);
 
     if (had_non_speculative_request_)
       proc_task_->set_had_non_speculative_request();
     // Start() could be called from within Resolve(), hence it must NOT directly
     // call OnProcTaskComplete, for example, on synchronous failure.
     proc_task_->Start();
   }
 
- private:
   // Called by ProcTask when it completes.
-  void OnProcTaskComplete(int net_error, int os_error,
-                          const AddressList& addrlist) {
-    DCHECK(is_running());
+  void OnProcTaskComplete(int net_error, const AddressList& addr_list) {
+    DCHECK(is_proc_running());
+    // |addr_list| will be destroyed once we destroy |proc_task_|.
+    AddressList list = addr_list;
     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);
-  }
-
-  // Completes all Requests. Calls OnJobFinished and deletes self.
-  void CompleteRequests(const AddressList& addrlist) {
-    CHECK(resolver_);
+    base::TimeDelta ttl = base::TimeDelta::FromSeconds(
+        kNegativeCacheEntryTTLSeconds);
+    if (net_error == OK)
+      ttl = base::TimeDelta::FromSeconds(kCacheEntryTTLSeconds);
 
     // 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, net_error, list, ttl);
+
+    CompleteRequests(net_error, list, ttl);
+  }
+
+  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);
+      dns_task_.reset();
+      StartProcTask();
+    }
+  }
+
+  // Called by DnsTask when it completes.
+  void OnDnsTaskComplete(int net_error,
+                         const AddressList& addr_list,
+                         base::TimeDelta ttl) {
+    DCHECK(is_dns_running());
+    // |addr_list| will be destroyed once we destroy |dns_task_|.
+    AddressList list = addr_list;
+    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;
+    }
+
+    // As in OnProcTaskComplete
+    scoped_ptr<Job> self_deleter(this);
+    resolver_->OnJobFinished(this, net_error, list, ttl);
+
+    CompleteRequests(net_error, list, ttl);
+  }
+
+  // Completes all Requests. Calls OnJobFinished and deletes self.
+  void CompleteRequests(int net_error,
+                        const AddressList& addr_list,
+                        base::TimeDelta ttl) {
+    CHECK(resolver_);
+    DCHECK(!is_running());
+    DCHECK(!is_queued());
+
+    // 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);
+
+    net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB,
+                                      net_error);
 
     // Complete all of the requests that were attached to the job.
     for (RequestsList::const_iterator it = requests_.begin();
          it != requests_.end(); ++it) {
       Request* req = *it;
 
       if (req->was_canceled())
         continue;
 
       DCHECK_EQ(this, req->job());
       // Update the net log and notify registered observers.
       LogFinishRequest(req->source_net_log(), req->request_net_log(),
-                       req->info(), net_error_, os_error_);
+                       req->info(), net_error);
 
-      req->OnComplete(net_error_, addrlist);
+      req->OnComplete(net_error, 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.
       if (!resolver_)
         return;
     }
   }
 
-  // Used to call OnJobFinished and RemoveJob.
+  // Used to call OnJobFinished.
   base::WeakPtr<HostResolverImpl> resolver_;
 
   Key key_;
 
   // Tracks the highest priority across |requests_|.
   PriorityTracker priority_tracker_;
 
   bool had_non_speculative_request_;
 
   BoundNetLog net_log_;
 
-  // Store result here in case the job fails fast in Resolve().
-  int net_error_;
-  int os_error_;
+  // Resolves the host using a HostResolverProc.
+  scoped_refptr<ProcTask> proc_task_;
 
-  // A ProcTask created and started when this Job is dispatched..
-  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_|.
+  // A handle used |HostResolverImpl::dispatcher_|.

[cbentzel, 2012/02/21 18:41:59]

Nit: used by

   PrioritizedDispatcher::Handle handle_;
 };
 
 //-----------------------------------------------------------------------------
 
 HostResolverImpl::ProcTaskParams::ProcTaskParams(
     HostResolverProc* resolver_proc,
     size_t max_retry_attempts)
     : resolver_proc(resolver_proc),
       max_retry_attempts(max_retry_attempts),
       unresponsive_delay(base::TimeDelta::FromMilliseconds(6000)),
       retry_factor(2) {
 }
 
 HostResolverImpl::ProcTaskParams::~ProcTaskParams() {}
 
 HostResolverImpl::HostResolverImpl(
     HostCache* cache,
     const PrioritizedDispatcher::Limits& job_limits,
     const ProcTaskParams& proc_params,
+    scoped_ptr<DnsConfigService> dns_config_service,
     NetLog* net_log)
     : cache_(cache),
       dispatcher_(job_limits),
       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));
 
   // Maximum of 4 retry attempts for host resolution.
   static const size_t kDefaultMaxRetryAttempts = 4u;
 
   if (proc_params_.max_retry_attempts == HostResolver::kDefaultRetryAttempts)
     proc_params_.max_retry_attempts = kDefaultMaxRetryAttempts;
 
 #if defined(OS_WIN)
   EnsureWinsockInit();
 #endif
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
   if (HaveOnlyLoopbackAddresses())
     additional_resolver_flags_ |= HOST_RESOLVER_LOOPBACK_ONLY;
 #endif
   NetworkChangeNotifier::AddIPAddressObserver(this);
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_OPENBSD)
 #if !defined(OS_ANDROID)
   EnsureDnsReloaderInit();
 #endif
   NetworkChangeNotifier::AddDNSObserver(this);
 #endif
+
+  if (dns_config_service_.get())
+    dns_config_service_->AddObserver(this);
 }
 
 HostResolverImpl::~HostResolverImpl() {
   DiscardIPv6ProbeJob();
 
   // This will also cancel all outstanding requests.
   STLDeleteValues(&jobs_);
 
   NetworkChangeNotifier::RemoveIPAddressObserver(this);
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_OPENBSD)
@@ skipping 21 matching lines @@
       NetLog::SOURCE_HOST_RESOLVER_IMPL_REQUEST);
 
   LogStartRequest(source_net_log, request_net_log, info);
 
   // Build a key that identifies the request in the cache and in the
   // outstanding jobs map.
   Key key = GetEffectiveKeyForRequest(info);
 
   int rv = ResolveHelper(key, info, addresses, request_net_log);
   if (rv != ERR_DNS_CACHE_MISS) {
-    LogFinishRequest(source_net_log, request_net_log, info, rv,
-                     0  /* os_error (unknown since from cache) */);
+    LogFinishRequest(source_net_log, request_net_log, info, rv);
     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;
   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());
 
     // Check for queue overflow.
     if (dispatcher_.num_queued_jobs() > max_queued_jobs_) {
       Job* evicted = static_cast<Job*>(dispatcher_.EvictOldestLowest());
       DCHECK(evicted);
       if (evicted == job) {
         delete job;
         rv = ERR_HOST_RESOLVER_QUEUE_TOO_LARGE;
-        LogFinishRequest(source_net_log, request_net_log, info, rv, 0);
+        LogFinishRequest(source_net_log, request_net_log, info, rv);
         return rv;
       }
       evicted->OnEvicted();  // Deletes |evicted|.
     }
-
     jobs_.insert(jobit, std::make_pair(key, job));
   } else {
     job = jobit->second;
   }
 
   // Can't complete synchronously. Create and attach request.
-  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()));
+  scoped_ptr<Request> req(new Request(source_net_log,
+                                      request_net_log,
+                                      info,
+                                      callback,
+                                      addresses));
   if (out_req)
-    *out_req = reinterpret_cast<RequestHandle>(req);
+    *out_req = reinterpret_cast<RequestHandle>(req.get());
 
-  DCHECK_EQ(ERR_IO_PENDING, job->net_error());
+  job->AddRequest(req.Pass());
   // Completion happens during Job::CompleteRequests().
   return ERR_IO_PENDING;
 }
 
 int HostResolverImpl::ResolveHelper(const Key& key,
                                     const RequestInfo& info,
                                     AddressList* addresses,
                                     const BoundNetLog& request_net_log) {
   // The result of |getaddrinfo| for empty hosts is inconsistent across systems.
   // On Windows it gives the default interface's address, whereas on Linux it
@@ skipping 18 matching lines @@
   // Make a log item for the request.
   BoundNetLog request_net_log = BoundNetLog::Make(net_log_,
       NetLog::SOURCE_HOST_RESOLVER_IMPL_REQUEST);
 
   // Update the net log and notify registered observers.
   LogStartRequest(source_net_log, request_net_log, info);
 
   Key key = GetEffectiveKeyForRequest(info);
 
   int rv = ResolveHelper(key, info, addresses, request_net_log);
-  LogFinishRequest(source_net_log, request_net_log, info, rv,
-                   0  /* os_error (unknown since from cache) */);
+  LogFinishRequest(source_net_log, request_net_log, info, rv);
   return rv;
 }
 
 void HostResolverImpl::CancelRequest(RequestHandle req_handle) {
   DCHECK(CalledOnValidThread());
   Request* req = reinterpret_cast<Request*>(req_handle);
   DCHECK(req);
 
   Job* job = req->job();
   DCHECK(job);
 
   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()));
-    } 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()) {
+  if (job->num_active_requests() == 0) {
+    RemoveJob(job);
+    if (job->is_running()) {
+      // If we were called from a Requests's callback within
+      // Job::CompleteRequests, that Request could not have been cancelled, so
+      // job->num_active_requests() could not be 0. Therefore, we are not in
+      // Job::CompleteRequests().

[mmenke, 2012/02/21 16:34:24]

nit:  Might want to move this comment up to "if (job->num_active_requests() ==
0) {", since the calls to "delete job" and "RemoveJob" both also depend on this.

       job->Abort();
     }
+    delete job;
   }
 }
 
 void HostResolverImpl::SetDefaultAddressFamily(AddressFamily address_family) {
   DCHECK(CalledOnValidThread());
   ipv6_probe_monitoring_ = false;
   DiscardIPv6ProbeJob();
   default_address_family_ = address_family;
 }
 
@@ skipping 54 matching lines @@
   if (!cache_entry)
     return false;
 
   request_net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_CACHE_HIT, NULL);
   *net_error = cache_entry->error;
   if (*net_error == OK)
     *addresses = CreateAddressListUsingPort(cache_entry->addrlist, info.port());
   return true;
 }
 
-void HostResolverImpl::OnJobFinished(Job* job, const AddressList& addrlist) {
+void HostResolverImpl::OnJobFinished(Job* job,
+                                     int net_error,
+                                     const AddressList& addr_list,
+                                     base::TimeDelta ttl) {
   DCHECK(job);
-  DCHECK(job->handle().is_null());
+  DCHECK(!job->is_queued());
+  DCHECK_NE(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE, net_error);
+  DCHECK_NE(ERR_ABORTED, net_error);
+
   RemoveJob(job);
-  if (job->net_error() == ERR_HOST_RESOLVER_QUEUE_TOO_LARGE)
-    return;
-
   // Signal dispatcher that a slot has opened.
   dispatcher_.OnJobFinished();
-  if (job->net_error() == ERR_ABORTED)
-    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,
+    cache_->Set(job->key(), net_error, addr_list,
                 base::TimeTicks::Now(), ttl);
   }
 }
 
 void HostResolverImpl::RemoveJob(Job* job) {
   DCHECK(job);
   jobs_.erase(job->key());
 }
 
 void HostResolverImpl::DiscardIPv6ProbeJob() {
@@ skipping 26 matching lines @@
       default_address_family_ != ADDRESS_FAMILY_UNSPECIFIED) {
     effective_address_family = default_address_family_;
     if (ipv6_probe_monitoring_)
       effective_flags |= HOST_RESOLVER_DEFAULT_FAMILY_SET_DUE_TO_NO_IPV6;
   }
   return Key(info.hostname(), effective_address_family, effective_flags);
 }
 
 void HostResolverImpl::AbortAllInProgressJobs() {
   base::WeakPtr<HostResolverImpl> self = AsWeakPtr();
-  // Scan |jobs_| for running jobs and abort them.
+  // In Abort, a Request callback could spawn new Jobs with matching keys, so
+  // first collect and remove all running jobs from |jobs_|.
+  std::vector<Job*> jobs_to_abort;
   for (JobMap::iterator it = jobs_.begin(); it != jobs_.end(); ) {
     Job* job = it->second;
-    // Advance the iterator before we might erase it.
-    ++it;
     if (job->is_running()) {
-      job->Abort();
-      // Check if resolver was deleted in a request callback.
-      if (!self)
-        return;
+      jobs_to_abort.push_back(job);
+      jobs_.erase(it++);
     } else {
-      // Keep it in |dispatch_|.
-      DCHECK(!job->handle().is_null());
+      DCHECK(job->is_queued());
+      ++it;
     }
   }
+
+  // Then Abort them and dispatch new Jobs.
+  for (size_t i = 0; i < jobs_to_abort.size(); ++i) {
+    jobs_to_abort[i]->Abort();
+    dispatcher_.OnJobFinished();
+  }
+  STLDeleteElements(&jobs_to_abort);
 }
 
 void HostResolverImpl::OnIPAddressChanged() {
   if (cache_.get())
     cache_->clear();
   if (ipv6_probe_monitoring_) {
     DiscardIPv6ProbeJob();
     ipv6_probe_job_ = new IPv6ProbeJob(this);
     ipv6_probe_job_->Start();
   }
@@ skipping 14 matching lines @@
   // 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())
     cache_->clear();
   // Existing jobs will have been sent to the original server so they need to
   // be aborted. TODO(Craig): Should these jobs be restarted?
   AbortAllInProgressJobs();
   // |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