Update UMA to track bypasses due to 4xx responses that are missing the proxy's via header and bypasses due to network errors.

BUG=376148

Review URL: https://codereview.chromium.org/298883011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273839 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 87 insertions, 25 deletions

diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index b2fcc0e..b70ea2c 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -47,6 +47,7 @@
 
 #if defined(SPDY_PROXY_AUTH_ORIGIN)
 #include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
 #endif
 
 using base::TimeDelta;
@@ -1168,6 +1169,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 +1193,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(
+        false, result->proxy_server(), net_error);
   }
 #endif
 
@@ -1439,6 +1445,25 @@ 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_SPARSE_SLOWLY(
+        "DataReductionProxy.BypassOnNetworkErrorPrimary",
+        std::abs(net_error));
+    return;
+  }
+  UMA_HISTOGRAM_SPARSE_SLOWLY(
+      "DataReductionProxy.BypassOnNetworkErrorFallback",
+      std::abs(net_error));
+}
 #endif  // defined(SPDY_PROXY_AUTH_ORIGIN)
 
 void ProxyService::OnProxyConfigChanged(
@@ -1581,13 +1606,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 +1635,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_);
   }
diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h
index 36b4589..6f85568 100644
--- a/net/proxy/proxy_service.h
+++ b/net/proxy/proxy_service.h
@@ -132,8 +132,10 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
   // This method is called after a failure to connect or resolve a host name.
   // It gives the proxy service an opportunity to reconsider the proxy to use.
   // The |results| parameter contains the results returned by an earlier call
-  // to ResolveProxy.  The semantics of this call are otherwise similar to
-  // ResolveProxy.
+  // to ResolveProxy.  The |net_error| parameter contains the network error
+  // code associated with the failure. See "net/base/net_error_list.h" for a
+  // list of possible values. The semantics of this call are otherwise
+  // similar to ResolveProxy.
   //
   // NULL can be passed for |pac_request| if the caller will not need to
   // cancel the request.
@@ -142,6 +144,7 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
   //
   // Profiling information for the request is saved to |net_log| if non-NULL.
   int ReconsiderProxyAfterError(const GURL& url,
+                                int net_error,
                                 ProxyInfo* results,
                                 const CompletionCallback& callback,
                                 PacRequest** pac_request,
@@ -286,6 +289,9 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
     // Bypass the proxy because responses appear not to be coming via it.
     MISSING_VIA_HEADER,
 
+    // Bypass the proxy because the proxy, not the origin, sent a 4xx response.
+    PROXY_4XX_BYPASS,
+
     // This must always be last.
     BYPASS_EVENT_TYPE_MAX
   };
@@ -296,6 +302,13 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
       bool is_primary,
       const ProxyServer& proxy_server,
       DataReductionProxyBypassEventType bypass_type) const;
+
+  // Records a net error code that resulted in bypassing the data reduction
+  // proxy (|is_primary| is true) or the data reduction proxy fallback.
+  void RecordDataReductionProxyBypassOnNetworkError(
+      bool is_primary,
+      const ProxyServer& proxy_server,
+      int net_error);
 #endif
 
  private:
@@ -458,6 +471,7 @@ class NET_EXPORT SyncProxyServiceHelper
                    ProxyInfo* proxy_info,
                    const BoundNetLog& net_log);
   int ReconsiderProxyAfterError(const GURL& url,
+                                int net_error,
                                 ProxyInfo* proxy_info,
                                 const BoundNetLog& net_log);
 
@@ -467,7 +481,9 @@ class NET_EXPORT SyncProxyServiceHelper
   virtual ~SyncProxyServiceHelper();
 
   void StartAsyncResolve(const GURL& url, const BoundNetLog& net_log);
-  void StartAsyncReconsider(const GURL& url, const BoundNetLog& net_log);
+  void StartAsyncReconsider(const GURL& url,
+                            int net_error,
+                            const BoundNetLog& net_log);
 
   void OnCompletion(int result);
 
diff --git a/net/proxy/proxy_service_unittest.cc b/net/proxy/proxy_service_unittest.cc
index bad5afd..cc245b6 100644
--- a/net/proxy/proxy_service_unittest.cc
+++ b/net/proxy/proxy_service_unittest.cc
@@ -315,7 +315,8 @@ TEST_F(ProxyServiceTest, PAC_FailoverWithoutDirect) {
   // DIRECT.
   TestCompletionCallback callback2;
   rv = service.ReconsiderProxyAfterError(
-      url, &info, callback2.callback(), NULL, BoundNetLog());
+      url, net::ERR_PROXY_CONNECTION_FAILED,
+      &info, callback2.callback(), NULL, BoundNetLog());
   // ReconsiderProxyAfterError returns error indicating nothing left.
   EXPECT_EQ(ERR_FAILED, rv);
   EXPECT_TRUE(info.is_empty());
@@ -410,7 +411,8 @@ TEST_F(ProxyServiceTest, PAC_FailoverAfterDirect) {
 
   // Fallback 1.
   TestCompletionCallback callback2;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback2.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback2.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
   EXPECT_FALSE(info.is_direct());
@@ -418,14 +420,16 @@ TEST_F(ProxyServiceTest, PAC_FailoverAfterDirect) {
 
   // Fallback 2.
   TestCompletionCallback callback3;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback3.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback3.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
   EXPECT_TRUE(info.is_direct());
 
   // Fallback 3.
   TestCompletionCallback callback4;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback4.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback4.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
   EXPECT_FALSE(info.is_direct());
@@ -433,7 +437,8 @@ TEST_F(ProxyServiceTest, PAC_FailoverAfterDirect) {
 
   // Fallback 4 -- Nothing to fall back to!
   TestCompletionCallback callback5;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback5.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback5.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(ERR_FAILED, rv);
   EXPECT_TRUE(info.is_empty());
@@ -728,7 +733,8 @@ TEST_F(ProxyServiceTest, ProxyFallback) {
 
   // Fake an error on the proxy.
   TestCompletionCallback callback2;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback2.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback2.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
 
@@ -770,7 +776,8 @@ TEST_F(ProxyServiceTest, ProxyFallback) {
 
   // We fake another error. It should now try the third one.
   TestCompletionCallback callback4;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback4.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback4.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
   EXPECT_EQ("foopy2:9090", info.proxy_server().ToURI());
@@ -779,7 +786,8 @@ TEST_F(ProxyServiceTest, ProxyFallback) {
   // proxy servers we thought were valid; next we try the proxy server
   // that was in our bad proxies map (foopy1:8080).
   TestCompletionCallback callback5;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback5.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback5.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
   EXPECT_EQ("foopy1:8080", info.proxy_server().ToURI());
@@ -787,7 +795,8 @@ TEST_F(ProxyServiceTest, ProxyFallback) {
   // Fake another error, the last proxy is gone, the list should now be empty,
   // so there is nothing left to try.
   TestCompletionCallback callback6;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback6.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback6.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(ERR_FAILED, rv);
   EXPECT_FALSE(info.is_direct());
@@ -861,7 +870,8 @@ TEST_F(ProxyServiceTest, ProxyFallbackToDirect) {
 
   // Fake an error on the proxy.
   TestCompletionCallback callback2;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback2.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback2.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
 
@@ -870,7 +880,8 @@ TEST_F(ProxyServiceTest, ProxyFallbackToDirect) {
 
   // Fake an error on this proxy as well.
   TestCompletionCallback callback3;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback3.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback3.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
 
@@ -883,7 +894,8 @@ TEST_F(ProxyServiceTest, ProxyFallbackToDirect) {
 
   // Now we tell the proxy service that even DIRECT failed.
   TestCompletionCallback callback4;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback4.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback4.callback(), NULL,
                                          BoundNetLog());
   // There was nothing left to try after DIRECT, so we are out of
   // choices.
@@ -931,7 +943,8 @@ TEST_F(ProxyServiceTest, ProxyFallback_NewSettings) {
       ProxyConfig::CreateFromCustomPacURL(GURL("http://foopy-new/proxy.pac")));
 
   TestCompletionCallback callback2;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback2.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback2.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(ERR_IO_PENDING, rv);
 
@@ -952,7 +965,8 @@ TEST_F(ProxyServiceTest, ProxyFallback_NewSettings) {
 
   // We fake another error. It should now ignore the first one.
   TestCompletionCallback callback3;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback3.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback3.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
   EXPECT_EQ("foopy2:9090", info.proxy_server().ToURI());
@@ -964,7 +978,8 @@ TEST_F(ProxyServiceTest, ProxyFallback_NewSettings) {
 
   // We fake another error. It should go back to the first proxy.
   TestCompletionCallback callback4;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback4.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback4.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(ERR_IO_PENDING, rv);
 
@@ -1023,7 +1038,8 @@ TEST_F(ProxyServiceTest, ProxyFallback_BadConfig) {
 
   // Fake a proxy error.
   TestCompletionCallback callback2;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback2.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback2.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
 
@@ -1055,7 +1071,8 @@ TEST_F(ProxyServiceTest, ProxyFallback_BadConfig) {
   // "just work" the next time we call it.
   ProxyInfo info3;
   TestCompletionCallback callback4;
-  rv = service.ReconsiderProxyAfterError(url, &info3, callback4.callback(),
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info3, callback4.callback(),
                                          NULL, BoundNetLog());
   EXPECT_EQ(ERR_IO_PENDING, rv);
 
@@ -1116,7 +1133,8 @@ TEST_F(ProxyServiceTest, ProxyFallback_BadConfigMandatory) {
 
   // Fake a proxy error.
   TestCompletionCallback callback2;
-  rv = service.ReconsiderProxyAfterError(url, &info, callback2.callback(), NULL,
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info, callback2.callback(), NULL,
                                          BoundNetLog());
   EXPECT_EQ(OK, rv);
 
@@ -1149,7 +1167,8 @@ TEST_F(ProxyServiceTest, ProxyFallback_BadConfigMandatory) {
   // "just work" the next time we call it.
   ProxyInfo info3;
   TestCompletionCallback callback4;
-  rv = service.ReconsiderProxyAfterError(url, &info3, callback4.callback(),
+  rv = service.ReconsiderProxyAfterError(url, net::ERR_PROXY_CONNECTION_FAILED,
+                                         &info3, callback4.callback(),
                                          NULL, BoundNetLog());
   EXPECT_EQ(ERR_IO_PENDING, rv);