BackoffEntry: Add the option to use a delay between requests

before backoff starts due to failures, for use with captive
portal code.

R=joi@chroimium.org
BUG=87100, 115487

Review URL: http://codereview.chromium.org/10173005

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

net/base/backoff_entry.cc

@@ -47,8 +47,11 @@ void BackoffEntry::InformOfRequest(bool succeeded) {
     // those failures will not reset the release time, further
     // requests will then need to wait the delay caused by the 2
     // failures.
+    base::TimeDelta delay;
+    if (policy_->always_use_initial_delay)
+      delay = base::TimeDelta::FromMilliseconds(policy_->initial_delay_ms);
     exponential_backoff_release_time_ = std::max(
-        ImplGetTimeNow(), exponential_backoff_release_time_);
+        ImplGetTimeNow() + delay, exponential_backoff_release_time_);
   }
 }
 
@@ -56,6 +59,13 @@ bool BackoffEntry::ShouldRejectRequest() const {
   return exponential_backoff_release_time_ > ImplGetTimeNow();
 }
 
+base::TimeDelta BackoffEntry::GetTimeUntilRelease() const {
+  base::TimeTicks now = ImplGetTimeNow();
+  if (exponential_backoff_release_time_ <= now)
+    return base::TimeDelta();
+  return exponential_backoff_release_time_ - now;
+}
+
 base::TimeTicks BackoffEntry::GetReleaseTime() const {
   return exponential_backoff_release_time_;
 }
@@ -107,6 +117,12 @@ base::TimeTicks BackoffEntry::ImplGetTimeNow() const {
 base::TimeTicks BackoffEntry::CalculateReleaseTime() const {
   int effective_failure_count =
       std::max(0, failure_count_ - policy_->num_errors_to_ignore);
+
+  // If always_use_initial_delay is true, it's equivalent to
+  // the effective_failure_count always being one greater than when it's false.
+  if (policy_->always_use_initial_delay)
+    ++effective_failure_count;
+
   if (effective_failure_count == 0) {
     // Never reduce previously set release horizon, e.g. due to Retry-After
     // header.
@@ -116,7 +132,7 @@ base::TimeTicks BackoffEntry::CalculateReleaseTime() const {
   // The delay is calculated with this formula:
   // delay = initial_backoff * multiply_factor^(
   //     effective_failure_count - 1) * Uniform(1 - jitter_factor, 1]
-  double delay = policy_->initial_backoff_ms;
+  double delay = policy_->initial_delay_ms;
   delay *= pow(policy_->multiply_factor, effective_failure_count - 1);
   delay -= base::RandDouble() * policy_->jitter_factor * delay;
 

net/base/backoff_entry.h

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef NET_BASE_BACKOFF_ITEM_H_
-#define NET_BASE_BACKOFF_ITEM_H_
+#ifndef NET_BASE_BACKOFF_ENTRY_H_
+#define NET_BASE_BACKOFF_ENTRY_H_
 #pragma once
 
 #include "base/threading/non_thread_safe.h"
@@ -17,8 +17,7 @@ namespace net {
 //
 // This utility class knows nothing about network specifics; it is
 // intended for reuse in various networking scenarios.
-class NET_EXPORT_PRIVATE BackoffEntry
-    : NON_EXPORTED_BASE(public base::NonThreadSafe) {
+class NET_EXPORT BackoffEntry : NON_EXPORTED_BASE(public base::NonThreadSafe) {
  public:
   // The set of parameters that define a back-off policy.
   struct Policy {
@@ -26,8 +25,11 @@ class NET_EXPORT_PRIVATE BackoffEntry
     // exponential back-off rules.
     int num_errors_to_ignore;
 
-    // Initial delay for exponential back-off.
-    int initial_backoff_ms;
+    // Initial delay.  The interpretation of this value depends on
+    // always_use_initial_delay.  It's either how long we wait between
+    // requests before backoff starts, or how much we delay the first request
+    // after backoff starts.
+    int initial_delay_ms;
 
     // Factor by which the waiting time will be multiplied.
     double multiply_factor;
@@ -43,6 +45,15 @@ class NET_EXPORT_PRIVATE BackoffEntry
     // Time to keep an entry from being discarded even when it
     // has no significant state, -1 to never discard.
     int64 entry_lifetime_ms;
+
+    // If true, we always use a delay of initial_delay_ms, even before
+    // we've seen num_errors_to_ignore errors.  Otherwise, initial_delay_ms
+    // is the first delay once we start exponential backoff.
+    //
+    // So if we're ignoring 1 error, we'll see (N, N, Nm, Nm^2, ...) if true,
+    // and (0, 0, N, Nm, ...) when false, where N is initial_backoff_ms and
+    // m is multiply_factor, assuming we've already seen one success.
+    bool always_use_initial_delay;
   };
 
   // Lifetime of policy must enclose lifetime of BackoffEntry. The
@@ -62,13 +73,16 @@ class NET_EXPORT_PRIVATE BackoffEntry
   // state) will no longer reject requests.
   base::TimeTicks GetReleaseTime() const;
 
+  // Returns the time until a request can be sent.
+  base::TimeDelta GetTimeUntilRelease() const;
+
   // Causes this object reject requests until the specified absolute time.
   // This can be used to e.g. implement support for a Retry-After header.
   void SetCustomReleaseTime(const base::TimeTicks& release_time);
 
   // Returns true if this object has no significant state (i.e. you could
   // just as well start with a fresh BackoffEntry object), and hasn't
-  // had for Policy::entry_lifetime_ms_.
+  // had for Policy::entry_lifetime_ms.
   bool CanDiscard() const;
 
   // Resets this entry to a fresh (as if just constructed) state.
@@ -89,7 +103,7 @@ class NET_EXPORT_PRIVATE BackoffEntry
   // allowed to start sending requests again.
   base::TimeTicks exponential_backoff_release_time_;
 
-  // Counts request errors; reset on success.
+  // Counts request errors; decremented on success.
   int failure_count_;
 
   const Policy* const policy_;
@@ -99,4 +113,4 @@ class NET_EXPORT_PRIVATE BackoffEntry
 
 }  // namespace net
 
-#endif  // NET_BASE_BACKOFF_ITEM_H_
+#endif  // NET_BASE_BACKOFF_ENTRY_H_

net/base/backoff_entry_unittest.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -11,7 +11,7 @@ using base::TimeDelta;
 using base::TimeTicks;
 using net::BackoffEntry;
 
-BackoffEntry::Policy base_policy = { 0, 1000, 2.0, 0.0, 20000, 2000 };
+BackoffEntry::Policy base_policy = { 0, 1000, 2.0, 0.0, 20000, 2000, false };
 
 class TestBackoffEntry : public BackoffEntry {
  public:
@@ -42,9 +42,11 @@ class TestBackoffEntry : public BackoffEntry {
 TEST(BackoffEntryTest, BaseTest) {
   TestBackoffEntry entry(&base_policy);
   EXPECT_FALSE(entry.ShouldRejectRequest());
+  EXPECT_EQ(TimeDelta(), entry.GetTimeUntilRelease());
 
   entry.InformOfRequest(false);
   EXPECT_TRUE(entry.ShouldRejectRequest());
+  EXPECT_EQ(TimeDelta::FromMilliseconds(1000), entry.GetTimeUntilRelease());
 }
 
 TEST(BackoffEntryTest, CanDiscardNeverExpires) {
@@ -83,6 +85,26 @@ TEST(BackoffEntryTest, CanDiscard) {
   EXPECT_TRUE(entry.CanDiscard());
 }
 
+TEST(BackoffEntryTest, CanDiscardAlwaysDelay) {
+  BackoffEntry::Policy always_delay_policy = base_policy;
+  always_delay_policy.always_use_initial_delay = true;
+  always_delay_policy.entry_lifetime_ms = 0;
+
+  TestBackoffEntry entry(&always_delay_policy);
+
+  // Because lifetime is non-zero, we shouldn't be able to discard yet.
+  entry.set_now(entry.GetReleaseTime() + TimeDelta::FromMilliseconds(2000));
+  EXPECT_TRUE(entry.CanDiscard());
+
+  // Even with no failures, we wait until the delay before we allow discard.
+  entry.InformOfRequest(true);
+  EXPECT_FALSE(entry.CanDiscard());
+
+  // Wait until the delay expires, and we can discard the entry again.
+  entry.set_now(entry.GetReleaseTime() + TimeDelta::FromMilliseconds(1000));
+  EXPECT_TRUE(entry.CanDiscard());
+}
+
 TEST(BackoffEntryTest, CanDiscardNotStored) {
   BackoffEntry::Policy no_store_policy = base_policy;
   no_store_policy.entry_lifetime_ms = 0;
@@ -95,10 +117,13 @@ TEST(BackoffEntryTest, ShouldIgnoreFirstTwo) {
   lenient_policy.num_errors_to_ignore = 2;
 
   BackoffEntry entry(&lenient_policy);
+
   entry.InformOfRequest(false);
   EXPECT_FALSE(entry.ShouldRejectRequest());
+
   entry.InformOfRequest(false);
   EXPECT_FALSE(entry.ShouldRejectRequest());
+
   entry.InformOfRequest(false);
   EXPECT_TRUE(entry.ShouldRejectRequest());
 }
@@ -114,16 +139,19 @@ TEST(BackoffEntryTest, ReleaseTimeCalculation) {
   entry.InformOfRequest(false);
   result = entry.GetReleaseTime();
   EXPECT_EQ(entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(1000), result);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(1000), entry.GetTimeUntilRelease());
 
   // 2 errors.
   entry.InformOfRequest(false);
   result = entry.GetReleaseTime();
   EXPECT_EQ(entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(2000), result);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(2000), entry.GetTimeUntilRelease());
 
   // 3 errors.
   entry.InformOfRequest(false);
   result = entry.GetReleaseTime();
   EXPECT_EQ(entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(4000), result);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(4000), entry.GetTimeUntilRelease());
 
   // 6 errors (to check it doesn't pass maximum).
   entry.InformOfRequest(false);
@@ -134,6 +162,42 @@ TEST(BackoffEntryTest, ReleaseTimeCalculation) {
       entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(20000), result);
 }
 
+TEST(BackoffEntryTest, ReleaseTimeCalculationAlwaysDelay) {
+  BackoffEntry::Policy always_delay_policy = base_policy;
+  always_delay_policy.always_use_initial_delay = true;
+  always_delay_policy.num_errors_to_ignore = 2;
+
+  TestBackoffEntry entry(&always_delay_policy);
+
+  // With previous requests, should return "now".
+  TimeTicks result = entry.GetReleaseTime();
+  EXPECT_EQ(TimeDelta(), entry.GetTimeUntilRelease());
+
+  // 1 error.
+  entry.InformOfRequest(false);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(1000), entry.GetTimeUntilRelease());
+
+  // 2 errors.
+  entry.InformOfRequest(false);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(1000), entry.GetTimeUntilRelease());
+
+  // 3 errors, exponential backoff starts.
+  entry.InformOfRequest(false);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(2000), entry.GetTimeUntilRelease());
+
+  // 4 errors.
+  entry.InformOfRequest(false);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(4000), entry.GetTimeUntilRelease());
+
+  // 8 errors (to check it doesn't pass maximum).
+  entry.InformOfRequest(false);
+  entry.InformOfRequest(false);
+  entry.InformOfRequest(false);
+  entry.InformOfRequest(false);
+  result = entry.GetReleaseTime();
+  EXPECT_EQ(TimeDelta::FromMilliseconds(20000), entry.GetTimeUntilRelease());
+}
+
 TEST(BackoffEntryTest, ReleaseTimeCalculationWithJitter) {
   for (int i = 0; i < 10; ++i) {
     BackoffEntry::Policy jittery_policy = base_policy;
@@ -172,6 +236,32 @@ TEST(BackoffEntryTest, FailureThenSuccess) {
             entry.GetReleaseTime());
 }
 
+TEST(BackoffEntryTest, FailureThenSuccessAlwaysDelay) {
+  BackoffEntry::Policy always_delay_policy = base_policy;
+  always_delay_policy.always_use_initial_delay = true;
+  always_delay_policy.num_errors_to_ignore = 1;
+
+  TestBackoffEntry entry(&always_delay_policy);
+
+  // Failure count 1.
+  entry.InformOfRequest(false);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(1000), entry.GetTimeUntilRelease());
+
+  // Failure count 2.
+  entry.InformOfRequest(false);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(2000), entry.GetTimeUntilRelease());
+  entry.set_now(entry.GetReleaseTime() + TimeDelta::FromMilliseconds(2000));
+
+  // Success.  We should go back to the original delay.
+  entry.InformOfRequest(true);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(1000), entry.GetTimeUntilRelease());
+
+  // Failure count reaches 2 again.  We should increase the delay once more.
+  entry.InformOfRequest(false);
+  EXPECT_EQ(TimeDelta::FromMilliseconds(2000), entry.GetTimeUntilRelease());
+  entry.set_now(entry.GetReleaseTime() + TimeDelta::FromMilliseconds(2000));
+}
+
 TEST(BackoffEntryTest, RetainCustomHorizon) {
   TestBackoffEntry custom(&base_policy);
   TimeTicks custom_horizon = TimeTicks() + TimeDelta::FromDays(3);

net/url_request/url_request_throttler_entry.cc

@@ -39,7 +39,7 @@ const int URLRequestThrottlerEntry::kDefaultMaxSendThreshold = 20;
 // avoid false positives.  It should help avoid back-off from kicking in e.g.
 // on flaky connections.
 const int URLRequestThrottlerEntry::kDefaultNumErrorsToIgnore = 2;
-const int URLRequestThrottlerEntry::kDefaultInitialBackoffMs = 700;
+const int URLRequestThrottlerEntry::kDefaultInitialDelayMs = 700;
 const double URLRequestThrottlerEntry::kDefaultMultiplyFactor = 1.4;
 const double URLRequestThrottlerEntry::kDefaultJitterFactor = 0.4;
 const int URLRequestThrottlerEntry::kDefaultMaximumBackoffMs = 15 * 60 * 1000;
@@ -116,12 +116,13 @@ URLRequestThrottlerEntry::URLRequestThrottlerEntry(
   DCHECK(manager_);
 
   Initialize();
-  backoff_policy_.initial_backoff_ms = initial_backoff_ms;
+  backoff_policy_.initial_delay_ms = initial_backoff_ms;
   backoff_policy_.multiply_factor = multiply_factor;
   backoff_policy_.jitter_factor = jitter_factor;
   backoff_policy_.maximum_backoff_ms = maximum_backoff_ms;
   backoff_policy_.entry_lifetime_ms = -1;
   backoff_policy_.num_errors_to_ignore = 0;
+  backoff_policy_.always_use_initial_delay = false;
 }
 
 bool URLRequestThrottlerEntry::IsEntryOutdated() const {
@@ -163,8 +164,7 @@ bool URLRequestThrottlerEntry::ShouldRejectRequest(int load_flags) const {
       GetBackoffEntry()->ShouldRejectRequest()) {
     int num_failures = GetBackoffEntry()->failure_count();
     int release_after_ms =
-        (GetBackoffEntry()->GetReleaseTime() - base::TimeTicks::Now())
-            .InMilliseconds();
+        GetBackoffEntry()->GetTimeUntilRelease().InMilliseconds();
 
     net_log_.AddEvent(
         NetLog::TYPE_THROTTLING_REJECTED_REQUEST,
@@ -271,11 +271,12 @@ URLRequestThrottlerEntry::~URLRequestThrottlerEntry() {
 void URLRequestThrottlerEntry::Initialize() {
   sliding_window_release_time_ = base::TimeTicks::Now();
   backoff_policy_.num_errors_to_ignore = kDefaultNumErrorsToIgnore;
-  backoff_policy_.initial_backoff_ms = kDefaultInitialBackoffMs;
+  backoff_policy_.initial_delay_ms = kDefaultInitialDelayMs;
   backoff_policy_.multiply_factor = kDefaultMultiplyFactor;
   backoff_policy_.jitter_factor = kDefaultJitterFactor;
   backoff_policy_.maximum_backoff_ms = kDefaultMaximumBackoffMs;
   backoff_policy_.entry_lifetime_ms = kDefaultEntryLifetimeMs;
+  backoff_policy_.always_use_initial_delay = false;
 
   // We pretend we just had a successful response so that we have a
   // starting point to our tracking. This is called from the

net/url_request/url_request_throttler_entry.h

@@ -43,7 +43,7 @@ class NET_EXPORT URLRequestThrottlerEntry
   static const int kDefaultNumErrorsToIgnore;
 
   // Initial delay for exponential back-off.
-  static const int kDefaultInitialBackoffMs;
+  static const int kDefaultInitialDelayMs;
 
   // Factor by which the waiting time will be multiplied.
   static const double kDefaultMultiplyFactor;

remoting/host/chromoting_host.cc

@@ -54,6 +54,9 @@ const net::BackoffEntry::Policy kDefaultBackoffPolicy = {
   // Time to keep an entry from being discarded even when it
   // has no significant state, -1 to never discard.
   -1,
+
+  // Don't use initial delay unless the last request was an error.
+  false,
 };
 
 }  // namespace