Add a command line option ("host-resolver-retry-attempts")

to specify the number of retry attempts to resolve host.


BUG=82580
TEST=host resolver unit tests
R=eroman
Review URL: http://codereview.chromium.org/7011044

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

10 files changed, 86 insertions, 55 deletions

diff --git a/net/base/host_resolver.h b/net/base/host_resolver.h
index 3148697..637e671 100644
--- a/net/base/host_resolver.h
+++ b/net/base/host_resolver.h
@@ -132,6 +132,11 @@ class NET_API HostResolver {
   // concurrency.
   static const size_t kDefaultParallelism = 0;
 
+  // This value can be passed into CreateSystemHostResolver as the
+  // |max_retry_attempts| parameter. This is the maximum number of times we
+  // will retry for host resolution.
+  static const size_t kDefaultRetryAttempts = -1;
+
   // If any completion callbacks are pending when the resolver is destroyed,
   // the host resolutions are cancelled, and the completion callbacks will not
   // be called.
@@ -242,7 +247,11 @@ class SingleRequestHostResolver {
 // |max_concurrent_resolves| is how many resolve requests will be allowed to
 // run in parallel. Pass HostResolver::kDefaultParallelism to choose a
 // default value.
+// |max_retry_attempts| is the maximum number of times we will retry for host
+// resolution. Pass HostResolver::kDefaultRetryAttempts to choose a default
+// value.
 NET_API HostResolver* CreateSystemHostResolver(size_t max_concurrent_resolves,
+                                               size_t max_retry_attempts,
                                                NetLog* net_log);
 
 }  // namespace net
diff --git a/net/base/host_resolver_impl.cc b/net/base/host_resolver_impl.cc
index 59d1b32..2f9e4bb0 100644
--- a/net/base/host_resolver_impl.cc
+++ b/net/base/host_resolver_impl.cc
@@ -97,6 +97,7 @@ HostCache* CreateDefaultCache() {
 
 // static
 HostResolver* CreateSystemHostResolver(size_t max_concurrent_resolves,
+                                       size_t max_retry_attempts,
                                        NetLog* net_log) {
   // Maximum of 8 concurrent resolver threads.
   // Some routers (or resolvers) appear to start to provide host-not-found if
@@ -108,8 +109,8 @@ HostResolver* CreateSystemHostResolver(size_t max_concurrent_resolves,
     max_concurrent_resolves = kDefaultMaxJobs;
 
   HostResolverImpl* resolver =
-      new HostResolverImpl(NULL, CreateDefaultCache(),
-                           max_concurrent_resolves, net_log);
+      new HostResolverImpl(NULL, CreateDefaultCache(), max_concurrent_resolves,
+                           max_retry_attempts, net_log);
 
   return resolver;
 }
@@ -450,10 +451,12 @@ class HostResolverImpl::Job
     // OnCheckForComplete has the potential for starting a new attempt on a
     // different worker thread if none of our outstanding attempts have
     // completed yet.
-    origin_loop_->PostDelayedTask(
-        FROM_HERE,
-        NewRunnableMethod(this, &Job::OnCheckForComplete),
-        unresponsive_delay_.InMilliseconds());
+    if (attempt_number_ <= resolver_->max_retry_attempts()) {
+      origin_loop_->PostDelayedTask(
+          FROM_HERE,
+          NewRunnableMethod(this, &Job::OnCheckForComplete),
+          unresponsive_delay_.InMilliseconds());
+    }
   }
 
   // Cancels the current job. The Job will be orphaned. Any outstanding resolve
@@ -553,16 +556,11 @@ class HostResolverImpl::Job
   void OnCheckForComplete() {
     DCHECK(origin_loop_->BelongsToCurrentThread());
 
-    if (was_cancelled() || was_completed())
+    if (was_completed() || was_cancelled())
       return;
 
     DCHECK(resolver_);
-    base::TimeDelta unresponsive_delay =
-        unresponsive_delay_ * resolver_->retry_factor();
-    if (unresponsive_delay >= resolver_->maximum_unresponsive_delay())
-      return;
-
-    unresponsive_delay_ = unresponsive_delay;
+    unresponsive_delay_ *= resolver_->retry_factor();
     StartLookupAttempt();
   }
 
@@ -1033,12 +1031,13 @@ HostResolverImpl::HostResolverImpl(
     HostResolverProc* resolver_proc,
     HostCache* cache,
     size_t max_jobs,
+    size_t max_retry_attempts,
     NetLog* net_log)
     : cache_(cache),
       max_jobs_(max_jobs),
+      max_retry_attempts_(max_retry_attempts),
       unresponsive_delay_(base::TimeDelta::FromMilliseconds(6000)),
       retry_factor_(2),
-      maximum_unresponsive_delay_(base::TimeDelta::FromMilliseconds(60000)),
       next_request_id_(0),
       next_job_id_(0),
       resolver_proc_(resolver_proc),
@@ -1049,6 +1048,12 @@ HostResolverImpl::HostResolverImpl(
       net_log_(net_log) {
   DCHECK_GT(max_jobs, 0u);
 
+  // Maximum of 4 retry attempts for host resolution.
+  static const size_t kDefaultMaxRetryAttempts = 4u;
+
+  if (max_retry_attempts_ == HostResolver::kDefaultRetryAttempts)
+    max_retry_attempts_ = kDefaultMaxRetryAttempts;
+
   // It is cumbersome to expose all of the constraints in the constructor,
   // so we choose some defaults, which users can override later.
   job_pools_[POOL_NORMAL] = new JobPool(max_jobs, 100u * max_jobs);
@@ -1537,7 +1542,7 @@ HostResolverImpl::Job* HostResolverImpl::CreateAndStartJob(Request* req) {
                                   NULL);
 
   scoped_refptr<Job> job(new Job(next_job_id_++, this, key,
-                                   req->request_net_log(), net_log_));
+                                 req->request_net_log(), net_log_));
   job->AddRequest(req);
   AddOutstandingJob(job);
   job->Start();
diff --git a/net/base/host_resolver_impl.h b/net/base/host_resolver_impl.h
index 9b8c1a6..19b4a2e 100644
--- a/net/base/host_resolver_impl.h
+++ b/net/base/host_resolver_impl.h
@@ -87,19 +87,22 @@ class NET_API HostResolverImpl
   // |max_jobs| specifies the maximum number of threads that the host resolver
   // will use (not counting potential duplicate attempts). Use
   // SetPoolConstraints() to specify finer-grain settings.
+  // |max_retry_attempts| is the maximum number of times we will retry for host
+  // resolution. Pass HostResolver::kDefaultRetryAttempts to choose a default
+  // value.
   //
   // For each attempt, we could start another attempt if host is not resolved
   // within unresponsive_delay_ time. We keep attempting to resolve the host
-  // until retry interval reaches maximum_unresponsive_delay_ time. For every
-  // retry attempt, we grow the unresponsive_delay_ by the retry_factor_ amount
-  // (that is retry interval is multiplied by the retry factor each time). Once
-  // retry interval exceeds maximum_unresponsive_delay_ time, we give up on
-  // additional attempts.
+  // for max_retry_attempts. For every retry attempt, we grow the
+  // unresponsive_delay_ by the retry_factor_ amount (that is retry interval is
+  // multiplied by the retry factor each time). Once we have retried
+  // max_retry_attempts, we give up on additional attempts.
   //
   // |net_log| must remain valid for the life of the HostResolverImpl.
   HostResolverImpl(HostResolverProc* resolver_proc,
                    HostCache* cache,
                    size_t max_jobs,
+                   size_t max_retry_attempts,
                    NetLog* net_log);
 
   // If any completion callbacks are pending when the resolver is destroyed,
@@ -252,13 +255,21 @@ class NET_API HostResolverImpl
   // NetworkChangeNotifier::IPAddressObserver methods:
   virtual void OnIPAddressChanged();
 
+  // Helper methods to get and set max_retry_attempts_.
+  size_t max_retry_attempts() const {
+    return max_retry_attempts_;
+  }
+  void set_max_retry_attempts(const size_t max_retry_attempts) {
+    max_retry_attempts_ = max_retry_attempts;
+  }
+
   // Helper methods for unit tests to get and set unresponsive_delay_.
   base::TimeDelta unresponsive_delay() const { return unresponsive_delay_; }
   void set_unresponsive_delay(const base::TimeDelta& unresponsive_delay) {
     unresponsive_delay_ = unresponsive_delay;
   }
 
-  // Helper methods to get and set retry_factor.
+  // Helper methods to get and set retry_factor_.
   uint32 retry_factor() const {
     return retry_factor_;
   }
@@ -266,15 +277,6 @@ class NET_API HostResolverImpl
     retry_factor_ = retry_factor;
   }
 
-  // Helper methods for unit tests to get and set maximum_unresponsive_delay_.
-  base::TimeDelta maximum_unresponsive_delay() const {
-    return maximum_unresponsive_delay_;
-  }
-  void set_maximum_unresponsive_delay(
-      const base::TimeDelta& maximum_unresponsive_delay) {
-    maximum_unresponsive_delay_ = maximum_unresponsive_delay;
-  }
-
   // Cache of host resolution results.
   scoped_ptr<HostCache> cache_;
 
@@ -285,6 +287,9 @@ class NET_API HostResolverImpl
   // create multiple concurrent resolve attempts for the hostname.
   size_t max_jobs_;
 
+  // Maximum number retry attempts to resolve the hostname.
+  size_t max_retry_attempts_;
+
   // This is the limit after which we make another attempt to resolve the host
   // if the worker thread has not responded yet. Allow unit tests to change the
   // value.
@@ -294,11 +299,6 @@ class NET_API HostResolverImpl
   // change the value.
   uint32 retry_factor_;
 
-  // This is the limit on how large we grow the retry interval. Once it exceeds
-  // this, we give up on additional attempts. Allow unit tests to change the
-  // value.
-  base::TimeDelta maximum_unresponsive_delay_;
-
   // The information to track pending requests for a JobPool, as well as
   // how many outstanding jobs the pool already has, and its constraints.
   JobPool* job_pools_[POOL_COUNT];
diff --git a/net/base/host_resolver_impl_unittest.cc b/net/base/host_resolver_impl_unittest.cc
index 7b72d1d..52b7f89 100644
--- a/net/base/host_resolver_impl_unittest.cc
+++ b/net/base/host_resolver_impl_unittest.cc
@@ -41,10 +41,11 @@ HostCache* CreateDefaultCache() {
 }
 
 static const size_t kMaxJobs = 10u;
+static const size_t kMaxRetryAttempts = 4u;
 
 HostResolverImpl* CreateHostResolverImpl(HostResolverProc* resolver_proc) {
   return new HostResolverImpl(resolver_proc, CreateDefaultCache(), kMaxJobs,
-                              NULL);
+                              kMaxRetryAttempts, NULL);
 }
 
 // Helper to create a HostResolver::RequestInfo.
@@ -451,6 +452,7 @@ TEST_F(HostResolverImplTest, CanceledAsynchronousLookup) {
         new HostResolverImpl(resolver_proc,
                              CreateDefaultCache(),
                              kMaxJobs,
+                             kMaxRetryAttempts,
                              &net_log));
     AddressList addrlist;
     const int kPortnum = 80;
@@ -897,7 +899,8 @@ TEST_F(HostResolverImplTest, StartWithinCallback) {
 
   // Turn off caching for this host resolver.
   scoped_ptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, NULL, kMaxJobs, NULL));
+      new HostResolverImpl(resolver_proc, NULL, kMaxJobs, kMaxRetryAttempts,
+                           NULL));
 
   // The class will receive callbacks for when each resolve completes. It
   // checks that the right things happened.
@@ -1197,7 +1200,8 @@ TEST_F(HostResolverImplTest, CancellationObserver) {
 // Test that IP address changes flush the cache.
 TEST_F(HostResolverImplTest, FlushCacheOnIPAddressChange) {
   scoped_ptr<HostResolver> host_resolver(
-      new HostResolverImpl(NULL, CreateDefaultCache(), kMaxJobs, NULL));
+      new HostResolverImpl(NULL, CreateDefaultCache(), kMaxJobs,
+                           kMaxRetryAttempts, NULL));
 
   AddressList addrlist;
 
@@ -1231,7 +1235,8 @@ TEST_F(HostResolverImplTest, AbortOnIPAddressChanged) {
       new WaitingHostResolverProc(NULL));
   HostCache* cache = CreateDefaultCache();
   scoped_ptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, cache, kMaxJobs, NULL));
+      new HostResolverImpl(resolver_proc, cache, kMaxJobs, kMaxRetryAttempts,
+                           NULL));
 
   // Resolve "host1".
   HostResolver::RequestInfo info(HostPortPair("host1", 70));
@@ -1354,9 +1359,10 @@ TEST_F(HostResolverImplTest, HigherPriorityRequestsStartedFirst) {
 
   // This HostResolverImpl will only allow 1 outstanding resolve at a time.
   size_t kMaxJobs = 1u;
+  const size_t kRetryAttempts = 0u;
   scoped_ptr<HostResolver> host_resolver(
       new HostResolverImpl(resolver_proc, CreateDefaultCache(), kMaxJobs,
-                           NULL));
+                           kRetryAttempts, NULL));
 
   CapturingObserver observer;
   host_resolver->AddObserver(&observer);
@@ -1439,9 +1445,10 @@ TEST_F(HostResolverImplTest, CancelPendingRequest) {
 
   // This HostResolverImpl will only allow 1 outstanding resolve at a time.
   const size_t kMaxJobs = 1u;
+  const size_t kRetryAttempts = 0u;
   scoped_ptr<HostResolver> host_resolver(
       new HostResolverImpl(resolver_proc, CreateDefaultCache(), kMaxJobs,
-                           NULL));
+                           kRetryAttempts, NULL));
 
   // Note that at this point the CapturingHostResolverProc is blocked, so any
   // requests we make will not complete.
@@ -1502,8 +1509,10 @@ TEST_F(HostResolverImplTest, QueueOverflow) {
 
   // This HostResolverImpl will only allow 1 outstanding resolve at a time.
   const size_t kMaxOutstandingJobs = 1u;
+  const size_t kRetryAttempts = 0u;
   scoped_ptr<HostResolverImpl> host_resolver(new HostResolverImpl(
-      resolver_proc, CreateDefaultCache(), kMaxOutstandingJobs, NULL));
+      resolver_proc, CreateDefaultCache(), kMaxOutstandingJobs, kRetryAttempts,
+      NULL));
 
   // Only allow up to 3 requests to be enqueued at a time.
   const size_t kMaxPendingRequests = 3u;
@@ -1580,8 +1589,10 @@ TEST_F(HostResolverImplTest, SetDefaultAddressFamily_IPv4) {
 
   // This HostResolverImpl will only allow 1 outstanding resolve at a time.
   const size_t kMaxOutstandingJobs = 1u;
+  const size_t kRetryAttempts = 0u;
   scoped_ptr<HostResolverImpl> host_resolver(new HostResolverImpl(
-      resolver_proc, CreateDefaultCache(), kMaxOutstandingJobs, NULL));
+      resolver_proc, CreateDefaultCache(), kMaxOutstandingJobs, kRetryAttempts,
+      NULL));
 
   host_resolver->SetDefaultAddressFamily(ADDRESS_FAMILY_IPV4);
 
@@ -1648,8 +1659,10 @@ TEST_F(HostResolverImplTest, SetDefaultAddressFamily_IPv6) {
 
   // This HostResolverImpl will only allow 1 outstanding resolve at a time.
   const size_t kMaxOutstandingJobs = 1u;
+  const size_t kRetryAttempts = 0u;
   scoped_ptr<HostResolverImpl> host_resolver(new HostResolverImpl(
-      resolver_proc, CreateDefaultCache(), kMaxOutstandingJobs, NULL));
+      resolver_proc, CreateDefaultCache(), kMaxOutstandingJobs, kRetryAttempts,
+      NULL));
 
   host_resolver->SetDefaultAddressFamily(ADDRESS_FAMILY_IPV6);
 
@@ -1713,9 +1726,8 @@ TEST_F(HostResolverImplTest, SetDefaultAddressFamily_Synchronous) {
   scoped_refptr<CapturingHostResolverProc> resolver_proc(
       new CapturingHostResolverProc(new EchoingHostResolverProc));
 
-  const size_t kMaxOutstandingJobs = 10u;
   scoped_ptr<HostResolverImpl> host_resolver(new HostResolverImpl(
-      resolver_proc, CreateDefaultCache(), kMaxOutstandingJobs, NULL));
+      resolver_proc, CreateDefaultCache(), kMaxJobs, kMaxRetryAttempts, NULL));
 
   host_resolver->SetDefaultAddressFamily(ADDRESS_FAMILY_IPV4);
 
@@ -1814,16 +1826,14 @@ TEST_F(HostResolverImplTest, MultipleAttempts) {
           NULL, kAttemptNumberToResolve, kTotalAttempts));
   HostCache* cache = CreateDefaultCache();
   scoped_ptr<HostResolverImpl> host_resolver(
-      new HostResolverImpl(resolver_proc, cache, kMaxJobs, NULL));
+      new HostResolverImpl(resolver_proc, cache, kMaxJobs, kMaxRetryAttempts,
+                           NULL));
 
-  // Specify smaller interval for unresponsive_delay_ and
-  // maximum_unresponsive_delay_ for HostResolverImpl so that unit test
-  // runs faster. For example, this test finishes in 1.5 secs (500ms * 3).
+  // Specify smaller interval for unresponsive_delay_ for HostResolverImpl so
+  // that unit test runs faster. For example, this test finishes in 1.5 secs
+  // (500ms * 3).
   TimeDelta kUnresponsiveTime = TimeDelta::FromMilliseconds(500);
-  TimeDelta kMaximumUnresponsiveTime = TimeDelta::FromMilliseconds(2500);
-
   host_resolver->set_unresponsive_delay(kUnresponsiveTime);
-  host_resolver->set_maximum_unresponsive_delay(kMaximumUnresponsiveTime);
 
   // Resolve "host1".
   HostResolver::RequestInfo info(HostPortPair("host1", 70));
diff --git a/net/base/mock_host_resolver.cc b/net/base/mock_host_resolver.cc
index cd14eff..d8f3acb 100644
--- a/net/base/mock_host_resolver.cc
+++ b/net/base/mock_host_resolver.cc
@@ -85,7 +85,7 @@ void MockHostResolverBase::Reset(HostResolverProc* interceptor) {
         base::TimeDelta::FromSeconds(0));
   }
 
-  impl_.reset(new HostResolverImpl(proc, cache, 50u, NULL));
+  impl_.reset(new HostResolverImpl(proc, cache, 50u, 4u, NULL));
 }
 
 int MockHostResolverBase::Resolve(const RequestInfo& info,
diff --git a/net/proxy/proxy_script_fetcher_impl_unittest.cc b/net/proxy/proxy_script_fetcher_impl_unittest.cc
index d1094bd..98fbd5e 100644
--- a/net/proxy/proxy_script_fetcher_impl_unittest.cc
+++ b/net/proxy/proxy_script_fetcher_impl_unittest.cc
@@ -45,6 +45,7 @@ class RequestContext : public URLRequestContext {
     ProxyConfig no_proxy;
     storage_.set_host_resolver(
         CreateSystemHostResolver(HostResolver::kDefaultParallelism,
+                                 HostResolver::kDefaultRetryAttempts,
                                  NULL));
     storage_.set_cert_verifier(new CertVerifier);
     storage_.set_proxy_service(ProxyService::CreateFixed(no_proxy));
diff --git a/net/socket/transport_client_socket_unittest.cc b/net/socket/transport_client_socket_unittest.cc
index a387db5..bbccd7c 100644
--- a/net/socket/transport_client_socket_unittest.cc
+++ b/net/socket/transport_client_socket_unittest.cc
@@ -119,6 +119,7 @@ void TransportClientSocketTest::SetUp() {
   AddressList addr;
   scoped_ptr<HostResolver> resolver(
       CreateSystemHostResolver(HostResolver::kDefaultParallelism,
+                               HostResolver::kDefaultRetryAttempts,
                                NULL));
   HostResolver::RequestInfo info(HostPortPair("localhost", listen_port_));
   int rv = resolver->Resolve(info, &addr, NULL, NULL, BoundNetLog());
diff --git a/net/test/test_server.cc b/net/test/test_server.cc
index bd24f73..aa704a8 100644
--- a/net/test/test_server.cc
+++ b/net/test/test_server.cc
@@ -192,7 +192,9 @@ bool TestServer::GetAddressList(AddressList* address_list) const {
   DCHECK(address_list);
 
   scoped_ptr<HostResolver> resolver(
-      CreateSystemHostResolver(HostResolver::kDefaultParallelism, NULL));
+      CreateSystemHostResolver(HostResolver::kDefaultParallelism,
+                               HostResolver::kDefaultRetryAttempts,
+                               NULL));
   HostResolver::RequestInfo info(host_port_pair_);
   int rv = resolver->Resolve(info, address_list, NULL, NULL, BoundNetLog());
   if (rv != net::OK) {
diff --git a/net/tools/fetch/fetch_client.cc b/net/tools/fetch/fetch_client.cc
index 2991ef8..4556080 100644
--- a/net/tools/fetch/fetch_client.cc
+++ b/net/tools/fetch/fetch_client.cc
@@ -139,6 +139,7 @@ int main(int argc, char** argv) {
 
   scoped_ptr<net::HostResolver> host_resolver(
       net::CreateSystemHostResolver(net::HostResolver::kDefaultParallelism,
+                                    net::HostResolver::kDefaultRetryAttempts,
                                     NULL));
 
   scoped_ptr<net::CertVerifier> cert_verifier(new net::CertVerifier);
diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc
index fff1d8b..c9e2fe4 100644
--- a/net/url_request/url_request_test_util.cc
+++ b/net/url_request/url_request_test_util.cc
@@ -16,6 +16,7 @@ TestURLRequestContext::TestURLRequestContext()
     : ALLOW_THIS_IN_INITIALIZER_LIST(context_storage_(this)) {
   context_storage_.set_host_resolver(
       net::CreateSystemHostResolver(net::HostResolver::kDefaultParallelism,
+                                    net::HostResolver::kDefaultRetryAttempts,
                                     NULL));
   context_storage_.set_proxy_service(net::ProxyService::CreateDirect());
   Init();
@@ -25,6 +26,7 @@ TestURLRequestContext::TestURLRequestContext(const std::string& proxy)
     : ALLOW_THIS_IN_INITIALIZER_LIST(context_storage_(this)) {
   context_storage_.set_host_resolver(
       net::CreateSystemHostResolver(net::HostResolver::kDefaultParallelism,
+                                    net::HostResolver::kDefaultRetryAttempts,
                                     NULL));
   net::ProxyConfig proxy_config;
   proxy_config.proxy_rules().ParseFromString(proxy);