Cleanup RateCounter

Previously RateCounter was using base::Time instead of base::TimeTicks,
which may cause it to generate incorrect values in some cases. Fxed it
to use TimeTicks. Also updated it to use TickClock interface for tests.

Review-Url: https://codereview.chromium.org/2561173004
Cr-Commit-Position: refs/heads/master@{#437712}

remoting/base/rate_counter.cc

@@ -9,40 +9,31 @@
 namespace remoting {
 
 RateCounter::RateCounter(base::TimeDelta time_window)
-    : time_window_(time_window),
-      sum_(0) {
-  DCHECK_GT(time_window.InMilliseconds(), 0);
+    : time_window_(time_window), sum_(0) {
+  DCHECK_GT(time_window, base::TimeDelta());
 }
 
-RateCounter::~RateCounter() {
-}
+RateCounter::~RateCounter() {}
 
 void RateCounter::Record(int64_t value) {
   DCHECK(CalledOnValidThread());
 
-  base::Time current_time = CurrentTime();
-  EvictOldDataPoints(current_time);
+  base::TimeTicks now = tick_clock_->NowTicks();
+  EvictOldDataPoints(now);
   sum_ += value;
-  data_points_.push(std::make_pair(current_time, value));
+  data_points_.push(std::make_pair(now, value));
 }
 
 double RateCounter::Rate() {
   DCHECK(CalledOnValidThread());
 
-  EvictOldDataPoints(CurrentTime());
+  EvictOldDataPoints(tick_clock_->NowTicks());
   return sum_ / time_window_.InSecondsF();
 }
 
-void RateCounter::SetCurrentTimeForTest(base::Time current_time) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(current_time >= current_time_for_test_);
-
-  current_time_for_test_ = current_time;
-}
-
-void RateCounter::EvictOldDataPoints(base::Time current_time) {
+void RateCounter::EvictOldDataPoints(base::TimeTicks now) {
   // Remove data points outside of the window.
-  base::Time window_start = current_time - time_window_;
+  base::TimeTicks window_start = now - time_window_;
 
   while (!data_points_.empty()) {
     if (data_points_.front().first > window_start)
@@ -53,10 +44,4 @@ void RateCounter::EvictOldDataPoints(base::Time current_time) {
   }
 }
 
-base::Time RateCounter::CurrentTime() const {
-  if (current_time_for_test_ == base::Time())
-    return base::Time::Now();
-  return current_time_for_test_;
-}
-
 }  // namespace remoting

remoting/base/rate_counter.h

@@ -12,6 +12,8 @@
 
 #include "base/macros.h"
 #include "base/threading/non_thread_safe.h"
+#include "base/time/default_tick_clock.h"
+#include "base/time/tick_clock.h"
 #include "base/time/time.h"
 
 namespace remoting {
@@ -32,18 +34,16 @@ class RateCounter : public base::NonThreadSafe {
   // Note that rates reported before |time_window| has elapsed are not accurate.
   double Rate();
 
-  // Overrides the current time for testing.
-  void SetCurrentTimeForTest(base::Time current_time);
+  void set_tick_clock_for_tests(base::TickClock* tick_clock) {
+    tick_clock_ = tick_clock;
+  }
 
  private:
   // Type used to store data points with timestamps.
-  typedef std::pair<base::Time, int64_t> DataPoint;
+  typedef std::pair<base::TimeTicks, int64_t> DataPoint;
 
   // Removes data points more than |time_window| older than |current_time|.
-  void EvictOldDataPoints(base::Time current_time);
-
-  // Returns the current time specified for test, if set, or base::Time::Now().
-  base::Time CurrentTime() const;
+  void EvictOldDataPoints(base::TimeTicks current_time);
 
   // Time window over which to calculate the rate.
   const base::TimeDelta time_window_;
@@ -54,8 +54,8 @@ class RateCounter : public base::NonThreadSafe {
   // Sum of values in |data_points_|.
   int64_t sum_;
 
-  // If set, used to calculate the running average, in place of Now().
-  base::Time current_time_for_test_;
+  base::DefaultTickClock default_tick_clock_;
+  base::TickClock* tick_clock_ = &default_tick_clock_;
 
   DISALLOW_COPY_AND_ASSIGN(RateCounter);
 };

remoting/base/rate_counter_unittest.cc

@@ -6,6 +6,7 @@
 #include <stdint.h>
 
 #include "base/macros.h"
+#include "base/test/simple_test_tick_clock.h"
 #include "remoting/base/rate_counter.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -18,10 +19,11 @@ TEST(RateCounterTest, OneSecondWindow) {
   RateCounter rate_counter(base::TimeDelta::FromSeconds(1));
   EXPECT_EQ(0, rate_counter.Rate());
 
-  base::Time now = base::Time::Now();
+  base::SimpleTestTickClock tick_clock;
+  rate_counter.set_tick_clock_for_tests(&tick_clock);
+
   for (size_t i = 0; i < arraysize(kTestValues); ++i) {
-    now += base::TimeDelta::FromSeconds(1);
-    rate_counter.SetCurrentTimeForTest(now);
+    tick_clock.Advance(base::TimeDelta::FromSeconds(1));
     rate_counter.Record(kTestValues[i]);
     EXPECT_EQ(static_cast<double>(kTestValues[i]), rate_counter.Rate());
   }
@@ -32,7 +34,8 @@ TEST(RateCounterTest, OneSecondWindowAllSamples) {
   RateCounter rate_counter(base::TimeDelta::FromSeconds(1));
   EXPECT_EQ(0, rate_counter.Rate());
 
-  rate_counter.SetCurrentTimeForTest(base::Time::Now());
+  base::SimpleTestTickClock tick_clock;
+  rate_counter.set_tick_clock_for_tests(&tick_clock);
 
   double expected = 0.0;
   for (size_t i = 0; i < arraysize(kTestValues); ++i) {
@@ -50,10 +53,11 @@ TEST(RateCounterTest, TwoSecondWindow) {
   RateCounter rate_counter(base::TimeDelta::FromSeconds(2));
   EXPECT_EQ(0, rate_counter.Rate());
 
-  base::Time now = base::Time::Now();
+  base::SimpleTestTickClock tick_clock;
+  rate_counter.set_tick_clock_for_tests(&tick_clock);
+
   for (size_t i = 0; i < arraysize(kTestValues); ++i) {
-    now += base::TimeDelta::FromSeconds(1);
-    rate_counter.SetCurrentTimeForTest(now);
+    tick_clock.Advance(base::TimeDelta::FromSeconds(1));
     rate_counter.Record(kTestValues[i]);
     double expected = kTestValues[i];
     if (i > 0)
@@ -71,11 +75,12 @@ TEST(RateCounterTest, LongWindow) {
   RateCounter rate_counter(base::TimeDelta::FromSeconds(kWindowSeconds));
   EXPECT_EQ(0, rate_counter.Rate());
 
+  base::SimpleTestTickClock tick_clock;
+  rate_counter.set_tick_clock_for_tests(&tick_clock);
+
   double expected = 0.0;
-  base::Time now = base::Time::Now();
   for (size_t i = 0; i < arraysize(kTestValues); ++i) {
-    now += base::TimeDelta::FromSeconds(1);
-    rate_counter.SetCurrentTimeForTest(now);
+    tick_clock.Advance(base::TimeDelta::FromSeconds(1));
     rate_counter.Record(kTestValues[i]);
     if (i != 0)
       expected += kTestValues[i];