DnsClient refactoring + features (timeout, suffix search, server rotation).

net/dns/dns_transaction.cc

Index: net/dns/dns_transaction.cc
diff --git a/net/dns/dns_transaction.cc b/net/dns/dns_transaction.cc
index 6d711b4a5a97cbcd3fe4d3b88cf1192a962a3177..d89e7822f9304c173167ed127495fb29523f79de 100644
--- a/net/dns/dns_transaction.cc
+++ b/net/dns/dns_transaction.cc
@@ -4,11 +4,26 @@
 
 #include "net/dns/dns_transaction.h"
 
+#include <deque>
+#include <string>
+#include <vector>
+
 #include "base/bind.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop.h"
 #include "base/rand_util.h"
-#include "base/values.h"
+#include "base/stl_util.h"
+#include "base/string_piece.h"
+#include "base/threading/non_thread_safe.h"
+#include "base/timer.h"
+#include "net/base/completion_callback.h"
+#include "net/base/dns_util.h"
 #include "net/base/io_buffer.h"
+#include "net/base/ip_endpoint.h"
 #include "net/base/net_errors.h"
+#include "net/base/net_log.h"
 #include "net/dns/dns_protocol.h"
 #include "net/dns/dns_query.h"
 #include "net/dns/dns_response.h"
@@ -20,259 +35,492 @@ namespace net {
 
 namespace {
 
-class DnsTransactionStartParameters : public NetLog::EventParameters {
+// Count labels in the fully-qualified name in DNS format.
+int CountLabels(const std::string& name) {
+  size_t count = 0;
+  for (size_t i = 0; i < name.size() && name[i]; i += name[i] + 1)
+    ++count;
+  return count;
+}
+
+bool IsIPLiteral(const std::string& hostname) {
+  IPAddressNumber ip;
+  return ParseIPLiteralToNumber(hostname, &ip);
+}
+
+class StartParameters : public NetLog::EventParameters {
  public:
-  DnsTransactionStartParameters(const IPEndPoint& dns_server,
-                                const base::StringPiece& qname,
-                                uint16 qtype,
-                                const NetLog::Source& source)
-      : dns_server_(dns_server),
-        qname_(qname.data(), qname.length()),
-        qtype_(qtype),
-        source_(source) {}
-
-  virtual Value* ToValue() const {
+  StartParameters(const std::string& hostname,
+                  uint16 qtype,
+                  const NetLog::Source& source)
+      : hostname_(hostname), qtype_(qtype), source_(source) {}
+
+  virtual Value* ToValue() const OVERRIDE {
     DictionaryValue* dict = new DictionaryValue();
-    dict->SetString("dns_server", dns_server_.ToString());
-    dict->SetString("hostname", qname_);
+    dict->SetString("hostname", hostname_);
     dict->SetInteger("query_type", qtype_);
-    if (source_.is_valid())
-      dict->Set("source_dependency", source_.ToValue());
+    dict->Set("source_dependency", source_.ToValue());
     return dict;
   }
 
  private:
-  IPEndPoint dns_server_;
-  std::string qname_;
-  uint16 qtype_;
+  const std::string hostname_;
+  const uint16 qtype_;
   const NetLog::Source source_;
 };
 
-class DnsTransactionFinishParameters : public NetLog::EventParameters {
+class ResponseParameters : public NetLog::EventParameters {
  public:
-  // TODO(szym): add rcode ?
-  DnsTransactionFinishParameters(int net_error, int answer_count)
-      : net_error_(net_error), answer_count_(answer_count) {}
+  ResponseParameters(int rcode, int answer_count, const NetLog::Source& source)
+      : rcode_(rcode), answer_count_(answer_count), source_(source) {}
 
-  virtual Value* ToValue() const {
+  virtual Value* ToValue() const OVERRIDE {
     DictionaryValue* dict = new DictionaryValue();
-    if (net_error_)
-      dict->SetInteger("net_error", net_error_);
+    dict->SetInteger("rcode", rcode_);
     dict->SetInteger("answer_count", answer_count_);
+    dict->Set("socket_source", source_.ToValue());
     return dict;
   }
 
  private:
-  const int net_error_;
+  const int rcode_;
   const int answer_count_;
+  const NetLog::Source source_;
 };
 
-class DnsTransactionRetryParameters : public NetLog::EventParameters {
+// ----------------------------------------------------------------------------
+
+// A single asynchronous DNS exchange over UDP, which consists of sending out a
+// DNS query, waiting for a response, and returning the response that it
+// matches. Logging is done in the socket and in the outer DnsTransaction.
+class DnsUDPAttempt {
  public:
-  DnsTransactionRetryParameters(int attempt_number,
-                                const NetLog::Source& source)
-      : attempt_number_(attempt_number), source_(source) {}
+  DnsUDPAttempt(scoped_ptr<DatagramClientSocket> socket,
+                const IPEndPoint& server,
+                scoped_ptr<DnsQuery> query,
+                const CompletionCallback& callback)
+      : next_state_(STATE_NONE),
+        socket_(socket.Pass()),
+        server_(server),
+        query_(query.Pass()),
+        callback_(callback) {
+  }
 
-  virtual Value* ToValue() const {
-    DictionaryValue* dict = new DictionaryValue();
-    dict->SetInteger("attempt_number", attempt_number_);
-    dict->Set("source_dependency", source_.ToValue());
-    return dict;
+  // Starts the attempt. Returns ERR_IO_PENDING if cannot complete synchronously
+  // and calls |callback| upon completion.
+  int Start() {
+    DCHECK_EQ(STATE_NONE, next_state_);
+    next_state_ = STATE_CONNECT;
+    return DoLoop(OK);
+  }
+
+  const DnsQuery* query() const {
+    return query_.get();
+  }
+
+  const DatagramClientSocket* socket() const {
+    return socket_.get();
+  }
+
+  // Returns the response or NULL if has not received a matching response from
+  // the server.
+  const DnsResponse* response() const {
+    const DnsResponse* resp = response_.get();
+    return (resp != NULL && resp->is_valid()) ? resp : NULL;
   }
 
  private:
-  const int attempt_number_;
-  const NetLog::Source source_;
-};
+  enum State {
+    STATE_CONNECT,
+    STATE_SEND_QUERY,
+    STATE_SEND_QUERY_COMPLETE,
+    STATE_READ_RESPONSE,
+    STATE_READ_RESPONSE_COMPLETE,
+    STATE_NONE,
+  };
+
+  int DoLoop(int result) {
+    DCHECK_NE(STATE_NONE, next_state_);
+    int rv = result;
+    do {
+      State state = next_state_;
+      next_state_ = STATE_NONE;
+      switch (state) {
+        case STATE_CONNECT:
+          rv = DoConnect();
+          break;
+        case STATE_SEND_QUERY:
+          rv = DoSendQuery();
+          break;
+        case STATE_SEND_QUERY_COMPLETE:
+          rv = DoSendQueryComplete(rv);
+          break;
+        case STATE_READ_RESPONSE:
+          rv = DoReadResponse();
+          break;
+        case STATE_READ_RESPONSE_COMPLETE:
+          rv = DoReadResponseComplete(rv);
+          break;
+        default:
+          NOTREACHED();
+          break;
+      }
+    } while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE);
 
-}  // namespace
+    return rv;
+  }
+
+  int DoConnect() {
+    next_state_ = STATE_SEND_QUERY;
+    return socket_->Connect(server_);

[cbentzel, 2012/01/13 22:17:25]

Why not DoConnectComplete? Is this guaranteed to complete synchronously?

[szym, 2012/01/13 22:32:44]

We don't actually have an API that would allow for asynchronous completion.
DatagramClientSocket::Connect() does not take callback. In fact, I think we
should rather DCHECK that the rv here is not ERR_IO_PENDING.

+  }
+
+  int DoSendQuery() {
+    next_state_ = STATE_SEND_QUERY_COMPLETE;
+    return socket_->Write(query_->io_buffer(),
+                          query_->io_buffer()->size(),
+                          base::Bind(&DnsUDPAttempt::OnIOComplete,
+                                     base::Unretained(this)));
+  }
+
+  int DoSendQueryComplete(int rv) {

[cbentzel, 2012/01/13 22:17:25]

DCHECK_NE(ERR_IO_PENDING, rv);

+    if (rv < 0)
+      return rv;
+
+    // Writing to UDP should not result in a partial datagram.
+    if (rv != query_->io_buffer()->size())
+      return ERR_MSG_TOO_BIG;
 
+    next_state_ = STATE_READ_RESPONSE;
+    return OK;
+  }
+
+  int DoReadResponse() {
+    next_state_ = STATE_READ_RESPONSE_COMPLETE;
+    response_.reset(new DnsResponse());
+    return socket_->Read(response_->io_buffer(),
+                         response_->io_buffer()->size(),
+                         base::Bind(&DnsUDPAttempt::OnIOComplete,
+                                    base::Unretained(this)));
+  }
+
+  int DoReadResponseComplete(int rv) {
+    DCHECK_NE(ERR_IO_PENDING, rv);
+    if (rv < 0)
+      return rv;
+
+    DCHECK(rv);
+    if (!response_->InitParse(rv, *query_))
+      return ERR_DNS_MALFORMED_RESPONSE;
+    if (response_->flags() & dns_protocol::kFlagTC)
+      return ERR_DNS_SERVER_REQUIRES_TCP;
+    if (response_->rcode() != dns_protocol::kRcodeNOERROR &&
+        response_->rcode() != dns_protocol::kRcodeNXDOMAIN) {
+      return ERR_DNS_SERVER_FAILED;
+    }
+    if (response_->answer_count() == 0)
+      return ERR_NAME_NOT_RESOLVED;
+
+    return OK;
+  }
+
+  void OnIOComplete(int rv) {
+    rv = DoLoop(rv);
+    if (rv != ERR_IO_PENDING)
+      callback_.Run(rv);
+  }
+
+  State next_state_;
+
+  scoped_ptr<DatagramClientSocket> socket_;
+  IPEndPoint server_;
+  scoped_ptr<DnsQuery> query_;
+
+  scoped_ptr<DnsResponse> response_;
+
+  CompletionCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(DnsUDPAttempt);
+};
+
+// ----------------------------------------------------------------------------
 
-DnsTransaction::DnsTransaction(DnsSession* session,
-                               const base::StringPiece& qname,
-                               uint16 qtype,
-                               const ResultCallback& callback,
-                               const BoundNetLog& source_net_log)
+// Implements DnsTransaction. Configuration is supplied by DnsSession.
+// The suffix list is built according to the DnsConfig from the session.
+// The timeout for each DnsUDPAttempt is given by DnsSession::NextTimeout.
+// The first server to attempt on each query is given by
+// DnsSession::NextFirstServerIndex, and the order is round-robin afterwards.
+// Each server is attempted DnsConfig::attempts times.
+class DnsTransactionImpl : public DnsTransaction, public base::NonThreadSafe {
+ public:
+  DnsTransactionImpl(DnsSession* session,
+                     const std::string& hostname,
+                     uint16 qtype,
+                     const DnsTransactionFactory::CallbackType& callback,
+                     const BoundNetLog& source_net_log)
     : session_(session),
-      dns_server_(session->NextServer()),
-      query_(new DnsQuery(session->NextId(), qname, qtype)),
+      hostname_(hostname),
+      qtype_(qtype),
       callback_(callback),
-      attempts_(0),
-      next_state_(STATE_NONE),
       net_log_(BoundNetLog::Make(session->net_log(),
-          NetLog::SOURCE_DNS_TRANSACTION)) {
-  net_log_.BeginEvent(
-      NetLog::TYPE_DNS_TRANSACTION,
-      make_scoped_refptr(
-          new DnsTransactionStartParameters(dns_server_,
-                                            qname,
-                                            qtype,
-                                            source_net_log.source())));
-}
+                                 NetLog::SOURCE_DNS_TRANSACTION)),
+      successful_attempt_(NULL),
+      first_server_index_(0) {
+    DCHECK(session_);
+    DCHECK(!hostname_.empty());
+    DCHECK(!callback_.is_null());
 
-DnsTransaction::~DnsTransaction() {}
+    DCHECK(!IsIPLiteral(hostname_));
 
-int DnsTransaction::Start() {
-  DCHECK_EQ(STATE_NONE, next_state_);
-  next_state_ = STATE_CONNECT;
-  return DoLoop(OK);
-}
+    net_log_.BeginEvent(NetLog::TYPE_DNS_TRANSACTION, make_scoped_refptr(
+        new StartParameters(hostname_, qtype_, source_net_log.source())));
+  }
 
-int DnsTransaction::DoLoop(int result) {
-  DCHECK_NE(STATE_NONE, next_state_);
-  int rv = result;
-  do {
-    State state = next_state_;
-    next_state_ = STATE_NONE;
-    switch (state) {
-      case STATE_CONNECT:
-        rv = DoConnect();
-        break;
-      case STATE_CONNECT_COMPLETE:
-        rv = DoConnectComplete(rv);
-        break;
-      case STATE_SEND_QUERY:
-        rv = DoSendQuery();
-        break;
-      case STATE_SEND_QUERY_COMPLETE:
-        rv = DoSendQueryComplete(rv);
-        break;
-      case STATE_READ_RESPONSE:
-        rv = DoReadResponse();
+  virtual ~DnsTransactionImpl() {
+    STLDeleteElements(&attempts_);
+    if (!callback_.is_null()) {
+      net_log_.AddEvent(NetLog::TYPE_CANCELLED, NULL);
+      net_log_.EndEventWithNetErrorCode(NetLog::TYPE_DNS_TRANSACTION,
+                                        ERR_ABORTED);
+    }
+  }
+
+  virtual const std::string& GetHostname() const OVERRIDE {
+    DCHECK(CalledOnValidThread());
+    return hostname_;
+  }
+
+  virtual uint16 GetType() const OVERRIDE {
+    DCHECK(CalledOnValidThread());
+    return qtype_;
+  }
+
+  virtual int Start() OVERRIDE {
+    int rv = PrepareSearch();
+    if (rv == OK)
+      rv = StartQuery();
+    DCHECK_NE(OK, rv);
+    return rv;
+  }
+
+ private:
+  // Prepares |qnames_| according to the DnsConfig.
+  int PrepareSearch() {
+    const DnsConfig& config = session_->config();
+
+    std::string labelled_hostname;
+    if (!DNSDomainFromDot(hostname_, &labelled_hostname))
+      return ERR_INVALID_ARGUMENT;
+
+    if (hostname_[hostname_.size() - 1] == '.') {
+      // It's a fully-qualified name, no suffix search.
+      qnames_.push_back(labelled_hostname);
+      return OK;
+    }
+
+    // Set true when |labelled_hostname| is put on the list.
+    bool had_hostname = false;
+
+    int ndots = CountLabels(labelled_hostname) - 1;
+    if (ndots >= config.ndots) {
+      qnames_.push_back(labelled_hostname);
+      had_hostname = true;
+    }
+
+    std::string qname;
+    for (size_t i = 0; i < config.search.size(); ++i) {
+      // Ignore invalid (too long) combinations.
+      if (!DNSDomainFromDot(hostname_ + "." + config.search[i], &qname))
+        continue;
+      if (qname.size() == labelled_hostname.size()) {
+        if (had_hostname)
+          continue;
+        had_hostname = true;
+      }
+      qnames_.push_back(qname);
+    }
+
+    if (!had_hostname)
+      qnames_.push_back(labelled_hostname);
+
+    return OK;
+  }
+
+  void DoCallback(int rv) {
+    if (callback_.is_null())
+      return;
+    DCHECK_NE(ERR_IO_PENDING, rv);
+    DCHECK(rv != OK || successful_attempt_ != NULL);
+
+    DnsTransactionFactory::CallbackType callback = callback_;
+    callback_.Reset();
+    net_log_.EndEventWithNetErrorCode(NetLog::TYPE_DNS_TRANSACTION, rv);
+    callback.Run(this,
+                 rv,
+                 successful_attempt_ ? successful_attempt_->response() : NULL);
+  }
+
+  // Makes another attempt at the current name, |qnames_.front()|, using the
+  // next nameserver.
+  int MakeAttempt() {
+    int attempt_number = attempts_.size();
+
+    scoped_ptr<DatagramClientSocket> socket(
+        session_->socket_factory()->CreateDatagramClientSocket(
+            DatagramSocket::RANDOM_BIND,
+            base::Bind(&base::RandInt),
+            net_log_.net_log(),
+            net_log_.source()));
+
+    uint16 id = session_->NextQueryId();
+    scoped_ptr<DnsQuery> query;
+    if (attempts_.empty()) {
+      query.reset(new DnsQuery(id, qnames_.front(), qtype_));
+    } else {
+      query.reset(attempts_[0]->query()->CloneWithNewId(id));
+    }
+
+    net_log_.AddEvent(NetLog::TYPE_DNS_TRANSACTION_ATTEMPT, make_scoped_refptr(
+        new NetLogSourceParameter("socket_source", socket->NetLog().source())));
+
+    const DnsConfig& config = session_->config();
+
+    int server_index = first_server_index_ +
+        (attempt_number % config.nameservers.size());
+
+    DnsUDPAttempt* attempt = new DnsUDPAttempt(
+        socket.Pass(),
+        config.nameservers[server_index],
+        query.Pass(),
+        base::Bind(&DnsTransactionImpl::OnAttemptComplete,
+                   base::Unretained(this),
+                   attempt_number));

[cbentzel, 2012/01/13 22:17:25]

Nice use of currying here.

+
+    base::TimeDelta timeout = session_->NextTimeout(attempt_number);
+    timer_.Start(FROM_HERE, timeout, this, &DnsTransactionImpl::OnTimeout);
+    attempts_.push_back(attempt);
+    return attempt->Start();
+  }
+
+  // Begins query for the current name. Makes the first attempt.
+  int StartQuery() {
+    std::string dotted_qname = DNSDomainToString(qnames_.front());
+    net_log_.BeginEvent(
+        NetLog::TYPE_DNS_TRANSACTION_QUERY,
+        make_scoped_refptr(new NetLogStringParameter("qname", dotted_qname)));
+
+    first_server_index_ = session_->NextFirstServerIndex();
+
+    STLDeleteElements(&attempts_);
+    return MakeAttempt();
+  }
+
+  void OnAttemptComplete(int attempt_number, int rv) {

[cbentzel, 2012/01/13 22:17:25]

DCHECK that attempt_number within reasonable range? implementations of vector
may do this range check in debug builds though. 

+    timer_.Stop();
+
+    const DnsUDPAttempt* attempt = attempts_[attempt_number];
+
+    if (attempt->response()) {
+      net_log_.AddEvent(
+          NetLog::TYPE_DNS_TRANSACTION_RESPONSE,
+          make_scoped_refptr(
+              new ResponseParameters(attempt->response()->rcode(),
+                                     attempt->response()->answer_count(),
+                                     attempt->socket()->NetLog().source())));
+    }
+
+    net_log_.EndEventWithNetErrorCode(NetLog::TYPE_DNS_TRANSACTION_QUERY, rv);
+
+    switch (rv) {
+      case ERR_NAME_NOT_RESOLVED:
+        // Try next suffix.
+        qnames_.pop_front();
+        if (qnames_.empty())
+          rv = ERR_NAME_NOT_RESOLVED;
+        else
+          rv = StartQuery();
         break;
-      case STATE_READ_RESPONSE_COMPLETE:
-        rv = DoReadResponseComplete(rv);
+      case OK:
+        successful_attempt_ = attempt;
         break;
       default:
-        NOTREACHED();
+        // TODO(szym): Some nameservers could fail and we should just ignore

[cbentzel, 2012/01/13 22:17:25]

It wouldn't be too hard to handle the ERR_DNS_SERVER_FAILED case in a naive way
which moves to the next server if available - perhaps do that now? 

+        // them.
         break;
     }
-  } while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE);
-
-  return rv;
-}
-
-void DnsTransaction::DoCallback(int result) {
-  DCHECK_NE(result, ERR_IO_PENDING);
-  int answer_count = (result == OK) ? response()->answer_count() : 0;
-  net_log_.EndEvent(
-      NetLog::TYPE_DNS_TRANSACTION,
-      make_scoped_refptr(
-          new DnsTransactionFinishParameters(result, answer_count)));
-  callback_.Run(this, result);
-}
+    if (rv != ERR_IO_PENDING)
+      DoCallback(rv);
+  }
 
-void DnsTransaction::OnIOComplete(int result) {
-  int rv = DoLoop(result);
-  if (rv != ERR_IO_PENDING)
-    DoCallback(rv);
-}
+  void OnTimeout() {
+    const DnsConfig& config = session_->config();
+    if (attempts_.size() == config.attempts * config.nameservers.size()) {
+      DoCallback(ERR_DNS_TIMED_OUT);
+      return;
+    }
+    int rv = MakeAttempt();
+    if (rv != ERR_IO_PENDING)
+      DoCallback(rv);
+  }
 
-int DnsTransaction::DoConnect() {
-  next_state_ = STATE_CONNECT_COMPLETE;
-
-  StartTimer(session_->NextTimeout(attempts_));
-  ++attempts_;
-
-  // TODO(szym): keep all sockets around in case the server responds
-  // after its timeout; state machine will need to change to handle that.
-  // The current plan is to move socket management out to DnsSession.
-  // Hence also move retransmissions to DnsClient::Request.
-  socket_.reset(session_->socket_factory()->CreateDatagramClientSocket(
-      DatagramSocket::RANDOM_BIND,
-      base::Bind(&base::RandInt),
-      net_log_.net_log(),
-      net_log_.source()));
-
-  net_log_.AddEvent(
-      NetLog::TYPE_DNS_TRANSACTION_ATTEMPT_STARTED,
-      make_scoped_refptr(
-          new DnsTransactionRetryParameters(attempts_,
-                                            socket_->NetLog().source())));
-
-  return socket_->Connect(dns_server_);
-}
+  scoped_refptr<DnsSession> session_;
+  std::string hostname_;
+  uint16 qtype_;
+  // Set to null once the transaction completes.
+  DnsTransactionFactory::CallbackType callback_;
 
-int DnsTransaction::DoConnectComplete(int rv) {
-  if (rv < 0)
-    return rv;
-  next_state_ = STATE_SEND_QUERY;
-  return OK;
-}
+  BoundNetLog net_log_;
 
-int DnsTransaction::DoSendQuery() {
-  next_state_ = STATE_SEND_QUERY_COMPLETE;
-  return socket_->Write(query_->io_buffer(),
-                        query_->io_buffer()->size(),
-                        base::Bind(&DnsTransaction::OnIOComplete,
-                                   base::Unretained(this)));
-}
+  // Search list of fully-qualified DNS names to query next (in DNS format).
+  std::deque<std::string> qnames_;
 
-int DnsTransaction::DoSendQueryComplete(int rv) {
-  if (rv < 0)
-    return rv;
+  // List of attempts for the current name.
+  std::vector<DnsUDPAttempt*> attempts_;
+  // The member of |attempts_| that succeeded first.
+  const DnsUDPAttempt* successful_attempt_;

[cbentzel, 2012/01/13 22:17:25]

|successful_attempt_| doesn't need to be a member - could be passed into
DoCallback from OnAttemptComplete instead.

 
-  // Writing to UDP should not result in a partial datagram.
-  if (rv != query_->io_buffer()->size())
-    return ERR_MSG_TOO_BIG;
+  // Index of the first server to try on each search query.
+  int first_server_index_;
 
-  next_state_ = STATE_READ_RESPONSE;
-  return OK;
-}
+  base::OneShotTimer<DnsTransactionImpl> timer_;
 
-int DnsTransaction::DoReadResponse() {
-  next_state_ = STATE_READ_RESPONSE_COMPLETE;
-  response_.reset(new DnsResponse());
-  return socket_->Read(response_->io_buffer(),
-                       response_->io_buffer()->size(),
-                       base::Bind(&DnsTransaction::OnIOComplete,
-                                  base::Unretained(this)));
-}
+  DISALLOW_COPY_AND_ASSIGN(DnsTransactionImpl);
+};
 
-int DnsTransaction::DoReadResponseComplete(int rv) {
-  DCHECK_NE(ERR_IO_PENDING, rv);
-  RevokeTimer();
-  if (rv < 0)
-    return rv;
+// ----------------------------------------------------------------------------
 
-  DCHECK(rv);
-  if (!response_->InitParse(rv, *query_))
-    return ERR_DNS_MALFORMED_RESPONSE;
-  // TODO(szym): define this flag value in dns_protocol
-  if (response_->flags1() & 2)
-    return ERR_DNS_SERVER_REQUIRES_TCP;
-  // TODO(szym): move this handling out of DnsTransaction?
-  if (response_->rcode() != dns_protocol::kRcodeNOERROR &&
-      response_->rcode() != dns_protocol::kRcodeNXDOMAIN) {
-    return ERR_DNS_SERVER_FAILED;
+// Implementation of DnsTransactionFactory that returns instances of
+// DnsTransactionImpl.
+class DnsTransactionFactoryImpl : public DnsTransactionFactory {
+ public:
+  explicit DnsTransactionFactoryImpl(DnsSession* session) {
+    session_ = session;
   }
-  // TODO(szym): add ERR_DNS_RR_NOT_FOUND?
-  if (response_->answer_count() == 0)
-    return ERR_NAME_NOT_RESOLVED;
 
-  return OK;
-}
+  virtual scoped_ptr<DnsTransaction> CreateTransaction(
+      const std::string& hostname,
+      uint16 qtype,
+      const CallbackType& callback,
+      const BoundNetLog& source_net_log) OVERRIDE {
+    return scoped_ptr<DnsTransaction>(new DnsTransactionImpl(session_,
+                                                             hostname,
+                                                             qtype,
+                                                             callback,
+                                                             source_net_log));
+  }
 
-void DnsTransaction::StartTimer(base::TimeDelta delay) {
-  timer_.Start(FROM_HERE, delay, this, &DnsTransaction::OnTimeout);
-}
+ private:
+  scoped_refptr<DnsSession> session_;
+};
 
-void DnsTransaction::RevokeTimer() {
-  timer_.Stop();
-}
+}  // namespace
 
-void DnsTransaction::OnTimeout() {
-  DCHECK(next_state_ == STATE_SEND_QUERY_COMPLETE ||
-         next_state_ == STATE_READ_RESPONSE_COMPLETE);
-  if (attempts_ == session_->config().attempts) {
-    DoCallback(ERR_DNS_TIMED_OUT);
-    return;
-  }
-  next_state_ = STATE_CONNECT;
-  query_.reset(query_->CloneWithNewId(session_->NextId()));
-  int rv = DoLoop(OK);
-  if (rv != ERR_IO_PENDING)
-    DoCallback(rv);
+// static
+scoped_ptr<DnsTransactionFactory> DnsTransactionFactory::CreateFactory(
+    DnsSession* session) {
+  return scoped_ptr<DnsTransactionFactory>(
+      new DnsTransactionFactoryImpl(session));
 }
 
 }  // namespace net
+