Simple fallback implementation.

chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc

Index: chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc
diff --git a/chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc b/chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc
index 4318d770545b5b7eb7818aaf49d33ad3db4b43d6..9bd884b0826dd33f8e72e53928777a5efc05393e 100644
--- a/chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc
+++ b/chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc
@@ -150,6 +150,17 @@ std::string DataReductionProxySettings::GetDataReductionProxyOrigin() {
 #endif
 }
 
+std::string DataReductionProxySettings::GetDataReductionProxyFallback() {

[bengr, 2013/10/20 18:19:31]

I think we should enforce that a fallback only has a value if the origin has a
value. If we don't do that, then you'll also have to go through the entire
codebase and change every 'defined(SPDY_PROXY_AUTH_ORIGIN)' to
'defined(SPDY_PROXY_AUTH_ORIGIN) || 'defined(DATA_REDUCTION_FALLBACK_HOST)' 

[marq (ping after 24h), 2013/10/20 20:43:52]

Yeah, good point. When I do the Great Renaming CL I'll refactor things to
distinguish features (proxy, proxy authentication) from parameters (hosts,
keys), which might simplify details like this. For now I'll enforce that this
only returns a value if GetDataReductionProxy() does. 

+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  if (command_line.HasSwitch(switches::kSpdyProxyAuthFallback))
+    return command_line.GetSwitchValueASCII(switches::kSpdyProxyAuthFallback);
+#if defined(DATA_REDUCTION_FALLBACK_HOST)

[bengr, 2013/10/20 18:19:31]

I hate names. If we are going to move over to using names that include "data
reduction," then I think we should do it everywhere. E.g., instead of
kSpdyProxyAuthFallback, we should instead use
"kDataReductionProxyServerFallback". Also, the switch for setting a proxy is
--proxy-server, so I'd love to have some consistency here.

Finally, I believe SPDY_PROXY_AUTH_ORIGIN requires a trailing slash whereas
--proxy-server might not. Again, we should be consistent, but if I recall
correctly, removing the slash will break auth. 

[marq (ping after 24h), 2013/10/20 20:43:52]

This implementation mirrors the one for getDataReductionProxyOrigin(), so no new
brokenness should be introduced here (we might need to adjust the private clank
auth code in another CL). 

[bengr, 2013/10/20 21:51:48]

Let's verify that we don't also need to adjust chromium auth code.

+  return DATA_REDUCTION_FALLBACK_HOST;
+#else
+  return std::string();
+#endif
+}
+

[bengr, 2013/10/20 18:19:31]

We should have a different SPDY_PROXY_AUTH_VALUE for the fallback. This,
unfortunately, will require some changes to the auth logic.

[bengr, 2013/10/20 18:19:31]

We should have a different SPDY_PROXY_AUTH_VALUE for the fallback. This,
unfortunately, will require some changes to the auth logic.

[marq (ping after 24h), 2013/10/20 20:43:52]

Ack.

 std::string DataReductionProxySettings::GetDataReductionProxyAuth() {
   if (!IsDataReductionProxyAllowed())
     return std::string();
@@ -237,17 +248,6 @@ void DataReductionProxySettings::OnURLFetchComplete(
   disabled_by_carrier_ = true;
 }
 
-std::string DataReductionProxySettings::GetDataReductionProxyOriginHostPort() {
-  std::string spdy_proxy = GetDataReductionProxyOrigin();
-  if (spdy_proxy.empty()) {
-    DLOG(ERROR) << "A SPDY proxy has not been set.";
-    return spdy_proxy;
-  }
-  // Remove a trailing slash from the proxy string if one exists as well as
-  // leading HTTPS scheme.
-  return net::HostPortPair::FromURL(GURL(spdy_proxy)).ToString();
-}
-
 void DataReductionProxySettings::OnIPAddressChanged() {
   if (enabled_by_user_) {
     DCHECK(IsDataReductionProxyAllowed());
@@ -324,12 +324,12 @@ void DataReductionProxySettings::MaybeActivateDataReductionProxy(
     ResetDataReductionStatistics();
   }
 
-  std::string spdy_proxy_origin = GetDataReductionProxyOriginHostPort();
-
+  std::string main_proxy = GetDataReductionProxyOrigin();
+  std::string fallback_proxy = GetDataReductionProxyFallback();
   // Configure use of the data reduction proxy if it is enabled and the proxy
   // origin is non-empty.
-  enabled_by_user_=
-      spdy_proxy_auth_enabled_.GetValue() && !spdy_proxy_origin.empty();
+  enabled_by_user_= spdy_proxy_auth_enabled_.GetValue()
+      && ( !main_proxy.empty() || !fallback_proxy.empty() );

[bengr, 2013/10/20 18:19:31]

Requiring both will be easier reason about.

[marq (ping after 24h), 2013/10/20 20:43:52]

Done.

   SetProxyConfigs(enabled_by_user_ && !disabled_by_carrier_, at_startup);
 
   // Check if the proxy has been disabled explicitly by the carrier.
@@ -345,8 +345,11 @@ void DataReductionProxySettings::SetProxyConfigs(bool enabled,
   DictionaryPrefUpdate update(prefs, prefs::kProxy);
   base::DictionaryValue* dict = update.Get();
   if (enabled) {
+    std::string fallback = GetDataReductionProxyFallback();
     std::string proxy_server_config =
-        "http=" + GetDataReductionProxyOrigin() + ",direct://;";
+        "http=" + GetDataReductionProxyOrigin() +
+        (fallback.empty() ? "" : "," + fallback) +
+        ",direct://;";
     dict->SetString("server", proxy_server_config);
     dict->SetString("mode",
                     ProxyModeToString(ProxyPrefs::MODE_FIXED_SERVERS));