diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc index 8954197117c9b3..c68d2bf7b816bb 100644 --- a/chrome/browser/metrics/chrome_metrics_service_client.cc +++ b/chrome/browser/metrics/chrome_metrics_service_client.cc @@ -11,6 +11,7 @@ #include "base/command_line.h" #include "base/files/file_path.h" #include "base/logging.h" +#include "base/metrics/field_trial.h" #include "base/metrics/histogram.h" #include "base/prefs/pref_registry_simple.h" #include "base/prefs/pref_service.h" @@ -98,6 +99,24 @@ metrics::SystemProfileProto::Channel AsProtobufChannel( return metrics::SystemProfileProto::CHANNEL_UNKNOWN; } +// Standard interval between log uploads, in seconds. +#if defined(OS_ANDROID) || defined(OS_IOS) +const int kStandardUploadIntervalSeconds = 5 * 60; // Five minutes. +const int kStandardUploadIntervalCellularSeconds = 15 * 60; // Fifteen minutes. +#else +const int kStandardUploadIntervalSeconds = 30 * 60; // Thirty minutes. +#endif + +#if defined(OS_ANDROID) || defined(OS_IOS) +// Returns true if the user is assigned to the experiment group for enabled +// cellular uploads. +bool IsCellularEnabledByExperiment() { + const std::string group_name = + base::FieldTrialList::FindFullName("UMA_EnableCellularLogUpload"); + return group_name == "Enabled"; +} +#endif + } // namespace ChromeMetricsServiceClient::ChromeMetricsServiceClient( @@ -246,6 +265,17 @@ ChromeMetricsServiceClient::CreateUploader( on_upload_complete)); } +base::TimeDelta ChromeMetricsServiceClient::GetStandardUploadInterval() { +#if defined(OS_ANDROID) || defined(OS_IOS) + bool is_cellular = false; + cellular_callback_.Run(&is_cellular); + + if (is_cellular && IsCellularEnabledByExperiment()) + return base::TimeDelta::FromSeconds(kStandardUploadIntervalCellularSeconds); +#endif + return base::TimeDelta::FromSeconds(kStandardUploadIntervalSeconds); +} + base::string16 ChromeMetricsServiceClient::GetRegistryBackupKey() { #if defined(OS_WIN) return L"Software\\" PRODUCT_STRING_PATH L"\\StabilityMetrics"; @@ -276,9 +306,7 @@ void ChromeMetricsServiceClient::Initialize() { scoped_ptr network_metrics_provider( new metrics::NetworkMetricsProvider( content::BrowserThread::GetBlockingPool())); - base::Callback cellular_callback = - network_metrics_provider->GetConnectionCallback(); - metrics_service_->SetConnectionTypeCallback(cellular_callback); + cellular_callback_ = network_metrics_provider->GetConnectionCallback(); metrics_service_->RegisterMetricsProvider(network_metrics_provider.Pass()); metrics_service_->RegisterMetricsProvider( @@ -289,7 +317,7 @@ void ChromeMetricsServiceClient::Initialize() { scoped_ptr(new metrics::GPUMetricsProvider())); profiler_metrics_provider_ = - new metrics::ProfilerMetricsProvider(cellular_callback); + new metrics::ProfilerMetricsProvider(cellular_callback_); metrics_service_->RegisterMetricsProvider( scoped_ptr(profiler_metrics_provider_)); diff --git a/chrome/browser/metrics/chrome_metrics_service_client.h b/chrome/browser/metrics/chrome_metrics_service_client.h index 2b95742b3cf670..c2f1c6e8bb2635 100644 --- a/chrome/browser/metrics/chrome_metrics_service_client.h +++ b/chrome/browser/metrics/chrome_metrics_service_client.h @@ -68,6 +68,7 @@ class ChromeMetricsServiceClient void CollectFinalMetrics(const base::Closure& done_callback) override; scoped_ptr CreateUploader( const base::Callback& on_upload_complete) override; + base::TimeDelta GetStandardUploadInterval() override; base::string16 GetRegistryBackupKey() override; metrics::MetricsService* metrics_service() { return metrics_service_.get(); } @@ -171,6 +172,10 @@ class ChromeMetricsServiceClient // MemoryDetails. MemoryGrowthTracker memory_growth_tracker_; + // Callback to determine whether or not a cellular network is currently being + // used. + base::Callback cellular_callback_; + base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(ChromeMetricsServiceClient); diff --git a/chromecast/browser/metrics/cast_metrics_service_client.cc b/chromecast/browser/metrics/cast_metrics_service_client.cc index 3e97217bea0c34..48bf45bbfd2e70 100644 --- a/chromecast/browser/metrics/cast_metrics_service_client.cc +++ b/chromecast/browser/metrics/cast_metrics_service_client.cc @@ -31,6 +31,10 @@ namespace chromecast { namespace metrics { +namespace { +const int kStandardUploadIntervalMinutes = 5; +} // namespace + // static scoped_ptr CastMetricsServiceClient::Create( base::TaskRunner* io_task_runner, @@ -150,6 +154,10 @@ CastMetricsServiceClient::CreateUploader( on_upload_complete)); } +base::TimeDelta CastMetricsServiceClient::GetStandardUploadInterval() { + return base::TimeDelta::FromMinutes(kStandardUploadIntervalMinutes); +} + void CastMetricsServiceClient::EnableMetricsService(bool enabled) { if (!metrics_service_loop_->BelongsToCurrentThread()) { metrics_service_loop_->PostTask( diff --git a/chromecast/browser/metrics/cast_metrics_service_client.h b/chromecast/browser/metrics/cast_metrics_service_client.h index 72ceedef952090..ffa17a3f8d16b7 100644 --- a/chromecast/browser/metrics/cast_metrics_service_client.h +++ b/chromecast/browser/metrics/cast_metrics_service_client.h @@ -62,6 +62,7 @@ class CastMetricsServiceClient : public ::metrics::MetricsServiceClient { void CollectFinalMetrics(const base::Closure& done_callback) override; scoped_ptr< ::metrics::MetricsLogUploader> CreateUploader( const base::Callback& on_upload_complete) override; + base::TimeDelta GetStandardUploadInterval() override; // Starts/stops the metrics service. void EnableMetricsService(bool enabled); diff --git a/components/metrics/metrics_reporting_scheduler.cc b/components/metrics/metrics_reporting_scheduler.cc index fb5461bd92e387..1318b22b3abbda 100644 --- a/components/metrics/metrics_reporting_scheduler.cc +++ b/components/metrics/metrics_reporting_scheduler.cc @@ -5,7 +5,6 @@ #include "components/metrics/metrics_reporting_scheduler.h" #include "base/compiler_specific.h" -#include "base/metrics/field_trial.h" #include "base/metrics/histogram.h" #include "base/strings/string_number_conversions.h" #include "components/variations/variations_associated_data.h" @@ -35,14 +34,6 @@ const int kUnsentLogsIntervalSeconds = 3; const int kUnsentLogsIntervalSeconds = 15; #endif -// Standard interval between log uploads, in seconds. -#if defined(OS_ANDROID) || defined(OS_IOS) -const int kStandardUploadIntervalSeconds = 5 * 60; // Five minutes. -const int kStandardUploadIntervalCellularSeconds = 15 * 60; // Fifteen minutes. -#else -const int kStandardUploadIntervalSeconds = 30 * 60; // Thirty minutes. -#endif - // When uploading metrics to the server fails, we progressively wait longer and // longer before sending the next log. This backoff process helps reduce load // on a server that is having issues. @@ -71,47 +62,23 @@ void LogActualUploadInterval(TimeDelta interval) { 50); } -// Returns upload interval specified for the current experiment running. -// TODO(gayane): Only for experimenting with upload interval for Android -// (bug: 17391128). Should be removed once the experiments are done. -base::TimeDelta GetUploadIntervalFromExperiment() { - std::string interval_str = variations::GetVariationParamValue( - "UMALogUploadInterval", "interval"); - int interval; - if (interval_str.empty() || !base::StringToInt(interval_str, &interval)) - return TimeDelta::FromSeconds(kStandardUploadIntervalSeconds); - - return TimeDelta::FromMinutes(interval); -} - -#if defined(OS_ANDROID) || defined(OS_IOS) -// Returns true if the user is assigned to the experiment group for enabled -// cellular uploads. -bool IsCellularEnabledByExperiment() { - const std::string group_name = - base::FieldTrialList::FindFullName("UMA_EnableCellularLogUpload"); - return group_name == "Enabled"; -} -#endif - } // anonymous namespace MetricsReportingScheduler::MetricsReportingScheduler( const base::Closure& upload_callback, - const base::Callback& cellular_callback) + const base::Callback& upload_interval_callback) : upload_callback_(upload_callback), upload_interval_(TimeDelta::FromSeconds(kInitialUploadIntervalSeconds)), running_(false), callback_pending_(false), init_task_complete_(false), waiting_for_init_task_complete_(false), - cellular_callback_(cellular_callback) { + upload_interval_callback_(upload_interval_callback) { } MetricsReportingScheduler::~MetricsReportingScheduler() {} void MetricsReportingScheduler::Start() { - GetUploadIntervalFromExperiment(); running_ = true; ScheduleNextUpload(); } @@ -206,15 +173,7 @@ void MetricsReportingScheduler::BackOffUploadInterval() { } base::TimeDelta MetricsReportingScheduler::GetStandardUploadInterval() { -#if defined(OS_ANDROID) || defined(OS_IOS) - bool is_cellular = false; - if (!cellular_callback_.is_null()) - cellular_callback_.Run(&is_cellular); - - if (is_cellular && IsCellularEnabledByExperiment()) - return TimeDelta::FromSeconds(kStandardUploadIntervalCellularSeconds); -#endif - return TimeDelta::FromSeconds(kStandardUploadIntervalSeconds); + return upload_interval_callback_.Run(); } } // namespace metrics diff --git a/components/metrics/metrics_reporting_scheduler.h b/components/metrics/metrics_reporting_scheduler.h index ddfe651d3ee29f..4d6102a14e7dfa 100644 --- a/components/metrics/metrics_reporting_scheduler.h +++ b/components/metrics/metrics_reporting_scheduler.h @@ -18,11 +18,12 @@ namespace metrics { class MetricsReportingScheduler { public: // Creates MetricsServiceScheduler object with the given |upload_callback| - // callback to call when uploading should happen and |cellular_callback| - // callback to get current network connection type. + // callback to call when uploading should happen and + // |upload_interval_callback| to determine the interval between uploads + // in steady state. MetricsReportingScheduler( const base::Closure& upload_callback, - const base::Callback& cellular_callback); + const base::Callback& upload_interval_callback); ~MetricsReportingScheduler(); // Starts scheduling uploads. This in a no-op if the scheduler is already @@ -57,10 +58,7 @@ class MetricsReportingScheduler { // where the server is having issues. void BackOffUploadInterval(); - // Returns upload interval based on the system and experiment that the user is - // assigned to. - // TODO(gayane): Only for experimenting with upload interval for Android - // (bug: 17391128). Should be removed once the experiments are done. + // Returns upload interval to use in steady state. base::TimeDelta GetStandardUploadInterval(); // The MetricsService method to call when uploading should happen. @@ -90,8 +88,8 @@ class MetricsReportingScheduler { // has been completed. bool waiting_for_init_task_complete_; - // Callback function used to get current network connection type. - base::Callback cellular_callback_; + // Callback function used to get the standard upload time. + base::Callback upload_interval_callback_; DISALLOW_COPY_AND_ASSIGN(MetricsReportingScheduler); }; diff --git a/components/metrics/metrics_reporting_scheduler_unittest.cc b/components/metrics/metrics_reporting_scheduler_unittest.cc index 2a8e7fafa0f546..630fc5c31e63ed 100644 --- a/components/metrics/metrics_reporting_scheduler_unittest.cc +++ b/components/metrics/metrics_reporting_scheduler_unittest.cc @@ -22,8 +22,8 @@ class MetricsReportingSchedulerTest : public testing::Test { base::Unretained(this)); } - base::Callback GetConnectionCallback() { - return base::Bind(&MetricsReportingSchedulerTest::SetConnectionTypeCallback, + base::Callback GetConnectionCallback() { + return base::Bind(&MetricsReportingSchedulerTest::GetStandardUploadInterval, base::Unretained(this)); } @@ -34,8 +34,8 @@ class MetricsReportingSchedulerTest : public testing::Test { ++callback_call_count_; } - void SetConnectionTypeCallback(bool* is_cellular_out) { - *is_cellular_out = false; + base::TimeDelta GetStandardUploadInterval() { + return base::TimeDelta::FromMinutes(5); } int callback_call_count_; diff --git a/components/metrics/metrics_service.cc b/components/metrics/metrics_service.cc index b4ee77ae32a58b..f86f33ab8b511b 100644 --- a/components/metrics/metrics_service.cc +++ b/components/metrics/metrics_service.cc @@ -301,7 +301,12 @@ void MetricsService::InitializeMetricsRecordingState() { base::Bind(&MetricsService::StartScheduledUpload, self_ptr_factory_.GetWeakPtr()); scheduler_.reset( - new MetricsReportingScheduler(upload_callback, is_cellular_callback_)); + new MetricsReportingScheduler( + upload_callback, + // MetricsServiceClient outlives MetricsService, and + // MetricsReportingScheduler is tied to the lifetime of |this|. + base::Bind(&MetricsServiceClient::GetStandardUploadInterval, + base::Unretained(client_)))); } void MetricsService::Start() { @@ -1130,10 +1135,4 @@ void MetricsService::RecordCurrentState(PrefService* pref) { base::Time::Now().ToTimeT()); } -void MetricsService::SetConnectionTypeCallback( - base::Callback is_cellular_callback) { - DCHECK(!scheduler_); - is_cellular_callback_ = is_cellular_callback; -} - } // namespace metrics diff --git a/components/metrics/metrics_service.h b/components/metrics/metrics_service.h index 7216df184fe34f..d7a612c83c1b6a 100644 --- a/components/metrics/metrics_service.h +++ b/components/metrics/metrics_service.h @@ -243,10 +243,6 @@ class MetricsService : public base::HistogramFlattener { // Clears the stability metrics that are saved in local state. void ClearSavedStabilityMetrics(); - // Sets the connection type callback used to pass to the scheduler. - void SetConnectionTypeCallback( - base::Callback is_cellular_callback); - protected: // Exposed for testing. MetricsLogManager* log_manager() { return &log_manager_; } @@ -480,9 +476,6 @@ class MetricsService : public base::HistogramFlattener { // exited-cleanly bit in the prefs. static ShutdownCleanliness clean_shutdown_status_; - // Callback function used to get current network connection type. - base::Callback is_cellular_callback_; - FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest, IsPluginProcess); FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest, PermutedEntropyCacheClearedWhenLowEntropyReset); diff --git a/components/metrics/metrics_service_client.h b/components/metrics/metrics_service_client.h index fa31c5e97998c3..1b481f998830ad 100644 --- a/components/metrics/metrics_service_client.h +++ b/components/metrics/metrics_service_client.h @@ -12,6 +12,7 @@ #include "base/callback_forward.h" #include "base/memory/scoped_ptr.h" #include "base/strings/string16.h" +#include "base/time/time.h" #include "components/metrics/proto/system_profile.pb.h" namespace metrics { @@ -66,6 +67,9 @@ class MetricsServiceClient { virtual scoped_ptr CreateUploader( const base::Callback& on_upload_complete) = 0; + // Returns the standard interval between upload attempts. + virtual base::TimeDelta GetStandardUploadInterval() = 0; + // Returns the name of a key under HKEY_CURRENT_USER that can be used to store // backups of metrics data. Unused except on Windows. virtual base::string16 GetRegistryBackupKey(); diff --git a/components/metrics/test_metrics_service_client.cc b/components/metrics/test_metrics_service_client.cc index 53cb1d93c22922..9dcb370eb44f8a 100644 --- a/components/metrics/test_metrics_service_client.cc +++ b/components/metrics/test_metrics_service_client.cc @@ -69,4 +69,8 @@ scoped_ptr TestMetricsServiceClient::CreateUploader( return scoped_ptr(); } +base::TimeDelta TestMetricsServiceClient::GetStandardUploadInterval() { + return base::TimeDelta::FromMinutes(5); +} + } // namespace metrics diff --git a/components/metrics/test_metrics_service_client.h b/components/metrics/test_metrics_service_client.h index 14c5518388b755..973d6e35af5b07 100644 --- a/components/metrics/test_metrics_service_client.h +++ b/components/metrics/test_metrics_service_client.h @@ -33,6 +33,7 @@ class TestMetricsServiceClient : public MetricsServiceClient { void CollectFinalMetrics(const base::Closure& done_callback) override; scoped_ptr CreateUploader( const base::Callback& on_upload_complete) override; + base::TimeDelta GetStandardUploadInterval() override; const std::string& get_client_id() const { return client_id_; } void set_version_string(const std::string& str) { version_string_ = str; }