Expose effective connection type to Cronet

net/nqe/network_quality_estimator.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/nqe/network_quality_estimator.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <utility>
@@ skipping 122 matching lines @@
   DCHECK_GT(max_limit, kLowerLimit);
   const size_t kBucketCount = 50;
 
   // Prefix of network quality estimator histograms.
   const char prefix[] = "NQE.";
   return base::Histogram::FactoryGet(
       prefix + statistic_name + GetNameForConnectionType(type), kLowerLimit,
       max_limit, kBucketCount, base::HistogramBase::kUmaTargetedHistogramFlag);
 }
 
-bool GetValueForVariationParam(
+// Sets |variations_value| to the value of |parameter_name| read from
+// |variation_params|. If the value is unavailable from |variation_params|, then
+// |variations_value| is set to |default_value|.
+void GetValueForVariationParam(
     const std::map<std::string, std::string>& variation_params,
     const std::string& parameter_name,
+    int32_t default_value,
     int32_t* variations_value) {
   const auto it = variation_params.find(parameter_name);
-  return it != variation_params.end() &&
-         base::StringToInt(it->second, variations_value);
+  if (it != variation_params.end() &&
+      base::StringToInt(it->second, variations_value)) {
+    return;
+  }
+  *variations_value = default_value;
 }
 
 // Returns the algorithm that should be used for computing effective connection
 // type based on field trial params. Returns an empty string if a valid
 // algorithm paramter is not present in the field trial params.
 std::string GetEffectiveConnectionTypeAlgorithm(
     const std::map<std::string, std::string>& variation_params) {
   const auto it = variation_params.find("effective_connection_type_algorithm");
   if (it == variation_params.end())
     return std::string();
@@ skipping 149 matching lines @@
       base::ThreadTaskRunnerHandle::Get(),
       base::Bind(&NetworkQualityEstimator::OnUpdatedRTTAvailable,
                  base::Unretained(this))));
 
   // 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));
+
+  // Below 33rd percentile of 2G transport RTT observations.
+  default_connection_thresholds_[EFFECTIVE_CONNECTION_TYPE_SLOW_2G] =
+      nqe::internal::NetworkQuality(nqe::internal::InvalidRTT(),
+                                    base::TimeDelta::FromMilliseconds(1870),

[RyanSturm, 2016/07/12 19:57:55]

nit: Can you move the 1870 and 1300 to a static const int (or some other
variable outside of this block of code)? It bothers me that the comments seem so
far from the actual value.

[tbansal1, 2016/07/12 20:32:25]

I would prefer to keep them here instead of using static const. Suppose, I was
setting HTTP RTT and downlink throughput threshold too, then it would be 3
static const variables per connection type (so may be 3*7 = 21 static consts).
What comments are you referring too?

[RyanSturm, 2016/07/12 20:44:47]

I think it would be more clear like:

// 1.87 seconds is the 33rd percentile of 2G transport RTT
// observations.
static const int slow_2g_rtt_in_milliseconds = 1870;
default_connection_thresholds_[...] = ...
base::TimeDelta::FromMilliseconds(slow_2g_rtt_in_milliseconds), ...;

I'm not sure if the style guide says anything about this.

wdyt?

[tbansal1, 2016/07/13 05:52:03]

I updated the comments.

+                                    nqe::internal::kInvalidThroughput);
+
+  // 50th percentile of 2G transport RTT observations.
+  default_connection_thresholds_[EFFECTIVE_CONNECTION_TYPE_2G] =
+      nqe::internal::NetworkQuality(nqe::internal::InvalidRTT(),
+                                    base::TimeDelta::FromMilliseconds(1300),
+                                    nqe::internal::kInvalidThroughput);
 }
 
 void NetworkQualityEstimator::ObtainOperatingParams(
     const std::map<std::string, std::string>& variation_params) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   for (size_t i = 0; i <= NetworkChangeNotifier::CONNECTION_LAST; ++i) {
     NetworkChangeNotifier::ConnectionType type =
         static_cast<NetworkChangeNotifier::ConnectionType>(i);
     DCHECK_EQ(nqe::internal::InvalidRTT(), default_observations_[i].http_rtt());
@@ skipping 45 matching lines @@
     DCHECK_EQ(nqe::internal::InvalidRTT(),
               connection_thresholds_[i].transport_rtt());
     DCHECK_EQ(nqe::internal::kInvalidThroughput,
               connection_thresholds_[i].downstream_throughput_kbps());
     if (effective_connection_type == EFFECTIVE_CONNECTION_TYPE_UNKNOWN)
       continue;
 
     std::string connection_type_name = std::string(
         GetNameForEffectiveConnectionType(effective_connection_type));
 
-    int32_t variations_value = kMinimumRTTVariationParameterMsec - 1;
-    if (GetValueForVariationParam(
-            variation_params, connection_type_name + kThresholdURLRTTMsecSuffix,
-            &variations_value) &&
-        variations_value >= kMinimumRTTVariationParameterMsec) {
-      connection_thresholds_[i].set_http_rtt(
-          base::TimeDelta(base::TimeDelta::FromMilliseconds(variations_value)));
+    int32_t variations_value;
+    GetValueForVariationParam(
+        variation_params, connection_type_name + kThresholdURLRTTMsecSuffix,
+        default_connection_thresholds_[i].http_rtt().InMilliseconds(),
+        &variations_value);
+    connection_thresholds_[i].set_http_rtt(
+        base::TimeDelta(base::TimeDelta::FromMilliseconds(variations_value)));
+    // Verify that the RTT values are in decreasing order as the network
+    // quality improves.
+    DCHECK(i == 0 ||
+           connection_thresholds_[i].http_rtt() ==
+               nqe::internal::InvalidRTT() ||
+           connection_thresholds_[i - 1].http_rtt() ==
+               nqe::internal::InvalidRTT() ||
+           connection_thresholds_[i].http_rtt() <=
+               connection_thresholds_[i - 1].http_rtt());
 
-      // Verify that the RTT values are in decreasing order as the network
-      // quality improves.
-      DCHECK(i == 0 ||
-             connection_thresholds_[i - 1].http_rtt() ==
-                 nqe::internal::InvalidRTT() ||
-             connection_thresholds_[i].http_rtt() <=
-                 connection_thresholds_[i - 1].http_rtt());
-    }
+    GetValueForVariationParam(
+        variation_params,
+        connection_type_name + kThresholdTransportRTTMsecSuffix,
+        default_connection_thresholds_[i].transport_rtt().InMilliseconds(),
+        &variations_value);
+    connection_thresholds_[i].set_transport_rtt(
+        base::TimeDelta(base::TimeDelta::FromMilliseconds(variations_value)));
+    // Verify that the transport RTT values are in decreasing order as the
+    // network quality improves.
+    DCHECK(i == 0 ||
+           connection_thresholds_[i].transport_rtt() ==
+               nqe::internal::InvalidRTT() ||
+           connection_thresholds_[i - 1].transport_rtt() ==
+               nqe::internal::InvalidRTT() ||
+           connection_thresholds_[i].transport_rtt() <=
+               connection_thresholds_[i - 1].transport_rtt());
 
-    variations_value = kMinimumRTTVariationParameterMsec - 1;
-    if (GetValueForVariationParam(
-            variation_params,
-            connection_type_name + kThresholdTransportRTTMsecSuffix,
-            &variations_value) &&
-        variations_value >= kMinimumRTTVariationParameterMsec) {
-      connection_thresholds_[i].set_transport_rtt(
-          base::TimeDelta(base::TimeDelta::FromMilliseconds(variations_value)));
-
-      // Verify that the transport RTT values are in decreasing order as the
-      // network quality improves.
-      DCHECK(i == 0 ||
-             connection_thresholds_[i - 1].transport_rtt() ==
-                 nqe::internal::InvalidRTT() ||
-             connection_thresholds_[i].transport_rtt() <=
-                 connection_thresholds_[i - 1].transport_rtt());
-    }
-
-    variations_value = kMinimumThroughputVariationParameterKbps - 1;
-    if (GetValueForVariationParam(variation_params,
-                                  connection_type_name + kThresholdKbpsSuffix,
-                                  &variations_value) &&
-        variations_value >= kMinimumThroughputVariationParameterKbps) {
-      connection_thresholds_[i].set_downstream_throughput_kbps(
-          variations_value);
-
-      // Verify that the throughput values are in increasing order as the
-      // network quality improves.
-      DCHECK(i == 0 ||
-             connection_thresholds_[i - 1].downstream_throughput_kbps() ==
-                 kMinimumThroughputVariationParameterKbps ||
-             connection_thresholds_[i].downstream_throughput_kbps() >=
-                 connection_thresholds_[i - 1].downstream_throughput_kbps());
-    }
+    GetValueForVariationParam(
+        variation_params, connection_type_name + kThresholdKbpsSuffix,
+        default_connection_thresholds_[i].downstream_throughput_kbps(),
+        &variations_value);
+    connection_thresholds_[i].set_downstream_throughput_kbps(variations_value);
+    // Verify that the throughput values are in increasing order as the
+    // network quality improves.
+    DCHECK(i == 0 ||
+           connection_thresholds_[i].downstream_throughput_kbps() ==
+               kMinimumThroughputVariationParameterKbps ||
+           connection_thresholds_[i - 1].downstream_throughput_kbps() ==
+               kMinimumThroughputVariationParameterKbps ||
+           connection_thresholds_[i].downstream_throughput_kbps() >=
+               connection_thresholds_[i - 1].downstream_throughput_kbps());
   }
 }
 
 void NetworkQualityEstimator::AddDefaultEstimates() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (default_observations_[current_network_id_.type].http_rtt() !=
       nqe::internal::InvalidRTT()) {
     RttObservation rtt_observation(
         default_observations_[current_network_id_.type].http_rtt(),
@@ skipping 1001 matching lines @@
 
 void NetworkQualityEstimator::MaybeRecomputeEffectiveConnectionType() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   const base::TimeTicks now = tick_clock_->NowTicks();
   // Recompute effective connection type only if
   // |effective_connection_type_recomputation_interval_| has passed since it was
   // last computed or a connection change event was observed since the last
   // computation. Strict inequalities are used to ensure that effective
   // connection type is recomputed on connection change events even if the clock
-  // has not updated.
+  // has not updated. If the algorithm uses transport RTT, then recompute the
+  // effective connection type if the effective connection type was previously
+  // unavailable. This is because the transport RTT observations are voluminous,
+  // so it may now be possible to compute the effective connection type.
   if (now - last_effective_connection_type_computation_ <
           effective_connection_type_recomputation_interval_ &&
-      last_connection_change_ < last_effective_connection_type_computation_) {
+      last_connection_change_ < last_effective_connection_type_computation_ &&
+      (effective_connection_type_ !=
+           NetworkQualityEstimator::EFFECTIVE_CONNECTION_TYPE_UNKNOWN ||
+       effective_connection_type_algorithm_ !=
+           EffectiveConnectionTypeAlgorithm::
+               TRANSPORT_RTT_OR_DOWNSTREAM_THROUGHOUT)) {
     return;
   }
 
   const EffectiveConnectionType past_type = effective_connection_type_;
   last_effective_connection_type_computation_ = now;
   effective_connection_type_ = GetEffectiveConnectionType();
 
   if (past_type != effective_connection_type_)
     NotifyObserversOfEffectiveConnectionTypeChanged();
 }
 
 void NetworkQualityEstimator::
     NotifyObserversOfEffectiveConnectionTypeChanged() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // TODO(tbansal): Add hysteresis in the notification.
   FOR_EACH_OBSERVER(
       EffectiveConnectionTypeObserver, effective_connection_type_observer_list_,
       OnEffectiveConnectionTypeChanged(effective_connection_type_));
 }
 
 }  // namespace net