NQE: Allow algorithm to be set using variation params

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..379ee545debab057300dbf4bacbc2389b6aae3ba 100644
--- a/net/nqe/network_quality_estimator.cc
+++ b/net/nqe/network_quality_estimator.cc
@@ -35,6 +35,16 @@
 
 namespace {
 
+// The algorithm that uses HTTP RTT and downstream throughput to determine the

[bengr, 2016/06/06 20:14:54]

Hmm. This will be hard to extend. I think instead you should do something like
this:

enum class Algorithm {
  UNKNOWN = 0,
  HTTP_RTT_AND_DOWNSTREAM_THROUGHOUT = 1
};

// If needed for UMA:
std::unordered_map<Algorithm, std::string> algorithm_name(
  { { HTTP_RTT_AND_DOWNSTREAM_THROUGHPUT,
      "HttpRTTAndDownstreamThroughput" } });

EffectiveConnectionType
GetRecentEffectiveConnectionType(
  EffectiveConnectionTypeAlgorithm algorithm) {
  EffectiveConnectionType ect = UNKNOWN; // ?
  switch (algorithm) {
    case HTTP_RTT_AND_DOWNSTREAM_THROUGHOUT:
    // fall-through:
    default:
      RunHttpRttAndDownstreamThroughputAlgorithm(&ect);
      break:
  }
  return ect;
}

[tbansal1, 2016/06/08 01:30:53]

I wanted to keep |algorithm_| as a const variable. So, I did something similar.

+// effective connection type.
+const char kHttpRTTDownstreamThroughputAlgorithm[] =
+    "HttpRTTAndDownstreamThroughput";
+
+// The default algorithm to be used if the algorithm value is not available
+// through field trial parameters.
+const char kDefaultEffectiveConnectionTypeAlgorithm[] =
+    "HttpRTTAndDownstreamThroughput";
+
 // Default value of the half life (in seconds) for computing time weighted
 // percentiles. Every half life, the weight of all observations reduces by
 // half. Lowering the half life would reduce the weight of older values faster.
@@ -135,11 +145,23 @@ bool GetValueForVariationParam(
     const std::map<std::string, std::string>& variation_params,
     const std::string& parameter_name,
     int32_t* variations_value) {
-  auto it = variation_params.find(parameter_name);
+  const auto it = variation_params.find(parameter_name);
   return it != variation_params.end() &&
          base::StringToInt(it->second, variations_value);
 }
 
+// Returns the value of the |parameter_name| in |variation_params|. If
+// |parameter_name| is not set in |variation_params|, then |default_value| is
+// returned.
+std::string GetValueForVariationParamWithDefaultValue(
+    const std::map<std::string, std::string>& variation_params,
+    const std::string& parameter_name,
+    const std::string& default_value) {
+  const auto it = variation_params.find(parameter_name);
+  return it != variation_params.end() && !it->second.empty() ? it->second
+                                                             : default_value;
+}
+
 net::NetworkQualityObservationSource ProtocolSourceToObservationSource(
     net::SocketPerformanceWatcherFactory::Protocol protocol) {
   switch (protocol) {
@@ -253,6 +275,10 @@ NetworkQualityEstimator::NetworkQualityEstimator(
       use_small_responses_(use_smaller_responses_for_tests),
       weight_multiplier_per_second_(
           GetWeightMultiplierPerSecond(variation_params)),
+      algorithm_(GetValueForVariationParamWithDefaultValue(
+          variation_params,
+          "algorithm",
+          kDefaultEffectiveConnectionTypeAlgorithm)),
       tick_clock_(new base::DefaultTickClock()),
       effective_connection_type_recomputation_interval_(
           base::TimeDelta::FromSeconds(15)),
@@ -755,6 +781,20 @@ NetworkQualityEstimator::GetRecentEffectiveConnectionType(
     const base::TimeTicks& start_time) const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  if (algorithm_ == kHttpRTTDownstreamThroughputAlgorithm) {
+    return GetRecentEffectiveConnectionTypeHttpRTTAndDownstreamThroughput(
+        start_time);
+  }
+  // Add additional algorithms here.
+  NOTREACHED();
+  return EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
+}
+
+NetworkQualityEstimator::EffectiveConnectionType NetworkQualityEstimator::
+    GetRecentEffectiveConnectionTypeHttpRTTAndDownstreamThroughput(
+        const base::TimeTicks& start_time) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   // If the device is currently offline, then return
   // EFFECTIVE_CONNECTION_TYPE_OFFLINE.
   if (GetCurrentNetworkID().type == NetworkChangeNotifier::CONNECTION_NONE)
@@ -768,7 +808,7 @@ NetworkQualityEstimator::GetRecentEffectiveConnectionType(
   if (!GetRecentMedianDownlinkThroughputKbps(start_time, &kbps))
     kbps = nqe::internal::kInvalidThroughput;
 
-  if (http_rtt == nqe::internal::InvalidRTT() &&
+  if (http_rtt == nqe::internal::InvalidRTT() ||
       kbps == nqe::internal::kInvalidThroughput) {
     // Quality of the current network is unknown.
     return EFFECTIVE_CONNECTION_TYPE_UNKNOWN;