Skip to content

Commit

Permalink
Remove some stale Variations params in DataUseTracker.
Browse files Browse the repository at this point in the history 
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 <asvitkine@chromium.org>
Commit-Queue: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1098110}
  • Loading branch information
Robert Kaplow authored and Chromium LUCI CQ committed Jan 27, 2023
1 parent 66e6811 commit 3d4f95b
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 86 deletions.
42 changes: 5 additions & 37 deletions components/metrics/data_use_tracker.cc
View file
Expand Up @@ -13,16 +13,15 @@
#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 {

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
Expand Down Expand Up @@ -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<double>(log_bytes + user_total_data_use) <=
uma_ratio;
kDefaultUMARatio;
}

void DataUseTracker::UpdateUsagePref(const std::string& pref_name,
Expand Down Expand Up @@ -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();
}
Expand Down
6 changes: 0 additions & 6 deletions components/metrics/data_use_tracker.h
View file
Expand Up @@ -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;

Expand Down
77 changes: 34 additions & 43 deletions components/metrics/data_use_tracker_unittest.cc
View file
Expand Up @@ -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));
Expand All @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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) {
Expand All @@ -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);
}

Expand Down

0 comments on commit 3d4f95b

Please sign in to comment.