NQE: Cleanup observation buffer code

net/nqe/observation_buffer.h

Index: net/nqe/observation_buffer.h
diff --git a/net/nqe/observation_buffer.h b/net/nqe/observation_buffer.h
index cc3f1d86dba8cda4d4313df5c38bf1ea69fc7b9a..587cca22d7239d2c5a8960256005d1b0521c23e8 100644
--- a/net/nqe/observation_buffer.h
+++ b/net/nqe/observation_buffer.h
@@ -9,12 +9,14 @@
 
 #include <algorithm>
 #include <deque>
+#include <memory>
 #include <vector>
 
-#include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/time/default_tick_clock.h"

[RyanSturm, 2016/09/26 18:44:55]

This pulls in base/time/tick_clock.h, but you should probably include it as well
due to the unique_ptr declared below.

[tbansal1, 2016/09/28 16:42:31]

Done.

 #include "base/time/time.h"
 #include "net/base/net_export.h"
+#include "net/nqe/network_quality_observation.h"

[RyanSturm, 2016/09/26 18:44:55]

Is network_quality_observation.h needed?

[tbansal1, 2016/09/28 16:42:31]

Yes. See Line 174:
std::deque<Observation<ValueType>> observations_;

 #include "net/nqe/network_quality_observation_source.h"
 #include "net/nqe/weighted_observation.h"
 
@@ -29,7 +31,8 @@ template <typename ValueType>
 class NET_EXPORT_PRIVATE ObservationBuffer {
  public:
   explicit ObservationBuffer(double weight_multiplier_per_second)
-      : weight_multiplier_per_second_(weight_multiplier_per_second) {
+      : weight_multiplier_per_second_(weight_multiplier_per_second),
+        tick_clock_(new base::DefaultTickClock()) {
     static_assert(kMaximumObservationsBufferSize > 0U,
                   "Minimum size of observation buffer must be > 0");
     DCHECK_GE(weight_multiplier_per_second_, 0.0);
@@ -116,6 +119,10 @@ class NET_EXPORT_PRIVATE ObservationBuffer {
     return true;
   }
 
+  void SetTickClockForTesting(std::unique_ptr<base::TickClock> tick_clock) {

[RyanSturm, 2016/09/26 18:44:55]

nit: Maybe add tick_clock as a parameter to the constructor. I prefer to avoid
ForTesting methods when possible.

[tbansal1, 2016/09/28 16:42:31]

Seems like net tends to use ForTesting() rather than passing it in constructor:
https://cs.chromium.org/search/?q=f:net+clock.*testing+-f:nqe&sq=package:chro...

+    tick_clock_ = std::move(tick_clock);
+  }
+
  private:
   // Maximum number of observations that can be held in the ObservationBuffer.
   static const size_t kMaximumObservationsBufferSize = 300;
@@ -136,7 +143,7 @@ class NET_EXPORT_PRIVATE ObservationBuffer {
 
     weighted_observations.clear();
     double total_weight_observations = 0.0;
-    base::TimeTicks now = base::TimeTicks::Now();
+    base::TimeTicks now = tick_clock_->NowTicks();
 
     for (const auto& observation : observations_) {
       if (observation.timestamp < begin_timestamp)
@@ -174,6 +181,8 @@ class NET_EXPORT_PRIVATE ObservationBuffer {
   //     weight_multiplier_per_second_ ^ kHalfLifeSeconds = 0.5
   const double weight_multiplier_per_second_;
 
+  std::unique_ptr<base::TickClock> tick_clock_;
+
   DISALLOW_COPY_AND_ASSIGN(ObservationBuffer);
 };