Metrics fix for data reduction proxy.

chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest.cc

 // Copyright 2013 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/spdyproxy/data_reduction_proxy_settings_unittest.h"
 
 #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"
 #include "base/prefs/testing_pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_settings.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_settings_android.h"
 #include "chrome/browser/prefs/proxy_prefs.h"
 #include "chrome/common/chrome_switches.h"
@@ skipping 24 matching lines @@
 const char kProbeURLWithNoResponse[] = "http://no.org/";
 
 // Transform "normal"-looking headers (\n-separated) to the appropriate
 // input format for ParseRawHeaders (\0-separated).
 void HeadersToRaw(std::string* headers) {
   std::replace(headers->begin(), headers->end(), '\n', '\0');
   if (!headers->empty())
     *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() {
 }
 
 DataReductionProxySettingsTestBase::~DataReductionProxySettingsTestBase() {}
 
 void DataReductionProxySettingsTestBase::AddProxyToCommandLine() {
   CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kSpdyProxyAuthOrigin, kDataReductionProxyOrigin);
   CommandLine::ForCurrentProcess()->AppendSwitchASCII(
@@ skipping 47 matching lines @@
 template void
 DataReductionProxySettingsTestBase::ResetSettings<DataReductionProxySettings>();
 template void
 DataReductionProxySettingsTestBase::ResetSettings<
     DataReductionProxySettingsAndroid>();
 
 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(
             GURL(test_url),
             settings,
             response,
             success ? net::HTTP_OK : net::HTTP_INTERNAL_SERVER_ERROR,
             success ? net::URLRequestStatus::SUCCESS :
                       net::URLRequestStatus::FAILED)));
   }
 }
 
 // Explicitly generate required instantiations.
 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,
     const std::string& expected_mode) {
   const DictionaryValue* dict = pref_service_.GetDictionary(prefs::kProxy);
   std::string mode;
   std::string server;
   dict->GetString("mode", &mode);
   ASSERT_EQ(expected_mode, mode);
   dict->GetString("server", &server);
@@ skipping 16 matching lines @@
 }
 
 void DataReductionProxySettingsTestBase::CheckProbe(
     bool initially_enabled,
     const std::string& probe_url,
     const std::string& response,
     bool request_success,
     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);
 }
 
 void DataReductionProxySettingsTestBase::CheckProbeOnIPChange(
     const std::string& probe_url,
     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);
 }
 
 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.
   CheckProxyConfigs(expected_enabled, false);
 }
 
 void DataReductionProxySettingsTestBase::CheckInitDataReductionProxy(
     bool enabled_at_startup) {
   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) {
     CheckProxyConfigs(enabled_at_startup, false);
   } else {
     // This presumes the proxy preference hadn't been set up by Chrome.
     CheckProxyPref(std::string(), std::string());
   }
 }
 
@@ skipping 250 matching lines @@
   CheckProbe(true, kProbeURLWithNoResponse, "", false, true, true);
   // The proxy is disabled initially. Probes should not be emitted to change
   // state.
   CheckProbe(false, kProbeURLWithOKResponse, "OK", true, false, false);
 }
 
 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);
   // IP address change triggers a probe that succeeds. Proxy remains
   // unrestricted.
   CheckProbeOnIPChange(kProbeURLWithOKResponse, "OK", true, false);
   // IP address change triggers a probe that fails. Proxy is restricted.
   CheckProbeOnIPChange(kProbeURLWithBadResponse, "Bad", true, true);
   // IP address change triggers a probe that fails. Proxy remains restricted.
   CheckProbeOnIPChange(kProbeURLWithBadResponse, "Bad", true, true);
   // IP address change triggers a probe that succeed. Proxy is unrestricted.
   CheckProbeOnIPChange(kProbeURLWithBadResponse, "OK", true, false);
 }
 
 TEST_F(DataReductionProxySettingsTest, TestOnProxyEnabledPrefChange) {
   AddProxyToCommandLine();
   settings_->InitPrefMembers();
   base::MessageLoopForUI loop;
   // The proxy is enabled initially.
   settings_->enabled_by_user_ = true;
   settings_->SetProxyConfigs(true, false, true);
   // The pref is disabled, so correspondingly should be the proxy.
   CheckOnPrefChange(false, false);
   // The pref is enabled, so correspondingly should be the proxy.
   CheckOnPrefChange(true, true);
 }
 
 TEST_F(DataReductionProxySettingsTest, TestInitDataReductionProxyOn) {
+  MockSettings* settings = static_cast<MockSettings*>(settings_.get());
+  EXPECT_CALL(*settings, RecordStartupState(spdyproxy::PROXY_ENABLED));
   CheckInitDataReductionProxy(true);
 }
 
 TEST_F(DataReductionProxySettingsTest, TestInitDataReductionProxyOff) {
   // InitDataReductionProxySettings with the preference off will directly call
   // 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);
 }
 
 TEST_F(DataReductionProxySettingsTest, TestGetDailyContentLengths) {
   DataReductionProxySettings::ContentLengthList result =
       settings_->GetDailyContentLengths(prefs::kDailyHttpOriginalContentLength);
 
   ASSERT_FALSE(result.empty());
   ASSERT_EQ(spdyproxy::kNumDaysInHistory, result.size());
 
@@ skipping 63 matching lines @@
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
     std::string headers(tests[i].headers);
     HeadersToRaw(&headers);
     scoped_refptr<net::HttpResponseHeaders> parsed(
         new net::HttpResponseHeaders(headers));
 
     EXPECT_EQ(tests[i].expected_result,
               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();
+}