Record UMA when googlezip proxies are removed from the proxy config.

This change records UMA about how often *.googlezip.net proxies are
found and removed from the effective proxy configuration.

This will allow us to recognize when the temporary fix to remove those
proxies from the proxy config is no longer necessary, and can be safely
deleted.

BUG=488208

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

Cr-Commit-Position: refs/heads/master@{#330131}

chrome/browser/net/pref_proxy_config_tracker_impl.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/net/pref_proxy_config_tracker_impl.h"
 
 #include "base/bind.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
@@ -30,10 +31,8 @@ bool IsGooglezipDataReductionProxy(const net::ProxyServer& proxy) {
 }
 
 // Removes any Data Reduction Proxies like *.googlezip.net from |proxy_list|.
-void RemoveGooglezipDataReductionProxiesFromList(net::ProxyList* proxy_list) {
-  if (proxy_list->IsEmpty())
-    return;
-
+// Returns the number of proxies that were removed from |proxy_list|.
+size_t RemoveGooglezipDataReductionProxiesFromList(net::ProxyList* proxy_list) {
   bool found_googlezip_proxy = false;
   for (const net::ProxyServer& proxy : proxy_list->GetAll()) {
     if (IsGooglezipDataReductionProxy(proxy)) {
@@ -42,17 +41,21 @@ void RemoveGooglezipDataReductionProxiesFromList(net::ProxyList* proxy_list) {
     }
   }
   if (!found_googlezip_proxy)
-    return;
+    return 0;
 
+  size_t num_removed_proxies = 0;
   net::ProxyList replacement_list;
   for (const net::ProxyServer& proxy : proxy_list->GetAll()) {
     if (!IsGooglezipDataReductionProxy(proxy))
       replacement_list.AddProxyServer(proxy);
+    else
+      ++num_removed_proxies;
   }
 
   if (replacement_list.IsEmpty())
     replacement_list.AddProxyServer(net::ProxyServer::Direct());
   *proxy_list = replacement_list;
+  return num_removed_proxies;
 }
 
 // Remove any Data Reduction Proxies like *.googlezip.net from |proxy_rules|.
@@ -61,17 +64,23 @@ void RemoveGooglezipDataReductionProxiesFromList(net::ProxyList* proxy_list) {
 // the Data Reduction Proxy without adding any of the necessary authentication
 // headers or applying the Data Reduction Proxy bypass logic. See
 // http://crbug.com/476610.
-// TODO(sclittle): Add UMA to record how often this method is called, and how
-// often it actually removes a *.googlezip.net proxy. This method should be
-// removed once it stops actually finding and removing *.googlezip.net proxies
-// from the proxy rules.
+// TODO(sclittle): This method should be removed once the UMA indicates that
+// *.googlezip.net proxies are no longer present in the |proxy_rules|.
 void RemoveGooglezipDataReductionProxies(
     net::ProxyConfig::ProxyRules* proxy_rules) {
-  RemoveGooglezipDataReductionProxiesFromList(&proxy_rules->fallback_proxies);
-  RemoveGooglezipDataReductionProxiesFromList(&proxy_rules->proxies_for_ftp);
-  RemoveGooglezipDataReductionProxiesFromList(&proxy_rules->proxies_for_http);
-  RemoveGooglezipDataReductionProxiesFromList(&proxy_rules->proxies_for_https);
+  size_t num_removed_proxies =
+      RemoveGooglezipDataReductionProxiesFromList(
+          &proxy_rules->fallback_proxies) +
+      RemoveGooglezipDataReductionProxiesFromList(
+          &proxy_rules->proxies_for_ftp) +
+      RemoveGooglezipDataReductionProxiesFromList(
+          &proxy_rules->proxies_for_http) +
+      RemoveGooglezipDataReductionProxiesFromList(
+          &proxy_rules->proxies_for_https) +
       RemoveGooglezipDataReductionProxiesFromList(&proxy_rules->single_proxies);
+
+  UMA_HISTOGRAM_COUNTS_100("Net.PrefProxyConfig.GooglezipProxyRemovalCount",
+                           num_removed_proxies);
 }
 
 }  // namespace

chrome/browser/net/pref_proxy_config_tracker_impl_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/testing_pref_service.h"
+#include "base/test/histogram_tester.h"
 #include "chrome/browser/prefs/pref_service_mock_factory.h"
 #include "chrome/browser/prefs/proxy_config_dictionary.h"
 #include "chrome/common/chrome_switches.h"
@@ -310,39 +311,58 @@ TEST_F(PrefProxyConfigTrackerImplTest, ExcludeGooglezipDataReductionProxies) {
       "https://proxy.googlezip.net:443,compress.googlezip.net,"
       "https://proxy-dev.googlezip.net:443,proxy-dev.googlezip.net,"
       "quic://proxy.googlezip.net";
+  const int kNumDataReductionProxies = 5;
 
   struct {
     std::string initial_proxy_rules;
     const char* http_proxy_info;
     const char* https_proxy_info;
     const char* ftp_proxy_info;
+    int expected_num_removed_proxies;
   } test_cases[] = {
       {"http=foopyhttp," + kDataReductionProxies +
            ",direct://;https=foopyhttps," + kDataReductionProxies +
            ",direct://;ftp=foopyftp," + kDataReductionProxies + ",direct://",
        "foopyhttp;direct://",
        "foopyhttps;direct://",
-       "foopyftp;direct://"},
+       "foopyftp;direct://",
+       kNumDataReductionProxies * 3},
 
       {"foopy," + kDataReductionProxies + ",direct://",
        "foopy;direct://",
        "foopy;direct://",
-       "foopy;direct://"},
+       "foopy;direct://",
+       kNumDataReductionProxies},
 
       {"http=" + kDataReductionProxies + ";https=" + kDataReductionProxies +
            ";ftp=" + kDataReductionProxies,
        "direct://",
        "direct://",
-       "direct://"},
+       "direct://",
+       kNumDataReductionProxies * 3},
 
       {"http=" + kDataReductionProxies + ",foopy,direct://",
        "foopy;direct://",
        "direct://",
-       "direct://"},
+       "direct://",
+       kNumDataReductionProxies},
+
+      {"foopy,direct://",
+       "foopy;direct://",
+       "foopy;direct://",
+       "foopy;direct://",
+       0},
+
+      {"direct://",
+       "direct://",
+       "direct://",
+       "direct://",
+       0},
   };
 
   // Test setting the proxy from a user pref.
   for (const auto& test : test_cases) {
+    base::HistogramTester histogram_tester;
     pref_service_->SetUserPref(prefs::kProxy,
                                ProxyConfigDictionary::CreateFixedServers(
                                    test.initial_proxy_rules, std::string()));
@@ -351,6 +371,9 @@ TEST_F(PrefProxyConfigTrackerImplTest, ExcludeGooglezipDataReductionProxies) {
     net::ProxyConfig config;
     EXPECT_EQ(net::ProxyConfigService::CONFIG_VALID,
               proxy_config_service_->GetLatestProxyConfig(&config));
+    histogram_tester.ExpectUniqueSample(
+        "Net.PrefProxyConfig.GooglezipProxyRemovalCount",
+        test.expected_num_removed_proxies, 1);
 
     CheckResolvedProxyMatches(&config, GURL("http://google.com"),
                               test.http_proxy_info);

tools/metrics/histograms/histograms.xml

@@ -20223,6 +20223,14 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="Net.PrefProxyConfig.GooglezipProxyRemovalCount">
+  <owner>sclittle@chromium.org</owner>
+  <summary>
+    Records how many *.googlezip.net Data Reduction Proxies were removed from
+    the effective proxy configuration when a proxy reconfiguration occurs.
+  </summary>
+</histogram>
+
 <histogram name="Net.Priority_High_Latency" units="milliseconds">
   <obsolete>
     Replaced by Net.Priority_High_Latency_b.