Bypassed Bytes UMAs

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 64b2ec1040c4cc533310f3826651f2c22525cdbd..183a37eb16a6716ba988ff9722a6f97c7ca30944 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
@@ -8,6 +8,7 @@
 #include "base/metrics/field_trial.h"
 #include "components/data_reduction_proxy/common/data_reduction_proxy_switches.h"
 #include "net/proxy/proxy_info.h"
+#include "net/proxy/proxy_retry_info.h"
 #include "net/proxy/proxy_service.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
@@ -18,6 +19,8 @@ namespace {
 const char kEnabled[] = "Enabled";
 }
 
+using namespace net;

[bengr, 2014/07/19 00:13:00]

Do you need this? If so, specify each class you are using, e.g.,
using net::URLRequest;

I typically do this if I'm referring to the class more than one or two times.

[megjablon, 2014/07/21 19:44:44]

Done.

+
 namespace data_reduction_proxy {
 
 // static
@@ -161,7 +164,6 @@ bool DataReductionProxyParams::Init(
 
 }
 
-
 void DataReductionProxyParams::InitWithoutChecks() {
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   std::string origin;
@@ -329,6 +331,90 @@ std::string DataReductionProxyParams::GetDefaultDevOrigin() const {
   return std::string();
 }
 
+bool DataReductionProxyParams::WereDataReductionProxiesBypassed(
+    const net::URLRequest& request, int64* delay_seconds) const {

[bengr, 2014/07/19 00:13:00]

why is this an int64 and not a base::TimeDelta?
Also, why an int64? That's a lot of seconds.

[megjablon, 2014/07/21 19:44:44]

We were using an int64 before in proxy metrics so I just kept it the same.
Changed.

+  DCHECK(request.context());
+  DCHECK(request.context()->proxy_service());

[bengr, 2014/07/19 00:13:00]

So this should never be called if the request does not have a valid proxy
service? 

[megjablon, 2014/07/21 19:44:44]

Switching back to how we checked previously.

+
+  const net::ProxyRetryInfoMap& retry_map =
+      request.context()->proxy_service()->proxy_retry_info();
+
+  bool ssl = request.url().SchemeIs(url::kHttpsScheme);
+
+  return WereProxiesBypassed(retry_map, ssl, delay_seconds);

[bengr, 2014/07/19 00:13:00]

Why not just:

return
WereProxiesBypassed(request.context()->proxy_service()->proxy_retry_info(),
                           request.url().SchemeIs(url::kHttpsScheme),
                           delay_seconds);

[megjablon, 2014/07/21 19:44:44]

Done.

+}
+
+bool DataReductionProxyParams::WereProxiesBypassed(
+    const net::ProxyRetryInfoMap& retry_map,
+    bool ssl,

[bengr, 2014/07/19 00:13:00]

rename ssl -> is_https

[megjablon, 2014/07/21 19:44:44]

Done.

+    int64* delay_seconds) const {
+  if (retry_map.size() == 0)
+    return false;
+
+  if (ssl && alt_allowed_) {
+    if (WerePrimaryAndFallbackBypassed(
+        retry_map, ssl_origin_, GURL(), delay_seconds)) {
+      return true;
+    }
+  } else {
+    if (allowed_) {
+      if (WerePrimaryAndFallbackBypassed(retry_map,
+                                         origin_,
+                                         fallback_origin_,
+                                         delay_seconds)) {
+        return true;
+      }
+    }
+
+    if (alt_allowed_) {
+      if (WerePrimaryAndFallbackBypassed(retry_map,
+                                         alt_origin_,
+                                         alt_fallback_origin_,
+                                         delay_seconds)) {
+        return true;
+      }
+    }
+  }
+
+    return false;

[bengr, 2014/07/19 00:13:00]

indentation here and below.

[megjablon, 2014/07/21 19:44:44]

Done.

+  }
+
+
+bool DataReductionProxyParams::WerePrimaryAndFallbackBypassed(
+    const net::ProxyRetryInfoMap& retry_map,
+    GURL primary,

[bengr, 2014/07/19 00:13:00]

const GURL&

[megjablon, 2014/07/21 19:44:44]

Done.

+    GURL fallback,
+    int64* delay_seconds) const {
+  int64 shortest_delay = 0;
+  net::ProxyRetryInfoMap::const_iterator found;
+
+  std::string proxy = net::HostPortPair::FromURL(primary).ToString() + "/";
+  // The retry list has the scheme prefix for https but not for http.

[bengr, 2014/07/19 00:13:01]

Maybe a better way to do this would be to create a ProxyServer from the GURL and
then search for ProxyServer::ToURI() in the retry_map. 

See:
https://code.google.com/p/chromium/codesearch#chromium/src/net/proxy/proxy_se...

Something like:

retry_map.find(ProxyServer(primary.SchemeIs("https") ? SCHEME_HTTPS :
SCHEME_HTTP,
                           HostPortPair::FromURL(primary))).ToURI()

[megjablon, 2014/07/21 19:44:44]

Done.

+  if (primary.SchemeIs(url::kHttpsScheme))
+    proxy = std::string("https://") + proxy;
+  found = retry_map.find(proxy);
+  if (!(found == retry_map.end())) {
+    shortest_delay = found->second.current_delay.InSeconds();
+    if (fallback_allowed_ && fallback.is_valid()) {
+      proxy = net::HostPortPair::FromURL(fallback).ToString() + "/";
+      found = retry_map.find(proxy);
+      if (!(found == retry_map.end())) {
+        if(shortest_delay > found->second.current_delay.InSeconds())
+          shortest_delay = found->second.current_delay.InSeconds();
+        if (delay_seconds != NULL)
+          *delay_seconds = shortest_delay;
+        return true;
+      }
+    } else {
+      if (delay_seconds != NULL)
+        *delay_seconds = shortest_delay;
+      return true;
+    }
+  }
+
+  return false;
+}
+
 std::string DataReductionProxyParams::GetDefaultOrigin() const {
 #if defined(SPDY_PROXY_AUTH_ORIGIN)
   return SPDY_PROXY_AUTH_ORIGIN;