Simple fallback implementation.

chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc

 // Copyright 2013 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/net/spdyproxy/data_reduction_proxy_settings.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
@@ skipping 132 matching lines @@
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kSpdyProxyAuthOrigin))
     return command_line.GetSwitchValueASCII(switches::kSpdyProxyAuthOrigin);
 #if defined(SPDY_PROXY_AUTH_ORIGIN)
   return SPDY_PROXY_AUTH_ORIGIN;
 #else
   return std::string();
 #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();
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kSpdyProxyAuthOrigin)) {
     // If an origin is provided via a switch, then only consider the value
     // that is provided by a switch. Do not use the preprocessor constant.
     // Don't expose SPDY_PROXY_AUTH_VALUE to a proxy passed in via the command
     // line.
     if (command_line.HasSwitch(switches::kSpdyProxyAuthValue))
@@ skipping 67 matching lines @@
   }
   DVLOG(1) << "The data reduction proxy is blocked.";
 
   if (enabled_by_user_ && !disabled_by_carrier_) {
     // Disable the proxy.
     SetProxyConfigs(false, false);
   }
   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());
     ProbeWhetherDataReductionProxyIsAvailable();
   }
 }
 
 void DataReductionProxySettings::OnProxyEnabledPrefChange() {
   if (!IsDataReductionProxyAllowed())
     return;
@@ skipping 56 matching lines @@
   PrefService* prefs = GetOriginalProfilePrefs();
 
   // TODO(marq): Consider moving this so stats are wiped the first time the
   // proxy settings are actually (not maybe) turned on.
   if (spdy_proxy_auth_enabled_.GetValue() &&
       !prefs->GetBoolean(prefs::kSpdyProxyAuthWasEnabledBefore)) {
     prefs->SetBoolean(prefs::kSpdyProxyAuthWasEnabledBefore, true);
     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.
   if (enabled_by_user_)
     ProbeWhetherDataReductionProxyIsAvailable();
 }
 
 void DataReductionProxySettings::SetProxyConfigs(bool enabled,
                                                  bool at_startup) {
   LogProxyState(enabled, at_startup);
   PrefService* prefs = GetOriginalProfilePrefs();
   DCHECK(prefs);
   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));
     dict->SetString("bypass_list", JoinString(bypass_rules_, ", "));
   } else {
     dict->SetString("mode", ProxyModeToString(ProxyPrefs::MODE_SYSTEM));
     dict->SetString("server", "");
     dict->SetString("bypass_list", "");
   }
 }
@@ skipping 95 matching lines @@
 }
 
 void
 DataReductionProxySettings::ProbeWhetherDataReductionProxyIsAvailable() {
   net::URLFetcher* fetcher = GetURLFetcher();
   if (!fetcher)
     return;
   fetcher_.reset(fetcher);
   fetcher_->Start();
 }