[net/dns] Handle failure in DatagramClientSocket::Connect in DnsUDPAttempt

DnsUDPAttempt::DoLoop did not handle the return value from the Connect call.

BUG=151896
TEST=./net_unittests --gtest_filter=DnsTransactionTest.ConnectFailure


Review URL: https://chromiumcodereview.appspot.com/10989055

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

2 files changed, 39 insertions, 15 deletions

diff --git a/net/dns/dns_transaction.cc b/net/dns/dns_transaction.cc
index 02add74..622680d 100644
--- a/net/dns/dns_transaction.cc
+++ b/net/dns/dns_transaction.cc
@@ -82,7 +82,11 @@ class DnsUDPAttempt {
   // and calls |callback| upon completion.
   int Start() {
     DCHECK_EQ(STATE_NONE, next_state_);
-    next_state_ = STATE_CONNECT;
+    int rv = socket_->Connect(server_);
+    DCHECK_NE(ERR_IO_PENDING, rv);
+    if (rv < 0)
+      return rv;
+    next_state_ = STATE_SEND_QUERY;
     return DoLoop(OK);
   }
 
@@ -116,7 +120,6 @@ class DnsUDPAttempt {
 
  private:
   enum State {
-    STATE_CONNECT,
     STATE_SEND_QUERY,
     STATE_SEND_QUERY_COMPLETE,
     STATE_READ_RESPONSE,
@@ -131,9 +134,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;
@@ -158,11 +158,6 @@ 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(),
diff --git a/net/dns/dns_transaction_unittest.cc b/net/dns/dns_transaction_unittest.cc
index dc140f2..379664e 100644
--- a/net/dns/dns_transaction_unittest.cc
+++ b/net/dns/dns_transaction_unittest.cc
@@ -111,6 +111,22 @@ class DnsSocketData {
 
 class TestSocketFactory;
 
+// A variant of MockUDPClientSocket which always fails to Connect.
+class FailingUDPClientSocket : public MockUDPClientSocket {
+ public:
+  FailingUDPClientSocket(SocketDataProvider* data,
+                         net::NetLog* net_log)
+      : MockUDPClientSocket(data, net_log) {
+  }
+  virtual ~FailingUDPClientSocket() {}
+  virtual int Connect(const IPEndPoint& endpoint) OVERRIDE {
+    return ERR_CONNECTION_REFUSED;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FailingUDPClientSocket);
+};
+
 // A variant of MockUDPClientSocket which notifies the factory OnConnect.
 class TestUDPClientSocket : public MockUDPClientSocket {
  public:
@@ -131,7 +147,7 @@ class TestUDPClientSocket : public MockUDPClientSocket {
 // Creates TestUDPClientSockets and keeps endpoints reported via OnConnect.
 class TestSocketFactory : public MockClientSocketFactory {
  public:
-  TestSocketFactory() {}
+  TestSocketFactory() : create_failing_sockets_(false) {}
   virtual ~TestSocketFactory() {}
 
   virtual DatagramClientSocket* CreateDatagramClientSocket(
@@ -139,6 +155,8 @@ class TestSocketFactory : public MockClientSocketFactory {
       const RandIntCallback& rand_int_cb,
       net::NetLog* net_log,
       const net::NetLog::Source& source) OVERRIDE {
+    if (create_failing_sockets_)
+      return new FailingUDPClientSocket(&empty_data_, net_log);
     SocketDataProvider* data_provider = mock_data().GetNext();
     TestUDPClientSocket* socket = new TestUDPClientSocket(this,
                                                           data_provider,
@@ -148,12 +166,15 @@ class TestSocketFactory : public MockClientSocketFactory {
   }
 
   void OnConnect(const IPEndPoint& endpoint) {
-    remote_endpoints.push_back(endpoint);
+    remote_endpoints_.push_back(endpoint);
   }
 
-  std::vector<IPEndPoint> remote_endpoints;
+  std::vector<IPEndPoint> remote_endpoints_;
+  bool create_failing_sockets_;
 
  private:
+  StaticSocketDataProvider empty_data_;
+
   DISALLOW_COPY_AND_ASSIGN(TestSocketFactory);
 };
 
@@ -379,9 +400,9 @@ class DnsTransactionTest : public testing::Test {
   // Checks if the sockets were connected in the order matching the indices in
   // |servers|.
   void CheckServerOrder(const unsigned* servers, size_t num_attempts) {
-    ASSERT_EQ(num_attempts, socket_factory_->remote_endpoints.size());
+    ASSERT_EQ(num_attempts, socket_factory_->remote_endpoints_.size());
     for (size_t i = 0; i < num_attempts; ++i) {
-      EXPECT_EQ(socket_factory_->remote_endpoints[i],
+      EXPECT_EQ(socket_factory_->remote_endpoints_[i],
                 session_->config().nameservers[servers[i]]);
     }
   }
@@ -802,6 +823,14 @@ TEST_F(DnsTransactionTest, SyncSearchQuery) {
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
+TEST_F(DnsTransactionTest, ConnectFailure) {
+  socket_factory_->create_failing_sockets_ = true;
+  transaction_ids_.push_back(0);  // Needed to make a DnsUDPAttempt.
+  TransactionHelper helper0("www.chromium.org", dns_protocol::kTypeA,
+                            ERR_CONNECTION_REFUSED);
+  EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
+}
+
 }  // namespace
 
 }  // namespace net