[Cronet] Write effective experimental options to NetLog

components/cronet/url_request_context_config.cc

 // Copyright 2014 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 "components/cronet/url_request_context_config.h"
 
 #include <utility>
 
 #include "base/json/json_reader.h"
 #include "base/logging.h"
@@ skipping 71 matching lines @@
 const char kHostResolverRulesFieldTrialName[] = "HostResolverRules";
 const char kHostResolverRules[] = "host_resolver_rules";
 
 // Disable IPv6. This should almost never be necessary because the network stack
 // has IPv6 detection logic. Please do not turn on this option without first
 // reporting a bug. See http://crbug.com/696569 for the currently known issue.
 const char kDisableIPv6[] = "disable_ipv6";
 
 const char kSSLKeyLogFile[] = "ssl_key_log_file";
 
-void ParseAndSetExperimentalOptions(
+// Returns the effective experimental options.
+std::unique_ptr<base::DictionaryValue> ParseAndSetExperimentalOptions(
     const std::string& experimental_options,
     net::URLRequestContextBuilder* context_builder,
     net::NetLog* net_log,
     const scoped_refptr<base::SequencedTaskRunner>& file_task_runner) {
   if (experimental_options.empty())
-    return;
+    return nullptr;
 
   DCHECK(net_log);
 
   DVLOG(1) << "Experimental Options:" << experimental_options;
   std::unique_ptr<base::Value> options =
       base::JSONReader::Read(experimental_options);
 
   if (!options) {
     DCHECK(false) << "Parsing experimental options failed: "
                   << experimental_options;
-    return;
+    return nullptr;
   }
 
   std::unique_ptr<base::DictionaryValue> dict =
       base::DictionaryValue::From(std::move(options));
 
   if (!dict) {
     DCHECK(false) << "Experimental options string is not a dictionary: "
                   << experimental_options;
-    return;
-  }
-
-  const base::DictionaryValue* quic_args = nullptr;
-  if (dict->GetDictionary(kQuicFieldTrialName, &quic_args)) {
-    std::string quic_connection_options;
-    if (quic_args->GetString(kQuicConnectionOptions,
-                             &quic_connection_options)) {
-      context_builder->set_quic_connection_options(
-          net::ParseQuicConnectionOptions(quic_connection_options));
-    }
-
-    // TODO(rtenneti): Delete this option after apps stop using it.
-    // Added this for backward compatibility.
-    bool quic_store_server_configs_in_properties = false;
-    if (quic_args->GetBoolean(kQuicStoreServerConfigsInProperties,
-                              &quic_store_server_configs_in_properties)) {
-      context_builder->set_quic_max_server_configs_stored_in_properties(
-          net::kMaxQuicServersToPersist);
-    }
-
-    int quic_max_server_configs_stored_in_properties = 0;
-    if (quic_args->GetInteger(kQuicMaxServerConfigsStoredInProperties,
-                              &quic_max_server_configs_stored_in_properties)) {
-      context_builder->set_quic_max_server_configs_stored_in_properties(
-          static_cast<size_t>(quic_max_server_configs_stored_in_properties));
-    }
-
-    bool quic_delay_tcp_race = false;
-    if (quic_args->GetBoolean(kQuicDelayTcpRace, &quic_delay_tcp_race)) {
-      context_builder->set_quic_delay_tcp_race(quic_delay_tcp_race);
-    }
-
-    int quic_idle_connection_timeout_seconds = 0;
-    if (quic_args->GetInteger(kQuicIdleConnectionTimeoutSeconds,
-                              &quic_idle_connection_timeout_seconds)) {
-      context_builder->set_quic_idle_connection_timeout_seconds(
-          quic_idle_connection_timeout_seconds);
-    }
-
-    std::string quic_host_whitelist;
-    if (quic_args->GetString(kQuicHostWhitelist, &quic_host_whitelist)) {
-      std::unordered_set<std::string> hosts;
-      for (const std::string& host :
-           base::SplitString(quic_host_whitelist, ",", base::TRIM_WHITESPACE,
-                             base::SPLIT_WANT_ALL)) {
-        hosts.insert(host);
-      }
-      context_builder->set_quic_host_whitelist(hosts);
-    }
-
-    bool quic_close_sessions_on_ip_change = false;
-    if (quic_args->GetBoolean(kQuicCloseSessionsOnIpChange,
-                              &quic_close_sessions_on_ip_change)) {
-      context_builder->set_quic_close_sessions_on_ip_change(
-          quic_close_sessions_on_ip_change);
-    }
-
-    bool quic_migrate_sessions_on_network_change = false;
-    if (quic_args->GetBoolean(kQuicMigrateSessionsOnNetworkChange,
-                              &quic_migrate_sessions_on_network_change)) {
-      context_builder->set_quic_migrate_sessions_on_network_change(
-          quic_migrate_sessions_on_network_change);
-    }
-
-    bool quic_prefer_aes = false;
-    if (quic_args->GetBoolean(kQuicPreferAes, &quic_prefer_aes)) {
-      context_builder->set_quic_prefer_aes(quic_prefer_aes);
-    }
-
-    std::string quic_user_agent_id;
-    if (quic_args->GetString(kQuicUserAgentId, &quic_user_agent_id)) {
-      context_builder->set_quic_user_agent_id(quic_user_agent_id);
-    }
-
-    bool quic_migrate_sessions_early = false;
-    if (quic_args->GetBoolean(kQuicMigrateSessionsEarly,
-                              &quic_migrate_sessions_early)) {
-      context_builder->set_quic_migrate_sessions_early(
-          quic_migrate_sessions_early);
-    }
-
-    bool quic_disable_bidirectional_streams = false;
-    if (quic_args->GetBoolean(kQuicDisableBidirectionalStreams,
-                              &quic_disable_bidirectional_streams)) {
-      context_builder->set_quic_disable_bidirectional_streams(
-          quic_disable_bidirectional_streams);
-    }
-
-    bool quic_race_cert_verification = false;
-    if (quic_args->GetBoolean(kQuicRaceCertVerification,
-                              &quic_race_cert_verification)) {
-      context_builder->set_quic_race_cert_verification(
-          quic_race_cert_verification);
-    }
+    return nullptr;
   }
 
   bool async_dns_enable = false;
   bool stale_dns_enable = false;
   bool host_resolver_rules_enable = false;
   bool disable_ipv6 = false;
   StaleHostResolver::StaleOptions stale_dns_options;
   std::string host_resolver_rules_string;
+  for (base::DictionaryValue::Iterator it(*dict.get()); !it.IsAtEnd();
+       it.Advance()) {
+    if (it.key() == kQuicFieldTrialName) {
+      const base::DictionaryValue* quic_args = nullptr;
+      if (!it.value().GetAsDictionary(&quic_args)) {
+        LOG(ERROR) << "Quic config params \"" << it.value()
+                   << "\" is not a dictionary value";
+        dict->Remove(it.key(), nullptr);
+        continue;
+      }
+      std::string quic_connection_options;
+      if (quic_args->GetString(kQuicConnectionOptions,
+                               &quic_connection_options)) {
+        context_builder->set_quic_connection_options(
+            net::ParseQuicConnectionOptions(quic_connection_options));
+      }
 
-  const base::DictionaryValue* async_dns_args = nullptr;
-  if (dict->GetDictionary(kAsyncDnsFieldTrialName, &async_dns_args))
-    async_dns_args->GetBoolean(kAsyncDnsEnable, &async_dns_enable);
+      // TODO(rtenneti): Delete this option after apps stop using it.
+      // Added this for backward compatibility.
+      bool quic_store_server_configs_in_properties = false;
+      if (quic_args->GetBoolean(kQuicStoreServerConfigsInProperties,
+                                &quic_store_server_configs_in_properties)) {
+        context_builder->set_quic_max_server_configs_stored_in_properties(
+            net::kMaxQuicServersToPersist);
+      }
 
-  const base::DictionaryValue* stale_dns_args = nullptr;
-  if (dict->GetDictionary(kStaleDnsFieldTrialName, &stale_dns_args)) {
-    if (stale_dns_args->GetBoolean(kStaleDnsEnable, &stale_dns_enable) &&
-        stale_dns_enable) {
-      int delay;
-      if (stale_dns_args->GetInteger(kStaleDnsDelayMs, &delay))
-        stale_dns_options.delay = base::TimeDelta::FromMilliseconds(delay);
-      int max_expired_time_ms;
-      if (stale_dns_args->GetInteger(kStaleDnsMaxExpiredTimeMs,
-                                     &max_expired_time_ms)) {
-        stale_dns_options.max_expired_time =
-            base::TimeDelta::FromMilliseconds(max_expired_time_ms);
+      int quic_max_server_configs_stored_in_properties = 0;
+      if (quic_args->GetInteger(
+              kQuicMaxServerConfigsStoredInProperties,
+              &quic_max_server_configs_stored_in_properties)) {
+        context_builder->set_quic_max_server_configs_stored_in_properties(
+            static_cast<size_t>(quic_max_server_configs_stored_in_properties));
       }
-      int max_stale_uses;
-      if (stale_dns_args->GetInteger(kStaleDnsMaxStaleUses, &max_stale_uses))
-        stale_dns_options.max_stale_uses = max_stale_uses;
-      bool allow_other_network;
-      if (stale_dns_args->GetBoolean(kStaleDnsAllowOtherNetwork,
-                                     &allow_other_network)) {
-        stale_dns_options.allow_other_network = allow_other_network;
+
+      bool quic_delay_tcp_race = false;
+      if (quic_args->GetBoolean(kQuicDelayTcpRace, &quic_delay_tcp_race)) {
+        context_builder->set_quic_delay_tcp_race(quic_delay_tcp_race);
       }
+
+      int quic_idle_connection_timeout_seconds = 0;
+      if (quic_args->GetInteger(kQuicIdleConnectionTimeoutSeconds,
+                                &quic_idle_connection_timeout_seconds)) {
+        context_builder->set_quic_idle_connection_timeout_seconds(
+            quic_idle_connection_timeout_seconds);
+      }
+
+      std::string quic_host_whitelist;
+      if (quic_args->GetString(kQuicHostWhitelist, &quic_host_whitelist)) {
+        std::unordered_set<std::string> hosts;
+        for (const std::string& host :
+             base::SplitString(quic_host_whitelist, ",", base::TRIM_WHITESPACE,
+                               base::SPLIT_WANT_ALL)) {
+          hosts.insert(host);
+        }
+        context_builder->set_quic_host_whitelist(hosts);
+      }
+
+      bool quic_close_sessions_on_ip_change = false;
+      if (quic_args->GetBoolean(kQuicCloseSessionsOnIpChange,
+                                &quic_close_sessions_on_ip_change)) {
+        context_builder->set_quic_close_sessions_on_ip_change(
+            quic_close_sessions_on_ip_change);
+      }
+
+      bool quic_migrate_sessions_on_network_change = false;
+      if (quic_args->GetBoolean(kQuicMigrateSessionsOnNetworkChange,
+                                &quic_migrate_sessions_on_network_change)) {
+        context_builder->set_quic_migrate_sessions_on_network_change(
+            quic_migrate_sessions_on_network_change);
+      }
+
+      bool quic_prefer_aes = false;
+      if (quic_args->GetBoolean(kQuicPreferAes, &quic_prefer_aes)) {
+        context_builder->set_quic_prefer_aes(quic_prefer_aes);
+      }
+
+      std::string quic_user_agent_id;
+      if (quic_args->GetString(kQuicUserAgentId, &quic_user_agent_id)) {
+        context_builder->set_quic_user_agent_id(quic_user_agent_id);
+      }
+
+      bool quic_migrate_sessions_early = false;
+      if (quic_args->GetBoolean(kQuicMigrateSessionsEarly,
+                                &quic_migrate_sessions_early)) {
+        context_builder->set_quic_migrate_sessions_early(
+            quic_migrate_sessions_early);
+      }
+
+      bool quic_disable_bidirectional_streams = false;
+      if (quic_args->GetBoolean(kQuicDisableBidirectionalStreams,
+                                &quic_disable_bidirectional_streams)) {
+        context_builder->set_quic_disable_bidirectional_streams(
+            quic_disable_bidirectional_streams);
+      }
+
+      bool quic_race_cert_verification = false;
+      if (quic_args->GetBoolean(kQuicRaceCertVerification,
+                                &quic_race_cert_verification)) {
+        context_builder->set_quic_race_cert_verification(
+            quic_race_cert_verification);
+      }
+    } else if (it.key() == kAsyncDnsFieldTrialName) {
+      const base::DictionaryValue* async_dns_args = nullptr;
+      if (!it.value().GetAsDictionary(&async_dns_args)) {
+        LOG(ERROR) << "\"" << it.key() << "\" config params \"" << it.value()
+                   << "\" is not a dictionary value";
+        dict->Remove(it.key(), nullptr);
+        continue;
+      }
+      async_dns_args->GetBoolean(kAsyncDnsEnable, &async_dns_enable);
+    } else if (it.key() == kStaleDnsFieldTrialName) {
+      const base::DictionaryValue* stale_dns_args = nullptr;
+      if (!it.value().GetAsDictionary(&stale_dns_args)) {
+        LOG(ERROR) << "\"" << it.key() << "\" config params \"" << it.value()
+                   << "\" is not a dictionary value";
+        dict->Remove(it.key(), nullptr);
+        continue;
+      }
+      if (stale_dns_args->GetBoolean(kStaleDnsEnable, &stale_dns_enable) &&
+          stale_dns_enable) {
+        int delay;
+        if (stale_dns_args->GetInteger(kStaleDnsDelayMs, &delay))
+          stale_dns_options.delay = base::TimeDelta::FromMilliseconds(delay);
+        int max_expired_time_ms;
+        if (stale_dns_args->GetInteger(kStaleDnsMaxExpiredTimeMs,
+                                       &max_expired_time_ms)) {
+          stale_dns_options.max_expired_time =
+              base::TimeDelta::FromMilliseconds(max_expired_time_ms);
+        }
+        int max_stale_uses;
+        if (stale_dns_args->GetInteger(kStaleDnsMaxStaleUses, &max_stale_uses))
+          stale_dns_options.max_stale_uses = max_stale_uses;
+        bool allow_other_network;
+        if (stale_dns_args->GetBoolean(kStaleDnsAllowOtherNetwork,
+                                       &allow_other_network)) {
+          stale_dns_options.allow_other_network = allow_other_network;
+        }
+      }
+    } else if (it.key() == kHostResolverRulesFieldTrialName) {
+      const base::DictionaryValue* host_resolver_rules_args = nullptr;
+      if (!it.value().GetAsDictionary(&host_resolver_rules_args)) {
+        LOG(ERROR) << "\"" << it.key() << "\" config params \"" << it.value()
+                   << "\" is not a dictionary value";
+        dict->Remove(it.key(), nullptr);
+        continue;
+      }
+      host_resolver_rules_enable = host_resolver_rules_args->GetString(
+          kHostResolverRules, &host_resolver_rules_string);
+    } else if (it.key() == kDisableIPv6) {
+      if (!it.value().GetAsBoolean(&disable_ipv6)) {
+        LOG(ERROR) << "\"" << it.key() << "\" config params \"" << it.value()
+                   << "\" is not a bool";
+        dict->Remove(it.key(), nullptr);
+        continue;
+      }
+    } else if (it.key() == kSSLKeyLogFile) {
+      std::string ssl_key_log_file_string;
+      if (it.value().GetAsString(&ssl_key_log_file_string)) {
+        DCHECK(file_task_runner);
+        base::FilePath ssl_key_log_file(ssl_key_log_file_string);
+        if (!ssl_key_log_file.empty() && file_task_runner) {
+          // SetSSLKeyLogFile is only safe to call before any SSLClientSockets
+          // are created. This should not be used if there are multiple
+          // CronetEngine.
+          // TODO(xunjieli): Expose this as a stable API after crbug.com/458365
+          // is resolved.
+          net::SSLClientSocket::SetSSLKeyLogFile(ssl_key_log_file,
+                                                 file_task_runner);
+        }
+      }
+    } else {
+      LOG(ERROR) << "Unrecognized Cronet experimental option \"" << it.key()

[kapishnikov, 2017/03/14 14:37:09]

Should we change the error to warning? It could be possible that some
experimental options will be removed in the future, so the new implementation
doesn't have them anymore.

[xunjieli, 2017/03/14 19:03:06]

This doesn't introduce crash. I think this one is an error (comparable to
Log.e() in Android) that clients should take care of. 

[kapishnikov, 2017/03/14 22:39:07]

There can be a scenario when the implementation is updated independently from
the client code. The implementation may not be necessary packaged inside the app
bundle but downloaded from somewhere else and updated independently after the
release of the app. If the implementation is updated to a version that does not
have the experimental option anymore, the app will suddenly start logging the
error but there is nothing that the developer can do. That is why I think that
the logging with the warning level might be better than the error one.

[xunjieli, 2017/03/14 23:10:46]

Done.

+                 << "\" with params \"" << it.value();
+      dict->Remove(it.key(), nullptr);
     }
   }
-
-  const base::DictionaryValue* host_resolver_rules_args = nullptr;
-  if (dict->GetDictionary(kHostResolverRulesFieldTrialName,
-                          &host_resolver_rules_args)) {
-    host_resolver_rules_enable = host_resolver_rules_args->GetString(
-        kHostResolverRules, &host_resolver_rules_string);
-  }
-
-  dict->GetBoolean(kDisableIPv6, &disable_ipv6);
-
   if (async_dns_enable || stale_dns_enable || host_resolver_rules_enable ||
       disable_ipv6) {
-    if (net_log == nullptr) {
-      CHECK(false) << "All DNS-related experiments require NetLog.";
-    }
+    CHECK(net_log) << "All DNS-related experiments require NetLog.";
     std::unique_ptr<net::HostResolver> host_resolver;
     if (stale_dns_enable) {
       DCHECK(!disable_ipv6);
       host_resolver.reset(new StaleHostResolver(
           net::HostResolver::CreateDefaultResolverImpl(net_log),
           stale_dns_options));
     } else {
       host_resolver = net::HostResolver::CreateDefaultResolver(net_log);
     }
     if (disable_ipv6)
       host_resolver->SetDefaultAddressFamily(net::ADDRESS_FAMILY_IPV4);
     if (async_dns_enable)
       host_resolver->SetDnsClientEnabled(true);
     if (host_resolver_rules_enable) {
       std::unique_ptr<net::MappedHostResolver> remapped_resolver(
           new net::MappedHostResolver(std::move(host_resolver)));
       remapped_resolver->SetRulesFromString(host_resolver_rules_string);
       host_resolver = std::move(remapped_resolver);
     }
     context_builder->set_host_resolver(std::move(host_resolver));
   }
-
-  std::string ssl_key_log_file_string;
-  if (dict->GetString(kSSLKeyLogFile, &ssl_key_log_file_string)) {
-    DCHECK(file_task_runner);
-    base::FilePath ssl_key_log_file(ssl_key_log_file_string);
-    if (!ssl_key_log_file.empty() && file_task_runner) {
-      // SetSSLKeyLogFile is only safe to call before any SSLClientSockets are
-      // created. This should not be used if there are multiple CronetEngine.
-      // TODO(xunjieli): Expose this as a stable API after crbug.com/458365 is
-      // resolved.
-      net::SSLClientSocket::SetSSLKeyLogFile(ssl_key_log_file,
-                                             file_task_runner);
-    }
-  }
+  return dict;
 }
 
 }  // namespace
 
 URLRequestContextConfig::QuicHint::QuicHint(const std::string& host,
                                             int port,
                                             int alternate_port)
     : host(host), port(port), alternate_port(alternate_port) {}
 
 URLRequestContextConfig::QuicHint::~QuicHint() {}
@@ skipping 71 matching lines @@
     context_builder->EnableHttpCache(cache_params);
   } else {
     context_builder->DisableHttpCache();
   }
   context_builder->set_user_agent(user_agent);
   context_builder->SetSpdyAndQuicEnabled(enable_spdy, enable_quic);
   context_builder->set_sdch_enabled(enable_sdch);
   if (enable_quic)
     context_builder->set_quic_user_agent_id(quic_user_agent_id);
 
-  ParseAndSetExperimentalOptions(experimental_options, context_builder, net_log,
-                                 file_task_runner);
+  effective_experimental_options = ParseAndSetExperimentalOptions(
+      experimental_options, context_builder, net_log, file_task_runner);
 
   std::unique_ptr<net::CertVerifier> cert_verifier;
   if (mock_cert_verifier) {
     // Because |context_builder| expects CachingCertVerifier, wrap
     // |mock_cert_verifier| into a CachingCertVerifier.
     cert_verifier = base::MakeUnique<net::CachingCertVerifier>(
         std::move(mock_cert_verifier));
   } else {
     // net::CertVerifier::CreateDefault() returns a CachingCertVerifier.
     cert_verifier = net::CertVerifier::CreateDefault();
@@ skipping 11 matching lines @@
       enable_quic, quic_user_agent_id, enable_spdy, enable_sdch, http_cache,
       http_cache_max_size, load_disable_cache, storage_path, user_agent,
       experimental_options, data_reduction_proxy_key,
       data_reduction_primary_proxy, data_reduction_fallback_proxy,
       data_reduction_secure_proxy_check_url, std::move(mock_cert_verifier),
       enable_network_quality_estimator,
       bypass_public_key_pinning_for_local_trust_anchors, cert_verifier_data);
 }
 
 }  // namespace cronet