Store network quality samples so we can compute percentiles.

net/base/network_quality_estimator.cc

Index: net/base/network_quality_estimator.cc
diff --git a/net/base/network_quality_estimator.cc b/net/base/network_quality_estimator.cc
index e012a99d72fdcc752cf4e9406f8e61651315598a..10c0ed153fea6bd4529bccb6651d6581f1480b89 100644
--- a/net/base/network_quality_estimator.cc
+++ b/net/base/network_quality_estimator.cc
@@ -6,12 +6,21 @@
 
 #include <string>
 
+#include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "net/base/net_util.h"
 #include "net/base/network_quality.h"
 #include "net/url_request/url_request.h"
 #include "url/gurl.h"
 
+namespace {
+
+// Maximum number of samples to hold in the buffer for network quality
+// estimation.
+const uint32_t kMaximumSamples = 300;
+
+}  // namespace
+
 namespace net {
 
 NetworkQualityEstimator::NetworkQualityEstimator()
@@ -27,6 +36,8 @@ NetworkQualityEstimator::NetworkQualityEstimator(
       peak_kbps_since_last_connection_change_(0) {
   static_assert(kMinRequestDurationMicroseconds > 0,
                 "Minimum request duration must be > 0");
+  static_assert(kMaximumSamples > 0,
+                "Buffer size for storing network quality samples must be > 0");
   NetworkChangeNotifier::AddConnectionTypeObserver(this);
 }
 
@@ -35,18 +46,23 @@ NetworkQualityEstimator::~NetworkQualityEstimator() {
   NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
 }
 
-void NetworkQualityEstimator::NotifyDataReceived(const URLRequest& request,
-                                                 int64_t prefilter_bytes_read) {
+void NetworkQualityEstimator::NotifyDataReceived(
+    const URLRequest& request,
+    int64_t cummulative_prefilter_bytes_read,
+    int64_t prefiltered_bytes_read) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK_GT(prefilter_bytes_read, 0);
+  DCHECK_GT(cummulative_prefilter_bytes_read, 0);
+  DCHECK_GT(prefiltered_bytes_read, 0);
 
   if (!request.url().is_valid() ||
       (!allow_localhost_requests_ && IsLocalhost(request.url().host())) ||
       !request.url().SchemeIsHTTPOrHTTPS() ||
       // Verify that response headers are received, so it can be ensured that
       // response is not cached.
-      request.response_info().response_time.is_null() || request.was_cached())
+      request.response_info().response_time.is_null() || request.was_cached() ||
+      request.creation_time() < last_connection_change_) {
     return;
+  }
 
   base::TimeTicks now = base::TimeTicks::Now();
   base::TimeDelta request_duration = now - request.creation_time();
@@ -58,16 +74,33 @@ void NetworkQualityEstimator::NotifyDataReceived(const URLRequest& request,
   if (request_duration < fastest_RTT_since_last_connection_change_)
     fastest_RTT_since_last_connection_change_ = request_duration;
 
+  // Only add RTT sample if this is the first read for this response.
+  if (cummulative_prefilter_bytes_read == prefiltered_bytes_read) {
+    // Pop the oldest element if the buffer is already full.
+    if (rtt_msec_.size() >= kMaximumSamples)

[bengr, 2015/06/01 22:55:42]

Perhaps use a while loop, for better future proofing.

[tbansal1, 2015/06/02 18:33:01]

Added
DCHECK_LE(samples_.size(), kMaximumSamples);
before this if check.

We should never need a while loop because the deque size
should never exceed the maximum size. So, when adding a
new sample, we should have to pop at most 1 existing sample.
Anything else implies there is something wrong with the
logic.

+      rtt_msec_.pop_front();
+
+    rtt_msec_.push_back(Sample(request_duration.InMilliseconds()));
+  }
+  DCHECK_LE(rtt_msec_.size(), kMaximumSamples);
+
   // Ignore tiny transfers which will not produce accurate rates.
   // Ignore short duration transfers.
-  if (prefilter_bytes_read >= kMinTransferSizeInBytes &&
+  if (cummulative_prefilter_bytes_read >= kMinTransferSizeInBytes &&
       request_duration >=
-          base::TimeDelta::FromMicroseconds(kMinRequestDurationMicroseconds) &&
-      request.creation_time() > last_connection_change_) {
-    uint64_t kbps = static_cast<uint64_t>(prefilter_bytes_read * 8 * 1000 /
-                                          request_duration.InMicroseconds());
+          base::TimeDelta::FromMicroseconds(kMinRequestDurationMicroseconds)) {
+    uint64_t kbps =
+        static_cast<uint64_t>(cummulative_prefilter_bytes_read * 8 * 1000 /
+                              request_duration.InMicroseconds());
     if (kbps > peak_kbps_since_last_connection_change_)
       peak_kbps_since_last_connection_change_ = kbps;
+
+    // Pop the oldest element if the buffer is already full.

[bengr, 2015/06/01 22:55:42]

You should probably write a ring buffer instead of duplicating code.

[tbansal1, 2015/06/02 18:33:01]

Done.

+    if (kbps_.size() >= kMaximumSamples)

[bengr, 2015/06/01 22:55:42]

Perhaps use a while loop, for better future proofing.

[tbansal1, 2015/06/02 18:33:01]

Please see the comment above.

+      kbps_.pop_front();
+
+    kbps_.push_back(Sample(kbps));
+    DCHECK_LE(kbps_.size(), kMaximumSamples);
   }
 }
 
@@ -160,7 +193,7 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
   current_connection_type_ = type;
 }
 
-NetworkQuality NetworkQualityEstimator::GetEstimate() const {
+NetworkQuality NetworkQualityEstimator::GetPeakEstimate() const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!bytes_read_since_last_connection_change_) {
@@ -175,4 +208,9 @@ NetworkQuality NetworkQualityEstimator::GetEstimate() const {
                         peak_kbps_since_last_connection_change_, 0.1);
 }
 
+NetworkQualityEstimator::Sample::Sample(int value)
+    : value_(value), timestamp_(base::TimeTicks::Now()) {
+  DCHECK_GE(value_, 0);
+}
+
 }  // namespace net