DataReductionProxyStatisticsPrefs should support WeakPtr

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc

Index: components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
index 50fc91a181b4bbdc2297cf9e3888766b3941f5e3..4e9271920e6f6c8b222d6101b324c30dddd2c963 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
@@ -13,9 +13,12 @@
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/test/histogram_tester.h"
+#include "base/test/test_simple_task_runner.h"
 #include "base/time/time.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_auth_request_handler.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers_test_utils.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h"
@@ -76,6 +79,28 @@ class DataReductionProxyNetworkDelegateTest : public testing::Test {
                                                      test_job_interceptor_));
     context_.set_job_factory(&test_job_factory_);
     event_store_.reset(new DataReductionProxyEventStore(message_loop_proxy()));
+
+    params_.reset(
+        new TestDataReductionProxyParams(
+            DataReductionProxyParams::kAllowed |
+            DataReductionProxyParams::kFallbackAllowed |
+            DataReductionProxyParams::kPromoAllowed,
+            TestDataReductionProxyParams::HAS_EVERYTHING &
+            ~TestDataReductionProxyParams::HAS_DEV_ORIGIN &
+            ~TestDataReductionProxyParams::HAS_DEV_FALLBACK_ORIGIN));
+
+    // loop_proxy_ is just the current message loop. This means loop_proxy_

[bengr, 2015/02/04 23:39:11]

What is loop_proxy_?

[megjablon, 2015/02/06 23:40:41]

Not sure what this comment was, just copied when moving this duplicate code.

+    // is the network thread used by DataReductionProxyAuthRequestHandler.
+    auth_handler_.reset(new DataReductionProxyAuthRequestHandler(
+        kClient, params_.get(), message_loop_proxy()));
+
+    configurator_.reset(new DataReductionProxyConfigurator(
+        message_loop_proxy(), net_log(), event_store()));
+
+    data_reduction_proxy_network_delegate_.reset(
+        new DataReductionProxyNetworkDelegate(
+            scoped_ptr<net::NetworkDelegate>(new TestNetworkDelegate()),
+            params_.get(), auth_handler_.get(), configurator_.get()));
   }
 
   const net::ProxyConfig& GetProxyConfig() const {
@@ -128,6 +153,12 @@ class DataReductionProxyNetworkDelegateTest : public testing::Test {
     return event_store_.get();
   }
 
+  scoped_ptr<TestDataReductionProxyParams> params_;
+  scoped_ptr<DataReductionProxyAuthRequestHandler> auth_handler_;
+  scoped_ptr<DataReductionProxyConfigurator> configurator_;
+  scoped_ptr<DataReductionProxyNetworkDelegate>
+      data_reduction_proxy_network_delegate_;
+
  private:
   base::MessageLoopForIO loop_;
   net::TestURLRequestContext context_;
@@ -143,40 +174,17 @@ class DataReductionProxyNetworkDelegateTest : public testing::Test {
 };
 
 TEST_F(DataReductionProxyNetworkDelegateTest, AuthenticationTest) {
-  scoped_ptr<TestDataReductionProxyParams> params(
-      new TestDataReductionProxyParams(
-          DataReductionProxyParams::kAllowed |
-          DataReductionProxyParams::kFallbackAllowed |
-          DataReductionProxyParams::kPromoAllowed,
-          TestDataReductionProxyParams::HAS_EVERYTHING &
-          ~TestDataReductionProxyParams::HAS_DEV_ORIGIN &
-          ~TestDataReductionProxyParams::HAS_DEV_FALLBACK_ORIGIN));
-  // loop_proxy_ is just the current message loop. This means loop_proxy_
-  // is the network thread used by DataReductionProxyAuthRequestHandler.
-  DataReductionProxyAuthRequestHandler auth_handler(
-      kClient, params.get(), message_loop_proxy());
-
-  scoped_ptr<DataReductionProxyConfigurator> configurator(
-      new DataReductionProxyConfigurator(message_loop_proxy(), net_log(),
-                                         event_store()));
-
-  scoped_ptr<DataReductionProxyNetworkDelegate> network_delegate(
-      new DataReductionProxyNetworkDelegate(
-          scoped_ptr<net::NetworkDelegate>(new TestNetworkDelegate()),
-          params.get(), &auth_handler,
-          configurator.get()));
-
-  set_network_delegate(network_delegate.get());
+  set_network_delegate(data_reduction_proxy_network_delegate_.get());
   scoped_ptr<net::URLRequest> fake_request(
       FetchURLRequest(GURL("http://www.google.com/"), std::string(), 0));
 
   net::ProxyInfo data_reduction_proxy_info;
   std::string data_reduction_proxy;
-  base::TrimString(params->DefaultOrigin(), "/", &data_reduction_proxy);
+  base::TrimString(params_->DefaultOrigin(), "/", &data_reduction_proxy);
   data_reduction_proxy_info.UseNamedProxy(data_reduction_proxy);
 
   net::HttpRequestHeaders headers;
-  network_delegate->NotifyBeforeSendProxyHeaders(
+  data_reduction_proxy_network_delegate_->NotifyBeforeSendProxyHeaders(
       fake_request.get(), data_reduction_proxy_info, &headers);
 
   EXPECT_TRUE(headers.HasHeader(kChromeProxyHeader));
@@ -207,30 +215,9 @@ TEST_F(DataReductionProxyNetworkDelegateTest, NetHistograms) {
   const int64 kResponseContentLength = 100;
   const int64 kOriginalContentLength = 200;
 
-  scoped_ptr<TestDataReductionProxyParams> params(
-      new TestDataReductionProxyParams(
-          DataReductionProxyParams::kAllowed |
-          DataReductionProxyParams::kFallbackAllowed |
-          DataReductionProxyParams::kPromoAllowed,
-          TestDataReductionProxyParams::HAS_EVERYTHING &
-          ~TestDataReductionProxyParams::HAS_DEV_ORIGIN &
-          ~TestDataReductionProxyParams::HAS_DEV_FALLBACK_ORIGIN));
-  // loop_proxy_ is just the current message loop. This means loop_proxy_
-  // is the network thread used by DataReductionProxyAuthRequestHandler.
-  DataReductionProxyAuthRequestHandler auth_handler(
-      kClient, params.get(), message_loop_proxy());
-
   base::HistogramTester histogram_tester;
-  scoped_ptr<DataReductionProxyConfigurator> configurator(
-      new DataReductionProxyConfigurator(message_loop_proxy(), net_log(),
-                                         event_store()));
-  scoped_ptr<DataReductionProxyNetworkDelegate> network_delegate(
-      new DataReductionProxyNetworkDelegate(
-          scoped_ptr<net::NetworkDelegate>(
-              new TestNetworkDelegate()), params.get(), &auth_handler,
-              configurator.get()));
 
-  set_network_delegate(network_delegate.get());
+  set_network_delegate(data_reduction_proxy_network_delegate_.get());
 
   std::string raw_headers =
       "HTTP/1.1 200 OK\n"
@@ -274,65 +261,6 @@ TEST_F(DataReductionProxyNetworkDelegateTest, NetHistograms) {
                                       kResponseContentLength, 1);
 }
 
-class DataReductionProxyHistoricNetworkStatsTest
-    : public testing::Test {
- public:
-  DataReductionProxyHistoricNetworkStatsTest() {}
-
-  void SetUp() override {
-    simple_pref_service_.registry()->RegisterInt64Pref(
-        prefs::kHttpReceivedContentLength, 0);
-    simple_pref_service_.registry()->RegisterInt64Pref(
-        prefs::kHttpOriginalContentLength, 0);
-  }
-
-  // Verify the pref values in |dict| are equal to that in
-  // |simple_pref_service|.
-  void VerifyPrefs(base::DictionaryValue* dict) {
-    base::string16 dict_pref_string;
-    int64 dict_pref;
-    int64 service_pref;
-
-    dict->GetString("historic_original_content_length", &dict_pref_string);
-    base::StringToInt64(dict_pref_string, &dict_pref);
-    service_pref = simple_pref_service_.GetInt64(
-        prefs::kHttpOriginalContentLength);
-    EXPECT_EQ(service_pref, dict_pref);
-
-    dict->GetString("historic_received_content_length", &dict_pref_string);
-    base::StringToInt64(dict_pref_string, &dict_pref);
-    service_pref = simple_pref_service_.GetInt64(
-        prefs::kHttpReceivedContentLength);
-    EXPECT_EQ(service_pref, dict_pref);
-  }
-
-  TestingPrefServiceSimple simple_pref_service_;
-};
-
-TEST_F(DataReductionProxyHistoricNetworkStatsTest,
-       HistoricNetworkStatsInfoToValue) {
-  const int64 kOriginalLength = 150;
-  const int64 kReceivedLength = 100;
-
-  base::DictionaryValue* dict = nullptr;
-  scoped_ptr<base::Value> stats_value(
-      DataReductionProxyNetworkDelegate::HistoricNetworkStatsInfoToValue(
-          &simple_pref_service_));
-  EXPECT_TRUE(stats_value->GetAsDictionary(&dict));
-  VerifyPrefs(dict);
-
-  simple_pref_service_.SetInt64(prefs::kHttpOriginalContentLength,
-                                kOriginalLength);
-  simple_pref_service_.SetInt64(prefs::kHttpReceivedContentLength,
-                                kReceivedLength);
-
-  stats_value.reset(
-      DataReductionProxyNetworkDelegate::HistoricNetworkStatsInfoToValue(
-          &simple_pref_service_));
-  EXPECT_TRUE(stats_value->GetAsDictionary(&dict));
-  VerifyPrefs(dict);
-}
-
 class BadEntropyProvider : public base::FieldTrial::EntropyProvider {
  public:
   ~BadEntropyProvider() override {}
@@ -347,21 +275,13 @@ TEST_F(DataReductionProxyNetworkDelegateTest, OnResolveProxyHandler) {
   int load_flags = net::LOAD_NORMAL;
   GURL url("http://www.google.com/");
 
-  TestDataReductionProxyParams test_params(
-            DataReductionProxyParams::kAllowed |
-            DataReductionProxyParams::kFallbackAllowed |
-            DataReductionProxyParams::kPromoAllowed,
-            TestDataReductionProxyParams::HAS_EVERYTHING &
-            ~TestDataReductionProxyParams::HAS_DEV_ORIGIN &
-            ~TestDataReductionProxyParams::HAS_DEV_FALLBACK_ORIGIN);
-
   // Data reduction proxy info
   net::ProxyInfo data_reduction_proxy_info;
   std::string data_reduction_proxy;
-  base::TrimString(test_params.DefaultOrigin(), "/", &data_reduction_proxy);
+  base::TrimString(params_->DefaultOrigin(), "/", &data_reduction_proxy);
   data_reduction_proxy_info.UsePacString(
       "PROXY " +
-      net::HostPortPair::FromURL(GURL(test_params.DefaultOrigin())).ToString() +
+      net::HostPortPair::FromURL(GURL(params_->DefaultOrigin())).ToString() +
       "; DIRECT");
   EXPECT_FALSE(data_reduction_proxy_info.is_empty());
 
@@ -397,7 +317,7 @@ TEST_F(DataReductionProxyNetworkDelegateTest, OnResolveProxyHandler) {
   // Another proxy is used. It should be used afterwards.
   result.Use(other_proxy_info);
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                       empty_proxy_retry_info, &test_params, &result);
+                        empty_proxy_retry_info, params_.get(), &result);
   EXPECT_EQ(other_proxy_info.proxy_server(), result.proxy_server());
 
   // A direct connection is used. The data reduction proxy should be used
@@ -406,7 +326,7 @@ TEST_F(DataReductionProxyNetworkDelegateTest, OnResolveProxyHandler) {
   result.Use(direct_proxy_info);
   net::ProxyConfig::ID prev_id = result.config_id();
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                       empty_proxy_retry_info, &test_params, &result);
+                       empty_proxy_retry_info, params_.get(), &result);
   EXPECT_EQ(data_reduction_proxy_info.proxy_server(), result.proxy_server());
   // Only the proxy list should be updated, not he proxy info.
   EXPECT_EQ(result.config_id(), prev_id);
@@ -416,7 +336,7 @@ TEST_F(DataReductionProxyNetworkDelegateTest, OnResolveProxyHandler) {
   result.Use(direct_proxy_info);
   prev_id = result.config_id();
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                       data_reduction_proxy_retry_info, &test_params, &result);
+                       data_reduction_proxy_retry_info, params_.get(), &result);
   EXPECT_TRUE(result.proxy_server().is_direct());
   EXPECT_EQ(result.config_id(), prev_id);
 
@@ -424,37 +344,37 @@ TEST_F(DataReductionProxyNetworkDelegateTest, OnResolveProxyHandler) {
   result.UseDirect();
   OnResolveProxyHandler(GURL("ws://echo.websocket.org/"),
                        load_flags, data_reduction_proxy_config,
-                       empty_proxy_retry_info, &test_params, &result);
+                       empty_proxy_retry_info, params_.get(), &result);
   EXPECT_TRUE(result.is_direct());
 
   OnResolveProxyHandler(GURL("wss://echo.websocket.org/"),
                        load_flags, data_reduction_proxy_config,
-                       empty_proxy_retry_info, &test_params, &result);
+                       empty_proxy_retry_info, params_.get(), &result);
   EXPECT_TRUE(result.is_direct());
 
   // Without DataCompressionProxyCriticalBypass Finch trial set, the
   // BYPASS_DATA_REDUCTION_PROXY load flag should be ignored.
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
+                        empty_proxy_retry_info, params_.get(),
                         &result);
   EXPECT_FALSE(result.is_direct());
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
                         empty_proxy_retry_info,
-                        &test_params, &other_proxy_info);
+                        params_.get(), &other_proxy_info);
   EXPECT_FALSE(other_proxy_info.is_direct());
 
   load_flags |= net::LOAD_BYPASS_DATA_REDUCTION_PROXY;
 
   result.UseDirect();
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
+                        empty_proxy_retry_info, params_.get(),
                         &result);
   EXPECT_FALSE(result.is_direct());
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
                         empty_proxy_retry_info,
-                        &test_params, &other_proxy_info);
+                        params_.get(), &other_proxy_info);
   EXPECT_FALSE(other_proxy_info.is_direct());
 
   // With Finch trial set, should only bypass if LOAD flag is set and the
@@ -469,12 +389,12 @@ TEST_F(DataReductionProxyNetworkDelegateTest, OnResolveProxyHandler) {
 
   result.UseDirect();
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
+                        empty_proxy_retry_info, params_.get(),
                         &result);
   EXPECT_FALSE(result.is_direct());
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
+                        empty_proxy_retry_info, params_.get(),
                         &other_proxy_info);
   EXPECT_FALSE(other_proxy_info.is_direct());
 
@@ -482,14 +402,159 @@ TEST_F(DataReductionProxyNetworkDelegateTest, OnResolveProxyHandler) {
 
   result.UseDirect();
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
+                        empty_proxy_retry_info, params_.get(),
                         &result);
   EXPECT_TRUE(result.is_direct());
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
+                        empty_proxy_retry_info, params_.get(),
                         &other_proxy_info);
   EXPECT_FALSE(other_proxy_info.is_direct());
 }
 
+TEST_F(DataReductionProxyNetworkDelegateTest, TotalLengths) {
+  const int64 kOriginalLength = 200;
+  const int64 kReceivedLength = 100;
+
+  TestingPrefServiceSimple pref_service;
+  scoped_ptr<DataReductionProxyStatisticsPrefs> statistics_prefs(
+      new DataReductionProxyStatisticsPrefs(
+          &pref_service,
+          scoped_refptr<base::TestSimpleTaskRunner>(
+              new base::TestSimpleTaskRunner()),
+          base::TimeDelta()));
+  PrefRegistrySimple* registry = pref_service.registry();

[bengr, 2015/02/04 23:39:10]

Can't you use the DRP registry functions, e.g.,
https://code.google.com/p/chromium/codesearch#chromium/src/components/data_re...

[megjablon, 2015/02/06 23:40:41]

Good to know about this

+  registry->RegisterInt64Pref(
+      data_reduction_proxy::prefs::kHttpReceivedContentLength, 0);
+  registry->RegisterInt64Pref(
+      data_reduction_proxy::prefs::kHttpOriginalContentLength, 0);
+  registry->RegisterListPref(
+      data_reduction_proxy::prefs::kDailyHttpOriginalContentLength);
+  registry->RegisterListPref(
+      data_reduction_proxy::prefs::kDailyHttpReceivedContentLength);
+  registry->RegisterListPref(
+      data_reduction_proxy::prefs::
+          kDailyOriginalContentLengthWithDataReductionProxyEnabled);
+  registry->RegisterListPref(
+      data_reduction_proxy::prefs::
+          kDailyContentLengthWithDataReductionProxyEnabled);
+  registry->RegisterListPref(
+      data_reduction_proxy::prefs::
+          kDailyContentLengthHttpsWithDataReductionProxyEnabled);
+  registry->RegisterListPref(
+      data_reduction_proxy::prefs::
+          kDailyContentLengthShortBypassWithDataReductionProxyEnabled);
+  registry->RegisterListPref(
+      data_reduction_proxy::prefs::
+          kDailyContentLengthLongBypassWithDataReductionProxyEnabled);
+  registry->RegisterListPref(
+      data_reduction_proxy::prefs::
+          kDailyContentLengthUnknownWithDataReductionProxyEnabled);
+  registry->RegisterListPref(
+      data_reduction_proxy::prefs::
+          kDailyOriginalContentLengthViaDataReductionProxy);
+  registry->RegisterListPref(
+      data_reduction_proxy::prefs::
+          kDailyContentLengthViaDataReductionProxy);
+  registry->RegisterInt64Pref(
+      data_reduction_proxy::prefs::
+          kDailyHttpContentLengthLastUpdateDate, 0L);
+  registry->RegisterBooleanPref(
+      data_reduction_proxy::prefs::kDataReductionProxyEnabled, false);
+
+  data_reduction_proxy_network_delegate_->
+      data_reduction_proxy_statistics_prefs_ =
+          statistics_prefs->GetWeakPtr();
+
+  data_reduction_proxy_network_delegate_->UpdateContentLengthPrefs(
+      kReceivedLength, kOriginalLength,
+      pref_service.GetBoolean(
+          data_reduction_proxy::prefs::kDataReductionProxyEnabled),
+      UNKNOWN_TYPE);
+
+  EXPECT_EQ(kReceivedLength,
+            statistics_prefs->GetInt64(
+                data_reduction_proxy::prefs::kHttpReceivedContentLength));
+  EXPECT_FALSE(pref_service.GetBoolean(
+      data_reduction_proxy::prefs::kDataReductionProxyEnabled));
+  EXPECT_EQ(kOriginalLength,
+            statistics_prefs->GetInt64(
+                data_reduction_proxy::prefs::kHttpOriginalContentLength));
+
+  // Record the same numbers again, and total lengths should be doubled.
+  data_reduction_proxy_network_delegate_->UpdateContentLengthPrefs(
+      kReceivedLength, kOriginalLength,
+      pref_service.GetBoolean(
+          data_reduction_proxy::prefs::kDataReductionProxyEnabled),
+      UNKNOWN_TYPE);
+
+  EXPECT_EQ(kReceivedLength * 2,
+            statistics_prefs->GetInt64(
+                data_reduction_proxy::prefs::kHttpReceivedContentLength));
+  EXPECT_FALSE(pref_service.GetBoolean(
+      data_reduction_proxy::prefs::kDataReductionProxyEnabled));
+  EXPECT_EQ(kOriginalLength * 2,
+            statistics_prefs->GetInt64(
+                data_reduction_proxy::prefs::kHttpOriginalContentLength));
+}
+
+class DataReductionProxyHistoricNetworkStatsTest
+    : public testing::Test {
+ public:
+  DataReductionProxyHistoricNetworkStatsTest() {}

[bengr, 2015/02/04 23:39:11]

Move end curly to new line.

[megjablon, 2015/02/06 23:40:42]

Done.

+
+  void SetUp() override {
+    simple_pref_service_.registry()->RegisterInt64Pref(
+        prefs::kHttpReceivedContentLength, 0);
+    simple_pref_service_.registry()->RegisterInt64Pref(
+        prefs::kHttpOriginalContentLength, 0);
+  }
+
+  // Verify the pref values in |dict| are equal to that in
+  // |simple_pref_service|.
+  void VerifyPrefs(base::DictionaryValue* dict) {
+    base::string16 dict_pref_string;
+    int64 dict_pref;
+    int64 service_pref;
+
+    dict->GetString("historic_original_content_length", &dict_pref_string);
+    base::StringToInt64(dict_pref_string, &dict_pref);
+    service_pref = simple_pref_service_.GetInt64(
+        prefs::kHttpOriginalContentLength);
+    EXPECT_EQ(service_pref, dict_pref);
+
+    dict->GetString("historic_received_content_length", &dict_pref_string);
+    base::StringToInt64(dict_pref_string, &dict_pref);
+    service_pref = simple_pref_service_.GetInt64(
+        prefs::kHttpReceivedContentLength);
+    EXPECT_EQ(service_pref, dict_pref);
+  }
+
+  TestingPrefServiceSimple simple_pref_service_;
+};
+
+TEST_F(DataReductionProxyHistoricNetworkStatsTest,
+       HistoricNetworkStatsInfoToValue) {
+  const int64 kOriginalLength = 150;
+  const int64 kReceivedLength = 100;
+
+  base::DictionaryValue* dict = nullptr;
+  scoped_ptr<base::Value> stats_value(
+      DataReductionProxyNetworkDelegate::HistoricNetworkStatsInfoToValue(
+          &simple_pref_service_));
+  EXPECT_TRUE(stats_value->GetAsDictionary(&dict));
+  VerifyPrefs(dict);
+
+  simple_pref_service_.SetInt64(prefs::kHttpOriginalContentLength,
+                                kOriginalLength);
+  simple_pref_service_.SetInt64(prefs::kHttpReceivedContentLength,
+                                kReceivedLength);
+
+  stats_value.reset(
+      DataReductionProxyNetworkDelegate::HistoricNetworkStatsInfoToValue(
+          &simple_pref_service_));
+  EXPECT_TRUE(stats_value->GetAsDictionary(&dict));
+  VerifyPrefs(dict);
+}
+
 }  // namespace data_reduction_proxy