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

chrome/browser/net/pref_proxy_config_tracker_impl.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/prefs/proxy_config_dictionary.h"
 #include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "net/proxy/proxy_list.h"
 #include "net/proxy/proxy_server.h"
 
 using content::BrowserThread;
 
 namespace {
 
 // Determine if |proxy| is of the form "*.googlezip.net".
 bool IsGooglezipDataReductionProxy(const net::ProxyServer& proxy) {
   return proxy.is_valid() && !proxy.is_direct() &&
          EndsWith(proxy.host_port_pair().host(), ".googlezip.net", true);
 }
 
 // Removes any Data Reduction Proxies like *.googlezip.net from |proxy_list|.
-void RemoveGooglezipDataReductionProxiesFromList(net::ProxyList* proxy_list) {
-  if (proxy_list->IsEmpty())
-    return;
-
+// Returns true if any proxies were removed from |proxy_list|, false otherwise.
+bool RemoveGooglezipDataReductionProxiesFromList(net::ProxyList* proxy_list) {
   bool found_googlezip_proxy = false;
   for (const net::ProxyServer& proxy : proxy_list->GetAll()) {
     if (IsGooglezipDataReductionProxy(proxy)) {
       found_googlezip_proxy = true;
       break;
     }
   }
   if (!found_googlezip_proxy)
-    return;
+    return false;
 
   net::ProxyList replacement_list;
   for (const net::ProxyServer& proxy : proxy_list->GetAll()) {
     if (!IsGooglezipDataReductionProxy(proxy))
       replacement_list.AddProxyServer(proxy);
   }
 
   if (replacement_list.IsEmpty())
     replacement_list.AddProxyServer(net::ProxyServer::Direct());
   *proxy_list = replacement_list;
+  return true;
+}
+
+void RecordGooglezipProxyRemovalResultHistogram(
+    PrefProxyConfigTrackerImpl::GooglezipProxyRemovalResult result) {
+  UMA_HISTOGRAM_ENUMERATION(
+      "Net.PrefProxyConfig.GooglezipProxyRemovalResult", result,
+      PrefProxyConfigTrackerImpl::GOOGLEZIP_PROXY_REMOVAL_RESULT_MAX);
 }
 
 // Remove any Data Reduction Proxies like *.googlezip.net from |proxy_rules|.
 // This is to prevent a Data Reduction Proxy from being activated in an
 // unsupported way, such as from a proxy pref, which could cause Chrome to use
 // 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);
-  RemoveGooglezipDataReductionProxiesFromList(&proxy_rules->single_proxies);
+  if (RemoveGooglezipDataReductionProxiesFromList(
+          &proxy_rules->fallback_proxies) |

[Alexei Svitkine (slow), 2015/05/14 20:46:34]

Nit: Are you using bitwise OR's to avoid short-circuit evaluation of logical
ORs? If so, add a comment, so someone doesn't go and "clean it up" later.
Otherwise, use logical ORs.

[sclittle, 2015/05/14 20:56:29]

Done.

+      RemoveGooglezipDataReductionProxiesFromList(
+          &proxy_rules->proxies_for_ftp) |
+      RemoveGooglezipDataReductionProxiesFromList(
+          &proxy_rules->proxies_for_http) |
+      RemoveGooglezipDataReductionProxiesFromList(
+          &proxy_rules->proxies_for_https) |
+      RemoveGooglezipDataReductionProxiesFromList(
+          &proxy_rules->single_proxies)) {
+    RecordGooglezipProxyRemovalResultHistogram(
+        PrefProxyConfigTrackerImpl::
+            GOOGLEZIP_PROXY_REMOVAL_RESULT_FOUND_AND_REMOVED);
+  } else {
+    RecordGooglezipProxyRemovalResultHistogram(
+        PrefProxyConfigTrackerImpl::GOOGLEZIP_PROXY_REMOVAL_RESULT_NOT_FOUND);
+  }
 }
 
 }  // namespace
 
 //============================= ChromeProxyConfigService =======================
 
 ChromeProxyConfigService::ChromeProxyConfigService(
     net::ProxyConfigService* base_service)
     : base_service_(base_service),
       pref_config_state_(ProxyPrefs::CONFIG_UNSET),
@@ skipping 330 matching lines @@
       (config_state_ != ProxyPrefs::CONFIG_UNSET &&
        !pref_config_.Equals(new_config))) {
     config_state_ = config_state;
     if (config_state_ != ProxyPrefs::CONFIG_UNSET)
       pref_config_ = new_config;
     update_pending_ = true;
   }
   if (update_pending_)
     OnProxyConfigChanged(config_state, new_config);
 }