Fixed DataReductionProxyParams::AreProxiesBypassed logic

components/data_reduction_proxy/browser/data_reduction_proxy_params.cc

Index: components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_params.cc b/components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
index 1074783a975a472fc2b87274aad69f540b7081be..ae71da1eff854bc70835bd66678b9da380c872c2 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
@@ -400,9 +400,15 @@ bool DataReductionProxyParams::AreProxiesBypassed(
   if (retry_map.size() == 0)
     return false;
 
-  if (is_https && alt_allowed_) {
-    return ArePrimaryAndFallbackBypassed(
-        retry_map, ssl_origin_, GURL(), min_retry_delay);
+  // If the request is https, consider only the ssl proxy.
+  if (is_https) {

[bengr, 2014/08/15 17:58:57]

I see this form in Chrome more often:

if (is_https) {
  if (alt_allowed_) {
    return ArePrimary...();
  }
  NOT_REACHED();
  return false;
}

[megjablon, 2014/08/15 19:04:23]

Done.

+    if (alt_allowed_) {
+      return ArePrimaryAndFallbackBypassed(
+          retry_map, ssl_origin_, GURL(), min_retry_delay);
+    } else {
+      NOTREACHED();
+      return false;
+    }
   }
 
   if (allowed_ && ArePrimaryAndFallbackBypassed(
@@ -423,34 +429,39 @@ bool DataReductionProxyParams::ArePrimaryAndFallbackBypassed(
     const GURL& primary,
     const GURL& fallback,
     base::TimeDelta* min_retry_delay) const {
+  // Look for the primary proxy in the retry map. This must be done before
+  // looking for the fallback in order to assign |min_retry_delay| if the
+  // primary proxy has a shorter delay.
+  base::TimeDelta delay = base::TimeDelta::Max();
   net::ProxyRetryInfoMap::const_iterator found = retry_map.find(
       net::ProxyServer(primary.SchemeIs(url::kHttpsScheme) ?
-          net::ProxyServer::SCHEME_HTTPS : net::ProxyServer::SCHEME_HTTP,
-          net::HostPortPair::FromURL(primary)).ToURI());
-
-  if (found == retry_map.end())
-    return false;
-
-  base::TimeDelta min_delay = found->second.current_delay;
-  if (!fallback_allowed_ || !fallback.is_valid()) {
-    if (min_retry_delay != NULL)
-      *min_retry_delay = min_delay;
-    return true;
+                           net::ProxyServer::SCHEME_HTTPS :
+                           net::ProxyServer::SCHEME_HTTP,
+                       net::HostPortPair::FromURL(primary)).ToURI());
+  if (found != retry_map.end()) {

[bengr, 2014/08/15 17:58:57]

Ah, you don't have to do this work if fallback_allowed_ and min_retry_delay ==
NULL

[megjablon, 2014/08/15 19:04:23]

Done.

+    delay = found->second.current_delay;
   }
 
-  found = retry_map.find(
-      net::ProxyServer(fallback.SchemeIs(url::kHttpsScheme) ?
-          net::ProxyServer::SCHEME_HTTPS : net::ProxyServer::SCHEME_HTTP,
-          net::HostPortPair::FromURL(fallback)).ToURI());
+  if (fallback_allowed_ && fallback.is_valid()) {
+    // If fallback is allowed, only the fallback proxy needs to be on the retry
+    // map to know if there was a bypass. We can reset found and forget if the
+    // primary was on the retry map.
+    found = retry_map.find(
+        net::ProxyServer(fallback.SchemeIs(url::kHttpsScheme) ?
+                             net::ProxyServer::SCHEME_HTTPS :
+                             net::ProxyServer::SCHEME_HTTP,
+                         net::HostPortPair::FromURL(fallback)).ToURI());
+    if (found != retry_map.end() &&
+        delay > found->second.current_delay) {
+      delay = found->second.current_delay;
+    }
+  }
 
-  if (found == retry_map.end())
-    return false;
+  if (min_retry_delay != NULL && found != retry_map.end()) {
+    *min_retry_delay = delay;

[bengr, 2014/08/15 17:58:57]

nit: you could write to min_retury_delay directly and get rid of delay.

[megjablon, 2014/08/15 19:04:23]

Done.

+  }
 
-  if (min_delay > found->second.current_delay)
-    min_delay = found->second.current_delay;
-  if (min_retry_delay != NULL)
-    *min_retry_delay = min_delay;
-  return true;
+  return found != retry_map.end();
 }
 
 std::string DataReductionProxyParams::GetDefaultOrigin() const {