Metrics fix for data reduction proxy.

chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest.cc

Index: chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest.cc
diff --git a/chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest.cc b/chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest.cc
index d725fc4a9f3e88a446c4b6c9471108528ef8049f..1c7e34187b5277d07aa1747da161f9d72f0df22f 100644
--- a/chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest.cc
+++ b/chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest.cc
@@ -7,6 +7,8 @@
 #include "base/command_line.h"
 #include "base/md5.h"
 #include "base/metrics/field_trial.h"
+#include "base/metrics/histogram_samples.h"
+#include "base/metrics/statistics_recorder.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
@@ -51,6 +53,15 @@ void HeadersToRaw(std::string* headers) {
     *headers += '\0';
 }
 
+ProbeURLFetchResult FetchResult(bool enabled, bool success) {
+  if (enabled)
+    return success ? spdyproxy::SUCCEEDED_PROXY_ALREADY_ENABLED
+                   : spdyproxy::FAILED_PROXY_DISABLED;

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

I'd rewrite these with if/else instead of the ternary operator.

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

Done.

+  else
+    return success ? spdyproxy::SUCCEEDED_PROXY_ENABLED
+                   : spdyproxy::FAILED_PROXY_ALREADY_DISABLED;
+}
+
 DataReductionProxySettingsTestBase::DataReductionProxySettingsTestBase()
     : testing::Test() {
 }
@@ -118,13 +129,16 @@ template <class C>
 void DataReductionProxySettingsTestBase::SetProbeResult(
     const std::string& test_url,
     const std::string& response,
+    ProbeURLFetchResult result,
     bool success,
     int expected_calls)  {
   MockDataReductionProxySettings<C>* settings =
       static_cast<MockDataReductionProxySettings<C>*>(settings_.get());
   if (0 == expected_calls) {
     EXPECT_CALL(*settings, GetURLFetcher()).Times(0);
+    EXPECT_CALL(*settings, RecordProbeURLFetchResult(_)).Times(0);
   } else {
+    EXPECT_CALL(*settings, RecordProbeURLFetchResult(result)).Times(1);
     EXPECT_CALL(*settings, GetURLFetcher())
         .Times(expected_calls)
         .WillRepeatedly(Return(new net::FakeURLFetcher(
@@ -142,14 +156,17 @@ template void
 DataReductionProxySettingsTestBase::SetProbeResult<DataReductionProxySettings>(
     const std::string& test_url,
     const std::string& response,
+    ProbeURLFetchResult result,
     bool success,
     int expected_calls);
 template void
 DataReductionProxySettingsTestBase::SetProbeResult<
-    DataReductionProxySettingsAndroid>(const std::string& test_url,
-                                       const std::string& response,
-                                       bool success,
-                                       int expected_calls);
+    DataReductionProxySettingsAndroid>(
+        const std::string& test_url,

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

Can this be moved up a line?

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

Done.

+        const std::string& response,
+        ProbeURLFetchResult result,
+        bool success,
+        int expected_calls);
 
 void DataReductionProxySettingsTestBase::CheckProxyPref(
     const std::string& expected_servers,
@@ -186,9 +203,15 @@ void DataReductionProxySettingsTestBase::CheckProbe(
     bool expected_enabled,
     bool expected_restricted) {
   pref_service_.SetBoolean(prefs::kSpdyProxyAuthEnabled, initially_enabled);
+  if (initially_enabled)
+    settings_->enabled_by_user_ = true;
   settings_->restricted_by_carrier_ = false;
-  SetProbeResult(
-      probe_url, response, request_success, initially_enabled ? 1 : 0);
+  SetProbeResult(probe_url,
+                 response,
+                 FetchResult(initially_enabled,
+                             request_success && (response == "OK")),

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

maybe rename as 'request_succeeded'

+                 request_success,
+                 initially_enabled ? 1 : 0);
   settings_->MaybeActivateDataReductionProxy(false);
   base::MessageLoop::current()->RunUntilIdle();
   CheckProxyConfigs(expected_enabled, expected_restricted);
@@ -199,7 +222,12 @@ void DataReductionProxySettingsTestBase::CheckProbeOnIPChange(
     const std::string& response,
     bool request_success,
     bool expected_restricted) {
-  SetProbeResult(probe_url, response, request_success, 1);
+  SetProbeResult(probe_url,
+                 response,
+                 FetchResult(!settings_->restricted_by_carrier_,
+                             request_success && (response == "OK")),
+                 request_success,
+                 1);
   settings_->OnIPAddressChanged();
   base::MessageLoop::current()->RunUntilIdle();
   CheckProxyConfigs(true, expected_restricted);
@@ -209,8 +237,11 @@ void DataReductionProxySettingsTestBase::CheckOnPrefChange(
     bool enabled,
     bool expected_enabled) {
   // Always have a sucessful probe for pref change tests.
-  SetProbeResult(
-      kProbeURLWithOKResponse, "OK", true, expected_enabled ? 1 : 0);
+  SetProbeResult(kProbeURLWithOKResponse,
+                 "OK",
+                 FetchResult(enabled, true),
+                 true,
+                 expected_enabled ? 1 : 0);
   pref_service_.SetBoolean(prefs::kSpdyProxyAuthEnabled, enabled);
   base::MessageLoop::current()->RunUntilIdle();
   // Never expect the proxy to be restricted for pref change tests.
@@ -222,8 +253,11 @@ void DataReductionProxySettingsTestBase::CheckInitDataReductionProxy(
   AddProxyToCommandLine();
   base::MessageLoopForUI loop;
   pref_service_.SetBoolean(prefs::kSpdyProxyAuthEnabled, enabled_at_startup);
-  SetProbeResult(
-      kProbeURLWithOKResponse, "OK", true, enabled_at_startup ? 1 : 0);
+  SetProbeResult(kProbeURLWithOKResponse,
+                 "OK",
+                 FetchResult(enabled_at_startup, true),
+                 true,
+                 enabled_at_startup ? 1 : 0);
   settings_->InitDataReductionProxySettings();
   base::MessageLoop::current()->RunUntilIdle();
   if (enabled_at_startup) {
@@ -494,6 +528,10 @@ TEST_F(DataReductionProxySettingsTest, TestOnIPAddressChanged) {
   AddProxyToCommandLine();
   base::MessageLoopForUI loop;
   // The proxy is enabled initially.
+  pref_service_.SetBoolean(prefs::kSpdyProxyAuthEnabled, true);
+  settings_->spdy_proxy_auth_enabled_.Init(
+      prefs::kSpdyProxyAuthEnabled,
+      settings_->GetOriginalProfilePrefs());
   settings_->enabled_by_user_ = true;
   settings_->restricted_by_carrier_ = false;
   settings_->SetProxyConfigs(true, false, true);
@@ -522,6 +560,8 @@ TEST_F(DataReductionProxySettingsTest, TestOnProxyEnabledPrefChange) {
 }
 
 TEST_F(DataReductionProxySettingsTest, TestInitDataReductionProxyOn) {
+  MockSettings* settings = static_cast<MockSettings*>(settings_.get());
+  EXPECT_CALL(*settings, RecordStartupState(spdyproxy::PROXY_ENABLED));
   CheckInitDataReductionProxy(true);
 }
 
@@ -530,6 +570,7 @@ TEST_F(DataReductionProxySettingsTest, TestInitDataReductionProxyOff) {
   // LogProxyState.
   MockSettings* settings = static_cast<MockSettings*>(settings_.get());
   EXPECT_CALL(*settings, LogProxyState(false, false, true)).Times(1);
+  EXPECT_CALL(*settings, RecordStartupState(spdyproxy::PROXY_DISABLED));
   CheckInitDataReductionProxy(false);
 }
 
@@ -613,3 +654,12 @@ TEST_F(DataReductionProxySettingsTest, WasFetchedViaProxy) {
               DataReductionProxySettings::WasFetchedViaProxy(parsed));
   }
 }
+
+TEST_F(DataReductionProxySettingsTest, CheckInitMetricsWhenNotAllowed) {
+  // We don't call |AddProxyToCommandLine()|, so the proxy feature

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

Don't use 'We'. Maybe: 

// The proxy feature should be unavailable, because |AddProxyToCommandLine()| is
not called.

+  // should be unavailable.
+  EXPECT_FALSE(DataReductionProxySettings::IsDataReductionProxyAllowed());
+  MockSettings* settings = static_cast<MockSettings*>(settings_.get());
+  EXPECT_CALL(*settings, RecordStartupState(spdyproxy::PROXY_NOT_AVAILABLE));
+  settings_->InitDataReductionProxySettings();
+}