Record NQE accuracy at main frame requests

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 f2247f048b1195104c95a750623e9433d1e09c92..249a8d7fb547004941d5bd57881132e23943176c 100644
--- a/net/nqe/network_quality_estimator.cc
+++ b/net/nqe/network_quality_estimator.cc
@@ -157,6 +157,32 @@ bool RequestSchemeIsHTTPOrHTTPS(const net::URLRequest& request) {
   return request.url().is_valid() && request.url().SchemeIsHTTPOrHTTPS();
 }
 
+// Returns the suffix of the histogram that should be used for recording the
+// accuracy when the observed RTT is |actual_rtt|. The width of i'th interval is
+// set to |20 * 2^(i - 1)|.
+std::string GetHistogramSuffixActualRTT(const base::TimeDelta& actual_rtt) {
+  const int rtt_milliseconds = actual_rtt.InMilliseconds();
+  DCHECK_GE(rtt_milliseconds, 0);
+
+  if (rtt_milliseconds <= 20)
+    return "0_20";
+  if (rtt_milliseconds <= 60)
+    return "20_60";
+  if (rtt_milliseconds <= 140)
+    return "60_140";
+  if (rtt_milliseconds <= 300)
+    return "140_300";
+  if (rtt_milliseconds <= 620)
+    return "300_620";
+  if (rtt_milliseconds <= 1260)
+    return "620_1260";
+  if (rtt_milliseconds <= 2540)
+    return "1260_2540";
+  if (rtt_milliseconds <= 5100)
+    return "2540_5100";
+  return "5100_Infinity";

[bengr, 2016/06/01 16:55:29]

How about:
const char* const kSuffixes[] = { "0_20", "20_60", ...}
for (int i = 0; i < arraylen(kSuffixes) - 1; ++i) {
  if (rtt_milliseconds <= 20 * 2^i)
    return kSuffixes[i];
return kSuffixes[arraylen(kSuffixes) - 1];


Alternatively, you could use StringPrintf("%d_%d", prev, curr)


Also, should the second interval be 21_60?

Do these suffixes need to be in sync with histograms.xml. Say so if so.

[tbansal1, 2016/06/01 21:49:02]

Done.
I think it is easier to understand the distribution of buckets if the strings
are explicit.
May be but that might be confusing too. e.g., it is not clear which histogram
should be used if actual_rtt is 20.5 milliseconds. I suppose it does not matter,
but if you feel strongly, I can change.
Done.

+}
+
 }  // namespace
 
 namespace net {
@@ -296,6 +322,13 @@ NetworkQualityEstimator::NetworkQualityEstimator(
 
   watcher_factory_.reset(new SocketWatcherFactory(
       base::ThreadTaskRunnerHandle::Get(), weak_ptr_factory_.GetWeakPtr()));
+
+  // Record accuracy at 3 different intervals. The values used here must remain
+  // in sync with the suffixes specified in
+  // tools/metrics/histograms/histograms.xml.
+  accuracy_recording_intervals_.push_back(base::TimeDelta::FromSeconds(15));
+  accuracy_recording_intervals_.push_back(base::TimeDelta::FromSeconds(30));
+  accuracy_recording_intervals_.push_back(base::TimeDelta::FromSeconds(60));
 }
 
 void NetworkQualityEstimator::ObtainOperatingParams(
@@ -431,6 +464,12 @@ NetworkQualityEstimator::~NetworkQualityEstimator() {
   NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
 }
 
+const std::vector<base::TimeDelta>&
+NetworkQualityEstimator::GetAccuracyRecordingIntervals() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return accuracy_recording_intervals_;
+}
+
 void NetworkQualityEstimator::NotifyStartTransaction(
     const URLRequest& request) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -451,24 +490,42 @@ void NetworkQualityEstimator::NotifyHeadersReceived(const URLRequest& request) {
     return;
   }
 
-  // Update |estimated_median_network_quality_| if this is a main frame request.
+  const base::TimeTicks now = tick_clock_->NowTicks();
+
+  // Update |estimated_quality_main_frame_| if this is a main frame request.
   if (request.load_flags() & LOAD_MAIN_FRAME) {
+    last_main_frame_request_ = now;
     base::TimeDelta estimated_http_rtt;
     if (!GetHttpRTTEstimate(&estimated_http_rtt))
       estimated_http_rtt = nqe::internal::InvalidRTT();
 
+    base::TimeDelta estimated_transport_rtt;
+    if (!GetTransportRTTEstimate(&estimated_transport_rtt))
+      estimated_transport_rtt = nqe::internal::InvalidRTT();
+
     int32_t downstream_throughput_kbps;
     if (!GetDownlinkThroughputKbpsEstimate(&downstream_throughput_kbps))
       downstream_throughput_kbps = nqe::internal::kInvalidThroughput;
 
-    estimated_median_network_quality_ = nqe::internal::NetworkQuality(
-        estimated_http_rtt, nqe::internal::InvalidRTT(),
+    estimated_quality_main_frame_ = nqe::internal::NetworkQuality(
+        estimated_http_rtt, estimated_transport_rtt,
         downstream_throughput_kbps);
 
     RecordMetricsOnMainFrameRequest();
+
+    // Post the tasks which will run in future and record the estimation

[bengr, 2016/06/01 16:55:29]

in -> in the

[tbansal1, 2016/06/01 21:49:02]

Done.

+    // accuracy based on the observations received between now and the time of
+    // task execution.

[bengr, 2016/06/01 16:55:29]

Say why you post instead of executing inline.

[tbansal1, 2016/06/01 21:49:02]

Done.

+    for (const base::TimeDelta& measuring_delay :
+         GetAccuracyRecordingIntervals()) {
+      base::MessageLoop::current()->task_runner()->PostDelayedTask(
+          FROM_HERE,
+          base::Bind(&NetworkQualityEstimator::RecordAccuracyAfterMainFrame,
+                     weak_ptr_factory_.GetWeakPtr(), measuring_delay),
+          measuring_delay);
+    }
   }
 
-  const base::TimeTicks now = tick_clock_->NowTicks();
   LoadTimingInfo load_timing_info;
   request.GetLoadTimingInfo(&load_timing_info);
 
@@ -496,14 +553,79 @@ void NetworkQualityEstimator::NotifyHeadersReceived(const URLRequest& request) {
   NotifyObserversOfRTT(http_rtt_observation);
 
   // Compare the RTT observation with the estimated value and record it.
-  if (estimated_median_network_quality_.http_rtt() !=
-      nqe::internal::InvalidRTT()) {
-    RecordHttpRTTUMA(
-        estimated_median_network_quality_.http_rtt().InMilliseconds(),
-        observed_http_rtt.InMilliseconds());
+  if (estimated_quality_main_frame_.http_rtt() != nqe::internal::InvalidRTT()) {
+    RecordHttpRTTUMA(estimated_quality_main_frame_.http_rtt().InMilliseconds(),
+                     observed_http_rtt.InMilliseconds());
   }
 }
 
+void NetworkQualityEstimator::RecordAccuracyAfterMainFrame(
+    const base::TimeDelta& measuring_duration) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_EQ(0, measuring_duration.InMilliseconds() % 1000);
+  DCHECK(ContainsValue(GetAccuracyRecordingIntervals(), measuring_duration));
+
+  const base::TimeTicks now = tick_clock_->NowTicks();
+
+  // Return if the time since |last_main_frame_request_| is less than
+  // |measuring_duration|. This may happen if another main frame request started
+  // during last |measuring_duration|. Returning here ensures that we do not
+  // take inaccurate readings.
+  if (now - last_main_frame_request_ < measuring_duration)
+    return;
+
+  // Return if the time since |last_main_frame_request_| is off by a factor of
+  // 2. This can happen if the task is executed much later than its scheduled
+  // time. Returning here ensures that we do not take inaccurate readings.
+  if (now - last_main_frame_request_ > 2 * measuring_duration)
+    return;
+
+  base::TimeDelta recent_http_rtt;
+  if (estimated_quality_main_frame_.http_rtt() != nqe::internal::InvalidRTT() &&
+      GetRecentHttpRTTMedian(last_main_frame_request_, &recent_http_rtt)) {
+    int estimated_actual_diff_milliseconds =
+        estimated_quality_main_frame_.http_rtt().InMilliseconds() -
+        recent_http_rtt.InMilliseconds();
+
+    const std::string prefix =
+        estimated_actual_diff_milliseconds >= 0
+            ? "NQE.Accuracy.HttpRTT.EstimatedActualDiff."
+            : "NQE.Accuracy.HttpRTT.ActualEstimatedDiff.";
+
+    base::HistogramBase* histogram = base::Histogram::FactoryGet(
+        prefix + base::IntToString(measuring_duration.InSeconds()) + "." +
+            GetHistogramSuffixActualRTT(recent_http_rtt),
+        1, 10 * 1000 /* 10 seconds */, 50 /* Number of buckets */,
+        base::HistogramBase::kUmaTargetedHistogramFlag);
+    histogram->Add(std::abs(estimated_actual_diff_milliseconds));
+  }
+
+  base::TimeDelta recent_transport_rtt;
+  if (estimated_quality_main_frame_.transport_rtt() !=
+          nqe::internal::InvalidRTT() &&
+      GetRecentTransportRTTMedian(last_main_frame_request_,
+                                  &recent_transport_rtt)) {
+    int estimated_actual_diff_milliseconds =
+        estimated_quality_main_frame_.transport_rtt().InMilliseconds() -
+        recent_transport_rtt.InMilliseconds();
+
+    const std::string prefix =
+        estimated_actual_diff_milliseconds >= 0
+            ? "NQE.Accuracy.TransportRTT.EstimatedActualDiff."
+            : "NQE.Accuracy.TransportRTT.ActualEstimatedDiff.";
+
+    base::HistogramBase* histogram = base::Histogram::FactoryGet(
+        prefix + base::IntToString(measuring_duration.InSeconds()) + "." +
+            GetHistogramSuffixActualRTT(recent_transport_rtt),
+        1, 10 * 1000 /* 10 seconds */, 50 /* Number of buckets */,
+        base::HistogramBase::kUmaTargetedHistogramFlag);
+    histogram->Add(std::abs(estimated_actual_diff_milliseconds));
+  }
+
+  // TODO(tbansal): Add histogram for downstream throughput and effective
+  // connection type.
+}
+
 void NetworkQualityEstimator::NotifyRequestCompleted(
     const URLRequest& request) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("net"),
@@ -639,7 +761,7 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
   // unavailable, add the default estimates.
   if (!ReadCachedNetworkQualityEstimate())
     AddDefaultEstimates();
-  estimated_median_network_quality_ = nqe::internal::NetworkQuality();
+  estimated_quality_main_frame_ = nqe::internal::NetworkQuality();
   throughput_analyzer_->OnConnectionTypeChanged();
   MaybeRecomputeEffectiveConnectionType();
 }
@@ -1009,7 +1131,7 @@ void NetworkQualityEstimator::OnUpdatedEstimateAvailable(
         EXTERNAL_ESTIMATE_PROVIDER_STATUS_RTT_AVAILABLE);
     UMA_HISTOGRAM_TIMES("NQE.ExternalEstimateProvider.RTT", rtt);
     rtt_observations_.AddObservation(
-        RttObservation(rtt, base::TimeTicks::Now(),
+        RttObservation(rtt, tick_clock_->NowTicks(),
                        NETWORK_QUALITY_OBSERVATION_SOURCE_EXTERNAL_ESTIMATE));
   }
 
@@ -1020,7 +1142,7 @@ void NetworkQualityEstimator::OnUpdatedEstimateAvailable(
                          downstream_throughput_kbps);
     downstream_throughput_kbps_observations_.AddObservation(
         ThroughputObservation(
-            downstream_throughput_kbps, base::TimeTicks::Now(),
+            downstream_throughput_kbps, tick_clock_->NowTicks(),
             NETWORK_QUALITY_OBSERVATION_SOURCE_EXTERNAL_ESTIMATE));
   }
 }