From 3d4f95beb6ea67548f7a594d90446a2af3181ec6 Mon Sep 17 00:00:00 2001 From: Robert Kaplow Date: Fri, 27 Jan 2023 21:19:47 +0000 Subject: [PATCH] Remove some stale Variations params in DataUseTracker. Let the tests just use the build in defaults, it turns out the tests were using 200 bytes instead of actual 200 * 1024. Changes tests to just directly use KB for simplicity. Bug: 1410319 Change-Id: Ibc0763da298a7699403dab488f5387b1c724de71 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4198682 Reviewed-by: Alexei Svitkine Commit-Queue: Robert Kaplow Cr-Commit-Position: refs/heads/main@{#1098110} --- components/metrics/data_use_tracker.cc | 42 ++-------- components/metrics/data_use_tracker.h | 6 -- .../metrics/data_use_tracker_unittest.cc | 77 ++++++++----------- 3 files changed, 39 insertions(+), 86 deletions(-) diff --git a/components/metrics/data_use_tracker.cc b/components/metrics/data_use_tracker.cc index 7c55350a3446d..b7aa6b7ecbb8b 100644 --- a/components/metrics/data_use_tracker.cc +++ b/components/metrics/data_use_tracker.cc @@ -13,7 +13,6 @@ #include "build/build_config.h" #include "components/metrics/metrics_pref_names.h" #include "components/prefs/scoped_user_pref_update.h" -#include "components/variations/variations_associated_data.h" namespace metrics { @@ -21,8 +20,8 @@ namespace { // Default weekly quota and allowed UMA ratio for UMA log uploads for Android. // These defaults will not be used for non-Android as |DataUseTracker| will not -// be initialized. Default values can be overridden by variation params. -const int kDefaultUMAWeeklyQuotaBytes = 204800; +// be initialized. +const int kDefaultUMAWeeklyQuotaBytes = 200 * 1024; // 200KB. const double kDefaultUMARatio = 0.05; } // namespace @@ -83,27 +82,20 @@ bool DataUseTracker::ShouldUploadLogOnCellular(int log_bytes) { RemoveExpiredEntries(); - int uma_weekly_quota_bytes; - if (!GetUmaWeeklyQuota(&uma_weekly_quota_bytes)) - return true; - int uma_total_data_use = ComputeTotalDataUse(prefs::kUmaCellDataUse); int new_uma_total_data_use = log_bytes + uma_total_data_use; // If the new log doesn't increase the total UMA traffic to be above the // allowed quota then the log should be uploaded. - if (new_uma_total_data_use <= uma_weekly_quota_bytes) - return true; - - double uma_ratio; - if (!GetUmaRatio(&uma_ratio)) + if (new_uma_total_data_use <= kDefaultUMAWeeklyQuotaBytes) { return true; + } int user_total_data_use = ComputeTotalDataUse(prefs::kUserCellDataUse); // If after adding the new log the uma ratio is still under the allowed ratio // then the log should be uploaded and vice versa. return new_uma_total_data_use / static_cast(log_bytes + user_total_data_use) <= - uma_ratio; + kDefaultUMARatio; } void DataUseTracker::UpdateUsagePref(const std::string& pref_name, @@ -156,30 +148,6 @@ int DataUseTracker::ComputeTotalDataUse(const std::string& pref_name) { return total_data_use; } -bool DataUseTracker::GetUmaWeeklyQuota(int* uma_weekly_quota_bytes) const { - DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - - std::string param_value_str = variations::GetVariationParamValue( - "UMA_EnableCellularLogUpload", "Uma_Quota"); - if (param_value_str.empty()) - *uma_weekly_quota_bytes = kDefaultUMAWeeklyQuotaBytes; - else - base::StringToInt(param_value_str, uma_weekly_quota_bytes); - return true; -} - -bool DataUseTracker::GetUmaRatio(double* ratio) const { - DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - - std::string param_value_str = variations::GetVariationParamValue( - "UMA_EnableCellularLogUpload", "Uma_Ratio"); - if (param_value_str.empty()) - *ratio = kDefaultUMARatio; - else - base::StringToDouble(param_value_str, ratio); - return true; -} - base::Time DataUseTracker::GetCurrentMeasurementDate() const { return base::Time::Now().LocalMidnight(); } diff --git a/components/metrics/data_use_tracker.h b/components/metrics/data_use_tracker.h index 9b7c1a9d8b0b6..ed7286fa9502c 100644 --- a/components/metrics/data_use_tracker.h +++ b/components/metrics/data_use_tracker.h @@ -72,12 +72,6 @@ class DataUseTracker { // pref. int ComputeTotalDataUse(const std::string& pref_name); - // Returns the weekly allowed quota for UMA data use. - virtual bool GetUmaWeeklyQuota(int* uma_weekly_quota_bytes) const; - - // Returns the allowed ratio for UMA data use over overall data use. - virtual bool GetUmaRatio(double* ratio) const; - // Returns the current date for measurement. virtual base::Time GetCurrentMeasurementDate() const; diff --git a/components/metrics/data_use_tracker_unittest.cc b/components/metrics/data_use_tracker_unittest.cc index f8a65de6eb4ae..a9253a5b444a1 100644 --- a/components/metrics/data_use_tracker_unittest.cc +++ b/components/metrics/data_use_tracker_unittest.cc @@ -40,16 +40,6 @@ class FakeDataUseTracker : public DataUseTracker { FakeDataUseTracker(const FakeDataUseTracker&) = delete; FakeDataUseTracker& operator=(const FakeDataUseTracker&) = delete; - bool GetUmaWeeklyQuota(int* uma_weekly_quota_bytes) const override { - *uma_weekly_quota_bytes = 200; - return true; - } - - bool GetUmaRatio(double* ratio) const override { - *ratio = 0.05; - return true; - } - base::Time GetCurrentMeasurementDate() const override { base::Time today_for_test; EXPECT_TRUE(base::Time::FromUTCString(kTodayStr, &today_for_test)); @@ -65,35 +55,35 @@ class FakeDataUseTracker : public DataUseTracker { // allowed ratio. void SetPrefTestValuesOverRatio(PrefService* local_state) { base::Value::Dict user_pref_dict; - user_pref_dict.Set(kTodayStr, 2 * 100); - user_pref_dict.Set(kYesterdayStr, 2 * 100); - user_pref_dict.Set(kExpiredDateStr1, 2 * 100); - user_pref_dict.Set(kExpiredDateStr2, 2 * 100); + user_pref_dict.Set(kTodayStr, 2 * 100 * 1024); + user_pref_dict.Set(kYesterdayStr, 2 * 100 * 1024); + user_pref_dict.Set(kExpiredDateStr1, 2 * 100 * 1024); + user_pref_dict.Set(kExpiredDateStr2, 2 * 100 * 1024); local_state->SetDict(prefs::kUserCellDataUse, std::move(user_pref_dict)); base::Value::Dict uma_pref_dict; - uma_pref_dict.Set(kTodayStr, 50); - uma_pref_dict.Set(kYesterdayStr, 50); - uma_pref_dict.Set(kExpiredDateStr1, 50); - uma_pref_dict.Set(kExpiredDateStr2, 50); + uma_pref_dict.Set(kTodayStr, 50 * 1024); + uma_pref_dict.Set(kYesterdayStr, 50 * 1024); + uma_pref_dict.Set(kExpiredDateStr1, 50 * 1024); + uma_pref_dict.Set(kExpiredDateStr2, 50 * 1024); local_state->SetDict(prefs::kUmaCellDataUse, std::move(uma_pref_dict)); } // Sets up data usage prefs with mock values which can be valid. void SetPrefTestValuesValidRatio(PrefService* local_state) { base::Value::Dict user_pref_dict; - user_pref_dict.Set(kTodayStr, 100 * 100); - user_pref_dict.Set(kYesterdayStr, 100 * 100); - user_pref_dict.Set(kExpiredDateStr1, 100 * 100); - user_pref_dict.Set(kExpiredDateStr2, 100 * 100); + user_pref_dict.Set(kTodayStr, 100 * 100 * 1024); + user_pref_dict.Set(kYesterdayStr, 100 * 100 * 1024); + user_pref_dict.Set(kExpiredDateStr1, 100 * 100 * 1024); + user_pref_dict.Set(kExpiredDateStr2, 100 * 100 * 1024); local_state->SetDict(prefs::kUserCellDataUse, std::move(user_pref_dict)); // Should be 4% of user traffic base::Value::Dict uma_pref_dict; - uma_pref_dict.Set(kTodayStr, 4 * 100); - uma_pref_dict.Set(kYesterdayStr, 4 * 100); - uma_pref_dict.Set(kExpiredDateStr1, 4 * 100); - uma_pref_dict.Set(kExpiredDateStr2, 4 * 100); + uma_pref_dict.Set(kTodayStr, 4 * 100 * 1024); + uma_pref_dict.Set(kYesterdayStr, 4 * 100 * 1024); + uma_pref_dict.Set(kExpiredDateStr1, 4 * 100 * 1024); + uma_pref_dict.Set(kExpiredDateStr2, 4 * 100 * 1024); local_state->SetDict(prefs::kUmaCellDataUse, std::move(uma_pref_dict)); } @@ -104,15 +94,15 @@ TEST(DataUseTrackerTest, CheckUpdateUsagePref) { FakeDataUseTracker data_use_tracker(&local_state); local_state.ClearDataUsePrefs(); - data_use_tracker.UpdateMetricsUsagePrefsInternal(2 * 100, true, false); - EXPECT_EQ(2 * 100, + data_use_tracker.UpdateMetricsUsagePrefsInternal(2 * 100 * 1024, true, false); + EXPECT_EQ(2 * 100 * 1024, local_state.GetDict(prefs::kUserCellDataUse).FindInt(kTodayStr)); EXPECT_FALSE(local_state.GetDict(prefs::kUmaCellDataUse).FindInt(kTodayStr)); - data_use_tracker.UpdateMetricsUsagePrefsInternal(100, true, true); - EXPECT_EQ(3 * 100, + data_use_tracker.UpdateMetricsUsagePrefsInternal(100 * 1024, true, true); + EXPECT_EQ(3 * 100 * 1024, local_state.GetDict(prefs::kUserCellDataUse).FindInt(kTodayStr)); - EXPECT_EQ(100, + EXPECT_EQ(100 * 1024, local_state.GetDict(prefs::kUmaCellDataUse).FindInt(kTodayStr)); } @@ -133,14 +123,15 @@ TEST(DataUseTrackerTest, CheckRemoveExpiredEntries) { EXPECT_FALSE( local_state.GetDict(prefs::kUmaCellDataUse).FindInt(kExpiredDateStr2)); - EXPECT_EQ(2 * 100, + EXPECT_EQ(2 * 100 * 1024, local_state.GetDict(prefs::kUserCellDataUse).FindInt(kTodayStr)); - EXPECT_EQ(50, local_state.GetDict(prefs::kUmaCellDataUse).FindInt(kTodayStr)); + EXPECT_EQ(50 * 1024, + local_state.GetDict(prefs::kUmaCellDataUse).FindInt(kTodayStr)); EXPECT_EQ( - 2 * 100, + 2 * 100 * 1024, local_state.GetDict(prefs::kUserCellDataUse).FindInt(kYesterdayStr)); - EXPECT_EQ(50, + EXPECT_EQ(50 * 1024, local_state.GetDict(prefs::kUmaCellDataUse).FindInt(kYesterdayStr)); } @@ -152,10 +143,10 @@ TEST(DataUseTrackerTest, CheckComputeTotalDataUse) { int user_data_use = data_use_tracker.ComputeTotalDataUse(prefs::kUserCellDataUse); - EXPECT_EQ(8 * 100, user_data_use); + EXPECT_EQ(8 * 100 * 1024, user_data_use); int uma_data_use = data_use_tracker.ComputeTotalDataUse(prefs::kUmaCellDataUse); - EXPECT_EQ(4 * 50, uma_data_use); + EXPECT_EQ(4 * 50 * 1024, uma_data_use); } TEST(DataUseTrackerTest, CheckShouldUploadLogOnCellular) { @@ -164,21 +155,21 @@ TEST(DataUseTrackerTest, CheckShouldUploadLogOnCellular) { local_state.ClearDataUsePrefs(); SetPrefTestValuesOverRatio(&local_state); - bool can_upload = data_use_tracker.ShouldUploadLogOnCellular(50); + bool can_upload = data_use_tracker.ShouldUploadLogOnCellular(50 * 1024); EXPECT_TRUE(can_upload); - can_upload = data_use_tracker.ShouldUploadLogOnCellular(100); + can_upload = data_use_tracker.ShouldUploadLogOnCellular(100 * 1024); EXPECT_TRUE(can_upload); - can_upload = data_use_tracker.ShouldUploadLogOnCellular(150); + can_upload = data_use_tracker.ShouldUploadLogOnCellular(150 * 1024); EXPECT_FALSE(can_upload); local_state.ClearDataUsePrefs(); SetPrefTestValuesValidRatio(&local_state); - can_upload = data_use_tracker.ShouldUploadLogOnCellular(100); + can_upload = data_use_tracker.ShouldUploadLogOnCellular(100 * 1024); EXPECT_TRUE(can_upload); // this is about 0.49% - can_upload = data_use_tracker.ShouldUploadLogOnCellular(200); + can_upload = data_use_tracker.ShouldUploadLogOnCellular(200 * 1024); EXPECT_TRUE(can_upload); - can_upload = data_use_tracker.ShouldUploadLogOnCellular(300); + can_upload = data_use_tracker.ShouldUploadLogOnCellular(300 * 1024); EXPECT_FALSE(can_upload); }