Commit 4b9de7c9 authored by Henrique Nakashima's avatar Henrique Nakashima Committed by Commit Bot

Revert "Flatten UpdateMetricsUsagePrefs - skip Metrics and Reporting service."

This reverts commit a061c829.

Reason for revert: Suspect in crbug.com/906242

Original change's description:
> Flatten UpdateMetricsUsagePrefs - skip Metrics and Reporting service.
> 
> UpdateMetricsUsagePrefs() is a chain of calls:
> - ChromeDataUseMeasurement::UpdateDataUseToMetricsService()
>   -> UpdateMetricsUsagePrefs()
>   -> MetricsService::UpdateMetricsUsagePrefs()
>   -> ReportingService::UpdateMetricsUsagePrefs()
>   -> DataUseTracker::UpdateMetricsUsagePrefs()
> 
> It can be reduced to:
> - ChromeDataUseMeasurement::UpdateDataUseToMetricsService()
>   -> UpdateMetricsUsagePrefs()
>   -> DataUseTracker::UpdateMetricsUsagePrefs()
> 
> This removes the dependency from ChromeDataUseMeasurement to
> MetricsService and ReportingService.
> 
> Bug: 902791
> Change-Id: I38a6d22d1ff823134c79bc342ea32be0f89cda77
> Reviewed-on: https://chromium-review.googlesource.com/c/1334267
> Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Reviewed-by: rajendrant <rajendrant@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#608567}

TBR=asvitkine@chromium.org,bcwhite@chromium.org,rajendrant@chromium.org,hnakashima@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 902791
Change-Id: Iad1f49a8bae42e60ae5a9c68509d4cb38db683ef
Reviewed-on: https://chromium-review.googlesource.com/c/1344817Reviewed-by: default avatarHenrique Nakashima <hnakashima@chromium.org>
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609861}
parent b4213429
...@@ -15,7 +15,7 @@ ...@@ -15,7 +15,7 @@
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "components/data_use_measurement/core/data_use_ascriber.h" #include "components/data_use_measurement/core/data_use_ascriber.h"
#include "components/data_use_measurement/core/url_request_classifier.h" #include "components/data_use_measurement/core/url_request_classifier.h"
#include "components/metrics/data_use_tracker.h" #include "components/metrics/metrics_service.h"
#include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/network_service_instance.h" #include "content/public/browser/network_service_instance.h"
...@@ -34,9 +34,14 @@ void UpdateMetricsUsagePrefs(int64_t total_bytes, ...@@ -34,9 +34,14 @@ void UpdateMetricsUsagePrefs(int64_t total_bytes,
bool is_cellular, bool is_cellular,
bool is_metrics_service_usage) { bool is_metrics_service_usage) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
metrics::DataUseTracker::UpdateMetricsUsagePrefs( // Some unit tests use IOThread but do not initialize MetricsService. In that
// case it's fine to skip the update.
auto* metrics_service = g_browser_process->metrics_service();
if (metrics_service) {
metrics_service->UpdateMetricsUsagePrefs(
base::saturated_cast<int>(total_bytes), is_cellular, base::saturated_cast<int>(total_bytes), is_cellular,
is_metrics_service_usage, g_browser_process->local_state()); is_metrics_service_usage);
}
} }
// This function is for forwarding metrics usage pref changes to the metrics // This function is for forwarding metrics usage pref changes to the metrics
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "build/build_config.h"
#include "components/metrics/metrics_pref_names.h" #include "components/metrics/metrics_pref_names.h"
#include "components/prefs/scoped_user_pref_update.h" #include "components/prefs/scoped_user_pref_update.h"
#include "components/variations/variations_associated_data.h" #include "components/variations/variations_associated_data.h"
...@@ -34,8 +33,6 @@ DataUseTracker::~DataUseTracker() {} ...@@ -34,8 +33,6 @@ DataUseTracker::~DataUseTracker() {}
std::unique_ptr<DataUseTracker> DataUseTracker::Create( std::unique_ptr<DataUseTracker> DataUseTracker::Create(
PrefService* local_state) { PrefService* local_state) {
std::unique_ptr<DataUseTracker> data_use_tracker; std::unique_ptr<DataUseTracker> data_use_tracker;
// Instantiate DataUseTracker only on Android. UpdateMetricsUsagePrefs() honors
// this rule too.
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
data_use_tracker.reset(new DataUseTracker(local_state)); data_use_tracker.reset(new DataUseTracker(local_state));
#endif #endif
...@@ -48,21 +45,7 @@ void DataUseTracker::RegisterPrefs(PrefRegistrySimple* registry) { ...@@ -48,21 +45,7 @@ void DataUseTracker::RegisterPrefs(PrefRegistrySimple* registry) {
registry->RegisterDictionaryPref(metrics::prefs::kUmaCellDataUse); registry->RegisterDictionaryPref(metrics::prefs::kUmaCellDataUse);
} }
// static
void DataUseTracker::UpdateMetricsUsagePrefs(int message_size, void DataUseTracker::UpdateMetricsUsagePrefs(int message_size,
bool is_cellular,
bool is_metrics_service_usage,
PrefService* local_state) {
// Instantiate DataUseTracker only on Android. Create() honors this rule too.
#if defined(OS_ANDROID)
metrics::DataUseTracker tracker(local_state);
tracker.UpdateMetricsUsagePrefsInternal(message_size, is_cellular,
is_metrics_service_usage);
#endif // defined(OS_ANDROID)
}
void DataUseTracker::UpdateMetricsUsagePrefsInternal(
int message_size,
bool is_cellular, bool is_cellular,
bool is_metrics_service_usage) { bool is_metrics_service_usage) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
......
...@@ -17,6 +17,9 @@ ...@@ -17,6 +17,9 @@
namespace metrics { namespace metrics {
typedef base::Callback<void(const std::string&, int, bool)>
UpdateUsagePrefCallbackType;
// Records the data use of user traffic and UMA traffic in user prefs. Taking // Records the data use of user traffic and UMA traffic in user prefs. Taking
// into account those prefs it can verify whether certain UMA log upload is // into account those prefs it can verify whether certain UMA log upload is
// allowed. // allowed.
...@@ -33,10 +36,9 @@ class DataUseTracker { ...@@ -33,10 +36,9 @@ class DataUseTracker {
static void RegisterPrefs(PrefRegistrySimple* registry); static void RegisterPrefs(PrefRegistrySimple* registry);
// Updates data usage tracking prefs with the specified values. // Updates data usage tracking prefs with the specified values.
static void UpdateMetricsUsagePrefs(int message_size, void UpdateMetricsUsagePrefs(int message_size,
bool is_cellular, bool is_cellular,
bool is_metrics_service_usage, bool is_metrics_service_usage);
PrefService* local_state);
// Returns whether a log with provided |log_bytes| can be uploaded according // Returns whether a log with provided |log_bytes| can be uploaded according
// to data use ratio and UMA quota provided by variations. // to data use ratio and UMA quota provided by variations.
...@@ -48,11 +50,6 @@ class DataUseTracker { ...@@ -48,11 +50,6 @@ class DataUseTracker {
FRIEND_TEST_ALL_PREFIXES(DataUseTrackerTest, CheckComputeTotalDataUse); FRIEND_TEST_ALL_PREFIXES(DataUseTrackerTest, CheckComputeTotalDataUse);
FRIEND_TEST_ALL_PREFIXES(DataUseTrackerTest, CheckCanUploadUMALog); FRIEND_TEST_ALL_PREFIXES(DataUseTrackerTest, CheckCanUploadUMALog);
// Updates data usage tracking prefs with the specified values.
void UpdateMetricsUsagePrefsInternal(int message_size,
bool is_cellular,
bool is_metrics_service_usage);
// Updates provided |pref_name| for a current date with the given message // Updates provided |pref_name| for a current date with the given message
// size. // size.
void UpdateUsagePref(const std::string& pref_name, int message_size); void UpdateUsagePref(const std::string& pref_name, int message_size);
......
...@@ -106,7 +106,7 @@ TEST(DataUseTrackerTest, CheckUpdateUsagePref) { ...@@ -106,7 +106,7 @@ TEST(DataUseTrackerTest, CheckUpdateUsagePref) {
int user_pref_value = 0; int user_pref_value = 0;
int uma_pref_value = 0; int uma_pref_value = 0;
data_use_tracker.UpdateMetricsUsagePrefsInternal(2 * 100, true, false); data_use_tracker.UpdateMetricsUsagePrefs(2 * 100, true, false);
local_state.GetDictionary(prefs::kUserCellDataUse) local_state.GetDictionary(prefs::kUserCellDataUse)
->GetInteger(kTodayStr, &user_pref_value); ->GetInteger(kTodayStr, &user_pref_value);
EXPECT_EQ(2 * 100, user_pref_value); EXPECT_EQ(2 * 100, user_pref_value);
...@@ -114,7 +114,7 @@ TEST(DataUseTrackerTest, CheckUpdateUsagePref) { ...@@ -114,7 +114,7 @@ TEST(DataUseTrackerTest, CheckUpdateUsagePref) {
->GetInteger(kTodayStr, &uma_pref_value); ->GetInteger(kTodayStr, &uma_pref_value);
EXPECT_EQ(0, uma_pref_value); EXPECT_EQ(0, uma_pref_value);
data_use_tracker.UpdateMetricsUsagePrefsInternal(100, true, true); data_use_tracker.UpdateMetricsUsagePrefs(100, true, true);
local_state.GetDictionary(prefs::kUserCellDataUse) local_state.GetDictionary(prefs::kUserCellDataUse)
->GetInteger(kTodayStr, &user_pref_value); ->GetInteger(kTodayStr, &user_pref_value);
EXPECT_EQ(3 * 100, user_pref_value); EXPECT_EQ(3 * 100, user_pref_value);
......
...@@ -452,6 +452,14 @@ void MetricsService::PushExternalLog(const std::string& log) { ...@@ -452,6 +452,14 @@ void MetricsService::PushExternalLog(const std::string& log) {
log_store()->StoreLog(log, MetricsLog::ONGOING_LOG); log_store()->StoreLog(log, MetricsLog::ONGOING_LOG);
} }
void MetricsService::UpdateMetricsUsagePrefs(int message_size,
bool is_cellular,
bool is_metrics_service_usage) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
reporting_service_.UpdateMetricsUsagePrefs(message_size, is_cellular,
is_metrics_service_usage);
}
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
// private methods // private methods
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
......
...@@ -169,6 +169,11 @@ class MetricsService : public base::HistogramFlattener { ...@@ -169,6 +169,11 @@ class MetricsService : public base::HistogramFlattener {
// Pushes a log that has been generated by an external component. // Pushes a log that has been generated by an external component.
void PushExternalLog(const std::string& log); void PushExternalLog(const std::string& log);
// Updates data usage tracking prefs with the specified values.
void UpdateMetricsUsagePrefs(int message_size,
bool is_cellular,
bool is_metrics_service_usage);
variations::SyntheticTrialRegistry* synthetic_trial_registry() { variations::SyntheticTrialRegistry* synthetic_trial_registry() {
return &synthetic_trial_registry_; return &synthetic_trial_registry_;
} }
......
...@@ -81,6 +81,16 @@ bool ReportingService::reporting_active() const { ...@@ -81,6 +81,16 @@ bool ReportingService::reporting_active() const {
return reporting_active_; return reporting_active_;
} }
void ReportingService::UpdateMetricsUsagePrefs(int message_size,
bool is_cellular,
bool is_metrics_service_usage) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (data_use_tracker_) {
data_use_tracker_->UpdateMetricsUsagePrefs(message_size, is_cellular,
is_metrics_service_usage);
}
}
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
// private methods // private methods
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
......
...@@ -66,6 +66,11 @@ class ReportingService { ...@@ -66,6 +66,11 @@ class ReportingService {
// True iff reporting is currently enabled. // True iff reporting is currently enabled.
bool reporting_active() const; bool reporting_active() const;
// Updates data usage tracking prefs with the specified values.
void UpdateMetricsUsagePrefs(int message_size,
bool is_cellular,
bool is_metrics_service_usage);
// Registers local state prefs used by this class. This should only be called // Registers local state prefs used by this class. This should only be called
// once. // once.
static void RegisterPrefs(PrefRegistrySimple* registry); static void RegisterPrefs(PrefRegistrySimple* registry);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment