Metrics fix for data reduction proxy.

chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc

Index: chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc
diff --git a/chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc b/chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc
index c2067f84a88617bddd9acff4b69a5e099b04843f..f2347da68091ebf384d2ede7b4433ffbb4a1efe8 100644
--- a/chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc
+++ b/chrome/browser/net/spdyproxy/data_reduction_proxy_settings.cc
@@ -37,49 +37,18 @@
 
 using base::FieldTrialList;
 using base::StringPrintf;
+using spdyproxy::ProbeURLFetchResult;
+using spdyproxy::ProxyStartupState;
 
 namespace {
 
 // Key of the UMA DataReductionProxy.StartupState histogram.
 const char kUMAProxyStartupStateHistogram[] =
     "DataReductionProxy.StartupState";
-// Values of the UMA DataReductionProxy.StartupState histogram.
-enum ProxyStartupState {
-  PROXY_NOT_AVAILABLE = 0,
-  PROXY_DISABLED,
-  PROXY_ENABLED,
-  PROXY_STARTUP_STATE_COUNT,
-};
 
 // Key of the UMA DataReductionProxy.ProbeURL histogram.
 const char kUMAProxyProbeURL[] = "DataReductionProxy.ProbeURL";
-// Values of the UMA DataReductionProxy.ProbeURL histogram.
-// This enum must remain synchronized with DataReductionProxyProbeURLFetchResult
-// in metrics/histograms/histograms.xml.
-enum ProbeURLFetchResult {
-  // The probe failed because the internet was disconnected.
-  INTERNET_DISCONNECTED = 0,
 
-  // The probe failed for any other reason, and as a result, the proxy was
-  // disabled.
-  FAILED_PROXY_DISABLED,
-
-  // The probe failed, but the proxy was already disabled.
-  FAILED_PROXY_ALREADY_DISABLED,
-
-  // THe probe succeeded, and as a result the proxy was enabled.
-  SUCCEEDED_PROXY_ENABLED,
-
-  // The probe succeeded, but the proxy was already enabled.
-  SUCCEEDED_PROXY_ALREADY_ENABLED,
-
-  // This must always be last.
-  FETCH_RESULT_COUNT
-};
-
-void RecordProbeURLFetchResult(ProbeURLFetchResult result) {
-  UMA_HISTOGRAM_ENUMERATION(kUMAProxyProbeURL, result, FETCH_RESULT_COUNT);
-}
 
 const char kEnabled[] = "Enabled";
 
@@ -126,6 +95,7 @@ void DataReductionProxySettings::InitPrefMembers() {
 
 void DataReductionProxySettings::InitDataReductionProxySettings() {
   InitPrefMembers();
+  RecordDataReductionInit();

[bengr, 2014/01/02 17:30:40]

Did you move this here so we record something even if the data reduction proxy
is not supported on the platform? Does this mean that every platform (Windows,
etc) will have UMA for the data reduction proxy?

[marq (ping after 24h), 2014/01/02 20:45:47]

No, because this code only exists for the mobile platforms because of how we
have it set up in .gyp, and because there's only code on those platforms to
create and init a DataReductionProxySettings object.

This change is the actual bug fix -- without it, we will never record the
PROXY_NOT_AVAILABLE startup state.

 
   // Disable the proxy if it is not allowed to be used.
   if (!IsDataReductionProxyAllowed())
@@ -140,7 +110,6 @@ void DataReductionProxySettings::InitDataReductionProxySettings() {
   // the feature via settings, in that once set, the preference will be sticky
   // across instances of Chrome. Disabling the feature can only be done through
   // the settings menu.
-  RecordDataReductionInit();
   if (spdy_proxy_auth_enabled_.GetValue() ||
       command_line.HasSwitch(switches::kEnableSpdyProxyAuth)) {
     MaybeActivateDataReductionProxy(true);
@@ -379,7 +348,7 @@ void DataReductionProxySettings::OnURLFetchComplete(
   net::URLRequestStatus status = source->GetStatus();
   if (status.status() == net::URLRequestStatus::FAILED &&
       status.error() == net::ERR_INTERNET_DISCONNECTED) {
-    RecordProbeURLFetchResult(INTERNET_DISCONNECTED);
+    RecordProbeURLFetchResult(spdyproxy::INTERNET_DISCONNECTED);
     return;
   }
 
@@ -398,9 +367,9 @@ void DataReductionProxySettings::OnURLFetchComplete(
         SetProxyConfigs(true /* enabled */,
                         false /* restricted */,
                         false /* at_startup */);
-        RecordProbeURLFetchResult(SUCCEEDED_PROXY_ENABLED);
+        RecordProbeURLFetchResult(spdyproxy::SUCCEEDED_PROXY_ENABLED);
       } else {
-        RecordProbeURLFetchResult(SUCCEEDED_PROXY_ALREADY_ENABLED);
+        RecordProbeURLFetchResult(spdyproxy::SUCCEEDED_PROXY_ALREADY_ENABLED);
       }
     }
     restricted_by_carrier_ = false;
@@ -415,9 +384,9 @@ void DataReductionProxySettings::OnURLFetchComplete(
       SetProxyConfigs(true /* enabled */,
                       true /* restricted */,
                       false /* at_startup */);
-      RecordProbeURLFetchResult(FAILED_PROXY_DISABLED);
+      RecordProbeURLFetchResult(spdyproxy::FAILED_PROXY_DISABLED);
     } else {
-      RecordProbeURLFetchResult(FAILED_PROXY_ALREADY_DISABLED);
+      RecordProbeURLFetchResult(spdyproxy::FAILED_PROXY_ALREADY_DISABLED);
     }
   }
   restricted_by_carrier_ = true;
@@ -551,12 +520,24 @@ void DataReductionProxySettings::SetProxyConfigs(
 
 // Metrics methods
 void DataReductionProxySettings::RecordDataReductionInit() {
-  ProxyStartupState state = PROXY_NOT_AVAILABLE;
+  ProxyStartupState state = spdyproxy::PROXY_NOT_AVAILABLE;
   if (IsDataReductionProxyAllowed())
-    state = IsDataReductionProxyEnabled() ? PROXY_ENABLED : PROXY_DISABLED;
+    state = IsDataReductionProxyEnabled() ? spdyproxy::PROXY_ENABLED

[bengr, 2014/01/02 17:30:40]

It's probably cleaner now to write this as:

if (IsDataReductionProxyExabled())
  state = spdyproxy::PROXY_ENABLED;
else
  state = spdyproxy::PROXY_DISABLED;

[marq (ping after 24h), 2014/01/02 20:45:47]

Done.

+                                          : spdyproxy::PROXY_DISABLED;
+  RecordStartupState(state);
+}
+
+void DataReductionProxySettings::RecordProbeURLFetchResult(
+    ProbeURLFetchResult result) {

[bengr, 2014/01/02 17:30:40]

Can this fit on the line above?

[marq (ping after 24h), 2014/01/02 20:45:47]

Not by nine characters.

+  UMA_HISTOGRAM_ENUMERATION(kUMAProxyProbeURL,
+                            result,
+                            spdyproxy::FETCH_RESULT_COUNT);
+}
+
+void DataReductionProxySettings::RecordStartupState(ProxyStartupState state) {
   UMA_HISTOGRAM_ENUMERATION(kUMAProxyStartupStateHistogram,
                             state,
-                            PROXY_STARTUP_STATE_COUNT);
+                            spdyproxy::PROXY_STARTUP_STATE_COUNT);
 }
 
 DataReductionProxySettings::ContentLengthList