Revert of Domain Reliability: Switch to BackoffEntry in Scheduler (patchset #4 id:60001 of https://codereview.chromium.org/704893002/)

Reason for revert:
Speculative revert to attempt to fix https://code.google.com/p/chromium/issues/detail?id=431608

Original issue's description:
> Domain Reliability: Switch to BackoffEntry in Scheduler
>
> Previously, Domain Reliability was using homegrown backoff handling code.
> Switch to using the existing BackoffEntry.
>
> BUG=
>
> Committed: https://crrev.com/dd6819b1522473ae11b98cb60b8b613af01f4b56
> Cr-Commit-Position: refs/heads/master@{#303296}

TBR=davidben@chromium.org,ttuttle@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.chromium.org/709333003

Cr-Commit-Position: refs/heads/master@{#303412}

5 files changed, 48 insertions, 74 deletions

diff --git a/components/domain_reliability/scheduler.cc b/components/domain_reliability/scheduler.cc
index fb5f857..636130c 100644
--- a/components/domain_reliability/scheduler.cc
+++ b/components/domain_reliability/scheduler.cc
@@ -24,11 +24,6 @@ const char* kMinimumUploadDelayFieldTrialName = "DomRel-MinimumUploadDelay";
 const char* kMaximumUploadDelayFieldTrialName = "DomRel-MaximumUploadDelay";
 const char* kUploadRetryIntervalFieldTrialName = "DomRel-UploadRetryInterval";
 
-// Fixed elements of backoff policy
-const double kMultiplyFactor = 2.0;
-const double kJitterFactor = 0.1;
-const int64 kMaximumBackoffMs = 60 * 1000 * 1000;
-
 unsigned GetUnsignedFieldTrialValueOrDefault(std::string field_trial_name,
                                              unsigned default_value) {
   if (!base::FieldTrialList::TrialExists(field_trial_name))
@@ -74,6 +69,7 @@ DomainReliabilityScheduler::DomainReliabilityScheduler(
     const Params& params,
     const ScheduleUploadCallback& callback)
     : time_(time),
+      collectors_(num_collectors),
       params_(params),
       callback_(callback),
       upload_pending_(false),
@@ -81,19 +77,6 @@ DomainReliabilityScheduler::DomainReliabilityScheduler(
       upload_running_(false),
       collector_index_(kInvalidCollectorIndex),
       last_upload_finished_(false) {
-  backoff_policy_.num_errors_to_ignore = 0;
-  backoff_policy_.initial_delay_ms =
-      params.upload_retry_interval.InMilliseconds();
-  backoff_policy_.multiply_factor = kMultiplyFactor;
-  backoff_policy_.jitter_factor = kJitterFactor;
-  backoff_policy_.maximum_backoff_ms = kMaximumBackoffMs;
-  backoff_policy_.entry_lifetime_ms = 0;
-  backoff_policy_.always_use_initial_delay = false;
-
-  for (size_t i = 0; i < num_collectors; ++i) {
-    collectors_.push_back(
-      new MockableTimeBackoffEntry(&backoff_policy_, time_));
-  }
 }
 
 DomainReliabilityScheduler::~DomainReliabilityScheduler() {}
@@ -133,21 +116,31 @@ void DomainReliabilityScheduler::OnUploadComplete(bool success) {
   VLOG(1) << "Upload to collector " << collector_index_
           << (success ? " succeeded." : " failed.");
 
-  net::BackoffEntry* backoff = collectors_[collector_index_];
+  CollectorState* collector = &collectors_[collector_index_];
   collector_index_ = kInvalidCollectorIndex;
-  backoff->InformOfRequest(success);
 
-  if (!success) {
+  if (success) {
+    collector->failures = 0;
+  } else {
     // Restore upload_pending_ and first_beacon_time_ to pre-upload state,
     // since upload failed.
     upload_pending_ = true;
     first_beacon_time_ = old_first_beacon_time_;
+
+    ++collector->failures;
   }
 
-  last_upload_end_time_ = time_->NowTicks();
+  base::TimeTicks now = time_->NowTicks();
+  base::TimeDelta retry_interval = GetUploadRetryInterval(collector->failures);
+  collector->next_upload = now + retry_interval;
+
+  last_upload_end_time_ = now;
   last_upload_success_ = success;
   last_upload_finished_ = true;
 
+  VLOG(1) << "Next upload to collector at least "
+          << retry_interval.InSeconds() << " seconds from now.";
+
   MaybeScheduleUpload();
 }
 
@@ -177,11 +170,10 @@ base::Value* DomainReliabilityScheduler::GetWebUIData() const {
 
   base::ListValue* collectors = new base::ListValue();
   for (size_t i = 0; i < collectors_.size(); ++i) {
-    const net::BackoffEntry* backoff = collectors_[i];
+    const CollectorState* state = &collectors_[i];
     base::DictionaryValue* value = new base::DictionaryValue();
-    value->SetInteger("failures", backoff->failure_count());
-    value->SetInteger("next_upload",
-        (backoff->GetReleaseTime() - now).InSeconds());
+    value->SetInteger("failures", state->failures);
+    value->SetInteger("next_upload", (state->next_upload - now).InSeconds());
     collectors->Append(value);
   }
   data->Set("collectors", collectors);
@@ -189,9 +181,7 @@ base::Value* DomainReliabilityScheduler::GetWebUIData() const {
   return data;
 }
 
-void DomainReliabilityScheduler::MakeDeterministicForTesting() {
-  backoff_policy_.jitter_factor = 0.0;
-}
+DomainReliabilityScheduler::CollectorState::CollectorState() : failures(0) {}
 
 void DomainReliabilityScheduler::MaybeScheduleUpload() {
   if (!upload_pending_ || upload_scheduled_ || upload_running_)
@@ -238,18 +228,16 @@ void DomainReliabilityScheduler::GetNextUploadTimeAndCollector(
   size_t min_index = kInvalidCollectorIndex;
 
   for (size_t i = 0; i < collectors_.size(); ++i) {
-    net::BackoffEntry* backoff = collectors_[i];
+    CollectorState* collector = &collectors_[i];
     // If a collector is usable, use the first one in the list.
-    if (!backoff->ShouldRejectRequest()) {
+    if (collector->failures == 0 || collector->next_upload <= now) {
       min_time = now;
       min_index = i;
       break;
-    }
-
     // If not, keep track of which will be usable soonest:
-    base::TimeTicks time = backoff->GetReleaseTime();
-    if (min_index == kInvalidCollectorIndex || time < min_time) {
-      min_time = time;
+    } else if (min_index == kInvalidCollectorIndex ||
+        collector->next_upload < min_time) {
+      min_time = collector->next_upload;
       min_index = i;
     }
   }
@@ -259,4 +247,16 @@ void DomainReliabilityScheduler::GetNextUploadTimeAndCollector(
   *collector_index_out = min_index;
 }
 
+base::TimeDelta DomainReliabilityScheduler::GetUploadRetryInterval(
+    unsigned failures) {
+  if (failures == 0)
+    return base::TimeDelta::FromSeconds(0);
+  else {
+    // Don't back off more than 64x the original delay.
+    if (failures > 7)
+      failures = 7;
+    return params_.upload_retry_interval * (1 << (failures - 1));
+  }
+}
+
 }  // namespace domain_reliability
diff --git a/components/domain_reliability/scheduler.h b/components/domain_reliability/scheduler.h
index 5d387c9..95b2fbf 100644
--- a/components/domain_reliability/scheduler.h
+++ b/components/domain_reliability/scheduler.h
@@ -8,10 +8,8 @@
 #include <vector>
 
 #include "base/callback.h"
-#include "base/memory/scoped_vector.h"
 #include "base/time/time.h"
 #include "components/domain_reliability/domain_reliability_export.h"
-#include "net/base/backoff_entry.h"
 
 namespace base {
 class Value;
@@ -73,22 +71,28 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityScheduler {
 
   base::Value* GetWebUIData() const;
 
-  // Disables jitter in BackoffEntries to make scheduling deterministic for
-  // unit tests.
-  void MakeDeterministicForTesting();
-
  private:
+  struct CollectorState {
+    CollectorState();
+
+    // The number of consecutive failures to upload to this collector, or 0 if
+    // the most recent upload succeeded.
+    unsigned failures;
+    base::TimeTicks next_upload;
+  };
+
   void MaybeScheduleUpload();
 
   void GetNextUploadTimeAndCollector(base::TimeTicks now,
                                      base::TimeTicks* upload_time_out,
                                      size_t* collector_index_out);
 
+  base::TimeDelta GetUploadRetryInterval(unsigned failures);
+
   MockableTime* time_;
+  std::vector<CollectorState> collectors_;
   Params params_;
   ScheduleUploadCallback callback_;
-  net::BackoffEntry::Policy backoff_policy_;
-  ScopedVector<net::BackoffEntry> collectors_;
 
   // Whether there are beacons that have not yet been uploaded. Set when a
   // beacon arrives or an upload fails, and cleared when an upload starts.
diff --git a/components/domain_reliability/scheduler_unittest.cc b/components/domain_reliability/scheduler_unittest.cc
index 80fbedb..16ae639 100644
--- a/components/domain_reliability/scheduler_unittest.cc
+++ b/components/domain_reliability/scheduler_unittest.cc
@@ -35,7 +35,6 @@ class DomainReliabilitySchedulerTest : public testing::Test {
         params_,
         base::Bind(&DomainReliabilitySchedulerTest::ScheduleUploadCallback,
                    base::Unretained(this))));
-    scheduler_->MakeDeterministicForTesting();
   }
 
   ::testing::AssertionResult CheckNoPendingUpload() {
diff --git a/components/domain_reliability/util.cc b/components/domain_reliability/util.cc
index 6fc50c5..68f6db6 100644
--- a/components/domain_reliability/util.cc
+++ b/components/domain_reliability/util.cc
@@ -136,17 +136,4 @@ scoped_ptr<MockableTime::Timer> ActualTime::CreateTimer() {
   return scoped_ptr<MockableTime::Timer>(new ActualTimer());
 }
 
-MockableTimeBackoffEntry::MockableTimeBackoffEntry(
-    const net::BackoffEntry::Policy* const policy,
-    MockableTime* time)
-    : net::BackoffEntry(policy),
-      time_(time) {
-}
-
-MockableTimeBackoffEntry::~MockableTimeBackoffEntry() {}
-
-base::TimeTicks MockableTimeBackoffEntry::ImplGetTimeNow() const {
-  return time_->NowTicks();
-}
-
 }  // namespace domain_reliability
diff --git a/components/domain_reliability/util.h b/components/domain_reliability/util.h
index 56ca229..80699d4 100644
--- a/components/domain_reliability/util.h
+++ b/components/domain_reliability/util.h
@@ -13,7 +13,6 @@
 #include "base/time/time.h"
 #include "base/tracked_objects.h"
 #include "components/domain_reliability/domain_reliability_export.h"
-#include "net/base/backoff_entry.h"
 #include "net/http/http_response_info.h"
 
 namespace domain_reliability {
@@ -83,21 +82,6 @@ class DOMAIN_RELIABILITY_EXPORT ActualTime : public MockableTime {
   scoped_ptr<MockableTime::Timer> CreateTimer() override;
 };
 
-// A subclass of BackoffEntry that uses a MockableTime to keep track of time.
-class MockableTimeBackoffEntry : public net::BackoffEntry {
- public:
-  MockableTimeBackoffEntry(const net::BackoffEntry::Policy* const policy,
-                           MockableTime* time);
-
-  virtual ~MockableTimeBackoffEntry();
-
- protected:
-  virtual base::TimeTicks ImplGetTimeNow() const override;
-
- private:
-  MockableTime* time_;
-};
-
 }  // namespace domain_reliability
 
 #endif  // COMPONENTS_DOMAIN_RELIABILITY_UTIL_H_