Keep pool of pre-connected DNS sockets

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 00d8064f6e8d6f263423659519e9691a99e406aa..45f2e9735f4813341fe5a6444757e8405c4b82d0 100644
--- a/net/dns/dns_transaction.cc
+++ b/net/dns/dns_transaction.cc
@@ -14,7 +14,6 @@
 #include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
-#include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/string_piece.h"
 #include "base/threading/non_thread_safe.h"
@@ -30,7 +29,6 @@
 #include "net/dns/dns_query.h"
 #include "net/dns/dns_response.h"
 #include "net/dns/dns_session.h"
-#include "net/socket/client_socket_factory.h"
 #include "net/udp/datagram_client_socket.h"
 
 namespace net {
@@ -66,13 +64,11 @@ Value* NetLogStartCallback(const std::string* hostname,
 // matches. Logging is done in the socket and in the outer DnsTransaction.
 class DnsUDPAttempt {
  public:
-  DnsUDPAttempt(scoped_ptr<DatagramClientSocket> socket,
-                const IPEndPoint& server,
+  DnsUDPAttempt(scoped_ptr<DnsSession::SocketLease> socket_lease,
                 scoped_ptr<DnsQuery> query,
                 const CompletionCallback& callback)
       : next_state_(STATE_NONE),
-        socket_(socket.Pass()),
-        server_(server),
+        socket_lease_(socket_lease.Pass()),
         query_(query.Pass()),
         callback_(callback) {
   }
@@ -81,7 +77,7 @@ class DnsUDPAttempt {
   // and calls |callback| upon completion.
   int Start() {
     DCHECK_EQ(STATE_NONE, next_state_);
-    next_state_ = STATE_CONNECT;
+    next_state_ = STATE_SEND_QUERY;
     return DoLoop(OK);
   }
 
@@ -90,7 +86,7 @@ class DnsUDPAttempt {
   }
 
   const DatagramClientSocket* socket() const {
-    return socket_.get();
+    return socket_lease_->socket();
   }
 
   // Returns the response or NULL if has not received a matching response from
@@ -109,13 +105,12 @@ class DnsUDPAttempt {
     DictionaryValue* dict = new DictionaryValue();
     dict->SetInteger("rcode", response_->rcode());
     dict->SetInteger("answer_count", response_->answer_count());
-    socket_->NetLog().source().AddToEventParameters(dict);
+    socket()->NetLog().source().AddToEventParameters(dict);
     return dict;
   }
 
  private:
   enum State {
-    STATE_CONNECT,
     STATE_SEND_QUERY,
     STATE_SEND_QUERY_COMPLETE,
     STATE_READ_RESPONSE,
@@ -123,6 +118,10 @@ class DnsUDPAttempt {
     STATE_NONE,
   };
 
+  DatagramClientSocket* my_socket() {
+    return socket_lease_->socket();
+  }
+
   int DoLoop(int result) {
     CHECK_NE(STATE_NONE, next_state_);
     int rv = result;
@@ -130,9 +129,6 @@ class DnsUDPAttempt {
       State state = next_state_;
       next_state_ = STATE_NONE;
       switch (state) {
-        case STATE_CONNECT:
-          rv = DoConnect();
-          break;
         case STATE_SEND_QUERY:
           rv = DoSendQuery();
           break;
@@ -154,17 +150,12 @@ class DnsUDPAttempt {
     return rv;
   }
 
-  int DoConnect() {
-    next_state_ = STATE_SEND_QUERY;
-    return socket_->Connect(server_);
-  }
-
   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)));
+    return my_socket()->Write(query_->io_buffer(),
+                              query_->io_buffer()->size(),
+                              base::Bind(&DnsUDPAttempt::OnIOComplete,
+                                         base::Unretained(this)));
   }
 
   int DoSendQueryComplete(int rv) {
@@ -183,10 +174,10 @@ class DnsUDPAttempt {
   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)));
+    return my_socket()->Read(response_->io_buffer(),
+                             response_->io_buffer()->size(),
+                             base::Bind(&DnsUDPAttempt::OnIOComplete,
+                                        base::Unretained(this)));
   }
 
   int DoReadResponseComplete(int rv) {
@@ -223,8 +214,7 @@ class DnsUDPAttempt {
 
   State next_state_;
 
-  scoped_ptr<DatagramClientSocket> socket_;
-  IPEndPoint server_;
+  scoped_ptr<DnsSession::SocketLease> socket_lease_;
   scoped_ptr<DnsQuery> query_;
 
   scoped_ptr<DnsResponse> response_;
@@ -384,21 +374,6 @@ class DnsTransactionImpl : public DnsTransaction,
   AttemptResult MakeAttempt() {
     unsigned attempt_number = attempts_.size();
 
-#if defined(OS_WIN)
-    // Avoid the Windows firewall warning about explicit UDP binding.
-    // TODO(szym): Reuse a pool of pre-bound sockets. http://crbug.com/107413
-    DatagramSocket::BindType bind_type = DatagramSocket::DEFAULT_BIND;
-#else
-    DatagramSocket::BindType bind_type = DatagramSocket::RANDOM_BIND;
-#endif
-
-    scoped_ptr<DatagramClientSocket> socket(
-        session_->socket_factory()->CreateDatagramClientSocket(
-            bind_type,
-            base::Bind(&base::RandInt),
-            net_log_.net_log(),
-            net_log_.source()));
-
     uint16 id = session_->NextQueryId();
     scoped_ptr<DnsQuery> query;
     if (attempts_.empty()) {
@@ -407,22 +382,22 @@ class DnsTransactionImpl : public DnsTransaction,
       query.reset(attempts_[0]->query()->CloneWithNewId(id));
     }
 
-    net_log_.AddEvent(NetLog::TYPE_DNS_TRANSACTION_ATTEMPT,
-                      socket->NetLog().source().ToEventParametersCallback());
-
     const DnsConfig& config = session_->config();
 
     unsigned server_index = first_server_index_ +
         (attempt_number % config.nameservers.size());
 
     DnsUDPAttempt* attempt = new DnsUDPAttempt(
-        socket.Pass(),
-        config.nameservers[server_index],
+        session_->AllocateSocket(server_index),
         query.Pass(),
         base::Bind(&DnsTransactionImpl::OnAttemptComplete,
                    base::Unretained(this),
                    attempt_number));
 
+    net_log_.AddEvent(

[cbentzel, 2012/08/29 16:48:06]

Why did this move?

[szym, 2012/08/29 16:57:08]

In my draft CL, this moved because it needs a reference to the socket which
|attempt| now owns. I thought it's not worth the complexity to first obtain the
socket, log the event, then pass it to the attempt.

+        NetLog::TYPE_DNS_TRANSACTION_ATTEMPT,
+        attempt->socket()->NetLog().source().ToEventParametersCallback());
+
     attempts_.push_back(attempt);
 
     int rv = attempt->Start();