Move EnableSpdy from HttpNetworkLayer to IOThread.

chrome/browser/io_thread.cc

 // Copyright (c) 2012 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 "chrome/browser/io_thread.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 633 matching lines @@
 
   if (parsed_command_line.HasSwitch(switches::kEnableWebSocketOverSpdy)) {
     // Enable WebSocket over SPDY.
     net::WebSocketJob::set_websocket_over_spdy_enabled(true);
   }
 
   bool used_spdy_switch = false;
   if (parsed_command_line.HasSwitch(switches::kUseSpdy)) {
     std::string spdy_mode =
       parsed_command_line.GetSwitchValueASCII(switches::kUseSpdy);
-    net::HttpNetworkLayer::EnableSpdy(spdy_mode);
+    EnableSpdy(spdy_mode);
     used_spdy_switch = true;
   }
   if (parsed_command_line.HasSwitch(switches::kEnableSpdy3)) {
     net::HttpStreamFactory::EnableNpnSpdy3();
     used_spdy_switch = true;
   } else if (parsed_command_line.HasSwitch(switches::kEnableNpn)) {
     net::HttpStreamFactory::EnableNpnSpdy();
     used_spdy_switch = true;
   } else if (parsed_command_line.HasSwitch(switches::kEnableNpnHttpOnly)) {
     net::HttpStreamFactory::EnableNpnHttpOnly();
     used_spdy_switch = true;
   }
   if (!used_spdy_switch) {
     net::HttpStreamFactory::EnableNpnSpdy3();
   }
 }
 
+void IOThread::EnableSpdy(const std::string& mode) {
+  static const char kOff[] = "off";
+  static const char kSSL[] = "ssl";
+  static const char kDisableSSL[] = "no-ssl";
+  static const char kDisablePing[] = "no-ping";
+  static const char kExclude[] = "exclude";  // Hosts to exclude
+  static const char kDisableCompression[] = "no-compress";
+  static const char kDisableAltProtocols[] = "no-alt-protocols";
+  static const char kForceAltProtocols[] = "force-alt-protocols";
+  static const char kSingleDomain[] = "single-domain";

[Ryan Sleevi, 2012/11/27 20:06:38]

You have many more options documented here than in the header. Is this
intentional (eg: legacy support?) If so, you should clarify this in a
file-comment, perhaps re-ordering these.

Otherwise, you should update the header level comment.

[Ryan Hamilton, 2012/11/27 20:52:53]

No idea what the history here is.  I've updated the comment to reflect the code.

+
+  static const char kInitialMaxConcurrentStreams[] = "init-max-streams";
+
+  std::vector<std::string> spdy_options;
+  base::SplitString(mode, ',', &spdy_options);
+
+  for (std::vector<std::string>::iterator it = spdy_options.begin();
+       it != spdy_options.end(); ++it) {
+    const std::string& element = *it;
+    std::vector<std::string> name_value;
+    base::SplitString(element, '=', &name_value);
+    const std::string& option = name_value[0];
+    const std::string value = name_value.size() > 1 ? name_value[1] : "";
+
+    if (option == kOff) {
+      net::HttpStreamFactory::set_spdy_enabled(false);
+    } else if (option == kDisableSSL) {
+      net::SpdySession::set_default_protocol(net::kProtoSPDY2);
+      net::HttpStreamFactory::set_force_spdy_over_ssl(false);
+      net::HttpStreamFactory::set_force_spdy_always(true);
+    } else if (option == kSSL) {
+      net::SpdySession::set_default_protocol(net::kProtoSPDY2);
+      net::HttpStreamFactory::set_force_spdy_over_ssl(true);
+      net::HttpStreamFactory::set_force_spdy_always(true);
+    } else if (option == kDisablePing) {
+      net::SpdySession::set_enable_ping_based_connection_checking(false);
+    } else if (option == kExclude) {
+      net::HttpStreamFactory::add_forced_spdy_exclusion(value);
+    } else if (option == kDisableCompression) {
+      net::BufferedSpdyFramer::set_enable_compression_default(false);
+    } else if (option == kDisableAltProtocols) {
+      net::HttpStreamFactory::set_use_alternate_protocols(false);
+    } else if (option == kForceAltProtocols) {
+      net::PortAlternateProtocolPair pair;
+      pair.port = 443;
+      pair.protocol = net::NPN_SPDY_2;
+      net::HttpServerPropertiesImpl::ForceAlternateProtocol(pair);
+    } else if (option == kSingleDomain) {
+      net::SpdySessionPool::ForceSingleDomain();
+      LOG(ERROR) << "FORCING SINGLE DOMAIN";

[Ryan Sleevi, 2012/11/27 20:06:38]

Does this *really* need to be a LOG(ERROR)?

[Ryan Hamilton, 2012/11/27 20:52:53]

Changed to DLOG(INFO).

+    } else if (option == kInitialMaxConcurrentStreams) {
+      int streams;
+      if (base::StringToInt(value, &streams) && streams > 0)
+        net::SpdySession::set_init_max_concurrent_streams(streams);
+    } else if (option.empty() && it == spdy_options.begin()) {
+      continue;

[Ryan Sleevi, 2012/11/27 20:06:38]

So ",foo" is a valid option string?

[Ryan Hamilton, 2012/11/27 20:52:53]

Actually that appears to crash.  I've fixed a bug in this method as a result. 
It doesn't seem harmful to allow. An extra trailing or leading "," in the
command line argument.  That being said, if you prefer to remove this case I'm
happy to  fold into the case below.

+    } else {
+      LOG(DFATAL) << "Unrecognized spdy option: " << option;

[Ryan Sleevi, 2012/11/27 20:06:38]

Should you just rewrite this as LOG(ERROR)? Trying to understand under what
situations an end user would ever benefit from the LOG(FATAL) crashing here,
since it's only enabled for debug

[Ryan Hamilton, 2012/11/27 20:52:53]

I'm happy to change this, but in my experience with --use-spdy I would much
prefer a crash when I typo the option because then I know right away that it's
not going what I want.  Otherwise, I might perform some testing and only later
realize that I used the wrong command line option and my tests were invalid.

+    }

[Ryan Sleevi, 2012/11/27 20:06:38]

It strikes me as odd that "off,ssl,no-ssl" is a perfectly valid string here.

[Ryan Hamilton, 2012/11/27 20:52:53]

Yes, this is a very strange piece of code.

+  }
+}
+
 // static
 void IOThread::RegisterPrefs(PrefService* local_state) {
   local_state->RegisterStringPref(prefs::kAuthSchemes,
                                   "basic,digest,ntlm,negotiate,"
                                   "spdyproxy");
   local_state->RegisterBooleanPref(prefs::kDisableAuthNegotiateCnameLookup,
                                    false);
   local_state->RegisterBooleanPref(prefs::kEnableAuthNegotiatePort, false);
   local_state->RegisterStringPref(prefs::kAuthServerWhitelist, "");
   local_state->RegisterStringPref(prefs::kAuthNegotiateDelegateWhitelist, "");
@@ skipping 126 matching lines @@
       new net::HttpNetworkLayer(
           new net::HttpNetworkSession(system_params)));
   globals_->system_ftp_transaction_factory.reset(
       new net::FtpNetworkLayer(globals_->host_resolver.get()));
   globals_->system_request_context.reset(
       ConstructSystemRequestContext(globals_, net_log_));
 
   sdch_manager_->set_sdch_fetcher(
       new SdchDictionaryFetcher(system_url_request_context_getter_.get()));
 }