[net/dns] Perform A/AAAA queries for AF_UNSPEC resolutions in parallel.

net/dns/host_resolver_impl_unittest.cc

Index: net/dns/host_resolver_impl_unittest.cc
===================================================================
--- net/dns/host_resolver_impl_unittest.cc	(revision 218288)
+++ net/dns/host_resolver_impl_unittest.cc	(working copy)
@@ -12,6 +12,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/condition_variable.h"
@@ -413,6 +414,20 @@
 
   HostResolverImplTest() : proc_(new MockHostResolverProc()) {}
 
+  void CreateResolver() {
+    CreateResolverWithLimitsAndParams(DefaultLimits(),
+                                      DefaultParams(proc_.get()));
+  }
+
+  // This HostResolverImpl will only allow 1 outstanding resolve at a time and
+  // perform no retries.
+  void CreateSerialResolver() {
+    HostResolverImpl::ProcTaskParams params = DefaultParams(proc_.get());
+    params.max_retry_attempts = 0u;
+    PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 1);
+    CreateResolverWithLimitsAndParams(limits, params);
+  }
+
  protected:
   // A Request::Handler which is a proxy to the HostResolverImplTest fixture.
   struct Handler : public Request::Handler {
@@ -435,26 +450,24 @@
     HostResolverImplTest* test;
   };
 
-  void CreateResolver() {
-    resolver_.reset(new HostResolverImpl(HostCache::CreateDefaultCache(),
-                                         DefaultLimits(),
-                                         DefaultParams(proc_.get()),
-                                         NULL));
+  // testing::Test implementation:
+  virtual void SetUp() OVERRIDE {
+    CreateResolver();
   }
 
-  // This HostResolverImpl will only allow 1 outstanding resolve at a time and
-  // perform no retries.
-  void CreateSerialResolver() {
-    HostResolverImpl::ProcTaskParams params = DefaultParams(proc_.get());
-    params.max_retry_attempts = 0u;
-    PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 1);
-    resolver_.reset(new HostResolverImpl(
-        HostCache::CreateDefaultCache(),
-        limits,
-        params,
-        NULL));
+  virtual void TearDown() OVERRIDE {
+    if (resolver_.get())
+      EXPECT_EQ(0u, resolver_->num_running_dispatcher_jobs_for_tests());
+    EXPECT_FALSE(proc_->HasBlockedRequests());
   }
 
+  virtual void CreateResolverWithLimitsAndParams(
+      const PrioritizedDispatcher::Limits& limits,
+      const HostResolverImpl::ProcTaskParams& params) {
+    resolver_.reset(new HostResolverImpl(HostCache::CreateDefaultCache(),
+                                         limits, params, NULL));
+  }
+
   // The Request will not be made until a call to |Resolve()|, and the Job will
   // not start until released by |proc_->SignalXXX|.
   Request* CreateRequest(const HostResolver::RequestInfo& info) {
@@ -487,25 +500,15 @@
     return CreateRequest(hostname, kDefaultPort);
   }
 
-  virtual void SetUp() OVERRIDE {
-    CreateResolver();
-  }
-
-  virtual void TearDown() OVERRIDE {
-    if (resolver_.get())
-      EXPECT_EQ(0u, resolver_->num_running_jobs_for_tests());
-    EXPECT_FALSE(proc_->HasBlockedRequests());
-  }
-
   void set_handler(Handler* handler) {
     handler_.reset(handler);
     handler_->test = this;
   }
 
   // Friendship is not inherited, so use proxies to access those.
-  size_t num_running_jobs() const {
+  size_t num_running_dispatcher_jobs() const {
     DCHECK(resolver_.get());
-    return resolver_->num_running_jobs_for_tests();
+    return resolver_->num_running_dispatcher_jobs_for_tests();
   }
 
   void set_fallback_to_proctask(bool fallback_to_proctask) {
@@ -889,7 +892,7 @@
 
   EXPECT_EQ(ERR_NETWORK_CHANGED, requests_[0]->WaitForResult());
 
-  EXPECT_EQ(1u, num_running_jobs());
+  EXPECT_EQ(1u, num_running_dispatcher_jobs());
 
   EXPECT_FALSE(requests_[1]->completed());
   EXPECT_FALSE(requests_[2]->completed());
@@ -1256,36 +1259,63 @@
 
 // Specialized fixture for tests of DnsTask.
 class HostResolverImplDnsTest : public HostResolverImplTest {
+ public:
+  HostResolverImplDnsTest() : dns_client_(NULL) {}
+
  protected:
+  // testing::Test implementation:
   virtual void SetUp() OVERRIDE {
-    AddDnsRule("nx", dns_protocol::kTypeA, MockDnsClientRule::FAIL);
-    AddDnsRule("nx", dns_protocol::kTypeAAAA, MockDnsClientRule::FAIL);
-    AddDnsRule("ok", dns_protocol::kTypeA, MockDnsClientRule::OK);
-    AddDnsRule("ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK);
-    AddDnsRule("4ok", dns_protocol::kTypeA, MockDnsClientRule::OK);
-    AddDnsRule("4ok", dns_protocol::kTypeAAAA, MockDnsClientRule::EMPTY);
-    AddDnsRule("6ok", dns_protocol::kTypeA, MockDnsClientRule::EMPTY);
-    AddDnsRule("6ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK);
-    AddDnsRule("4nx", dns_protocol::kTypeA, MockDnsClientRule::OK);
-    AddDnsRule("4nx", dns_protocol::kTypeAAAA, MockDnsClientRule::FAIL);
+    AddDnsRule("nx", dns_protocol::kTypeA, MockDnsClientRule::FAIL, false);
+    AddDnsRule("nx", dns_protocol::kTypeAAAA, MockDnsClientRule::FAIL, false);
+    AddDnsRule("ok", dns_protocol::kTypeA, MockDnsClientRule::OK, false);
+    AddDnsRule("ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK, false);
+    AddDnsRule("4ok", dns_protocol::kTypeA, MockDnsClientRule::OK, false);
+    AddDnsRule("4ok", dns_protocol::kTypeAAAA, MockDnsClientRule::EMPTY, false);
+    AddDnsRule("6ok", dns_protocol::kTypeA, MockDnsClientRule::EMPTY, false);
+    AddDnsRule("6ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK, false);
+    AddDnsRule("4nx", dns_protocol::kTypeA, MockDnsClientRule::OK, false);
+    AddDnsRule("4nx", dns_protocol::kTypeAAAA, MockDnsClientRule::FAIL, false);
+
+    AddDnsRule("4slow_ok", dns_protocol::kTypeA, MockDnsClientRule::OK, true);
+    AddDnsRule("4slow_ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK,
+               false);
+    AddDnsRule("6slow_ok", dns_protocol::kTypeA, MockDnsClientRule::OK, false);
+    AddDnsRule("6slow_ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK,
+               true);
+    AddDnsRule("4slow_4ok", dns_protocol::kTypeA, MockDnsClientRule::OK, true);
+    AddDnsRule("4slow_4ok", dns_protocol::kTypeAAAA, MockDnsClientRule::EMPTY,
+               false);
+    AddDnsRule("4slow_4timeout", dns_protocol::kTypeA,
+               MockDnsClientRule::TIMEOUT, true);
+    AddDnsRule("4slow_4timeout", dns_protocol::kTypeAAAA, MockDnsClientRule::OK,
+               false);
+    AddDnsRule("4slow_6timeout", dns_protocol::kTypeA,
+               MockDnsClientRule::OK, true);
+    AddDnsRule("4slow_6timeout", dns_protocol::kTypeAAAA,
+               MockDnsClientRule::TIMEOUT, false);
     CreateResolver();
   }
 
-  void CreateResolver() {
+  // HostResolverImplTest implementation:
+  virtual void CreateResolverWithLimitsAndParams(
+      const PrioritizedDispatcher::Limits& limits,
+      const HostResolverImpl::ProcTaskParams& params) OVERRIDE {
     resolver_.reset(new HostResolverImpl(HostCache::CreateDefaultCache(),
-                                         DefaultLimits(),
-                                         DefaultParams(proc_.get()),
+                                         limits,
+                                         params,
                                          NULL));
     // Disable IPv6 support probing.
     resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED);
-    resolver_->SetDnsClient(CreateMockDnsClient(DnsConfig(), dns_rules_));
+    dns_client_ = new MockDnsClient(DnsConfig(), dns_rules_);
+    resolver_->SetDnsClient(scoped_ptr<DnsClient>(dns_client_));
   }
 
   // Adds a rule to |dns_rules_|. Must be followed by |CreateResolver| to apply.
   void AddDnsRule(const std::string& prefix,
                   uint16 qtype,
-                  MockDnsClientRule::Result result) {
-    dns_rules_.push_back(MockDnsClientRule(prefix, qtype, result));
+                  MockDnsClientRule::Result result,
+                  bool delay) {
+    dns_rules_.push_back(MockDnsClientRule(prefix, qtype, result, delay));
   }
 
   void ChangeDnsConfig(const DnsConfig& config) {
@@ -1295,6 +1325,8 @@
   }
 
   MockDnsClientRuleList dns_rules_;
+  // Owned by |resolver_|.
+  MockDnsClient* dns_client_;
 };
 
 // TODO(szym): Test AbortAllInProgressJobs due to DnsConfig change.
@@ -1362,7 +1394,8 @@
 
   // Simulate the case when the preference or policy has disabled the DNS client
   // causing AbortDnsTasks.
-  resolver_->SetDnsClient(CreateMockDnsClient(DnsConfig(), dns_rules_));
+  resolver_->SetDnsClient(
+      scoped_ptr<DnsClient>(new MockDnsClient(DnsConfig(), dns_rules_)));
   ChangeDnsConfig(CreateValidDnsConfig());
 
   // First request is resolved by MockDnsClient, others should fail due to
@@ -1586,7 +1619,8 @@
                                        DefaultLimits(),
                                        DefaultParams(proc.get()),
                                        NULL));
-  resolver_->SetDnsClient(CreateMockDnsClient(DnsConfig(), dns_rules_));
+  resolver_->SetDnsClient(
+      scoped_ptr<DnsClient>(new MockDnsClient(DnsConfig(), dns_rules_)));
   resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_IPV4);
 
   // Get the expected output.
@@ -1640,4 +1674,186 @@
   EXPECT_EQ(saw_ipv6, req->HasAddress("::1", 80));
 }
 
+// Cancel a request with a single DNS transaction active.
+TEST_F(HostResolverImplDnsTest, CancelWithOneTransactionActive) {
+  resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_IPV4);
+  ChangeDnsConfig(CreateValidDnsConfig());
+
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80)->Resolve());
+  EXPECT_EQ(1u, num_running_dispatcher_jobs());
+  requests_[0]->Cancel();
+
+  // Dispatcher state checked in TearDown.
+}
+
+// Cancel a request with a single DNS transaction active and another pending.
+TEST_F(HostResolverImplDnsTest, CancelWithOneTransactionActiveOnePending) {
+  CreateSerialResolver();
+  resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED);
+  ChangeDnsConfig(CreateValidDnsConfig());
+
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80)->Resolve());
+  EXPECT_EQ(1u, num_running_dispatcher_jobs());
+  requests_[0]->Cancel();
+
+  // Dispatcher state checked in TearDown.
+}
+
+// Cancel a request with two DNS transactions active.
+TEST_F(HostResolverImplDnsTest, CancelWithTwoTransactionsActive) {
+  resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED);
+  ChangeDnsConfig(CreateValidDnsConfig());
+
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80)->Resolve());
+  EXPECT_EQ(2u, num_running_dispatcher_jobs());
+  requests_[0]->Cancel();
+
+  // Dispatcher state checked in TearDown.
+}
+
+// Delete a resolver with some active requests and some queued requests.
+TEST_F(HostResolverImplDnsTest, DeleteWithActiveTransactions) {
+  // At most 10 Jobs active at once.
+  CreateResolverWithLimitsAndParams(
+      PrioritizedDispatcher::Limits(NUM_PRIORITIES, 10u),
+      DefaultParams(proc_.get()));
+
+  resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED);
+  ChangeDnsConfig(CreateValidDnsConfig());
+
+  // First active job is an IPv4 request.
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80, MEDIUM,
+                                          ADDRESS_FAMILY_IPV4)->Resolve());
+
+  // Add 10 more DNS lookups for different hostnames.  First 4 should have two
+  // active jobs, next one has a single active job, and one pending.  Others
+  // should all be queued.
+  for (int i = 0; i < 10; ++i) {
+    EXPECT_EQ(ERR_IO_PENDING, CreateRequest(
+        base::StringPrintf("ok%i", i))->Resolve());
+  }
+  EXPECT_EQ(10u, num_running_dispatcher_jobs());
+
+  resolver_.reset();
+}
+
+// Cancel a request with only the IPv6 transaction active.
+TEST_F(HostResolverImplDnsTest, CancelWithIPv6TransactionActive) {
+  resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED);
+  ChangeDnsConfig(CreateValidDnsConfig());
+
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("6slow_ok", 80)->Resolve());
+  EXPECT_EQ(2u, num_running_dispatcher_jobs());
+
+  // The IPv4 request should complete, the IPv6 request is still pending.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1u, num_running_dispatcher_jobs());
+  requests_[0]->Cancel();
+
+  // Dispatcher state checked in TearDown.
+}
+
+// Cancel a request with only the IPv4 transaction pending.
+TEST_F(HostResolverImplDnsTest, CancelWithIPv4TransactionPending) {
+  set_fallback_to_proctask(false);
+  resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED);
+  ChangeDnsConfig(CreateValidDnsConfig());
+
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("4slow_ok", 80)->Resolve());
+  EXPECT_EQ(2u, num_running_dispatcher_jobs());
+
+  // The IPv6 request should complete, the IPv4 request is still pending.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1u, num_running_dispatcher_jobs());
+
+  requests_[0]->Cancel();
+}
+
+// Test cases where AAAA completes first.
+TEST_F(HostResolverImplDnsTest, AAAACompletesFirst) {
+  set_fallback_to_proctask(false);
+  resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED);
+  ChangeDnsConfig(CreateValidDnsConfig());
+
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("4slow_ok", 80)->Resolve());
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("4slow_4ok", 80)->Resolve());
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("4slow_4timeout", 80)->Resolve());
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("4slow_6timeout", 80)->Resolve());
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(requests_[0]->completed());
+  EXPECT_FALSE(requests_[1]->completed());
+  EXPECT_FALSE(requests_[2]->completed());
+  // The IPv6 of the third request should have failed and resulted in cancelling
+  // the IPv4 request.
+  EXPECT_TRUE(requests_[3]->completed());
+  EXPECT_EQ(ERR_DNS_TIMED_OUT, requests_[3]->result());
+  EXPECT_EQ(3u, num_running_dispatcher_jobs());
+
+  dns_client_->CompleteDelayedTransactions();
+  EXPECT_TRUE(requests_[0]->completed());
+  EXPECT_EQ(OK, requests_[0]->result());
+  EXPECT_EQ(2u, requests_[0]->NumberOfAddresses());
+  EXPECT_TRUE(requests_[0]->HasAddress("127.0.0.1", 80));
+  EXPECT_TRUE(requests_[0]->HasAddress("::1", 80));
+
+  EXPECT_TRUE(requests_[1]->completed());
+  EXPECT_EQ(OK, requests_[1]->result());
+  EXPECT_EQ(1u, requests_[1]->NumberOfAddresses());
+  EXPECT_TRUE(requests_[1]->HasAddress("127.0.0.1", 80));
+
+  EXPECT_TRUE(requests_[2]->completed());
+  EXPECT_EQ(ERR_DNS_TIMED_OUT, requests_[2]->result());
+}
+
+// Test the case where only a single transaction slot is available.
+TEST_F(HostResolverImplDnsTest, SerialResolver) {
+  CreateSerialResolver();
+  set_fallback_to_proctask(false);
+  resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED);
+  ChangeDnsConfig(CreateValidDnsConfig());
+
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80)->Resolve());
+  EXPECT_EQ(1u, num_running_dispatcher_jobs());
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(requests_[0]->completed());
+  EXPECT_EQ(OK, requests_[0]->result());
+  EXPECT_EQ(2u, requests_[0]->NumberOfAddresses());
+  EXPECT_TRUE(requests_[0]->HasAddress("127.0.0.1", 80));
+  EXPECT_TRUE(requests_[0]->HasAddress("::1", 80));
+}
+
+// Test the case where the AAAA query is started when another transaction
+// completes.
+TEST_F(HostResolverImplDnsTest, AAAAStartsAfterOtherJobFinishes) {
+  CreateResolverWithLimitsAndParams(
+      PrioritizedDispatcher::Limits(NUM_PRIORITIES, 2),
+      DefaultParams(proc_.get()));
+  set_fallback_to_proctask(false);
+  resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED);
+  ChangeDnsConfig(CreateValidDnsConfig());
+
+  EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80, MEDIUM,
+                                          ADDRESS_FAMILY_IPV4)->Resolve());
+  EXPECT_EQ(ERR_IO_PENDING,
+            CreateRequest("4slow_ok", 80, MEDIUM)->Resolve());
+  // An IPv4 request should have been started pending for each job.
+  EXPECT_EQ(2u, num_running_dispatcher_jobs());
+
+  // Request 0's IPv4 request should complete, starting Request 1's IPv6
+  // request, which should also complete.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1u, num_running_dispatcher_jobs());
+  EXPECT_TRUE(requests_[0]->completed());
+  EXPECT_FALSE(requests_[1]->completed());
+
+  dns_client_->CompleteDelayedTransactions();
+  EXPECT_TRUE(requests_[1]->completed());
+  EXPECT_EQ(OK, requests_[1]->result());
+  EXPECT_EQ(2u, requests_[1]->NumberOfAddresses());
+  EXPECT_TRUE(requests_[1]->HasAddress("127.0.0.1", 80));
+  EXPECT_TRUE(requests_[1]->HasAddress("::1", 80));
+}
+
 }  // namespace net