NQE: Add accuracy histogram for external estimate provider

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 7750e2faff832b75c6063e3551e24aa924905d94..bdc91014f0e907ead4cb5fdb66dc71f71ee36a75 100644
--- a/net/nqe/network_quality_estimator.cc
+++ b/net/nqe/network_quality_estimator.cc
@@ -517,6 +517,7 @@ void NetworkQualityEstimator::NotifyHeadersReceived(const URLRequest& request) {
         GetEffectiveConnectionType();
 
     RecordMetricsOnMainFrameRequest();
+    MaybeQueryExternalEstimateProvider();
 
     // 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
@@ -672,6 +673,27 @@ void NetworkQualityEstimator::RecordAccuracyAfterMainFrame(
         base::HistogramBase::kUmaTargetedHistogramFlag);
     histogram->Add(std::abs(estimated_observed_diff));
   }
+
+  // Add histogram for accuracy of external estimate provider.

[bengr, 2016/07/20 23:51:28]

for -> to evaluate the 
of -> of the

[tbansal1, 2016/07/21 22:00:50]

Done.

+  if (external_estimate_provider_quality_.http_rtt() !=
+          nqe::internal::InvalidRTT() &&
+      recent_http_rtt != nqe::internal::InvalidRTT()) {
+    const int estimated_observed_diff_milliseconds =
+        external_estimate_provider_quality_.http_rtt().InMilliseconds() -
+        recent_http_rtt.InMilliseconds();
+
+    const std::string sign_suffix =
+        estimated_observed_diff_milliseconds >= 0 ? "Positive." : "Negative.";

[bengr, 2016/07/20 23:51:28]

You should probably create a helper called GetSignSuffix() and use it
everywhere.

[tbansal1, 2016/07/21 22:00:50]

Done.

+
+    base::HistogramBase* histogram = base::Histogram::FactoryGet(

[bengr, 2016/07/20 23:51:28]

You get similar histograms all over this method. Consider refactoring into a
helper.

[tbansal1, 2016/07/21 22:00:50]

Done.

+        "NQE.ExternalEstimateProvider.RTT.Accuracy."

[bengr, 2016/07/20 23:51:28]

Here and everywhere in this method, consider using StringPrintf.

[tbansal1, 2016/07/21 22:00:50]

Done.

+        "EstimatedObservedDiff." +
+            sign_suffix + base::IntToString(measuring_duration.InSeconds()) +
+            "." + GetHistogramSuffixObservedRTT(recent_http_rtt),
+        1, 10 * 1000 /* 10 seconds */, 50 /* Number of buckets */,
+        base::HistogramBase::kUmaTargetedHistogramFlag);
+    histogram->Add(std::abs(estimated_observed_diff_milliseconds));
+  }
 }
 
 void NetworkQualityEstimator::NotifyRequestCompleted(
@@ -783,6 +805,19 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
 
   current_network_id_ = GetCurrentNetworkID();
 
+  MaybeQueryExternalEstimateProvider();
+
+  // Read any cached estimates for the new network. If cached estimates are
+  // unavailable, add the default estimates.
+  if (!ReadCachedNetworkQualityEstimate())
+    AddDefaultEstimates();
+  estimated_quality_at_last_main_frame_ = nqe::internal::NetworkQuality();
+  throughput_analyzer_->OnConnectionTypeChanged();
+  MaybeRecomputeEffectiveConnectionType();
+  UpdateSignalStrength();
+}
+
+void NetworkQualityEstimator::MaybeQueryExternalEstimateProvider() const {
   // Query the external estimate provider on certain connection types. Once the
   // updated estimates are available, OnUpdatedEstimateAvailable will be called
   // by |external_estimate_provider_| with updated estimates.
@@ -795,15 +830,6 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
         EXTERNAL_ESTIMATE_PROVIDER_STATUS_QUERIED);
     external_estimate_provider_->Update();
   }
-
-  // Read any cached estimates for the new network. If cached estimates are
-  // unavailable, add the default estimates.
-  if (!ReadCachedNetworkQualityEstimate())
-    AddDefaultEstimates();
-  estimated_quality_at_last_main_frame_ = nqe::internal::NetworkQuality();
-  throughput_analyzer_->OnConnectionTypeChanged();
-  MaybeRecomputeEffectiveConnectionType();
-  UpdateSignalStrength();
 }
 
 void NetworkQualityEstimator::UpdateSignalStrength() {
@@ -1263,6 +1289,8 @@ void NetworkQualityEstimator::OnUpdatedEstimateAvailable(
   RecordExternalEstimateProviderMetrics(
       EXTERNAL_ESTIMATE_PROVIDER_STATUS_CALLBACK);
 
+  external_estimate_provider_quality_ = nqe::internal::NetworkQuality();
+
   if (rtt > base::TimeDelta()) {
     RecordExternalEstimateProviderMetrics(
         EXTERNAL_ESTIMATE_PROVIDER_STATUS_RTT_AVAILABLE);
@@ -1270,6 +1298,7 @@ void NetworkQualityEstimator::OnUpdatedEstimateAvailable(
     rtt_observations_.AddObservation(
         RttObservation(rtt, tick_clock_->NowTicks(),
                        NETWORK_QUALITY_OBSERVATION_SOURCE_EXTERNAL_ESTIMATE));
+    external_estimate_provider_quality_.set_http_rtt(rtt);
   }
 
   if (downstream_throughput_kbps > 0) {
@@ -1281,6 +1310,8 @@ void NetworkQualityEstimator::OnUpdatedEstimateAvailable(
         ThroughputObservation(
             downstream_throughput_kbps, tick_clock_->NowTicks(),
             NETWORK_QUALITY_OBSERVATION_SOURCE_EXTERNAL_ESTIMATE));
+    external_estimate_provider_quality_.set_downstream_throughput_kbps(
+        downstream_throughput_kbps);
   }
 }