Add functionality for embedders to configure NQE

components/cronet/url_request_context_config.cc

Index: components/cronet/url_request_context_config.cc
diff --git a/components/cronet/url_request_context_config.cc b/components/cronet/url_request_context_config.cc
index e4b4a77376f2a3eb9641456d0839af42befffda9..093af06563e4eff252660e0b2606a5b45597e061 100644
--- a/components/cronet/url_request_context_config.cc
+++ b/components/cronet/url_request_context_config.cc
@@ -4,8 +4,10 @@
 
 #include "components/cronet/url_request_context_config.h"
 
+#include <map>
 #include <utility>
 
+#include "base/compiler_specific.h"
 #include "base/json/json_reader.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
@@ -23,6 +25,7 @@
 #include "net/dns/host_resolver.h"
 #include "net/dns/mapped_host_resolver.h"
 #include "net/http/http_server_properties.h"
+#include "net/nqe/network_quality_estimator_params.h"
 #include "net/quic/chromium/quic_utils_chromium.h"
 #include "net/quic/core/quic_packets.h"
 #include "net/socket/ssl_client_socket.h"
@@ -79,6 +82,9 @@ const char kStaleDnsAllowOtherNetwork[] = "allow_other_network";
 const char kHostResolverRulesFieldTrialName[] = "HostResolverRules";
 const char kHostResolverRules[] = "host_resolver_rules";
 
+// NetworkQualityEstimator experiment dictionary name.
+const char kNetworkQualityEstimatorFieldTrialName[] = "NetworkQualityEstimator";
+
 // Disable IPv6 when on WiFi. This is a workaround for a known issue on certain
 // Android phones, and should not be necessary when not on one of those devices.
 // See https://crbug.com/696569 for details.
@@ -130,12 +136,12 @@ URLRequestContextConfig::URLRequestContextConfig(
       load_disable_cache(load_disable_cache),
       storage_path(storage_path),
       user_agent(user_agent),
-      experimental_options(experimental_options),
       mock_cert_verifier(std::move(mock_cert_verifier)),
       enable_network_quality_estimator(enable_network_quality_estimator),
       bypass_public_key_pinning_for_local_trust_anchors(
           bypass_public_key_pinning_for_local_trust_anchors),
-      cert_verifier_data(cert_verifier_data) {}
+      cert_verifier_data(cert_verifier_data),
+      experimental_options(experimental_options) {}
 
 URLRequestContextConfig::~URLRequestContextConfig() {}
 
@@ -143,6 +149,8 @@ void URLRequestContextConfig::ParseAndSetExperimentalOptions(
     net::URLRequestContextBuilder* context_builder,
     net::NetLog* net_log,
     const scoped_refptr<base::SequencedTaskRunner>& file_task_runner) {
+  nqe_params.reset(new net::NetworkQualityEstimatorParams(

[mgersh, 2017/06/28 18:26:50]

base::MakeUnique is preferred over raw new.

[tbansal1, 2017/06/29 01:17:58]

Done.

+      std::map<std::string, std::string>()));
   if (experimental_options.empty())
     return;
 
@@ -325,6 +333,42 @@ void URLRequestContextConfig::ParseAndSetExperimentalOptions(
                                                  file_task_runner);
         }
       }
+    } else if (it.key() == kNetworkQualityEstimatorFieldTrialName) {
+      const base::DictionaryValue* nqe_args = nullptr;
+      if (!it.value().GetAsDictionary(&nqe_args)) {
+        LOG(ERROR) << "\"" << it.key() << "\" config params \"" << it.value()
+                   << "\" is not a dictionary value";
+        effective_experimental_options->Remove(it.key(), nullptr);
+        continue;
+      }
+
+      std::string nqe_option;
+      if (nqe_args->GetString("persistent_cache_reading_enabled",

[mgersh, 2017/06/28 18:26:50]

This string can be moved to the top with the other string constants.

[tbansal1, 2017/06/29 01:17:58]

Done.

+                              &nqe_option)) {
+        nqe_params->set_persistent_cache_reading_enabled(
+            nqe_option == "true" ? true : false);

[mgersh, 2017/06/28 18:26:50]

This option could be a boolean instead of a string.

[tbansal1, 2017/06/29 01:17:58]

Thanks, that's much better.

+      }
+
+      if (nqe_args->GetString("effective_connection_type_algorithm",

[mgersh, 2017/06/28 18:26:50]

This string too.

[tbansal1, 2017/06/29 01:17:58]

Done.

+                              &nqe_option)) {
+        nqe_params->SetEffectiveConnectionTypeAlgorithm(
+            net::NetworkQualityEstimatorParams::
+                GetEffectiveConnectionTypeAlgorithmFromString(nqe_option));
+      }
+
+      if (nqe_args->GetString(net::kForceEffectiveConnectionType,
+                              &nqe_option)) {
+        net::EffectiveConnectionType forced_effective_connection_type =
+            net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
+        bool effective_connection_type_available =
+            net::GetEffectiveConnectionTypeForName(
+                nqe_option, &forced_effective_connection_type);
+        DCHECK(effective_connection_type_available);

[mgersh, 2017/06/28 18:26:50]

This depends on a user-provided value being correct, so it shouldn't be a
DCHECK, which is for conditions that should be impossible. I'd check it and log
a warning if it fails, and then only set the option in params if it succeeds.

[tbansal1, 2017/06/29 01:17:58]

Done.

+        ALLOW_UNUSED_LOCAL(effective_connection_type_available);
+        nqe_params->SetForcedEffectiveConnectionType(
+            forced_effective_connection_type);
+      }
+
     } else {
       LOG(WARNING) << "Unrecognized Cronet experimental option \"" << it.key()
                    << "\" with params \"" << it.value();