Independently enable SPDY versions from field trial.

chrome/browser/io_thread.cc

Index: chrome/browser/io_thread.cc
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index ed2f0d670fb2f6e31dd1a7e48ab5c8932fb18b66..e17197c2f1ff0760d29767b9b5a35267aa4dd7d1 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -136,6 +136,7 @@ const char kSpdyFieldTrialName[] = "SPDY";
 const char kSpdyFieldTrialHoldbackGroupNamePrefix[] = "SpdyDisabled";
 const char kSpdyFieldTrialSpdy31GroupNamePrefix[] = "Spdy31Enabled";
 const char kSpdyFieldTrialSpdy4GroupNamePrefix[] = "Spdy4Enabled";
+const char kSpdyFieldTrialParametrizedPrefix[] = "Parametrized";
 
 // Field trial for Cache-Control: stale-while-revalidate directive.
 const char kStaleWhileRevalidateFieldTrialName[] = "StaleWhileRevalidate";
@@ -865,28 +866,12 @@ void IOThread::InitializeNetworkOptions(const base::CommandLine& command_line) {
     if (trial)
       trial->Disable();
   } else {
-    if (command_line.HasSwitch(switches::kTrustedSpdyProxy)) {
-      globals_->trusted_spdy_proxy.set(
-          command_line.GetSwitchValueASCII(switches::kTrustedSpdyProxy));
-    }
-    if (command_line.HasSwitch(switches::kIgnoreUrlFetcherCertRequests))
-      net::URLFetcher::SetIgnoreCertificateRequests(true);
-
-    if (command_line.HasSwitch(switches::kUseSpdy)) {
-      std::string spdy_mode =
-          command_line.GetSwitchValueASCII(switches::kUseSpdy);
-      EnableSpdy(spdy_mode);
-    } else if (command_line.HasSwitch(switches::kEnableSpdy4)) {
-      globals_->next_protos = net::NextProtosSpdy4Http2();
-      globals_->use_alternate_protocols.set(true);
-    } else if (command_line.HasSwitch(switches::kEnableNpnHttpOnly)) {
-      globals_->next_protos = net::NextProtosHttpOnly();
-      globals_->use_alternate_protocols.set(false);
-    } else {
-      // No SPDY command-line flags have been specified. Examine trial groups.
-      ConfigureSpdyFromTrial(
-          base::FieldTrialList::FindFullName(kSpdyFieldTrialName), globals_);
+    std::string group = base::FieldTrialList::FindFullName(kSpdyFieldTrialName);
+    VariationParameters params;
+    if (!variations::GetVariationParams(kSpdyFieldTrialName, &params)) {
+      params.clear();
     }
+    ConfigureSpdyGlobals(command_line, group, params, globals_);
   }
 
   ConfigureTCPFastOpen(command_line);
@@ -930,27 +915,65 @@ void IOThread::ConfigureSdch() {
   }
 }
 
-void IOThread::ConfigureSpdyFromTrial(base::StringPiece spdy_trial_group,
-                                      Globals* globals) {
-  if (spdy_trial_group.starts_with(kSpdyFieldTrialHoldbackGroupNamePrefix)) {
-    // TODO(jgraettinger): Use net::NextProtosHttpOnly() instead?
-    net::HttpStreamFactory::set_spdy_enabled(false);
-  } else if (spdy_trial_group.starts_with(
-                 kSpdyFieldTrialSpdy31GroupNamePrefix)) {
-    globals->next_protos = net::NextProtosSpdy31();
-    globals->use_alternate_protocols.set(true);
-  } else if (spdy_trial_group.starts_with(
-                 kSpdyFieldTrialSpdy4GroupNamePrefix)) {
-    globals->next_protos = net::NextProtosSpdy4Http2();
+// static
+void IOThread::ConfigureSpdyGlobals(
+    const base::CommandLine& command_line,
+    base::StringPiece spdy_trial_group,
+    const VariationParameters& spdy_trial_params,
+    IOThread::Globals* globals) {
+  if (command_line.HasSwitch(switches::kTrustedSpdyProxy)) {
+    globals->trusted_spdy_proxy.set(
+        command_line.GetSwitchValueASCII(switches::kTrustedSpdyProxy));
+  }
+  if (command_line.HasSwitch(switches::kIgnoreUrlFetcherCertRequests))
+    net::URLFetcher::SetIgnoreCertificateRequests(true);
+
+  if (command_line.HasSwitch(switches::kUseSpdy)) {

[Ryan Hamilton, 2015/03/20 23:38:07]

Let's use early-return to clean up this mess :>

if (command_line.HasSwitch(switches::kUseSpdy)) {
  ...
  return;
}

if (command_line.HasSwitch(switches::kEnableSpdy4)) {
  ...
  return;
}

if (command_line.HasSwitch(switches::kEnableNpnHttpOnly)) { 
  ...
  return;
}

if (spdy_trial_group.starts_with(kSpdyFieldTrialHoldbackGroupNamePrefix)) {
  ...
  return;
}

if (spdy_trial_group.starts_with(kSpdyFieldTrialSpdy31GroupNamePrefix)) {
  ...
  return;
}
  
if (spdy_trial_group.starts_with(kSpdyFieldTrialSpdy4GroupNamePrefix)) {
  ...
  return;
}

if (spdy_trial_group.starts_with(kSpdyFieldTrialParametrizedPrefix)) {
  ...
  return;
}

// By default, enable HTTP/2.
globals->next_protos = net::NextProtos(true, true, true, true);
globals->use_alternate_protocols.set(true);
}

[Bence, 2015/03/23 14:19:13]

Done.

+    std::string spdy_mode =
+        command_line.GetSwitchValueASCII(switches::kUseSpdy);
+    EnableSpdy(spdy_mode, globals);

[Ryan Hamilton, 2015/03/20 23:38:07]

Ah, so here's the one place this method is called. Perhaps it should be called
something like ConfigureSpdyGlobalsFromUseSpdyArgument() or some such.

Since it's a helper of ConfigureSpdy, you could also make it a method in an
anon-namespace of this class. Your call.

[Bence, 2015/03/23 14:19:13]

Great idea.  Done.

+  } else if (command_line.HasSwitch(switches::kEnableSpdy4)) {
+    globals->next_protos = net::NextProtos(true, true, true, true);
     globals->use_alternate_protocols.set(true);
+  } else if (command_line.HasSwitch(switches::kEnableNpnHttpOnly)) {
+    globals->next_protos = net::NextProtos(false, false, false, false);
+    globals->use_alternate_protocols.set(false);
   } else {
-    // By default, enable HTTP/2.
-    globals->next_protos = net::NextProtosSpdy4Http2();
-    globals->use_alternate_protocols.set(true);
+    // No SPDY command-line flags have been specified. Examine trial groups.
+    if (spdy_trial_group.starts_with(kSpdyFieldTrialHoldbackGroupNamePrefix)) {
+      // TODO(jgraettinger): Use net::NextProtosHttpOnly() instead?
+      net::HttpStreamFactory::set_spdy_enabled(false);
+    } else if (spdy_trial_group.starts_with(
+                   kSpdyFieldTrialSpdy31GroupNamePrefix)) {
+      globals->next_protos = net::NextProtosSpdy31();
+      globals->use_alternate_protocols.set(true);
+    } else if (spdy_trial_group.starts_with(
+                   kSpdyFieldTrialSpdy4GroupNamePrefix)) {
+      globals->next_protos = net::NextProtos(true, true, true, true);
+      globals->use_alternate_protocols.set(true);
+    } else if (spdy_trial_group.starts_with(
+                   kSpdyFieldTrialParametrizedPrefix)) {
+      bool quic_enabled = LowerCaseEqualsASCII(
+          GetVariationParam(spdy_trial_params, "quic_enabled"), "true");

[Ryan Hamilton, 2015/03/20 23:38:07]

I think this is going to fight with the QUIC field trial which tries to be
responsible for enabling QUIC.

[Bence, 2015/03/23 14:19:13]

Good point.  I have the impression that this is preexisting problem.  I'm
solving it by adding QUIC to next_protos iff enable_quic as determined earlier
by ConfigureQuic.

Done.

+      bool spdy31_enabled = LowerCaseEqualsASCII(
+          GetVariationParam(spdy_trial_params, "spdy31_enabled"), "true");
+      bool h2_14_enabled = LowerCaseEqualsASCII(
+          GetVariationParam(spdy_trial_params, "h2_14_enabled"), "true");
+      bool h2_enabled = LowerCaseEqualsASCII(
+          GetVariationParam(spdy_trial_params, "h2_enabled"), "true");
+      globals->next_protos = net::NextProtos(quic_enabled, spdy31_enabled,
+                                             h2_14_enabled, h2_enabled);
+      globals->use_alternate_protocols.set(true);
+    } else {
+      // By default, enable HTTP/2.
+      globals->next_protos = net::NextProtos(true, true, true, true);
+      globals->use_alternate_protocols.set(true);
+    }
   }
 }
 
-void IOThread::EnableSpdy(const std::string& mode) {
+// static
+void IOThread::EnableSpdy(const std::string& mode, IOThread::Globals* globals) {
   static const char kOff[] = "off";
   static const char kSSL[] = "ssl";
   static const char kDisableSSL[] = "no-ssl";
@@ -979,32 +1002,32 @@ void IOThread::EnableSpdy(const std::string& mode) {
     if (option == kOff) {
       net::HttpStreamFactory::set_spdy_enabled(false);
     } else if (option == kDisableSSL) {
-      globals_->spdy_default_protocol.set(net::kProtoSPDY31);
-      globals_->force_spdy_over_ssl.set(false);
-      globals_->force_spdy_always.set(true);
+      globals->spdy_default_protocol.set(net::kProtoSPDY31);
+      globals->force_spdy_over_ssl.set(false);
+      globals->force_spdy_always.set(true);
     } else if (option == kSSL) {
-      globals_->spdy_default_protocol.set(net::kProtoSPDY31);
-      globals_->force_spdy_over_ssl.set(true);
-      globals_->force_spdy_always.set(true);
+      globals->spdy_default_protocol.set(net::kProtoSPDY31);
+      globals->force_spdy_over_ssl.set(true);
+      globals->force_spdy_always.set(true);
     } else if (option == kDisablePing) {
-      globals_->enable_spdy_ping_based_connection_checking.set(false);
+      globals->enable_spdy_ping_based_connection_checking.set(false);
     } else if (option == kExclude) {
-      globals_->forced_spdy_exclusions.insert(
+      globals->forced_spdy_exclusions.insert(
           net::HostPortPair::FromURL(GURL(value)));
     } else if (option == kDisableCompression) {
-      globals_->enable_spdy_compression.set(false);
+      globals->enable_spdy_compression.set(false);
     } else if (option == kDisableAltProtocols) {
-      globals_->use_alternate_protocols.set(false);
+      globals->use_alternate_protocols.set(false);
     } else if (option == kForceAltProtocols) {
       net::AlternateProtocolInfo pair(443, net::NPN_SPDY_3, 1);
       net::HttpServerPropertiesImpl::ForceAlternateProtocol(pair);
     } else if (option == kSingleDomain) {
       DVLOG(1) << "FORCING SINGLE DOMAIN";
-      globals_->force_spdy_single_domain.set(true);
+      globals->force_spdy_single_domain.set(true);
     } else if (option == kInitialMaxConcurrentStreams) {
       int streams;
       if (base::StringToInt(value, &streams))
-        globals_->initial_max_spdy_concurrent_streams.set(streams);
+        globals->initial_max_spdy_concurrent_streams.set(streams);
     } else if (option.empty() && it == spdy_options.begin()) {
       continue;
     } else {