diff options
author | jeremyim <jeremyim@chromium.org> | 2015-05-15 11:13:06 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-05-15 18:14:21 +0000 |
commit | 0323138fb70b031af8b60d7bc07152c3e2ddb7b8 (patch) | |
tree | 68e770de91eba13c7dd6b6e555882a1323cad7aa /components | |
parent | 040db4f47768b171ba2feced77bdedac341c5644 (diff) | |
download | chromium_src-0323138fb70b031af8b60d7bc07152c3e2ddb7b8.zip chromium_src-0323138fb70b031af8b60d7bc07152c3e2ddb7b8.tar.gz chromium_src-0323138fb70b031af8b60d7bc07152c3e2ddb7b8.tar.bz2 |
Add DataReductionProxyExperimentsStats and UMA for measuring potentially non-compressed bytes.
If no valid Data Reduction Proxy configuration is available to the client,
then the client does not get the benefit of the Data Reduction Proxy during
the period when a configuration is being retrieved. This CL sets up a
configurable simulation where all requests will continue to go through the
Data Reduction Proxy, but if the request is initiated during a period when
the client would request a configuration from the service, we will record
the reduction statistics to determine the overall effect on data compression.
BUG=484864
Review URL: https://codereview.chromium.org/1127893002
Cr-Commit-Position: refs/heads/master@{#330133}
Diffstat (limited to 'components')
23 files changed, 1089 insertions, 16 deletions
diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 1578e70..c5ca9d3 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -110,9 +110,11 @@ 'data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_compression_stats_unittest.cc', + 'data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params_unittest.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_configurator_unittest.cc', + 'data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats_unittest.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_io_data_unittest.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_metrics_unittest.cc', diff --git a/components/cronet/android/cronet_data_reduction_proxy.cc b/components/cronet/android/cronet_data_reduction_proxy.cc index 1a05234..f6f47ef 100644 --- a/components/cronet/android/cronet_data_reduction_proxy.cc +++ b/components/cronet/android/cronet_data_reduction_proxy.cc @@ -116,7 +116,7 @@ void CronetDataReductionProxy::Init(bool enable, scoped_ptr<data_reduction_proxy::DataReductionProxyService> data_reduction_proxy_service( new data_reduction_proxy::DataReductionProxyService( - compression_stats.Pass(), settings_.get(), + compression_stats.Pass(), settings_.get(), prefs_.get(), url_request_context_getter_.get(), task_runner_)); io_data_->SetDataReductionProxyService( data_reduction_proxy_service->GetWeakPtr()); diff --git a/components/data_reduction_proxy.gypi b/components/data_reduction_proxy.gypi index 2fa94c2..4742760 100644 --- a/components/data_reduction_proxy.gypi +++ b/components/data_reduction_proxy.gypi @@ -17,12 +17,16 @@ 'data_reduction_proxy/core/browser/data_reduction_proxy_compression_stats.h', 'data_reduction_proxy/core/browser/data_reduction_proxy_config.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_config.h', + 'data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.cc', + 'data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.h', 'data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.h', 'data_reduction_proxy/core/browser/data_reduction_proxy_configurator.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h', 'data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_delegate.h', + 'data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.cc', + 'data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.h', 'data_reduction_proxy/core/browser/data_reduction_proxy_interceptor.cc', 'data_reduction_proxy/core/browser/data_reduction_proxy_interceptor.h', 'data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc', diff --git a/components/data_reduction_proxy/DEPS b/components/data_reduction_proxy/DEPS index 54740c7..596361d 100644 --- a/components/data_reduction_proxy/DEPS +++ b/components/data_reduction_proxy/DEPS @@ -1,5 +1,6 @@ include_rules = [ "+components/pref_registry", + "+components/variations", "+crypto", "+google_apis", "+net", diff --git a/components/data_reduction_proxy/core/browser/BUILD.gn b/components/data_reduction_proxy/core/browser/BUILD.gn index a292f6b..22e939a 100644 --- a/components/data_reduction_proxy/core/browser/BUILD.gn +++ b/components/data_reduction_proxy/core/browser/BUILD.gn @@ -12,6 +12,8 @@ static_library("browser") { "data_reduction_proxy_compression_stats.h", "data_reduction_proxy_config.cc", "data_reduction_proxy_config.h", + "data_reduction_proxy_config_retrieval_params.cc", + "data_reduction_proxy_config_retrieval_params.h", "data_reduction_proxy_config_service_client.cc", "data_reduction_proxy_config_service_client.h", "data_reduction_proxy_configurator.cc", @@ -19,6 +21,8 @@ static_library("browser") { "data_reduction_proxy_debug_ui_service.h", "data_reduction_proxy_delegate.cc", "data_reduction_proxy_delegate.h", + "data_reduction_proxy_experiments_stats.cc", + "data_reduction_proxy_experiments_stats.h", "data_reduction_proxy_interceptor.cc", "data_reduction_proxy_interceptor.h", "data_reduction_proxy_io_data.cc", @@ -93,9 +97,11 @@ source_set("unit_tests") { "data_reduction_proxy_bypass_protocol_unittest.cc", "data_reduction_proxy_bypass_stats_unittest.cc", "data_reduction_proxy_compression_stats_unittest.cc", + "data_reduction_proxy_config_retrieval_params_unittest.cc", "data_reduction_proxy_config_service_client_unittest.cc", "data_reduction_proxy_config_unittest.cc", "data_reduction_proxy_configurator_unittest.cc", + "data_reduction_proxy_experiments_stats_unittest.cc", "data_reduction_proxy_interceptor_unittest.cc", "data_reduction_proxy_io_data_unittest.cc", "data_reduction_proxy_metrics_unittest.cc", diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.cc new file mode 100644 index 0000000..9379f67 --- /dev/null +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.cc @@ -0,0 +1,270 @@ +// Copyright 2015 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 "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.h" + +#include "base/metrics/field_trial.h" +#include "base/metrics/histogram.h" +#include "base/prefs/pref_service.h" +#include "base/strings/string_number_conversions.h" +#include "base/strings/string_piece.h" +#include "components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h" +#include "components/variations/variations_associated_data.h" + +namespace { + +// The trial group prefix for which an experiment is considered enabled. +const char kEnabled[] = "Enabled"; + +// The minimum interval (in seconds) at which the config retrieval can occur. +const int kConfigFetchMinimumIntervalSeconds = 300; + +// The default latency base for retrieving the config. +const int kConfigFetchDefaultRoundtripMillisecondsBase = 100; + +// The default latency base for retrieving the config. +const double kConfigFetchDefaultRoundtripMultiplier = 1.0; + +// The default latency base for retrieving the config. +const int kConfigFetchDefaultRoundtripMillisecondsIncrement = 100; + +// The minimum Data Reduction Proxy configuration expiration. +const int kConfigFetchMinimumExpirationSeconds = 5 * 60; + +// The default Data Reduction Proxy configuration expiration. +const int kConfigFetchDefaultExpirationSeconds = 24 * 60 * 60; + +// Based on a histogram prefix |histogram| and |suffix|, retrieves the +// histogram for the expanded histogram name. The histogram is identical to +// the used in UMA_HISTOGRAM_COUNTS. +base::HistogramBase* GetHistogramWithSuffix(const char* histogram, int suffix) { + std::string full_histogram_name = histogram + base::IntToString(suffix); + return base::Histogram::FactoryGet( + full_histogram_name, 1, 1000000, 50, + base::HistogramBase::kUmaTargetedHistogramFlag); +} + +// Retrieves the boolean stored in |variation| from the field trial group +// |group|. If the value is not present or cannot be parsed, returns +// |default_value|. +bool GetVariationBoolWithDefault(const char* group, + const char* variation, + bool default_value) { + std::string variation_value = + variations::GetVariationParamValue(group, variation); + int64 variation_numeric; + if (variation_value.empty() || + !base::StringToInt64(variation_value, &variation_numeric)) { + return default_value; + } + + return variation_numeric != 0; +} + +// Retrieves the int64 stored in |variation| from the field trial group +// |group|. If the value is not present, cannot be parsed, or is less than +// |min_value|, returns |default_value|. +int64 GetVariationInt64WithDefault(const char* group, + const char* variation, + int64 default_value, + int64 min_value) { + DCHECK(default_value >= min_value); + std::string variation_value = + variations::GetVariationParamValue(group, variation); + int64 variation_numeric; + if (variation_value.empty() || + !base::StringToInt64(variation_value, &variation_numeric) || + variation_numeric < min_value) { + return default_value; + } + + return variation_numeric; +} + +// Retrieves the double stored in |variation| from the field trial group +// |group|. If the value is not present, cannot be parsed, or is less than +// |min_value|, returns |default_value|. +double GetVariationDoubleWithDefault(const char* group, + const char* variation, + double default_value, + double min_value) { + DCHECK(default_value >= min_value); + std::string variation_value = + variations::GetVariationParamValue(group, variation); + double variation_numeric; + if (variation_value.empty() || + !base::StringToDouble(variation_value, &variation_numeric) || + variation_numeric < min_value) { + return default_value; + } + + return variation_numeric; +} + +} // namespace + +namespace data_reduction_proxy { + +const int kConfigFetchGroups = 10; + +const char kConfigFetchTrialGroup[] = "DataReductionProxyConfigFetchBytes"; + +const char kConfigRoundtripMillisecondsBaseParam[] = + "config_roundtrip_milliseconds_base"; + +const char kConfigRoundtripMultiplierParam[] = "config_roundtrip_multiplier"; + +const char kConfigRoundtripMillisecondsIncrementParam[] = + "config_roundtrip_milliseconds_increment"; + +const char kConfigExpirationSecondsParam[] = "config_expiration_seconds"; + +const char kConfigAlwaysStaleParam[] = "always_stale"; + +const int kConfigFetchBufferSeconds = 300; + +// static +scoped_ptr<DataReductionProxyConfigRetrievalParams> +DataReductionProxyConfigRetrievalParams::Create(PrefService* pref_service) { + std::string group_value = + base::FieldTrialList::FindFullName(kConfigFetchTrialGroup); + base::StringPiece group = group_value; + if (!group.starts_with(kEnabled)) + return scoped_ptr<DataReductionProxyConfigRetrievalParams>(); + + base::Time now = base::Time::Now(); + base::Time config_retrieve; + bool config_always_stale = GetVariationBoolWithDefault( + kConfigFetchTrialGroup, kConfigAlwaysStaleParam, false); + if (config_always_stale) { + config_retrieve = base::Time(); + } else { + int64 config_retrieve_value = + pref_service->GetInt64(prefs::kSimulatedConfigRetrieveTime); + config_retrieve = base::Time::FromInternalValue(config_retrieve_value); + if (config_retrieve > now) + config_retrieve = base::Time(); + } + + int64 config_expiration_interval_seconds = GetVariationInt64WithDefault( + kConfigFetchTrialGroup, kConfigExpirationSecondsParam, + kConfigFetchDefaultExpirationSeconds, + kConfigFetchMinimumExpirationSeconds); + base::TimeDelta config_expiration_interval = + base::TimeDelta::FromSeconds(config_expiration_interval_seconds); + base::Time config_expiration = config_retrieve + config_expiration_interval; + std::vector<Variation> variations; + bool expired_config = (now > config_expiration); + if (expired_config) { + config_expiration = now + config_expiration_interval; + + int64 config_roundtrip_milliseconds = GetVariationInt64WithDefault( + kConfigFetchTrialGroup, kConfigRoundtripMillisecondsBaseParam, + kConfigFetchDefaultRoundtripMillisecondsBase, 0); + double config_roundtrip_multiplier = GetVariationDoubleWithDefault( + kConfigFetchTrialGroup, kConfigRoundtripMultiplierParam, + kConfigFetchDefaultRoundtripMultiplier, 1.0); + int64 roundtrip_milliseconds_increment = GetVariationInt64WithDefault( + kConfigFetchTrialGroup, kConfigRoundtripMillisecondsIncrementParam, + kConfigFetchDefaultRoundtripMillisecondsIncrement, 0); + + for (int params_index = 0; params_index < kConfigFetchGroups; + ++params_index) { + base::Time config_retrieved = now + base::TimeDelta::FromMilliseconds( + config_roundtrip_milliseconds); + variations.push_back(Variation(params_index, config_retrieved)); + config_roundtrip_milliseconds *= config_roundtrip_multiplier; + config_roundtrip_milliseconds += roundtrip_milliseconds_increment; + } + } + + return scoped_ptr<DataReductionProxyConfigRetrievalParams>( + new DataReductionProxyConfigRetrievalParams(expired_config, variations, + config_expiration, + config_expiration_interval)); +} + +DataReductionProxyConfigRetrievalParams::Variation::Variation( + int index, + const base::Time& simulated_config_retrieved) + : simulated_config_retrieved_(simulated_config_retrieved) { + lost_bytes_ocl_ = GetHistogramWithSuffix( + "DataReductionProxy.ConfigFetchLostBytesOCL_", index); + lost_bytes_rcl_ = GetHistogramWithSuffix( + "DataReductionProxy.ConfigFetchLostBytesCL_", index); + lost_bytes_diff_ = GetHistogramWithSuffix( + "DataReductionProxy.ConfigFetchLostBytesDiff_", index); +} + +DataReductionProxyConfigRetrievalParams::ConfigState +DataReductionProxyConfigRetrievalParams::Variation::GetState( + const base::Time& request_time, + const base::Time& config_expiration) const { + if (!simulated_config_retrieved_.is_null() && + request_time < simulated_config_retrieved_) { + return DataReductionProxyConfigRetrievalParams::RETRIEVING; + } else if (request_time < config_expiration) { + return DataReductionProxyConfigRetrievalParams::VALID; + } + + return DataReductionProxyConfigRetrievalParams::EXPIRED; +} + +void DataReductionProxyConfigRetrievalParams::Variation::RecordStats( + int64 received_content_length, + int64 original_content_length) const { + lost_bytes_rcl_->Add(received_content_length); + lost_bytes_ocl_->Add(original_content_length); + int64 content_length_diff = original_content_length - received_content_length; + if (content_length_diff > 0) + lost_bytes_diff_->Add(content_length_diff); +} + +DataReductionProxyConfigRetrievalParams:: + DataReductionProxyConfigRetrievalParams( + bool loaded_expired_config, + const std::vector<Variation>& variations, + const base::Time& config_expiration, + const base::TimeDelta& config_expiration_interval) + : loaded_expired_config_(loaded_expired_config), + config_expiration_(config_expiration), + config_expiration_interval_(config_expiration_interval), + variations_(variations) { + config_refresh_interval_ = + config_expiration_interval_ - + base::TimeDelta::FromSeconds(kConfigFetchBufferSeconds); + if (config_refresh_interval_.InSeconds() < + kConfigFetchMinimumIntervalSeconds) { + config_refresh_interval_ = + base::TimeDelta::FromSeconds(kConfigFetchMinimumIntervalSeconds); + } +} + +DataReductionProxyConfigRetrievalParams:: + ~DataReductionProxyConfigRetrievalParams() { +} + +void DataReductionProxyConfigRetrievalParams::RecordStats( + const base::Time& request_time, + int64 received_content_length, + int64 original_content_length) const { + for (const auto& variation : variations_) { + switch (variation.GetState(request_time, config_expiration_)) { + case VALID: + break; + case RETRIEVING: + case EXPIRED: + variation.RecordStats(received_content_length, original_content_length); + break; + default: + NOTREACHED(); + } + } +} + +void DataReductionProxyConfigRetrievalParams::RefreshConfig() { + config_expiration_ = base::Time::Now() + config_expiration_interval_; +} + +} // namespace data_reduction_proxy diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.h new file mode 100644 index 0000000..53aa10a --- /dev/null +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.h @@ -0,0 +1,154 @@ +// Copyright 2015 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. + +#ifndef COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_CONFIG_RETRIEVAL_PARAMS_H_ +#define COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_CONFIG_RETRIEVAL_PARAMS_H_ + +#include <vector> + +#include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" +#include "base/time/time.h" + +class PrefService; + +namespace base { +class HistogramBase; +} + +namespace data_reduction_proxy { + +// The number of config fetch variations supported. Must match the number of +// suffixes in histograms.xml. +extern const int kConfigFetchGroups; + +// The name of the trial group for measuring lost bytes during fetching the +// configuration. +extern const char kConfigFetchTrialGroup[]; + +// The config retrieval round trip time is calculated as: +// (previous base) * (multiplier) + increment. +// Multipler is >= 1. Increment is >= 0. +// The name of the param prefix for determining the base round trip time in +// milliseconds for simulating a configuration fetch. +extern const char kConfigRoundtripMillisecondsBaseParam[]; + +// The name of the param prefix for determining the base round trip time in +// milliseconds for simulating a configuration fetch. +extern const char kConfigRoundtripMultiplierParam[]; + +// The name of the param prefix for determining the base round trip time in +// milliseconds for simulating a configuration fetch. +extern const char kConfigRoundtripMillisecondsIncrementParam[]; + +// The name of the param for determining the expiration interval of the +// configuration in seconds. +extern const char kConfigExpirationSecondsParam[]; + +// The name of the param for determining if the config should be considered +// stale on startup (regardless of whether it is stale or not). +extern const char kConfigAlwaysStaleParam[]; + +// The number of seconds by which the expiration interval exceeds the refresh +// interval so that we maintain a valid configuration. +extern const int kConfigFetchBufferSeconds; + +// Parameters for setting up an experiment to measure potentially uncompressed +// bytes during the retrieval of the Data Reduction Proxy configuration. +class DataReductionProxyConfigRetrievalParams { + public: + enum ConfigState { + VALID = 0, + RETRIEVING = 1, + EXPIRED = 2, + }; + + // A variation of the simulated configuration retrieval time, permitting the + // experiment to measure multiple values. + class Variation { + public: + Variation(int index, const base::Time& simulated_config_retrieved); + + // Returns the current state of the Data Reduction Proxy configuration. + // Virtual for testing. + virtual ConfigState GetState(const base::Time& request_time, + const base::Time& config_expiration) const; + + // Records content length statistics if potentially uncompressed bytes have + // been detected for this variation. + void RecordStats(int64 received_content_length, + int64 original_content_length) const; + + // Visible for testing. + const base::Time& simulated_config_retrieved() const { + return simulated_config_retrieved_; + } + + private: + // The time at which the simulated Data Reduction Proxy configuration is + // considered to have been received. + base::Time simulated_config_retrieved_; + + // Histograms for recording stats. + base::HistogramBase* lost_bytes_ocl_; + base::HistogramBase* lost_bytes_rcl_; + base::HistogramBase* lost_bytes_diff_; + }; + + static scoped_ptr<DataReductionProxyConfigRetrievalParams> Create( + PrefService* pref_service); + + virtual ~DataReductionProxyConfigRetrievalParams(); + + // Records content length statistics against any variations for which + // potentially uncompressed bytes have been detected. + void RecordStats(const base::Time& request_time, + int64 received_content_length, + int64 original_content_length) const; + + // Simulates retrieving a new configuration. + void RefreshConfig(); + + bool loaded_expired_config() const { return loaded_expired_config_; } + + // Returns the expiration time of the current configuration. + const base::Time& config_expiration() const { return config_expiration_; } + + // Returns how often the configuration should be retrieved. + const base::TimeDelta& refresh_interval() const { + return config_refresh_interval_; + } + + // Visible for testing. + const std::vector<Variation>& variations() const { return variations_; } + + private: + DataReductionProxyConfigRetrievalParams( + bool loaded_expired_config, + const std::vector<Variation>& variations, + const base::Time& config_expiration, + const base::TimeDelta& config_refresh_interval); + + // Indicates that the Data Reduction Proxy configuration when loaded has + // expired. + bool loaded_expired_config_; + + // The time at which the simulated Data Reduction Proxy configuration + // expires. + base::Time config_expiration_; + + // The duration for which a Data Reduction Proxy configuration is considered + // valid. + base::TimeDelta config_expiration_interval_; + + // The duration after which a simulated retrieval of a new Data Reduction + // Proxy configuration should be performed. + base::TimeDelta config_refresh_interval_; + + // The different variations on roundtrip time that can take place. + std::vector<Variation> variations_; +}; + +} // namespace data_reduction_proxy +#endif // COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_CONFIG_RETRIEVAL_PARAMS_H_ diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params_unittest.cc new file mode 100644 index 0000000..02f82cb --- /dev/null +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params_unittest.cc @@ -0,0 +1,228 @@ +// Copyright 2015 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 "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.h" + +#include <map> + +#include "base/message_loop/message_loop.h" +#include "base/prefs/pref_service.h" +#include "base/strings/string_number_conversions.h" +#include "base/time/time.h" +#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h" +#include "components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h" +#include "components/variations/variations_associated_data.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +// Test value for how long a Data Reduction Proxy configuration should be valid. +const int64 kConfigExpirationSeconds = 60 * 60 * 24; + +// Checks that |actual| falls in the range of |min| + |delta| and +// |max| + |delta|. +void ExpectInTimeRange(const base::Time& min, + const base::Time& max, + const base::TimeDelta& delta, + const base::Time& actual) { + EXPECT_GE(actual, min + delta); + EXPECT_LE(actual, max + delta); +} + +} // namespace + +namespace data_reduction_proxy { + +class DataReductionProxyConfigRetrievalParamsTest : public testing::Test { + protected: + void SetUp() override { + test_context_ = DataReductionProxyTestContext::Builder().Build(); + } + + void SetConfigExperimentValues( + bool enabled, + const base::TimeDelta& retrieve_offset_from_now, + int64 roundtrip_milliseconds_base, + double roundtrip_multiplier, + int64 roundtrip_milliseconds_increment, + int64 expiration_seconds, + bool always_stale) { + variations::testing::ClearAllVariationParams(); + std::map<std::string, std::string> variation_params; + variation_params[kConfigRoundtripMillisecondsBaseParam] = + base::Int64ToString(roundtrip_milliseconds_base); + variation_params[kConfigRoundtripMultiplierParam] = + base::DoubleToString(roundtrip_multiplier); + variation_params[kConfigRoundtripMillisecondsIncrementParam] = + base::Int64ToString(roundtrip_milliseconds_increment); + variation_params[kConfigExpirationSecondsParam] = + base::Int64ToString(expiration_seconds); + variation_params[kConfigAlwaysStaleParam] = always_stale ? "1" : "0:"; + if (enabled) { + ASSERT_TRUE(variations::AssociateVariationParams( + kConfigFetchTrialGroup, "Enabled", variation_params)); + } + pref_service()->SetInt64( + prefs::kSimulatedConfigRetrieveTime, + (base::Time::Now() + retrieve_offset_from_now).ToInternalValue()); + } + + PrefService* pref_service() { return test_context_->pref_service(); } + + private: + base::MessageLoopForIO message_loop_; + scoped_ptr<DataReductionProxyTestContext> test_context_; +}; + +TEST_F(DataReductionProxyConfigRetrievalParamsTest, ExpectedVariations) { + struct { + bool experiment_enabled; + base::TimeDelta retrieve_offset_from_now; + int64 expiration_seconds; + int64 roundtrip_milliseconds_base; + double roundtrip_multiplier; + int64 roundtrip_milliseconds_increment; + bool always_stale; + bool expected_has_variations; + base::TimeDelta expected_config_expiration_from_now; + base::TimeDelta expected_delta_0; + base::TimeDelta expected_delta_2; + base::TimeDelta expected_delta_5; + } test_cases[] = { + // Experiment is disabled. + {false, + base::TimeDelta::FromHours(-2), + kConfigExpirationSeconds, + 100, + 2.0, + 100, + false, + false, + base::TimeDelta(), + base::TimeDelta(), + base::TimeDelta(), + base::TimeDelta()}, + // Experiment is enabled, but not marked always stale and the last + // retrieval is such that the configuration is still valid. + {true, + base::TimeDelta::FromHours(-2), + kConfigExpirationSeconds, + 100, + 2.0, + 100, + false, + false, + base::TimeDelta::FromHours(22), + base::TimeDelta(), + base::TimeDelta(), + base::TimeDelta()}, + // Experiment is enabled, but not marked always stale with a retrieval + // time sufficiently in the past such that the configuration is expired. + { + true, + base::TimeDelta::FromHours(-26), + kConfigExpirationSeconds, + 100, + 2.0, + 100, + true, + true, + base::TimeDelta::FromSeconds(kConfigExpirationSeconds), + base::TimeDelta::FromMilliseconds(100), + base::TimeDelta::FromMilliseconds(700), + base::TimeDelta::FromMilliseconds(6300), + }, + // Experiment is enabled, marked always stale and the retrieval time is + // recent enough that a configuration would be still valid. + { + true, + base::TimeDelta::FromHours(-2), + kConfigExpirationSeconds, + 100, + 2.0, + 100, + true, + true, + base::TimeDelta::FromSeconds(kConfigExpirationSeconds), + base::TimeDelta::FromMilliseconds(100), + base::TimeDelta::FromMilliseconds(700), + base::TimeDelta::FromMilliseconds(6300), + }, + // Experiment is enabled but values are invalid. + { + true, + base::TimeDelta::FromHours(-2), + -400, + -500, + 0.5, + -300, + true, + true, + base::TimeDelta::FromSeconds(kConfigExpirationSeconds), + base::TimeDelta::FromMilliseconds(100), + base::TimeDelta::FromMilliseconds(300), + base::TimeDelta::FromMilliseconds(600), + }, + // Experiment is enabled, but the last config retrieval time is in the + // future and thus ignored. + { + true, + base::TimeDelta::FromHours(26), + kConfigExpirationSeconds, + 100, + 2.0, + 100, + true, + true, + base::TimeDelta::FromSeconds(kConfigExpirationSeconds), + base::TimeDelta::FromMilliseconds(100), + base::TimeDelta::FromMilliseconds(700), + base::TimeDelta::FromMilliseconds(6300), + }, + }; + + for (const auto& test_case : test_cases) { + base::FieldTrialList field_trial_list(nullptr); + base::FieldTrialList::CreateFieldTrial( + kConfigFetchTrialGroup, + test_case.experiment_enabled ? "Enabled" : "Disabled"); + base::Time min_now = base::Time::Now(); + SetConfigExperimentValues( + test_case.experiment_enabled, test_case.retrieve_offset_from_now, + test_case.roundtrip_milliseconds_base, test_case.roundtrip_multiplier, + test_case.roundtrip_milliseconds_increment, kConfigExpirationSeconds, + test_case.always_stale); + scoped_ptr<DataReductionProxyConfigRetrievalParams> config_params = + DataReductionProxyConfigRetrievalParams::Create(pref_service()); + base::Time max_now = base::Time::Now(); + if (!test_case.experiment_enabled) { + EXPECT_EQ(nullptr, config_params.get()); + continue; + } + + EXPECT_NE(nullptr, config_params.get()); + ExpectInTimeRange(min_now, max_now, + test_case.expected_config_expiration_from_now, + config_params->config_expiration()); + EXPECT_EQ(config_params->refresh_interval(), + base::TimeDelta::FromSeconds(kConfigExpirationSeconds) - + base::TimeDelta::FromSeconds(kConfigFetchBufferSeconds)); + if (test_case.expected_has_variations) { + EXPECT_EQ(kConfigFetchGroups, (int)config_params->variations().size()); + ExpectInTimeRange( + min_now, max_now, test_case.expected_delta_0, + config_params->variations()[0].simulated_config_retrieved()); + ExpectInTimeRange( + min_now, max_now, test_case.expected_delta_2, + config_params->variations()[2].simulated_config_retrieved()); + ExpectInTimeRange( + min_now, max_now, test_case.expected_delta_5, + config_params->variations()[5].simulated_config_retrieved()); + } else { + EXPECT_EQ(0u, config_params->variations().size()); + } + } +} + +} // namespace data_reduction_proxy diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.cc new file mode 100644 index 0000000..ddf3de1 --- /dev/null +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.cc @@ -0,0 +1,67 @@ +// Copyright 2015 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 "components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.h" + +#include "base/logging.h" +#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.h" +#include "components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h" + +namespace data_reduction_proxy { + +DataReductionProxyExperimentsStats::DataReductionProxyExperimentsStats( + const Int64ValueSetter& value_setter) + : value_setter_(value_setter), initialized_(false) { + // Constructed on the UI thread, but should be checked on the IO thread. + thread_checker_.DetachFromThread(); +} + +DataReductionProxyExperimentsStats::~DataReductionProxyExperimentsStats() { +} + +void DataReductionProxyExperimentsStats::InitializeOnUIThread(scoped_ptr< + DataReductionProxyConfigRetrievalParams> config_retrieval_params) { + DCHECK(!initialized_); + config_retrieval_params_ = config_retrieval_params.Pass(); + + // This method may be called from the UI thread, but should be checked on the + // IO thread. + thread_checker_.DetachFromThread(); +} + +void DataReductionProxyExperimentsStats::InitializeOnIOThread() { + DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK(!initialized_); + initialized_ = true; + if (config_retrieval_params_) { + if (config_retrieval_params_->loaded_expired_config()) + UpdateSimulatedConfig(); + config_refresh_time_.Start( + FROM_HERE, config_retrieval_params_->refresh_interval(), this, + &DataReductionProxyExperimentsStats::UpdateSimulatedConfig); + } +} + +void DataReductionProxyExperimentsStats::RecordBytes( + const base::Time& request_time, + DataReductionProxyRequestType request_type, + int64 received_content_length, + int64 original_content_length) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (config_retrieval_params_) { + // Only measure requests which flowed through the Data Reduction Proxy. + if (request_type == VIA_DATA_REDUCTION_PROXY) { + config_retrieval_params_->RecordStats( + request_time, received_content_length, original_content_length); + } + } +} + +void DataReductionProxyExperimentsStats::UpdateSimulatedConfig() { + DCHECK(config_retrieval_params_); + value_setter_.Run(prefs::kSimulatedConfigRetrieveTime, + base::Time::Now().ToInternalValue()); +} + +} // namespace data_reduction_proxy diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.h new file mode 100644 index 0000000..1806d51 --- /dev/null +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.h @@ -0,0 +1,80 @@ +// Copyright 2015 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. + +#ifndef COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_EXPERIMENTS_STATS_H_ +#define COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_EXPERIMENTS_STATS_H_ + +#include <string> + +#include "base/basictypes.h" +#include "base/callback.h" +#include "base/macros.h" +#include "base/memory/scoped_ptr.h" +#include "base/threading/thread_checker.h" +#include "base/timer/timer.h" +#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.h" + +namespace base { +class Time; +} + +namespace data_reduction_proxy { + +class DataReductionProxyConfigRetrievalParams; + +typedef base::Callback<void(const std::string&, int64)> Int64ValueSetter; + +// Collects statistics specific to experiments of which a client may be part. +// Any calls into this class should be as lightweight as possible such that if +// no experiments are being performed, there should be minimal code being +// executed. +// Currently supported experiments are: +// - Measuring potentially uncompressed bytes during simulated config retrieval. +class DataReductionProxyExperimentsStats { + public: + DataReductionProxyExperimentsStats(const Int64ValueSetter& value_setter); + virtual ~DataReductionProxyExperimentsStats(); + + // Initializes |this| on the UI thread. If called, must be called prior to + // InitializeOnIOThread. + void InitializeOnUIThread(scoped_ptr<DataReductionProxyConfigRetrievalParams> + config_retrieval_params); + + // Initializes |this| on the IO thread. + void InitializeOnIOThread(); + + // Collect received bytes for the experiment. + void RecordBytes(const base::Time& request_time, + DataReductionProxyRequestType request_type, + int64 received_content_length, + int64 original_content_length); + + private: + // Updates the simulated expiration of the Data Reduction Proxy configuration + // if |config_retrieval_| is set. + void UpdateSimulatedConfig(); + + // Allows an experiment to persist data to preferences. + Int64ValueSetter value_setter_; + + // Enables measuring of potentially uncompressed bytes during a simulated Data + // Reduction Proxy configuration retrieval. + scoped_ptr<DataReductionProxyConfigRetrievalParams> config_retrieval_params_; + + // If set, periodically updates the simulated expiration of the Data Reduction + // Proxy configuration. + base::RepeatingTimer<DataReductionProxyExperimentsStats> config_refresh_time_; + + // Enforce initialization order. + bool initialized_; + + // Enforce usage on the IO thread. + base::ThreadChecker thread_checker_; + + DISALLOW_COPY_AND_ASSIGN(DataReductionProxyExperimentsStats); +}; + +} // namespace data_reduction_proxy + +#endif // COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_EXPERIMENTS_STATS_H_ diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats_unittest.cc new file mode 100644 index 0000000..6b1d2d3 --- /dev/null +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats_unittest.cc @@ -0,0 +1,163 @@ +// Copyright 2015 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 "components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.h" + +#include <map> + +#include "base/metrics/field_trial.h" +#include "base/prefs/pref_service.h" +#include "base/strings/string_number_conversions.h" +#include "base/test/histogram_tester.h" +#include "base/time/time.h" +#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_retrieval_params.h" +#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h" +#include "components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h" +#include "components/variations/variations_associated_data.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +// Test value for how long a Data Reduction Proxy configuration should be valid. +const int64 kConfigExpirationSeconds = 60 * 60 * 24; + +} // namespace + +namespace data_reduction_proxy { + +class DataReductionProxyExperimentsStatsTest : public testing::Test { + public: + void SetPrefValue(const std::string& path, int64 value) {} + + protected: + void SetUp() override { + test_context_ = DataReductionProxyTestContext::Builder().Build(); + } + + void SetConfigExperimentValues( + bool enabled, + const base::TimeDelta& retrieve_offset_from_now, + int64 roundtrip_milliseconds_base, + double roundtrip_multiplier, + int64 roundtrip_milliseconds_increment, + int64 expiration_seconds, + bool always_stale) { + variations::testing::ClearAllVariationParams(); + std::map<std::string, std::string> variation_params; + variation_params[kConfigRoundtripMillisecondsBaseParam] = + base::Int64ToString(roundtrip_milliseconds_base); + variation_params[kConfigRoundtripMultiplierParam] = + base::DoubleToString(roundtrip_multiplier); + variation_params[kConfigRoundtripMillisecondsIncrementParam] = + base::Int64ToString(roundtrip_milliseconds_increment); + variation_params[kConfigExpirationSecondsParam] = + base::Int64ToString(expiration_seconds); + variation_params[kConfigAlwaysStaleParam] = always_stale ? "1" : "0:"; + if (enabled) { + ASSERT_TRUE(variations::AssociateVariationParams( + kConfigFetchTrialGroup, "Enabled", variation_params)); + } + pref_service()->SetInt64( + prefs::kSimulatedConfigRetrieveTime, + (base::Time::Now() + retrieve_offset_from_now).ToInternalValue()); + } + + PrefService* pref_service() { return test_context_->pref_service(); } + + private: + base::MessageLoopForIO message_loop_; + scoped_ptr<DataReductionProxyTestContext> test_context_; +}; + +TEST_F(DataReductionProxyExperimentsStatsTest, CheckHistogramsNoExperiments) { + scoped_ptr<DataReductionProxyExperimentsStats> experiments_stats( + new DataReductionProxyExperimentsStats( + base::Bind(&DataReductionProxyExperimentsStatsTest::SetPrefValue, + base::Unretained(this)))); + experiments_stats->InitializeOnUIThread( + scoped_ptr<DataReductionProxyConfigRetrievalParams>()); + base::HistogramTester histogram_tester; + experiments_stats->RecordBytes(base::Time::Now(), VIA_DATA_REDUCTION_PROXY, + 500, 1000); + histogram_tester.ExpectTotalCount( + "DataReductionProxy.ConfigFetchLostBytesOCL_0", 0); + histogram_tester.ExpectTotalCount( + "DataReductionProxy.ConfigFetchLostBytesCL_0", 0); + histogram_tester.ExpectTotalCount( + "DataReductionProxy.ConfigFetchLostBytesDiff_0", 0); +} + +TEST_F(DataReductionProxyExperimentsStatsTest, CheckConfigHistograms) { + struct { + DataReductionProxyRequestType request_type; + base::Time request_time; + int64 received_content_length; + int64 original_content_length; + int64 expected_received_content_length; + int64 expected_original_content_length; + int64 expected_diff; + } test_cases[] = { + { + VIA_DATA_REDUCTION_PROXY, + base::Time::Now() + base::TimeDelta::FromMinutes(5), + 500, + 1000, + -1, + -1, + -1, + }, + { + VIA_DATA_REDUCTION_PROXY, base::Time::Now(), 500, 1000, 500, 1000, 500, + }, + { + HTTPS, base::Time::Now(), 500, 1000, -1, -1, -1, + }, + }; + + base::FieldTrialList field_trial_list(nullptr); + base::FieldTrialList::CreateFieldTrial(kConfigFetchTrialGroup, "Enabled"); + SetConfigExperimentValues(true, base::TimeDelta::FromHours(-2), 100, 2.0, + 1000, kConfigExpirationSeconds, true); + scoped_ptr<DataReductionProxyConfigRetrievalParams> config_params = + DataReductionProxyConfigRetrievalParams::Create(pref_service()); + scoped_ptr<DataReductionProxyExperimentsStats> experiments_stats( + new DataReductionProxyExperimentsStats( + base::Bind(&DataReductionProxyExperimentsStatsTest::SetPrefValue, + base::Unretained(this)))); + experiments_stats->InitializeOnUIThread(config_params.Pass()); + for (const auto& test_case : test_cases) { + base::HistogramTester histogram_tester; + experiments_stats->RecordBytes( + test_case.request_time, test_case.request_type, + test_case.received_content_length, test_case.original_content_length); + if (test_case.expected_received_content_length == -1) { + histogram_tester.ExpectTotalCount( + "DataReductionProxy.ConfigFetchLostBytesCL_0", 0); + } else { + histogram_tester.ExpectUniqueSample( + "DataReductionProxy.ConfigFetchLostBytesCL_0", + test_case.expected_received_content_length, 1); + } + + if (test_case.expected_original_content_length == -1) { + histogram_tester.ExpectTotalCount( + "DataReductionProxy.ConfigFetchLostBytesOCL_0", 0); + } else { + histogram_tester.ExpectUniqueSample( + "DataReductionProxy.ConfigFetchLostBytesOCL_0", + test_case.expected_original_content_length, 1); + } + + if (test_case.expected_diff == -1) { + histogram_tester.ExpectTotalCount( + "DataReductionProxy.ConfigFetchLostBytesDiff_0", 0); + } else { + histogram_tester.ExpectUniqueSample( + "DataReductionProxy.ConfigFetchLostBytesDiff_0", + test_case.expected_diff, 1); + } + } +} + +} // namespace data_reduction_proxy diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc index 50a303c..5a0e52f 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc @@ -14,6 +14,7 @@ #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.h" +#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h" @@ -142,6 +143,11 @@ DataReductionProxyIOData::DataReductionProxyIOData( proxy_delegate_.reset( new DataReductionProxyDelegate(request_options_.get(), config_.get())); + // It is safe to use base::Unretained here, since it gets executed + // synchronously on the IO thread, and |this| outlives the caller (since the + // caller is owned by |this|. + experiments_stats_.reset(new DataReductionProxyExperimentsStats(base::Bind( + &DataReductionProxyIOData::SetInt64Pref, base::Unretained(this)))); } DataReductionProxyIOData::DataReductionProxyIOData() @@ -177,6 +183,7 @@ void DataReductionProxyIOData::InitializeOnIOThread() { config_->InitializeOnIOThread(basic_url_request_context_getter_.get()); if (config_client_.get()) config_client_->InitializeOnIOThread(url_request_context_getter_); + experiments_stats_->InitializeOnIOThread(); ui_task_runner_->PostTask( FROM_HERE, base::Bind(&DataReductionProxyService::SetIOData, @@ -209,7 +216,8 @@ DataReductionProxyIOData::CreateNetworkDelegate( scoped_ptr<DataReductionProxyNetworkDelegate> network_delegate( new DataReductionProxyNetworkDelegate( wrapped_network_delegate.Pass(), config_.get(), - request_options_.get(), configurator_.get())); + request_options_.get(), configurator_.get(), + experiments_stats_.get())); if (track_proxy_bypass_statistics) network_delegate->InitIODataAndUMA(this, bypass_stats_.get()); return network_delegate.Pass(); @@ -279,4 +287,12 @@ void DataReductionProxyIOData::SetUnreachable(bool unreachable) { service_, unreachable)); } +void DataReductionProxyIOData::SetInt64Pref(const std::string& pref_path, + int64 value) { + DCHECK(io_task_runner_->BelongsToCurrentThread()); + ui_task_runner_->PostTask( + FROM_HERE, base::Bind(&DataReductionProxyService::SetInt64Pref, service_, + pref_path, value)); +} + } // namespace data_reduction_proxy diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h index d68001f..fbfce47 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h @@ -34,6 +34,7 @@ class DataReductionProxyConfig; class DataReductionProxyConfigServiceClient; class DataReductionProxyConfigurator; class DataReductionProxyEventCreator; +class DataReductionProxyExperimentsStats; class DataReductionProxyService; // Contains and initializes all Data Reduction Proxy objects that operate on @@ -60,7 +61,8 @@ class DataReductionProxyIOData : public DataReductionProxyEventStorageDelegate { void ShutdownOnUIThread(); // Sets the Data Reduction Proxy service after it has been created. - void SetDataReductionProxyService( + // Virtual for testing. + virtual void SetDataReductionProxyService( base::WeakPtr<DataReductionProxyService> data_reduction_proxy_service); void RetrieveConfig(); @@ -124,6 +126,10 @@ class DataReductionProxyIOData : public DataReductionProxyEventStorageDelegate { return config_client_.get(); } + DataReductionProxyExperimentsStats* experiments_stats() const { + return experiments_stats_.get(); + } + net::ProxyDelegate* proxy_delegate() const { return proxy_delegate_.get(); } @@ -165,6 +171,9 @@ class DataReductionProxyIOData : public DataReductionProxyEventStorageDelegate { // Records that the data reduction proxy is unreachable or not. void SetUnreachable(bool unreachable); + // Stores an int64 value in preferences storage. + void SetInt64Pref(const std::string& pref_path, int64 value); + // The type of Data Reduction Proxy client. Client client_; @@ -197,6 +206,9 @@ class DataReductionProxyIOData : public DataReductionProxyEventStorageDelegate { // Requests new Data Reduction Proxy configurations from a remote service. scoped_ptr<DataReductionProxyConfigServiceClient> config_client_; + // Used to track stats for experiments. + scoped_ptr<DataReductionProxyExperimentsStats> experiments_stats_; + // A net log. net::NetLog* net_log_; diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc index 9a66acd..706f172 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc @@ -12,6 +12,7 @@ #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h" +#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h" #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h" @@ -76,7 +77,8 @@ DataReductionProxyNetworkDelegate::DataReductionProxyNetworkDelegate( scoped_ptr<net::NetworkDelegate> network_delegate, DataReductionProxyConfig* config, DataReductionProxyRequestOptions* request_options, - const DataReductionProxyConfigurator* configurator) + const DataReductionProxyConfigurator* configurator, + DataReductionProxyExperimentsStats* experiments_stats) : LayeredNetworkDelegate(network_delegate.Pass()), received_content_length_(0), original_content_length_(0), @@ -84,9 +86,11 @@ DataReductionProxyNetworkDelegate::DataReductionProxyNetworkDelegate( data_reduction_proxy_bypass_stats_(nullptr), data_reduction_proxy_request_options_(request_options), data_reduction_proxy_io_data_(nullptr), - configurator_(configurator) { + configurator_(configurator), + experiments_stats_(experiments_stats) { DCHECK(data_reduction_proxy_config_); DCHECK(data_reduction_proxy_request_options_); + DCHECK(experiments_stats_); } DataReductionProxyNetworkDelegate::~DataReductionProxyNetworkDelegate() { @@ -185,6 +189,9 @@ void DataReductionProxyNetworkDelegate::OnCompletedInternal( RecordContentLengthHistograms(received_content_length, original_content_length, freshness_lifetime); + experiments_stats_->RecordBytes(request->request_time(), request_type, + received_content_length, + original_content_length); if (data_reduction_proxy_io_data_ && data_reduction_proxy_bypass_stats_) { data_reduction_proxy_bypass_stats_->RecordBytesHistograms( diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h index 0290d6e..da13b28 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h @@ -31,6 +31,7 @@ namespace data_reduction_proxy { class DataReductionProxyBypassStats; class DataReductionProxyConfig; class DataReductionProxyConfigurator; +class DataReductionProxyExperimentsStats; class DataReductionProxyIOData; class DataReductionProxyRequestOptions; @@ -52,7 +53,8 @@ class DataReductionProxyNetworkDelegate : public net::LayeredNetworkDelegate { scoped_ptr<net::NetworkDelegate> network_delegate, DataReductionProxyConfig* config, DataReductionProxyRequestOptions* handler, - const DataReductionProxyConfigurator* configurator); + const DataReductionProxyConfigurator* configurator, + DataReductionProxyExperimentsStats* experiments_stats); ~DataReductionProxyNetworkDelegate() override; // Initializes member variables to record data reduction proxy prefs and @@ -122,6 +124,8 @@ class DataReductionProxyNetworkDelegate : public net::LayeredNetworkDelegate { const DataReductionProxyConfigurator* configurator_; + DataReductionProxyExperimentsStats* experiments_stats_; + DISALLOW_COPY_AND_ASSIGN(DataReductionProxyNetworkDelegate); }; 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 b7402b0..897162c 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 @@ -82,7 +82,8 @@ class DataReductionProxyNetworkDelegateTest : public testing::Test { new DataReductionProxyNetworkDelegate( scoped_ptr<net::NetworkDelegate>(new TestNetworkDelegate()), config(), test_context_->io_data()->request_options(), - test_context_->configurator())); + test_context_->configurator(), + test_context_->io_data()->experiments_stats())); } const net::ProxyConfig& GetProxyConfig() const { diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_prefs.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_prefs.cc index 2b59e3a..ac95358 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_prefs.cc +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_prefs.cc @@ -59,6 +59,7 @@ void RegisterSyncableProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { prefs::kDailyOriginalContentLengthViaDataReductionProxy); registry->RegisterListPref(prefs::kDailyContentLengthViaDataReductionProxy); registry->RegisterInt64Pref(prefs::kDailyHttpContentLengthLastUpdateDate, 0L); + registry->RegisterInt64Pref(prefs::kSimulatedConfigRetrieveTime, 0L); } void RegisterSimpleProfilePrefs(PrefRegistrySimple* registry) { @@ -105,6 +106,7 @@ void RegisterPrefs(PrefRegistrySimple* registry) { prefs::kDailyContentLengthViaDataReductionProxy); registry->RegisterInt64Pref( prefs::kDailyHttpContentLengthLastUpdateDate, 0L); + registry->RegisterInt64Pref(prefs::kSimulatedConfigRetrieveTime, 0L); } void MigrateStatisticsPrefs(PrefService* local_state_prefs, diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc index 374489e..9f6b4b0 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc @@ -6,6 +6,7 @@ #include "base/bind.h" #include "base/location.h" +#include "base/prefs/pref_service.h" #include "base/sequenced_task_runner.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_compression_stats.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h" @@ -18,10 +19,12 @@ namespace data_reduction_proxy { DataReductionProxyService::DataReductionProxyService( scoped_ptr<DataReductionProxyCompressionStats> compression_stats, DataReductionProxySettings* settings, + PrefService* prefs, net::URLRequestContextGetter* request_context_getter, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) : url_request_context_getter_(request_context_getter), settings_(settings), + prefs_(prefs), io_task_runner_(io_task_runner), initialized_(false), weak_factory_(this) { @@ -53,8 +56,10 @@ void DataReductionProxyService::EnableCompressionStatisticsLogging( const base::TimeDelta& commit_delay) { DCHECK(CalledOnValidThread()); DCHECK(!compression_stats_); + DCHECK(!prefs_); + prefs_ = prefs; compression_stats_.reset(new DataReductionProxyCompressionStats( - prefs, ui_task_runner, commit_delay)); + prefs_, ui_task_runner, commit_delay)); } void DataReductionProxyService::UpdateContentLengths( @@ -100,6 +105,12 @@ void DataReductionProxyService::SetUnreachable(bool unreachable) { settings_->SetUnreachable(unreachable); } +void DataReductionProxyService::SetInt64Pref(const std::string& pref_path, + int64 value) { + if (prefs_) + prefs_->SetInt64(pref_path, value); +} + void DataReductionProxyService::SetProxyPrefs(bool enabled, bool alternative_enabled, bool at_startup) { diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h index 2fbc18d..df2b38b 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h @@ -43,14 +43,16 @@ class DataReductionProxyService : public base::NonThreadSafe, public DataReductionProxyEventStorageDelegate { public: - // The caller must ensure that |settings| and |request_context| remain alive - // for the lifetime of the |DataReductionProxyService| instance. This instance + // The caller must ensure that |settings|, |prefs|, |request_context|, and + // |io_task_runner| remain alive for the lifetime of the + // |DataReductionProxyService| instance. |prefs| may be null. This instance // will take ownership of |compression_stats|. // TODO(jeremyim): DataReductionProxyService should own // DataReductionProxySettings and not vice versa. DataReductionProxyService( scoped_ptr<DataReductionProxyCompressionStats> compression_stats, DataReductionProxySettings* settings, + PrefService* prefs, net::URLRequestContextGetter* request_context_getter, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner); @@ -89,6 +91,9 @@ class DataReductionProxyService // Records whether the Data Reduction Proxy is unreachable or not. void SetUnreachable(bool unreachable); + // Stores an int64 value in |prefs_|. + void SetInt64Pref(const std::string& pref_path, int64 value); + // Bridge methods to safely call to the UI thread objects. // Virtual for testing. virtual void SetProxyPrefs(bool enabled, @@ -129,6 +134,9 @@ class DataReductionProxyService DataReductionProxySettings* settings_; + // A prefs service for storing data. + PrefService* prefs_; + // Used to post tasks to |io_data_|. scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_; diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc index a9d620d..706648f 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc @@ -9,6 +9,7 @@ #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator_test_utils.h" +#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_experiments_stats.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h" @@ -166,10 +167,14 @@ void TestDataReductionProxyConfigServiceClient::TestTickClock::SetTime( MockDataReductionProxyService::MockDataReductionProxyService( scoped_ptr<DataReductionProxyCompressionStats> compression_stats, DataReductionProxySettings* settings, + PrefService* prefs, net::URLRequestContextGetter* request_context, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) - : DataReductionProxyService( - compression_stats.Pass(), settings, request_context, io_task_runner) { + : DataReductionProxyService(compression_stats.Pass(), + settings, + prefs, + request_context, + io_task_runner) { } MockDataReductionProxyService::~MockDataReductionProxyService() { @@ -182,8 +187,9 @@ TestDataReductionProxyIOData::TestDataReductionProxyIOData( scoped_ptr<DataReductionProxyRequestOptions> request_options, scoped_ptr<DataReductionProxyConfigurator> configurator, scoped_ptr<DataReductionProxyConfigServiceClient> config_client, + scoped_ptr<DataReductionProxyExperimentsStats> experiments_stats, bool enabled) - : DataReductionProxyIOData() { + : DataReductionProxyIOData(), service_set_(false) { io_task_runner_ = task_runner; ui_task_runner_ = task_runner; config_ = config.Pass(); @@ -191,6 +197,7 @@ TestDataReductionProxyIOData::TestDataReductionProxyIOData( request_options_ = request_options.Pass(); configurator_ = configurator.Pass(); config_client_ = config_client.Pass(); + experiments_stats_ = experiments_stats.Pass(); bypass_stats_.reset(new DataReductionProxyBypassStats( config_.get(), base::Bind(&DataReductionProxyIOData::SetUnreachable, base::Unretained(this)))); @@ -202,6 +209,15 @@ TestDataReductionProxyIOData::TestDataReductionProxyIOData( TestDataReductionProxyIOData::~TestDataReductionProxyIOData() { } +void TestDataReductionProxyIOData::SetDataReductionProxyService( + base::WeakPtr<DataReductionProxyService> data_reduction_proxy_service) { + if (!service_set_) + DataReductionProxyIOData::SetDataReductionProxyService( + data_reduction_proxy_service); + + service_set_ = true; +} + DataReductionProxyTestContext::Builder::Builder() : params_flags_(DataReductionProxyParams::kAllowed | DataReductionProxyParams::kFallbackAllowed | @@ -391,11 +407,14 @@ DataReductionProxyTestContext::Builder::Build() { RegisterSimpleProfilePrefs(pref_service->registry()); + scoped_ptr<DataReductionProxyExperimentsStats> experiments_stats( + new DataReductionProxyExperimentsStats(base::Bind( + &PrefService::SetInt64, base::Unretained(pref_service.get())))); scoped_ptr<TestDataReductionProxyIOData> io_data( new TestDataReductionProxyIOData( task_runner, config.Pass(), event_creator.Pass(), request_options.Pass(), configurator.Pass(), config_client.Pass(), - true /* enabled */)); + experiments_stats.Pass(), true /* enabled */)); io_data->SetSimpleURLRequestContextGetter(request_context_getter); scoped_ptr<DataReductionProxyTestContext> test_context( @@ -476,11 +495,11 @@ DataReductionProxyTestContext::CreateDataReductionProxyServiceInternal() { if (test_context_flags_ & DataReductionProxyTestContext::USE_MOCK_SERVICE) { return make_scoped_ptr(new MockDataReductionProxyService( - compression_stats.Pass(), settings_.get(), + compression_stats.Pass(), settings_.get(), simple_pref_service_.get(), request_context_getter_.get(), task_runner_)); } else { return make_scoped_ptr(new DataReductionProxyService( - compression_stats.Pass(), settings_.get(), + compression_stats.Pass(), settings_.get(), simple_pref_service_.get(), request_context_getter_.get(), task_runner_)); } } diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h index 004a3a5..5898569 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h @@ -43,6 +43,7 @@ namespace data_reduction_proxy { class DataReductionProxyConfigurator; class DataReductionProxyEventCreator; +class DataReductionProxyExperimentsStats; class DataReductionProxyMutableConfigValues; class DataReductionProxyRequestOptions; class DataReductionProxySettings; @@ -150,6 +151,7 @@ class MockDataReductionProxyService : public DataReductionProxyService { MockDataReductionProxyService( scoped_ptr<DataReductionProxyCompressionStats> compression_stats, DataReductionProxySettings* settings, + PrefService* prefs, net::URLRequestContextGetter* request_context, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner); ~MockDataReductionProxyService() override; @@ -170,9 +172,13 @@ class TestDataReductionProxyIOData : public DataReductionProxyIOData { scoped_ptr<DataReductionProxyRequestOptions> request_options, scoped_ptr<DataReductionProxyConfigurator> configurator, scoped_ptr<DataReductionProxyConfigServiceClient> config_client, + scoped_ptr<DataReductionProxyExperimentsStats> experiments_stats, bool enabled); ~TestDataReductionProxyIOData() override; + void SetDataReductionProxyService(base::WeakPtr<DataReductionProxyService> + data_reduction_proxy_service) override; + DataReductionProxyConfigurator* configurator() const { return configurator_.get(); } @@ -189,6 +195,10 @@ class TestDataReductionProxyIOData : public DataReductionProxyIOData { base::WeakPtr<DataReductionProxyIOData> GetWeakPtr() { return weak_factory_.GetWeakPtr(); } + + private: + // Allowed SetDataReductionProxyService to be re-entrant. + bool service_set_; }; // Builds a test version of the Data Reduction Proxy stack for use in tests. diff --git a/components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.cc b/components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.cc index a086117..9979f38 100644 --- a/components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.cc +++ b/components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.cc @@ -88,6 +88,13 @@ const char kHttpReceivedContentLength[] = "http_received_content_length"; // received over the network. const char kHttpOriginalContentLength[] = "http_original_content_length"; +// Pref to store the retrieval time of the last simulated Data Reduction Proxy +// configuration. This is part of an experiment to see how many bytes are lost +// if the Data Reduction Proxy is not used due to configuration being expired +// or not available. +const char kSimulatedConfigRetrieveTime[] = + "data_reduction.simulated_config_retrieve_time"; + // A boolean specifying whether the data reduction proxy statistics preferences // have migrated from local state to the profile. const char kStatisticsPrefsMigrated[] = diff --git a/components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h b/components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h index 9a9b36a..1766046 100644 --- a/components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h +++ b/components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h @@ -28,6 +28,7 @@ extern const char kDataReductionProxyAltEnabled[]; extern const char kDataReductionProxyWasEnabledBefore[]; extern const char kHttpOriginalContentLength[]; extern const char kHttpReceivedContentLength[]; +extern const char kSimulatedConfigRetrieveTime[]; extern const char kStatisticsPrefsMigrated[]; extern const char kUpdateDailyReceivedContentLengths[]; |