Record errors that trigger a data reduction proxy bypass

net/proxy/proxy_service.cc

Index: net/proxy/proxy_service.cc
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index b2fcc0e401c953cae4c38089582d667b3f46650f..d1d571594bc60e614a1c345ced4e2c2c37e47025 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -1168,6 +1168,7 @@ void ProxyService::OnInitProxyResolverComplete(int result) {
 }
 
 int ProxyService::ReconsiderProxyAfterError(const GURL& url,
+                                            int net_error,
                                             ProxyInfo* result,
                                             const CompletionCallback& callback,
                                             PacRequest** pac_request,
@@ -1191,9 +1192,13 @@ int ProxyService::ReconsiderProxyAfterError(const GURL& url,
   if (result->proxy_server().isDataReductionProxy()) {
     RecordDataReductionProxyBypassInfo(
         true, result->proxy_server(), ERROR_BYPASS);
+    RecordDataReductionProxyBypassOnNetworkError(
+        true, result->proxy_server(), net_error);
   } else if (result->proxy_server().isDataReductionProxyFallback()) {
     RecordDataReductionProxyBypassInfo(
         false, result->proxy_server(), ERROR_BYPASS);
+    RecordDataReductionProxyBypassOnNetworkError(
+        true, result->proxy_server(), net_error);
   }
 #endif
 
@@ -1439,6 +1444,27 @@ void ProxyService::RecordDataReductionProxyBypassInfo(
                               bypass_type, BYPASS_EVENT_TYPE_MAX);
   }
 }
+
+void ProxyService::RecordDataReductionProxyBypassOnNetworkError(
+    bool is_primary,
+    const ProxyServer& proxy_server,
+    int net_error) {
+  // Only record UMA if the proxy isn't already on the retry list.
+  if (proxy_retry_info_.find(proxy_server.ToURI()) != proxy_retry_info_.end())
+    return;
+
+  if (is_primary) {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+        "DataReductionProxy.BypassOnNetworkErrorPrimary",
+        std::abs(net_error),
+        GetAllErrorCodesForUma());
+    return;
+  }
+  UMA_HISTOGRAM_CUSTOM_ENUMERATION(

[Matt Welsh, 2014/05/23 23:56:30]

Shouldn't there be an else here to prevent double-counting in the is_primary
case?

[bengr, 2014/05/24 01:22:02]

I return in the first case.

+      "DataReductionProxy.BypassOnNetworkErrorFallback",
+      std::abs(net_error),
+      GetAllErrorCodesForUma());
+}
 #endif  // defined(SPDY_PROXY_AUTH_ORIGIN)
 
 void ProxyService::OnProxyConfigChanged(
@@ -1581,13 +1607,14 @@ int SyncProxyServiceHelper::ResolveProxy(const GURL& url,
 }
 
 int SyncProxyServiceHelper::ReconsiderProxyAfterError(
-    const GURL& url, ProxyInfo* proxy_info, const BoundNetLog& net_log) {
+    const GURL& url, int net_error, ProxyInfo* proxy_info,
+    const BoundNetLog& net_log) {
   DCHECK(io_message_loop_ != base::MessageLoop::current());
 
   io_message_loop_->PostTask(
       FROM_HERE,
       base::Bind(&SyncProxyServiceHelper::StartAsyncReconsider, this, url,
-                 net_log));
+                 net_error, net_log));
 
   event_.Wait();
 
@@ -1609,9 +1636,10 @@ void SyncProxyServiceHelper::StartAsyncResolve(const GURL& url,
 }
 
 void SyncProxyServiceHelper::StartAsyncReconsider(const GURL& url,
+                                                  int net_error,
                                                   const BoundNetLog& net_log) {
   result_ = proxy_service_->ReconsiderProxyAfterError(
-      url, &proxy_info_, callback_, NULL, net_log);
+      url, net_error, &proxy_info_, callback_, NULL, net_log);
   if (result_ != net::ERR_IO_PENDING) {
     OnCompletion(result_);
   }