Store network quality samples so we can compute percentiles.

net/base/network_quality_estimator.h

Index: net/base/network_quality_estimator.h
diff --git a/net/base/network_quality_estimator.h b/net/base/network_quality_estimator.h
index ecfa08af6b8341b3a9bb06897a89baa42651c7b7..bc77bd60707218d4d835a7343717ae7677dadd88 100644
--- a/net/base/network_quality_estimator.h
+++ b/net/base/network_quality_estimator.h
@@ -7,6 +7,8 @@
 
 #include <stdint.h>
 
+#include <deque>
+
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/threading/thread_checker.h"
@@ -33,21 +35,58 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator
 
   ~NetworkQualityEstimator() override;
 
-  // Returns an estimate of the current network quality.
-  NetworkQuality GetEstimate() const;
+  // Returns the peak estimates (fastest RTT and peak throughput) of the
+  // current network.
+  NetworkQuality GetPeakEstimate() const;
 
   // Notifies NetworkQualityEstimator that a response has been received.
-  // |prefilter_bytes_read| is the count of the bytes received prior to
-  // applying filters (e.g. decompression, SDCH) from request creation time
+  // |cummulative_prefilter_bytes_read| is the count of the bytes received prior
+  // to applying filters (e.g. decompression, SDCH) from request creation time
   // until now.
+  // |prefiltered_bytes_read| is the count of the bytes received prior
+  // to applying filters in the most recent read.
   void NotifyDataReceived(const URLRequest& request,
-                          int64_t prefilter_bytes_read);
+                          int64_t cummulative_prefilter_bytes_read,
+                          int64_t prefiltered_bytes_read);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest,
                            TestPeakKbpsFastestRTTUpdates);
   FRIEND_TEST_ALL_PREFIXES(URLRequestTestHTTP, NetworkQualityEstimator);
 
+  // Sample is used to store RTT and Kbps samples in buffers. A sample has a
+  // value and a timestamp (when the sample was taken).
+  class Sample {
+   public:
+    explicit Sample(int value);

[mmenke, 2015/06/03 18:31:40]

Suggest taking the time here, to make things a but more explicit.  Also remember
someone saying something about possibly loading state from disk.

[tbansal1, 2015/06/05 01:50:08]

Done.

+
+    virtual ~Sample();

[mmenke, 2015/06/03 18:31:39]

--virtual

[tbansal1, 2015/06/05 01:50:08]

Done.

+
+    // Value of the sample.
+    const int value_;

[mmenke, 2015/06/03 18:31:39]

Classes aren't allowed to have public member variables.  This should either be a
struct, or these should be private with public accessors.

Since they're in an stl container, and you need them to be copiable, I'd go with
structs.

[tbansal1, 2015/06/05 01:50:08]

Done.

+
+    // Time when the sample was taken.
+    const base::TimeTicks timestamp_;
+  };
+
+  // Ring Buffer is used to store samples sorted by time.
+  class RingBuffer {

[mmenke, 2015/06/03 18:31:39]

Think it's more important to name classes after what they contain, rather than
the data format they use to do it, which is more of an internal implementation
detail.

Maybe just SampleBuffer, SampleCache, SampleSet, or even SampleRingBuffer, if
you really think that's important.

[tbansal1, 2015/06/05 01:50:08]

Done.

[bengr, 2015/06/05 20:44:13]

I like SampleBuffer better. Also, can we use the word "Observation" instead of
"Sample" everywhere?

[tbansal1, 2015/06/08 20:27:51]

Done.

+   public:
+    RingBuffer();
+
+    virtual ~RingBuffer();

[mmenke, 2015/06/03 18:31:39]

--virtual

[tbansal1, 2015/06/05 01:50:08]

Done.

+
+    void AddSample(int value);
+
+   private:
+    // Maximum number of samples to hold in the RingBuffer.
+    const uint32_t kMaximumSamples = 300;

[mmenke, 2015/06/03 18:31:40]

static?

[tbansal1, 2015/06/05 01:50:08]

Done.

+
+    // Buffer that holds samples sorted by time.
+    // Oldest sample is at the front of the queue.
+    std::deque<Sample> samples_;
+  };
+
   // Tiny transfer sizes may give inaccurate throughput results.
   // Minimum size of the transfer over which the throughput is computed.
   static const int kMinTransferSizeInBytes = 10000;
@@ -93,6 +132,12 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator
   // 2) The transfer time includes at least one RTT while no bytes are read.
   uint64_t peak_kbps_since_last_connection_change_;
 
+  // Buffer that holds Kbps samples.
+  RingBuffer kbps_;

[mmenke, 2015/06/03 18:31:39]

Maybe kbps_samples_, and same for the next one?

[tbansal1, 2015/06/05 01:50:08]

Done.

+
+  // Buffer that holds RTT (in milliseconds) samples.
+  RingBuffer rtt_msec_;
+
   base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkQualityEstimator);