Removed data compression UMA from ProxyService

This change removes logic specific to the data reduction proxy
from net::ProxyService and net::ProxyServer. To do so, it introduces
a new method, net::NetworkDelegate::OnProxyFallback.

BUG=396699

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

Cr-Commit-Position: refs/heads/master@{#288603}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288603 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 84 insertions, 154 deletions

diff --git a/net/proxy/proxy_server.cc b/net/proxy/proxy_server.cc
index b0997e0..09c2538 100644
--- a/net/proxy/proxy_server.cc
+++ b/net/proxy/proxy_server.cc
@@ -219,27 +219,6 @@ ProxyServer::Scheme ProxyServer::GetSchemeFromURI(const std::string& scheme) {
   return GetSchemeFromURIInternal(scheme.begin(), scheme.end());
 }
 
-// TODO(bengr): Use |scheme_| to indicate that this is the data reduction proxy.
-#if defined(SPDY_PROXY_AUTH_ORIGIN)
-bool ProxyServer::isDataReductionProxy() const {
-    bool dev_host = false;
-#if defined (DATA_REDUCTION_DEV_HOST)
-    dev_host = host_port_pair_.Equals(
-        HostPortPair::FromURL(GURL(DATA_REDUCTION_DEV_HOST)));
-#endif
-  return dev_host || host_port_pair_.Equals(
-      HostPortPair::FromURL(GURL(SPDY_PROXY_AUTH_ORIGIN)));
-}
-
-bool ProxyServer::isDataReductionProxyFallback() const {
-#if defined(DATA_REDUCTION_FALLBACK_HOST)
-  return host_port_pair_.Equals(
-      HostPortPair::FromURL(GURL(DATA_REDUCTION_FALLBACK_HOST)));
-#endif  // defined(DATA_REDUCTION_FALLBACK_HOST)
-  return false;
-}
-#endif  // defined(SPDY_PROXY_AUTH_ORIGIN)
-
 // static
 ProxyServer ProxyServer::FromSchemeHostAndPort(
     Scheme scheme,
diff --git a/net/proxy/proxy_server.h b/net/proxy/proxy_server.h
index 27b8b04..1ff6536 100644
--- a/net/proxy/proxy_server.h
+++ b/net/proxy/proxy_server.h
@@ -154,14 +154,6 @@ class NET_EXPORT ProxyServer {
     return host_port_pair_ < other.host_port_pair_;
   }
 
-#if defined(SPDY_PROXY_AUTH_ORIGIN)
-  // Returns true if this proxy server is the data reduction proxy or its
-  // fallback, respectively, as configured in gyp. These functions will return
-  // false for data reduction proxy servers specified on the command line.
-  bool isDataReductionProxy() const;
-  bool isDataReductionProxyFallback() const;
-#endif  // defined(SPDY_PROXY_AUTH_ORIGIN)
-
  private:
   // Creates a ProxyServer given a scheme, and host/port string. If parsing the
   // host/port string fails, the returned instance will be invalid.
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index a1d634a7..4006110 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -13,8 +13,6 @@
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
-#include "base/metrics/histogram.h"
-#include "base/metrics/sparse_histogram.h"
 #include "base/strings/string_util.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/values.h"
@@ -1197,6 +1195,7 @@ int ProxyService::ReconsiderProxyAfterError(const GURL& url,
   // want to re-run ResolveProxy in two cases: 1) we have a new config, or 2) a
   // direct connection failed and we never tried the current config.
 
+  DCHECK(result);
   bool re_resolve = result->config_id_ != config_.id();
 
   if (re_resolve) {
@@ -1207,24 +1206,17 @@ int ProxyService::ReconsiderProxyAfterError(const GURL& url,
                         network_delegate, net_log);
   }
 
-#if defined(SPDY_PROXY_AUTH_ORIGIN)
-  if (result->proxy_server().isDataReductionProxy()) {
-    RecordDataReductionProxyBypassInfo(
-        true, false, result->proxy_server(), NETWORK_ERROR);
-    RecordDataReductionProxyBypassOnNetworkError(
-        true, result->proxy_server(), net_error);
-  } else if (result->proxy_server().isDataReductionProxyFallback()) {
-    RecordDataReductionProxyBypassInfo(
-        false, false, result->proxy_server(), NETWORK_ERROR);
-    RecordDataReductionProxyBypassOnNetworkError(
-        false, result->proxy_server(), net_error);
-  }
-#endif
+  DCHECK(!result->is_empty());
+  ProxyServer bad_proxy = result->proxy_server();
 
   // We don't have new proxy settings to try, try to fallback to the next proxy
   // in the list.
   bool did_fallback = result->Fallback(net_log);
 
+  if (network_delegate) {
+      network_delegate->NotifyProxyFallback(bad_proxy, net_error, did_fallback);
+  }
+
   // Return synchronous failure if there is nothing left to fall-back to.
   // TODO(eroman): This is a yucky API, clean it up.
   return did_fallback ? OK : ERR_FAILED;
@@ -1459,53 +1451,6 @@ scoped_ptr<ProxyService::PacPollPolicy>
   return scoped_ptr<PacPollPolicy>(new DefaultPollPolicy());
 }
 
-void ProxyService::RecordDataReductionProxyBypassInfo(
-    bool is_primary,
-    bool bypass_all,
-    const ProxyServer& proxy_server,
-    DataReductionProxyBypassType bypass_type) const {
-  // 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 (bypass_all) {
-    if (is_primary) {
-      UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.BlockTypePrimary",
-                                bypass_type, BYPASS_EVENT_TYPE_MAX);
-    } else {
-      UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.BlockTypeFallback",
-                                bypass_type, BYPASS_EVENT_TYPE_MAX);
-    }
-  } else {
-    if (is_primary) {
-      UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.BypassTypePrimary",
-                                bypass_type, BYPASS_EVENT_TYPE_MAX);
-    } else {
-      UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.BypassTypeFallback",
-                                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));
-}
-
 void ProxyService::OnProxyConfigChanged(
     const ProxyConfig& config,
     ProxyConfigService::ConfigAvailability availability) {
diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h
index baca9da..7b3e5d2 100644
--- a/net/proxy/proxy_service.h
+++ b/net/proxy/proxy_service.h
@@ -273,65 +273,6 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
 
   bool quick_check_enabled() const { return quick_check_enabled_; }
 
-  // Values of the UMA DataReductionProxy.BypassType{Primary|Fallback}
-  // and DataReductionProxy.BlockType{Primary|Fallback} histograms.
-  // This enum must remain synchronized with the enum of the same
-  // name in metrics/histograms/histograms.xml.
-  enum DataReductionProxyBypassType {
-    // Bypass due to explicit instruction for the current request.
-    // Not yet supported.
-    CURRENT_BYPASS = 0,
-
-    // Bypass the proxy for less than one minute.
-    SHORT_BYPASS = 1,
-
-    // Bypass the proxy for one to five minutes.
-    MEDIUM_BYPASS = 2,
-
-    // Bypass the proxy for more than five minutes.
-    LONG_BYPASS = 3,
-
-    // Bypass due to a 4xx missing via header.
-    MISSING_VIA_HEADER_4XX = 4,
-
-    // Bypass due to other missing via header, excluding 4xx errors.
-    MISSING_VIA_HEADER_OTHER = 5,
-
-    // Bypass due to 407 response from proxy without a challenge.
-    MALFORMED_407 = 6,
-
-    // Bypass due to a 500 internal server error
-    STATUS_500_HTTP_INTERNAL_SERVER_ERROR = 7,
-
-    // Bypass because the request URI was too long.
-    STATUS_502_HTTP_BAD_GATEWAY = 8,
-
-    // Bypass due to a 503 response.
-    STATUS_503_HTTP_SERVICE_UNAVAILABLE = 9,
-
-    // Bypass due to any network error.
-    NETWORK_ERROR = 10,
-
-    // This must always be last.
-    BYPASS_EVENT_TYPE_MAX = 11
-  };
-
-  // Records a |DataReductionProxyBypassEventType|  or
-  // |DataReductionProxyBlockEventType| for either the data reduction
-  // proxy (|is_primary| is true) or the data reduction proxy fallback.
-  void RecordDataReductionProxyBypassInfo(
-      bool is_primary,
-      bool bypass_all,
-      const ProxyServer& proxy_server,
-      DataReductionProxyBypassType block_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);
-
  private:
   FRIEND_TEST_ALL_PREFIXES(ProxyServiceTest, UpdateConfigAfterFailedAutodetect);
   FRIEND_TEST_ALL_PREFIXES(ProxyServiceTest, UpdateConfigFromPACToDirect);
diff --git a/net/proxy/proxy_service_unittest.cc b/net/proxy/proxy_service_unittest.cc
index 69ab4ce..a2dcf3f 100644
--- a/net/proxy/proxy_service_unittest.cc
+++ b/net/proxy/proxy_service_unittest.cc
@@ -193,6 +193,48 @@ class TestResolveProxyNetworkDelegate : public NetworkDelegate {
   bool remove_proxy_;
 };
 
+// A test network delegate that exercises the OnProxyFallback callback.
+class TestProxyFallbackNetworkDelegate : public NetworkDelegate {
+ public:
+  TestProxyFallbackNetworkDelegate()
+      : on_proxy_fallback_called_(false),
+        proxy_fallback_net_error_(0),
+        proxy_did_fallback_(false) {
+  }
+
+  virtual void OnProxyFallback(
+      const ProxyServer& proxy_server,
+      int net_error,
+      bool did_fallback) OVERRIDE {
+    proxy_server_ = proxy_server;
+    proxy_fallback_net_error_ = net_error;
+    proxy_did_fallback_ = did_fallback;
+    on_proxy_fallback_called_ = true;
+  }
+
+  bool on_proxy_fallback_called() const {
+    return on_proxy_fallback_called_;
+  }
+
+  const ProxyServer& proxy_server() const {
+    return proxy_server_;
+  }
+
+  int proxy_fallback_net_error() const {
+    return proxy_fallback_net_error_;
+  }
+
+  bool proxy_did_fallback() const {
+    return proxy_did_fallback_;
+  }
+
+ private:
+  bool on_proxy_fallback_called_;
+  ProxyServer proxy_server_;
+  int proxy_fallback_net_error_;
+  bool proxy_did_fallback_;
+};
+
 }  // namespace
 
 TEST_F(ProxyServiceTest, Direct) {
@@ -462,13 +504,20 @@ TEST_F(ProxyServiceTest, PAC_FailoverWithoutDirect) {
   // Now, imagine that connecting to foopy:8080 fails: there is nothing
   // left to fallback to, since our proxy list was NOT terminated by
   // DIRECT.
+  TestProxyFallbackNetworkDelegate network_delegate;
   TestCompletionCallback callback2;
+  ProxyServer expected_proxy_server = info.proxy_server();
   rv = service.ReconsiderProxyAfterError(
       url, net::LOAD_NORMAL, net::ERR_PROXY_CONNECTION_FAILED,
-      &info, callback2.callback(), NULL, NULL, BoundNetLog());
+      &info, callback2.callback(), NULL, &network_delegate, BoundNetLog());
   // ReconsiderProxyAfterError returns error indicating nothing left.
   EXPECT_EQ(ERR_FAILED, rv);
   EXPECT_TRUE(info.is_empty());
+  EXPECT_TRUE(network_delegate.on_proxy_fallback_called());
+  EXPECT_EQ(expected_proxy_server, network_delegate.proxy_server());
+  EXPECT_EQ(net::ERR_PROXY_CONNECTION_FAILED,
+            network_delegate.proxy_fallback_net_error());
+  EXPECT_FALSE(network_delegate.proxy_did_fallback());
 }
 
 // Test that if the execution of the PAC script fails (i.e. javascript runtime
@@ -561,6 +610,7 @@ TEST_F(ProxyServiceTest, PAC_FailoverAfterDirect) {
   EXPECT_TRUE(info.is_direct());
 
   // Fallback 1.
+  TestProxyFallbackNetworkDelegate network_delegate2;
   TestCompletionCallback callback2;
   rv = service.ReconsiderProxyAfterError(url, net::LOAD_NORMAL,
                                          net::ERR_PROXY_CONNECTION_FAILED,
@@ -569,34 +619,57 @@ TEST_F(ProxyServiceTest, PAC_FailoverAfterDirect) {
   EXPECT_EQ(OK, rv);
   EXPECT_FALSE(info.is_direct());
   EXPECT_EQ("foobar:10", info.proxy_server().ToURI());
+  // No network delegate provided.
+  EXPECT_FALSE(network_delegate2.on_proxy_fallback_called());
 
   // Fallback 2.
+  TestProxyFallbackNetworkDelegate network_delegate3;
+  ProxyServer expected_proxy_server3 = info.proxy_server();
   TestCompletionCallback callback3;
   rv = service.ReconsiderProxyAfterError(url, net::LOAD_NORMAL,
                                          net::ERR_PROXY_CONNECTION_FAILED,
                                          &info, callback3.callback(), NULL,
-                                         NULL, BoundNetLog());
+                                         &network_delegate3, BoundNetLog());
   EXPECT_EQ(OK, rv);
   EXPECT_TRUE(info.is_direct());
+  EXPECT_TRUE(network_delegate3.on_proxy_fallback_called());
+  EXPECT_EQ(expected_proxy_server3, network_delegate3.proxy_server());
+  EXPECT_EQ(net::ERR_PROXY_CONNECTION_FAILED,
+            network_delegate3.proxy_fallback_net_error());
+  EXPECT_TRUE(network_delegate3.proxy_did_fallback());
 
   // Fallback 3.
+  TestProxyFallbackNetworkDelegate network_delegate4;
+  ProxyServer expected_proxy_server4 = info.proxy_server();
   TestCompletionCallback callback4;
   rv = service.ReconsiderProxyAfterError(url, net::LOAD_NORMAL,
                                          net::ERR_PROXY_CONNECTION_FAILED,
                                          &info, callback4.callback(), NULL,
-                                         NULL, BoundNetLog());
+                                         &network_delegate4, BoundNetLog());
   EXPECT_EQ(OK, rv);
   EXPECT_FALSE(info.is_direct());
   EXPECT_EQ("foobar:20", info.proxy_server().ToURI());
+  EXPECT_TRUE(network_delegate4.on_proxy_fallback_called());
+  EXPECT_EQ(expected_proxy_server4, network_delegate4.proxy_server());
+  EXPECT_EQ(net::ERR_PROXY_CONNECTION_FAILED,
+            network_delegate4.proxy_fallback_net_error());
+  EXPECT_TRUE(network_delegate4.proxy_did_fallback());
 
   // Fallback 4 -- Nothing to fall back to!
+  TestProxyFallbackNetworkDelegate network_delegate5;
+  ProxyServer expected_proxy_server5 = info.proxy_server();
   TestCompletionCallback callback5;
   rv = service.ReconsiderProxyAfterError(url, net::LOAD_NORMAL,
                                          net::ERR_PROXY_CONNECTION_FAILED,
                                          &info, callback5.callback(), NULL,
-                                         NULL, BoundNetLog());
+                                         &network_delegate5, BoundNetLog());
   EXPECT_EQ(ERR_FAILED, rv);
   EXPECT_TRUE(info.is_empty());
+  EXPECT_TRUE(network_delegate5.on_proxy_fallback_called());
+  EXPECT_EQ(expected_proxy_server5, network_delegate5.proxy_server());
+  EXPECT_EQ(net::ERR_PROXY_CONNECTION_FAILED,
+            network_delegate5.proxy_fallback_net_error());
+  EXPECT_FALSE(network_delegate5.proxy_did_fallback());
 }
 
 TEST_F(ProxyServiceTest, PAC_ConfigSourcePropagates) {