Record UMA about the DRP pref migration action taken.

This change adds a histogram to track the rate at which different Data
Reduction Proxy pref migration actions are taken, including cases where
no migration action is taken.

BUG=488778

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

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

4 files changed, 141 insertions, 23 deletions

diff --git a/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc b/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc
index db900de..f01181a 100644
--- a/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc
+++ b/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc
@@ -7,6 +7,7 @@
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/strings/string_util.h"
@@ -67,42 +68,57 @@ bool ContainsDataReductionProxyDefaultHostSuffix(
 // prefs, if present. |proxy_pref_name| is the name of the proxy pref.
 void DataReductionProxyChromeSettings::MigrateDataReductionProxyOffProxyPrefs(
     PrefService* prefs) {
+  ProxyPrefMigrationResult proxy_pref_status =
+      MigrateDataReductionProxyOffProxyPrefsHelper(prefs);
+  UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.ProxyPrefMigrationResult",
+                            proxy_pref_status,
+                            DataReductionProxyChromeSettings::PROXY_PREF_MAX);
+}
+
+DataReductionProxyChromeSettings::ProxyPrefMigrationResult
+DataReductionProxyChromeSettings::MigrateDataReductionProxyOffProxyPrefsHelper(
+    PrefService* prefs) {
   base::DictionaryValue* dict =
-      (base::DictionaryValue*) prefs->GetUserPrefValue(prefs::kProxy);
+      (base::DictionaryValue*)prefs->GetUserPrefValue(prefs::kProxy);
   if (!dict)
-    return;
+    return PROXY_PREF_NOT_CLEARED;
 
   // Clear empty "proxy" dictionary created by a bug. See http://crbug/448172.
   if (dict->empty()) {
     prefs->ClearPref(prefs::kProxy);
-    return;
+    return PROXY_PREF_CLEARED_EMPTY;
   }
 
   std::string mode;
   if (!dict->GetString("mode", &mode))
-    return;
+    return PROXY_PREF_NOT_CLEARED;
   // Clear "system" proxy entry since this is the default. This entry was
   // created by bug (http://crbug/448172).
   if (ProxyModeToString(ProxyPrefs::MODE_SYSTEM) == mode) {
     prefs->ClearPref(prefs::kProxy);
-    return;
+    return PROXY_PREF_CLEARED_MODE_SYSTEM;
   }
   if (ProxyModeToString(ProxyPrefs::MODE_FIXED_SERVERS) != mode)
-    return;
+    return PROXY_PREF_NOT_CLEARED;
   std::string proxy_server;
   if (!dict->GetString("server", &proxy_server))
-    return;
+    return PROXY_PREF_NOT_CLEARED;
   net::ProxyConfig::ProxyRules proxy_rules;
   proxy_rules.ParseFromString(proxy_server);
   // Clear the proxy pref if it matches a currently configured Data Reduction
   // Proxy, or if the proxy host ends with ".googlezip.net", in order to ensure
   // that any DRP in the pref is cleared even if the DRP configuration was
   // changed. See http://crbug.com/476610.
-  if (!Config()->ContainsDataReductionProxy(proxy_rules) &&
-      !ContainsDataReductionProxyDefaultHostSuffix(proxy_rules)) {
-    return;
-  }
+  ProxyPrefMigrationResult rv;
+  if (Config()->ContainsDataReductionProxy(proxy_rules))
+    rv = PROXY_PREF_CLEARED_DRP;
+  else if (ContainsDataReductionProxyDefaultHostSuffix(proxy_rules))
+    rv = PROXY_PREF_CLEARED_GOOGLEZIP;
+  else
+    return PROXY_PREF_NOT_CLEARED;
+
   prefs->ClearPref(prefs::kProxy);
+  return rv;
 }
 
 DataReductionProxyChromeSettings::DataReductionProxyChromeSettings()
diff --git a/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h b/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h
index c7dc701..c5cf8f1 100644
--- a/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h
+++ b/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h
@@ -31,6 +31,19 @@ class DataReductionProxyChromeSettings
     : public data_reduction_proxy::DataReductionProxySettings,
       public KeyedService {
  public:
+  // Enum values that can be reported for the
+  // DataReductionProxy.ProxyPrefMigrationResult histogram. These values must be
+  // kept in sync with their counterparts in histograms.xml. Visible here for
+  // testing purposes.
+  enum ProxyPrefMigrationResult {
+    PROXY_PREF_NOT_CLEARED = 0,
+    PROXY_PREF_CLEARED_EMPTY,
+    PROXY_PREF_CLEARED_MODE_SYSTEM,
+    PROXY_PREF_CLEARED_DRP,
+    PROXY_PREF_CLEARED_GOOGLEZIP,
+    PROXY_PREF_MAX
+  };
+
   // Constructs a settings object. Construction and destruction must happen on
   // the UI thread.
   DataReductionProxyChromeSettings();
@@ -56,6 +69,12 @@ class DataReductionProxyChromeSettings
   void MigrateDataReductionProxyOffProxyPrefs(PrefService* prefs);
 
  private:
+  // Helper method for migrating the Data Reduction Proxy away from using the
+  // proxy pref. Returns the ProxyPrefMigrationResult value indicating the
+  // migration action taken.
+  ProxyPrefMigrationResult MigrateDataReductionProxyOffProxyPrefsHelper(
+      PrefService* prefs);
+
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyChromeSettings);
 };
 
diff --git a/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_unittest.cc b/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_unittest.cc
index e13020f..a445898 100644
--- a/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_unittest.cc
+++ b/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/testing_pref_service.h"
+#include "base/test/histogram_tester.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h"
 #include "chrome/common/pref_names.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h"
@@ -25,18 +26,6 @@ class DataReductionProxyChromeSettingsTest : public testing::Test {
         make_scoped_ptr(new DataReductionProxyChromeSettings());
     test_context_ =
         data_reduction_proxy::DataReductionProxyTestContext::Builder()
-            .WithParamsFlags(
-                data_reduction_proxy::DataReductionProxyParams::kAllowed |
-                data_reduction_proxy::DataReductionProxyParams::
-                    kFallbackAllowed |
-                data_reduction_proxy::DataReductionProxyParams::kPromoAllowed)
-            .WithParamsDefinitions(
-                data_reduction_proxy::TestDataReductionProxyParams::
-                    HAS_EVERYTHING &
-                ~data_reduction_proxy::TestDataReductionProxyParams::
-                    HAS_DEV_ORIGIN &
-                ~data_reduction_proxy::TestDataReductionProxyParams::
-                    HAS_DEV_FALLBACK_ORIGIN)
             .WithMockConfig()
             .SkipSettingsInitialization()
             .Build();
@@ -55,15 +44,74 @@ class DataReductionProxyChromeSettingsTest : public testing::Test {
   data_reduction_proxy::MockDataReductionProxyConfig* config_;
 };
 
+TEST_F(DataReductionProxyChromeSettingsTest, MigrateNonexistentProxyPref) {
+  base::HistogramTester histogram_tester;
+  EXPECT_CALL(*config_, ContainsDataReductionProxy(_)).Times(0);
+  drp_chrome_settings_->MigrateDataReductionProxyOffProxyPrefs(
+      test_context_->pref_service());
+
+  EXPECT_EQ(NULL, test_context_->pref_service()->GetUserPref(prefs::kProxy));
+  histogram_tester.ExpectUniqueSample(
+      "DataReductionProxy.ProxyPrefMigrationResult",
+      DataReductionProxyChromeSettings::PROXY_PREF_NOT_CLEARED, 1);
+}
+
+TEST_F(DataReductionProxyChromeSettingsTest, MigrateBadlyFormedProxyPref) {
+  const struct {
+    // NULL indicates that mode is unset.
+    const char* proxy_mode_string;
+    // NULL indicates that server is unset.
+    const char* proxy_server_string;
+  } test_cases[] = {
+      // The pref should not be cleared if mode is unset.
+      {nullptr, "http=compress.googlezip.net"},
+      // The pref should not be cleared for modes other than "fixed_servers".
+      {"pac_script", "http=compress.googlezip.net"},
+      // The pref should not be cleared when the server field is unset.
+      {"fixed_servers", nullptr},
+  };
+
+  for (const auto& test : test_cases) {
+    base::HistogramTester histogram_tester;
+    dict_.reset(new base::DictionaryValue());
+    if (test.proxy_mode_string)
+      dict_->SetString("mode", test.proxy_mode_string);
+    if (test.proxy_server_string)
+      dict_->SetString("server", test.proxy_server_string);
+    test_context_->pref_service()->Set(prefs::kProxy, *dict_.get());
+
+    EXPECT_CALL(*config_, ContainsDataReductionProxy(_)).Times(0);
+    drp_chrome_settings_->MigrateDataReductionProxyOffProxyPrefs(
+        test_context_->pref_service());
+
+    const base::DictionaryValue* final_value;
+    test_context_->pref_service()
+        ->GetUserPref(prefs::kProxy)
+        ->GetAsDictionary(&final_value);
+    EXPECT_NE(nullptr, final_value);
+    EXPECT_TRUE(dict_->Equals(final_value));
+
+    histogram_tester.ExpectUniqueSample(
+        "DataReductionProxy.ProxyPrefMigrationResult",
+        DataReductionProxyChromeSettings::PROXY_PREF_NOT_CLEARED, 1);
+  }
+}
+
 TEST_F(DataReductionProxyChromeSettingsTest, MigrateEmptyProxy) {
+  base::HistogramTester histogram_tester;
+  test_context_->pref_service()->Set(prefs::kProxy, *dict_.get());
   EXPECT_CALL(*config_, ContainsDataReductionProxy(_)).Times(0);
   drp_chrome_settings_->MigrateDataReductionProxyOffProxyPrefs(
       test_context_->pref_service());
 
   EXPECT_EQ(NULL, test_context_->pref_service()->GetUserPref(prefs::kProxy));
+  histogram_tester.ExpectUniqueSample(
+      "DataReductionProxy.ProxyPrefMigrationResult",
+      DataReductionProxyChromeSettings::PROXY_PREF_CLEARED_EMPTY, 1);
 }
 
 TEST_F(DataReductionProxyChromeSettingsTest, MigrateSystemProxy) {
+  base::HistogramTester histogram_tester;
   dict_->SetString("mode", "system");
   test_context_->pref_service()->Set(prefs::kProxy, *dict_.get());
   EXPECT_CALL(*config_, ContainsDataReductionProxy(_)).Times(0);
@@ -72,6 +120,9 @@ TEST_F(DataReductionProxyChromeSettingsTest, MigrateSystemProxy) {
       test_context_->pref_service());
 
   EXPECT_EQ(NULL, test_context_->pref_service()->GetUserPref(prefs::kProxy));
+  histogram_tester.ExpectUniqueSample(
+      "DataReductionProxy.ProxyPrefMigrationResult",
+      DataReductionProxyChromeSettings::PROXY_PREF_CLEARED_MODE_SYSTEM, 1);
 }
 
 TEST_F(DataReductionProxyChromeSettingsTest, MigrateDataReductionProxy) {
@@ -80,6 +131,7 @@ TEST_F(DataReductionProxyChromeSettingsTest, MigrateDataReductionProxy) {
                                       "https=https://tunneldrp.com"};
 
   for (const std::string& test_server : kTestServers) {
+    base::HistogramTester histogram_tester;
     dict_.reset(new base::DictionaryValue());
     dict_->SetString("mode", "fixed_servers");
     dict_->SetString("server", test_server);
@@ -92,6 +144,9 @@ TEST_F(DataReductionProxyChromeSettingsTest, MigrateDataReductionProxy) {
         test_context_->pref_service());
 
     EXPECT_EQ(NULL, test_context_->pref_service()->GetUserPref(prefs::kProxy));
+    histogram_tester.ExpectUniqueSample(
+        "DataReductionProxy.ProxyPrefMigrationResult",
+        DataReductionProxyChromeSettings::PROXY_PREF_CLEARED_DRP, 1);
   }
 }
 
@@ -103,6 +158,7 @@ TEST_F(DataReductionProxyChromeSettingsTest,
       "https=https://tunnel.googlezip.net"};
 
   for (const std::string& test_server : kTestServers) {
+    base::HistogramTester histogram_tester;
     dict_.reset(new base::DictionaryValue());
     // The proxy pref is set to a Data Reduction Proxy that doesn't match the
     // currently configured DRP, but the pref should still be cleared.
@@ -117,6 +173,9 @@ TEST_F(DataReductionProxyChromeSettingsTest,
         test_context_->pref_service());
 
     EXPECT_EQ(NULL, test_context_->pref_service()->GetUserPref(prefs::kProxy));
+    histogram_tester.ExpectUniqueSample(
+        "DataReductionProxy.ProxyPrefMigrationResult",
+        DataReductionProxyChromeSettings::PROXY_PREF_CLEARED_GOOGLEZIP, 1);
   }
 }
 
@@ -128,6 +187,7 @@ TEST_F(DataReductionProxyChromeSettingsTest, MigrateIgnoreOtherProxy) {
       "https=http://arbitraryprefixgooglezip.net"};
 
   for (const std::string& test_server : kTestServers) {
+    base::HistogramTester histogram_tester;
     dict_.reset(new base::DictionaryValue());
     dict_->SetString("mode", "fixed_servers");
     dict_->SetString("server", test_server);
@@ -148,5 +208,9 @@ TEST_F(DataReductionProxyChromeSettingsTest, MigrateIgnoreOtherProxy) {
     std::string server;
     EXPECT_TRUE(value->GetString("server", &server));
     EXPECT_EQ(test_server, server);
+
+    histogram_tester.ExpectUniqueSample(
+        "DataReductionProxy.ProxyPrefMigrationResult",
+        DataReductionProxyChromeSettings::PROXY_PREF_NOT_CLEARED, 1);
   }
 }
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index b0476b1..e82f3e2 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -4629,6 +4629,16 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="DataReductionProxy.ProxyPrefMigrationResult"
+    enum="DataReductionProxyProxyPrefMigrationResult">
+  <owner>sclittle@chromium.org</owner>
+  <summary>
+    Records the result of migrating the Data Reduction Proxy away from being
+    configured via a proxy preference, including cases when no migration action
+    was taken.
+  </summary>
+</histogram>
+
 <histogram name="DataReductionProxy.RequestCompletionErrorCodes"
     enum="NetErrorCodes">
   <owner>sclittle@chromium.org</owner>
@@ -49962,6 +49972,15 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   <int value="3" label="Enabled from second screen"/>
 </enum>
 
+<enum name="DataReductionProxyProxyPrefMigrationResult" type="int">
+  <int value="0" label="Proxy pref not cleared"/>
+  <int value="1" label="Empty proxy pref was cleared"/>
+  <int value="2" label="System proxy pref was cleared"/>
+  <int value="3" label="Proxy pref containing a DRP was cleared"/>
+  <int value="4"
+      label="Proxy pref containing a *.googlezip.net proxy was cleared"/>
+</enum>
+
 <enum name="DataReductionProxyResponseProxyServerStatus" type="int">
   <int value="0" label="Empty proxy server"/>
   <int value="1" label="DRP proxy server"/>