Record RTT prediction accuracy when using average algorithms

net/nqe/network_quality_estimator.cc

Index: net/nqe/network_quality_estimator.cc
diff --git a/net/nqe/network_quality_estimator.cc b/net/nqe/network_quality_estimator.cc
index cd3dc1c2c5b0e9d63456537d91e4014209c3b728..a4a151c79858ebf1642fa48d439bdb92305990a5 100644
--- a/net/nqe/network_quality_estimator.cc
+++ b/net/nqe/network_quality_estimator.cc
@@ -345,6 +345,9 @@ NetworkQualityEstimator::NetworkQualityEstimator(
   // remain in sync with the suffixes specified in
   // tools/metrics/histograms/histograms.xml.
   accuracy_recording_intervals_.push_back(base::TimeDelta::FromSeconds(15));
+
+  for (int i = 0; i < STATISTIC_LAST; ++i)
+    http_rtt_at_last_main_frame_[i] = nqe::internal::InvalidRTT();
 }
 
 void NetworkQualityEstimator::ObtainOperatingParams(
@@ -425,6 +428,21 @@ void NetworkQualityEstimator::NotifyStartTransaction(
     effective_connection_type_at_last_main_frame_ = effective_connection_type_;
     estimated_quality_at_last_main_frame_ = network_quality_;
 
+    // Record the HTTP at the last main frame for experimental statistics.
+    for (int i = 0; i < STATISTIC_LAST; ++i) {
+      Statistic statistic = static_cast<Statistic>(i);
+      switch (statistic) {
+        case STATISTIC_LAST:
+          NOTREACHED();
+          break;
+        case STATISTIC_WEIGHTED_AVERAGE:
+          http_rtt_at_last_main_frame_[i] =
+              GetRTTEstimateInternal(disallowed_observation_sources_for_http_,
+                                     base::TimeTicks(), statistic, 50);
+          break;
+      }
+    }
+
     // Post the tasks which will run in the future and record the estimation
     // accuracy based on the observations received between now and the time of
     // task execution. Posting the task at different intervals makes it
@@ -515,6 +533,32 @@ void NetworkQualityEstimator::RecordAccuracyAfterMainFrame(
     return;
 
   base::TimeDelta recent_http_rtt;
+
+  // Record the HTTP prediction accuracy for experimental statistics.
+  for (int i = 0; i < STATISTIC_LAST; ++i) {
+    Statistic statistic = static_cast<Statistic>(i);
+    switch (statistic) {

[RyanSturm, 2017/02/24 19:54:18]

nit: can you get rid of this switch statement if you just add a statistic to
string method that returns "NQE.WeightedAverage.Accuracy.HttpRTT" for average?

[tbansal1, 2017/02/25 03:04:01]

Done.

+      case STATISTIC_LAST:
+        NOTREACHED();
+        break;
+      case STATISTIC_WEIGHTED_AVERAGE:
+        recent_http_rtt =
+            GetRTTEstimateInternal(disallowed_observation_sources_for_http_,
+                                   last_main_frame_request_, statistic, 50);
+        if (recent_http_rtt != nqe::internal::InvalidRTT() &&
+            http_rtt_at_last_main_frame_[i] != nqe::internal::InvalidRTT()) {
+          int estimated_observed_diff_milliseconds =
+              http_rtt_at_last_main_frame_[i].InMilliseconds() -
+              recent_http_rtt.InMilliseconds();
+
+          RecordRTTAccuracy("NQE.WeightedAverage.Accuracy.HttpRTT",
+                            estimated_observed_diff_milliseconds,
+                            measuring_duration, recent_http_rtt);
+        }
+        break;
+    }
+  }
+
   if (!GetRecentHttpRTT(last_main_frame_request_, &recent_http_rtt))
     recent_http_rtt = nqe::internal::InvalidRTT();
 
@@ -850,6 +894,10 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
   if (!ReadCachedNetworkQualityEstimate())
     AddDefaultEstimates();
   estimated_quality_at_last_main_frame_ = nqe::internal::NetworkQuality();
+
+  for (int i = 0; i < STATISTIC_LAST; ++i)
+    http_rtt_at_last_main_frame_[i] = nqe::internal::InvalidRTT();
+
   throughput_analyzer_->OnConnectionTypeChanged();
   MaybeComputeEffectiveConnectionType();
 }
@@ -908,8 +956,9 @@ void NetworkQualityEstimator::RecordMetricsOnConnectionTypeChanged() const {
     // Add the remaining percentile values.
     static const int kPercentiles[] = {0, 10, 90, 100};
     for (size_t i = 0; i < arraysize(kPercentiles); ++i) {
-      rtt = GetRTTEstimateInternal(disallowed_observation_sources_for_http_,
-                                   base::TimeTicks(), kPercentiles[i]);
+      rtt = GetRTTEstimateInternal(
+          disallowed_observation_sources_for_http_, base::TimeTicks(),
+          base::Optional<Statistic>(), kPercentiles[i]);
 
       rtt_percentile = GetHistogram(
           "RTT.Percentile" + base::IntToString(kPercentiles[i]) + ".",
@@ -927,9 +976,9 @@ void NetworkQualityEstimator::RecordMetricsOnConnectionTypeChanged() const {
     // Add the remaining percentile values.
     static const int kPercentiles[] = {0, 10, 90, 100};
     for (size_t i = 0; i < arraysize(kPercentiles); ++i) {
-      rtt =
-          GetRTTEstimateInternal(disallowed_observation_sources_for_transport_,
-                                 base::TimeTicks(), kPercentiles[i]);
+      rtt = GetRTTEstimateInternal(
+          disallowed_observation_sources_for_transport_, base::TimeTicks(),
+          base::Optional<Statistic>(), kPercentiles[i]);
 
       transport_rtt_percentile = GetHistogram(
           "TransportRTT.Percentile" + base::IntToString(kPercentiles[i]) + ".",
@@ -1010,6 +1059,22 @@ void NetworkQualityEstimator::RecordMetricsOnMainFrameRequest() const {
 
   effective_connection_type_histogram->Add(
       effective_connection_type_at_last_main_frame_);
+
+  // Record the HTTP RTT at the main frames for experimental statistics.
+  for (int i = 0; i < STATISTIC_LAST; ++i) {

[RyanSturm, 2017/02/24 19:54:18]

nit: this for loop/switch is so awkward.

I assume its for compile time assertions, but I don't like it.

[tbansal1, 2017/02/25 03:04:01]

Done.

+    Statistic statistic = static_cast<Statistic>(i);
+    switch (statistic) {
+      case STATISTIC_LAST:
+        NOTREACHED();
+        break;
+      case STATISTIC_WEIGHTED_AVERAGE:
+        if (http_rtt_at_last_main_frame_[i] != nqe::internal::InvalidRTT()) {
+          UMA_HISTOGRAM_TIMES("NQE.WeightedAverage.MainFrame.RTT",
+                              http_rtt_at_last_main_frame_[i]);
+        }
+        break;
+    }
+  }
 }
 
 void NetworkQualityEstimator::ComputeEffectiveConnectionType() {
@@ -1285,7 +1350,7 @@ bool NetworkQualityEstimator::GetRecentHttpRTT(
     base::TimeDelta* rtt) const {
   DCHECK(thread_checker_.CalledOnValidThread());
   *rtt = GetRTTEstimateInternal(disallowed_observation_sources_for_http_,
-                                start_time, 50);
+                                start_time, base::Optional<Statistic>(), 50);
   return (*rtt != nqe::internal::InvalidRTT());
 }
 
@@ -1294,7 +1359,7 @@ bool NetworkQualityEstimator::GetRecentTransportRTT(
     base::TimeDelta* rtt) const {
   DCHECK(thread_checker_.CalledOnValidThread());
   *rtt = GetRTTEstimateInternal(disallowed_observation_sources_for_transport_,
-                                start_time, 50);
+                                start_time, base::Optional<Statistic>(), 50);
   return (*rtt != nqe::internal::InvalidRTT());
 }
 
@@ -1309,19 +1374,39 @@ bool NetworkQualityEstimator::GetRecentDownlinkThroughputKbps(
 base::TimeDelta NetworkQualityEstimator::GetRTTEstimateInternal(
     const std::vector<NetworkQualityObservationSource>&
         disallowed_observation_sources,
-    const base::TimeTicks& start_time,
+    base::TimeTicks start_time,
+    const base::Optional<Statistic>& statistic,
     int percentile) const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // RTT observations are sorted by duration from shortest to longest, thus
   // a higher percentile RTT will have a longer RTT than a lower percentile.
   base::TimeDelta rtt = nqe::internal::InvalidRTT();
-  if (!rtt_observations_.GetPercentile(start_time, signal_strength_dbm_, &rtt,
-                                       percentile,
-                                       disallowed_observation_sources)) {
-    return nqe::internal::InvalidRTT();
+
+  if (!statistic) {
+    // Use default statistic algorithm.
+    if (!rtt_observations_.GetPercentile(start_time, signal_strength_dbm_, &rtt,
+                                         percentile,
+                                         disallowed_observation_sources)) {
+      return nqe::internal::InvalidRTT();
+    }
+    return rtt;
   }
-  return rtt;
+
+  switch (statistic.value()) {
+    case STATISTIC_LAST:
+      NOTREACHED();
+      return nqe::internal::InvalidRTT();
+    case STATISTIC_WEIGHTED_AVERAGE:
+      if (!rtt_observations_.GetWeightedAverage(
+              start_time, signal_strength_dbm_, disallowed_observation_sources,
+              &rtt)) {
+        return nqe::internal::InvalidRTT();
+      }
+      return rtt;
+  }
+  NOTREACHED();
+  return nqe::internal::InvalidRTT();
 }
 
 int32_t NetworkQualityEstimator::GetDownlinkThroughputKbpsEstimateInternal(