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

net/dns/dns_client_unittest.cc

Index: net/dns/dns_client_unittest.cc
diff --git a/net/dns/dns_client_unittest.cc b/net/dns/dns_client_unittest.cc
index fdbae8f91a3cf817c57d2f60c5c34ec2e67792b7..1663a6fa64ba9a272e7f03e9f5c1ede958ed99c1 100644
--- a/net/dns/dns_client_unittest.cc
+++ b/net/dns/dns_client_unittest.cc
@@ -15,28 +15,28 @@
 #include "net/socket/socket_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-// TODO(szym): test DnsClient::Request::Start with synchronous failure
-// TODO(szym): test suffix search and server fallback once implemented
+// TODO(szym): test timeout.
+// TODO(szym): test server fallback.
+// TODO(szym): test suffix search.
 
 namespace net {
 
 namespace {
 
-class DnsClientTest : public testing::Test {
+class DnsTransactionTest : public testing::Test {
  public:
-  class TestRequestHelper {
+  class TestHelper {
    public:
     // If |answer_count| < 0, it is the expected error code.
-    TestRequestHelper(const char* name,
-                      uint16 type,
-                      const MockWrite& write,
-                      const MockRead& read,
-                      int answer_count) {
-      // Must include the terminating \x00.
-      qname = std::string(name, strlen(name) + 1);
-      qtype = type;
-      expected_answer_count = answer_count;
-      completed = false;
+    TestHelper(const char* name,
+               uint16 type,
+               const MockWrite& write,
+               const MockRead& read,
+               int answer_count)
+        : qname(name),
+          qtype(type),
+          expected_answer_count(answer_count),
+          completed(false) {
       writes.push_back(write);
       reads.push_back(read);
       ReadBigEndian<uint16>(write.data, &transaction_id);
@@ -44,29 +44,33 @@ class DnsClientTest : public testing::Test {
                                               &writes[0], writes.size()));
     }
 
-    void MakeRequest(DnsClient* client) {
-      EXPECT_EQ(NULL, request.get());
-      request.reset(client->CreateRequest(
+    void MakeRequest(DnsTransactionFactory* client) {
+      EXPECT_EQ(NULL, transaction.get());
+      transaction = client->CreateTransaction(
           qname,
           qtype,
-          base::Bind(&TestRequestHelper::OnRequestComplete,
+          base::Bind(&TestHelper::OnTransactionComplete,
                      base::Unretained(this)),
-          BoundNetLog()));
-      EXPECT_EQ(qname, request->qname());
-      EXPECT_EQ(qtype, request->qtype());
-      EXPECT_EQ(ERR_IO_PENDING, request->Start());
+          BoundNetLog());
+      EXPECT_EQ(qname, transaction->GetHostname());
+      EXPECT_EQ(qtype, transaction->GetType());
+      int rv = transaction->Start();
+      if (rv != ERR_IO_PENDING) {
+        EXPECT_NE(OK, rv);
+        EXPECT_EQ(expected_answer_count, rv);
+      }
     }
 
     void Cancel() {
-      ASSERT_TRUE(request.get() != NULL);
-      request.reset(NULL);
+      ASSERT_TRUE(transaction.get() != NULL);
+      transaction.reset(NULL);
     }
 
-    void OnRequestComplete(DnsClient::Request* req,
-                           int rv,
-                           const DnsResponse* response) {
+    void OnTransactionComplete(DnsTransaction* t,
+                               int rv,
+                               const DnsResponse* response) {
       EXPECT_FALSE(completed);
-      EXPECT_EQ(request.get(), req);
+      EXPECT_EQ(transaction.get(), t);
 
       if (expected_answer_count >= 0) {
         EXPECT_EQ(OK, rv);
@@ -87,9 +91,9 @@ class DnsClientTest : public testing::Test {
       completed = true;
     }
 
-    void CancelOnRequestComplete(DnsClient::Request* req,
-                                 int rv,
-                                 const DnsResponse* response) {
+    void CancelOnTransactionComplete(DnsTransaction* req,
+                                     int rv,
+                                     const DnsResponse* response) {
       EXPECT_FALSE(completed);
       Cancel();
     }
@@ -100,15 +104,15 @@ class DnsClientTest : public testing::Test {
     std::vector<MockRead> reads;
     uint16 transaction_id;  // Id from first write.
     scoped_ptr<StaticSocketDataProvider> data;
-    scoped_ptr<DnsClient::Request> request;
+    scoped_ptr<DnsTransaction> transaction;
     int expected_answer_count;
 
     bool completed;
   };
 
   virtual void SetUp() OVERRIDE {
-    helpers_.push_back(new TestRequestHelper(
-        kT0DnsName,
+    helpers_.push_back(new TestHelper(
+        kT0HostName,
         kT0Qtype,
         MockWrite(true, reinterpret_cast<const char*>(kT0QueryDatagram),
                   arraysize(kT0QueryDatagram)),
@@ -116,8 +120,8 @@ class DnsClientTest : public testing::Test {
                   arraysize(kT0ResponseDatagram)),
         arraysize(kT0IpAddresses) + 1));  // +1 for CNAME RR
 
-    helpers_.push_back(new TestRequestHelper(
-        kT1DnsName,
+    helpers_.push_back(new TestHelper(
+        kT1HostName,
         kT1Qtype,
         MockWrite(true, reinterpret_cast<const char*>(kT1QueryDatagram),
                   arraysize(kT1QueryDatagram)),
@@ -125,8 +129,8 @@ class DnsClientTest : public testing::Test {
                   arraysize(kT1ResponseDatagram)),
         arraysize(kT1IpAddresses) + 1));  // +1 for CNAME RR
 
-    helpers_.push_back(new TestRequestHelper(
-        kT2DnsName,
+    helpers_.push_back(new TestHelper(
+        kT2HostName,
         kT2Qtype,
         MockWrite(true, reinterpret_cast<const char*>(kT2QueryDatagram),
                   arraysize(kT2QueryDatagram)),
@@ -134,8 +138,8 @@ class DnsClientTest : public testing::Test {
                   arraysize(kT2ResponseDatagram)),
         arraysize(kT2IpAddresses) + 1));  // +1 for CNAME RR
 
-    helpers_.push_back(new TestRequestHelper(
-        kT3DnsName,
+    helpers_.push_back(new TestHelper(
+        kT3HostName,
         kT3Qtype,
         MockWrite(true, reinterpret_cast<const char*>(kT3QueryDatagram),
                   arraysize(kT3QueryDatagram)),
@@ -143,15 +147,15 @@ class DnsClientTest : public testing::Test {
                   arraysize(kT3ResponseDatagram)),
         arraysize(kT3IpAddresses) + 2));  // +2 for CNAME RR
 
-    CreateClient();
+    CreateFactory();
   }
 
-  void CreateClient() {
-    MockClientSocketFactory* factory = new MockClientSocketFactory();
+  void CreateFactory() {
+    MockClientSocketFactory* socket_factory = new MockClientSocketFactory();
 
     transaction_ids_.clear();
     for (unsigned i = 0; i < helpers_.size(); ++i) {
-      factory->AddSocketDataProvider(helpers_[i]->data.get());
+      socket_factory->AddSocketDataProvider(helpers_[i]->data.get());
       transaction_ids_.push_back(static_cast<int>(helpers_[i]->transaction_id));
     }
 
@@ -166,11 +170,11 @@ class DnsClientTest : public testing::Test {
 
     DnsSession* session = new DnsSession(
         config,
-        factory,
-        base::Bind(&DnsClientTest::GetNextId, base::Unretained(this)),
+        socket_factory,
+        base::Bind(&DnsTransactionTest::GetNextId, base::Unretained(this)),
         NULL /* NetLog */);
 
-    client_.reset(DnsClient::CreateClient(session));
+    factory_ = DnsTransactionFactory::CreateFactory(session);
   }
 
   virtual void TearDown() OVERRIDE {
@@ -187,13 +191,13 @@ class DnsClientTest : public testing::Test {
   }
 
  protected:
-  std::vector<TestRequestHelper*> helpers_;
+  std::vector<TestHelper*> helpers_;
   std::deque<int> transaction_ids_;
-  scoped_ptr<DnsClient> client_;
+  scoped_ptr<DnsTransactionFactory> factory_;
 };
 
-TEST_F(DnsClientTest, Lookup) {
-  helpers_[0]->MakeRequest(client_.get());
+TEST_F(DnsTransactionTest, Lookup) {
+  helpers_[0]->MakeRequest(factory_.get());
 
   // Wait until result.
   MessageLoop::current()->RunAllPending();
@@ -201,9 +205,9 @@ TEST_F(DnsClientTest, Lookup) {
   EXPECT_TRUE(helpers_[0]->completed);
 }
 
-TEST_F(DnsClientTest, ConcurrentLookup) {
+TEST_F(DnsTransactionTest, ConcurrentLookup) {
   for (unsigned i = 0; i < helpers_.size(); ++i) {
-    helpers_[i]->MakeRequest(client_.get());
+    helpers_[i]->MakeRequest(factory_.get());
   }
 
   MessageLoop::current()->RunAllPending();
@@ -213,9 +217,9 @@ TEST_F(DnsClientTest, ConcurrentLookup) {
   }
 }
 
-TEST_F(DnsClientTest, CancelLookup) {
+TEST_F(DnsTransactionTest, CancelLookup) {
   for (unsigned i = 0; i < helpers_.size(); ++i) {
-    helpers_[i]->MakeRequest(client_.get());
+    helpers_[i]->MakeRequest(factory_.get());
   }
 
   helpers_[0]->Cancel();
@@ -229,13 +233,13 @@ TEST_F(DnsClientTest, CancelLookup) {
   EXPECT_TRUE(helpers_[3]->completed);
 }
 
-TEST_F(DnsClientTest, DestroyClient) {
+TEST_F(DnsTransactionTest, DestroyClient) {
   for (unsigned i = 0; i < helpers_.size(); ++i) {
-    helpers_[i]->MakeRequest(client_.get());
+    helpers_[i]->MakeRequest(factory_.get());
   }
 
   // Destroying the client does not affect running requests.
-  client_.reset(NULL);
+  factory_.reset(NULL);
 
   MessageLoop::current()->RunAllPending();
 
@@ -244,18 +248,20 @@ TEST_F(DnsClientTest, DestroyClient) {
   }
 }
 
-TEST_F(DnsClientTest, DestroyRequestFromCallback) {
+TEST_F(DnsTransactionTest, DestroyRequestFromCallback) {
   // Custom callback to cancel the completing request.
-  helpers_[0]->request.reset(client_->CreateRequest(
+  helpers_[0]->transaction = factory_->CreateTransaction(
       helpers_[0]->qname,
       helpers_[0]->qtype,
-      base::Bind(&TestRequestHelper::CancelOnRequestComplete,
+      base::Bind(&TestHelper::CancelOnTransactionComplete,
                  base::Unretained(helpers_[0])),
-      BoundNetLog()));
-  helpers_[0]->request->Start();
+      BoundNetLog());
+
+  int rv = helpers_[0]->transaction->Start();
+  EXPECT_EQ(ERR_IO_PENDING, rv);
 
   for (unsigned i = 1; i < helpers_.size(); ++i) {
-    helpers_[i]->MakeRequest(client_.get());
+    helpers_[i]->MakeRequest(factory_.get());
   }
 
   MessageLoop::current()->RunAllPending();
@@ -266,11 +272,11 @@ TEST_F(DnsClientTest, DestroyRequestFromCallback) {
   }
 }
 
-TEST_F(DnsClientTest, HandleFailure) {
+TEST_F(DnsTransactionTest, HandleFailure) {
   STLDeleteElements(&helpers_);
   // Wrong question.
-  helpers_.push_back(new TestRequestHelper(
-      kT0DnsName,
+  helpers_.push_back(new TestHelper(
+      kT0HostName,
       kT0Qtype,
       MockWrite(true, reinterpret_cast<const char*>(kT0QueryDatagram),
                 arraysize(kT0QueryDatagram)),
@@ -283,8 +289,8 @@ TEST_F(DnsClientTest, HandleFailure) {
   memcpy(nxdomain_response, kT0QueryDatagram, arraysize(nxdomain_response));
   nxdomain_response[2] &= 0x80;  // Response bit.
   nxdomain_response[3] &= 0x03;  // NXDOMAIN bit.
-  helpers_.push_back(new TestRequestHelper(
-      kT0DnsName,
+  helpers_.push_back(new TestHelper(
+      kT0HostName,
       kT0Qtype,
       MockWrite(true, reinterpret_cast<const char*>(kT0QueryDatagram),
                 arraysize(kT0QueryDatagram)),
@@ -292,10 +298,10 @@ TEST_F(DnsClientTest, HandleFailure) {
                 arraysize(nxdomain_response)),
       ERR_NAME_NOT_RESOLVED));
 
-  CreateClient();
+  CreateFactory();
 
   for (unsigned i = 0; i < helpers_.size(); ++i) {
-    helpers_[i]->MakeRequest(client_.get());
+    helpers_[i]->MakeRequest(factory_.get());
   }
 
   MessageLoop::current()->RunAllPending();