From 86b55b05b7d263dac62c785dd669a73e42a2342a Mon Sep 17 00:00:00 2001 From: Kevin Kuehler Date: Tue, 2 Jun 2020 16:47:25 -0700 Subject: [PATCH] stats: Add dtoi param to usage ping Resolves https://github.com/brave/brave-browser/issues/10061 --- browser/brave_stats_updater.cc | 2 + browser/brave_stats_updater_browsertest.cc | 2 + browser/brave_stats_updater_params.cc | 21 ++++++ browser/brave_stats_updater_params.h | 12 ++-- browser/brave_stats_updater_unittest.cc | 81 ++++++++++++++++++++++ browser/brave_stats_updater_util.cc | 24 +++++++ browser/brave_stats_updater_util.h | 3 + 7 files changed, 141 insertions(+), 4 deletions(-) diff --git a/browser/brave_stats_updater.cc b/browser/brave_stats_updater.cc index 242dc35acaa2..b0b539987f63 100644 --- a/browser/brave_stats_updater.cc +++ b/browser/brave_stats_updater.cc @@ -56,6 +56,8 @@ GURL GetUpdateURL(const GURL& base_update_url, update_url, "first", stats_updater_params.GetFirstCheckMadeParam()); update_url = net::AppendQueryParameter( update_url, "woi", stats_updater_params.GetWeekOfInstallationParam()); + update_url = net::AppendQueryParameter( + update_url, "dtoi", stats_updater_params.GetDateOfInstallationParam()); update_url = net::AppendQueryParameter( update_url, "ref", stats_updater_params.GetReferralCodeParam()); return update_url; diff --git a/browser/brave_stats_updater_browsertest.cc b/browser/brave_stats_updater_browsertest.cc index 31d0187937bc..93c35930f428 100644 --- a/browser/brave_stats_updater_browsertest.cc +++ b/browser/brave_stats_updater_browsertest.cc @@ -5,7 +5,9 @@ #include "base/environment.h" #include "base/path_service.h" +#include "base/time/time.h" #include "brave/browser/brave_stats_updater.h" +#include "brave/browser/brave_stats_updater_params.h" #include "brave/common/pref_names.h" #include "brave/components/brave_referrals/browser/brave_referrals_service.h" #include "chrome/browser/ui/browser.h" diff --git a/browser/brave_stats_updater_params.cc b/browser/brave_stats_updater_params.cc index fea8fb72131e..d7271d63a008 100644 --- a/browser/brave_stats_updater_params.cc +++ b/browser/brave_stats_updater_params.cc @@ -17,6 +17,7 @@ namespace brave { base::Time BraveStatsUpdaterParams::g_current_time; +bool BraveStatsUpdaterParams::g_force_first_run = false; BraveStatsUpdaterParams::BraveStatsUpdaterParams(PrefService* pref_service) : BraveStatsUpdaterParams(pref_service, @@ -58,6 +59,12 @@ std::string BraveStatsUpdaterParams::GetWeekOfInstallationParam() const { return week_of_installation_; } +std::string BraveStatsUpdaterParams::GetDateOfInstallationParam() const { + return (GetCurrentTimeNow() - date_of_installation_ >= g_dtoi_delete_delta) + ? "null" + : brave::GetDateAsYMD(date_of_installation_); +} + std::string BraveStatsUpdaterParams::GetReferralCodeParam() const { return referral_promo_code_.empty() ? "none" : referral_promo_code_; } @@ -70,6 +77,10 @@ void BraveStatsUpdaterParams::LoadPrefs() { week_of_installation_ = pref_service_->GetString(kWeekOfInstallation); if (week_of_installation_.empty()) week_of_installation_ = GetLastMondayAsYMD(); + date_of_installation_ = brave::GetFirstRunTime(pref_service_); + DCHECK(!date_of_installation_.is_null()); + if (ShouldForceFirstRun()) + date_of_installation_ = GetCurrentTimeNow(); #if BUILDFLAG(ENABLE_BRAVE_REFERRALS) referral_promo_code_ = pref_service_->GetString(kReferralPromoCode); #endif @@ -119,10 +130,20 @@ base::Time BraveStatsUpdaterParams::GetCurrentTimeNow() const { return g_current_time.is_null() ? base::Time::Now() : g_current_time; } +// static +bool BraveStatsUpdaterParams::ShouldForceFirstRun() const { + return g_force_first_run; +} + // static void BraveStatsUpdaterParams::SetCurrentTimeForTest( const base::Time& current_time) { g_current_time = current_time; } +// static +void BraveStatsUpdaterParams::SetFirstRunForTest(bool first_run) { + g_force_first_run = first_run; +} + } // namespace brave diff --git a/browser/brave_stats_updater_params.h b/browser/brave_stats_updater_params.h index 26dabbd762a2..b5d6b4f164c0 100644 --- a/browser/brave_stats_updater_params.h +++ b/browser/brave_stats_updater_params.h @@ -9,14 +9,11 @@ #include #include "base/macros.h" +#include "base/time/time.h" class BraveStatsUpdaterTest; class PrefService; -namespace base { -class Time; -} - namespace brave { class BraveStatsUpdaterParams { @@ -33,6 +30,7 @@ class BraveStatsUpdaterParams { std::string GetMonthlyParam() const; std::string GetFirstCheckMadeParam() const; std::string GetWeekOfInstallationParam() const; + std::string GetDateOfInstallationParam() const; std::string GetReferralCodeParam() const; void SavePrefs(); @@ -48,8 +46,12 @@ class BraveStatsUpdaterParams { int last_check_month_; bool first_check_made_; std::string week_of_installation_; + base::Time date_of_installation_; std::string referral_promo_code_; static base::Time g_current_time; + static bool g_force_first_run; + static constexpr base::TimeDelta g_dtoi_delete_delta = + base::TimeDelta::FromSeconds(14 * 24 * 60 * 60); void LoadPrefs(); @@ -61,8 +63,10 @@ class BraveStatsUpdaterParams { int GetCurrentMonth() const; int GetCurrentISOWeekNumber() const; base::Time GetCurrentTimeNow() const; + bool ShouldForceFirstRun() const; static void SetCurrentTimeForTest(const base::Time& current_time); + static void SetFirstRunForTest(bool first_run); DISALLOW_COPY_AND_ASSIGN(BraveStatsUpdaterParams); }; diff --git a/browser/brave_stats_updater_unittest.cc b/browser/brave_stats_updater_unittest.cc index 8de85e438621..256577e12c6a 100644 --- a/browser/brave_stats_updater_unittest.cc +++ b/browser/brave_stats_updater_unittest.cc @@ -44,6 +44,10 @@ class BraveStatsUpdaterTest: public testing::Test { brave::BraveStatsUpdaterParams::SetCurrentTimeForTest(current_time); } + void SetFirstRunForTest(bool first_run) { + brave::BraveStatsUpdaterParams::SetFirstRunForTest(first_run); + } + private: TestingPrefServiceSimple testing_local_state_; }; @@ -148,6 +152,83 @@ TEST_F(BraveStatsUpdaterTest, IsMonthlyUpdateNeededLastCheckedNextMonth) { ASSERT_EQ(GetLocalState()->GetInteger(kLastCheckMonth), kThisMonth); } +TEST_F(BraveStatsUpdaterTest, HasDateOfInstallationFirstRun) { + base::Time::Exploded exploded; + base::Time current_time; + + // Set date to 2018-11-04 (ISO week #44) + exploded.hour = 0; + exploded.minute = 0; + exploded.second = 0; + exploded.millisecond = 0; + exploded.day_of_week = 0; + exploded.year = 2018; + exploded.month = 11; + exploded.day_of_month = 4; + + ASSERT_TRUE(base::Time::FromLocalExploded(exploded, ¤t_time)); + SetCurrentTimeForTest(current_time); + SetFirstRunForTest(true); + + brave::BraveStatsUpdaterParams brave_stats_updater_params( + GetLocalState(), kToday, kThisWeek, kThisMonth); + ASSERT_EQ(brave_stats_updater_params.GetDateOfInstallationParam(), + "2018-11-04"); +} + +TEST_F(BraveStatsUpdaterTest, HasDailyRetention) { + base::Time::Exploded exploded; + base::Time current_time, dtoi_time; + + // Set date to 2018-11-04 + exploded.hour = 0; + exploded.minute = 0; + exploded.second = 0; + exploded.millisecond = 0; + exploded.day_of_week = 0; + exploded.year = 2018; + exploded.month = 11; + exploded.day_of_month = 4; + + ASSERT_TRUE(base::Time::FromLocalExploded(exploded, &dtoi_time)); + // Make first run date 6 days earlier (still within 14 day window) + exploded.day_of_month = 10; + ASSERT_TRUE(base::Time::FromLocalExploded(exploded, ¤t_time)); + + SetCurrentTimeForTest(dtoi_time); + brave::BraveStatsUpdaterParams brave_stats_updater_params( + GetLocalState(), kToday, kThisWeek, kThisMonth); + SetCurrentTimeForTest(current_time); + ASSERT_EQ(brave_stats_updater_params.GetDateOfInstallationParam(), + "2018-11-04"); +} + +TEST_F(BraveStatsUpdaterTest, HasDailyRetentionExpiration) { + base::Time::Exploded exploded; + base::Time current_time, dtoi_time; + + // Set date to 2018-11-04 + exploded.hour = 0; + exploded.minute = 0; + exploded.second = 0; + exploded.millisecond = 0; + exploded.day_of_week = 0; + exploded.year = 2018; + exploded.month = 11; + exploded.day_of_month = 4; + + ASSERT_TRUE(base::Time::FromLocalExploded(exploded, &dtoi_time)); + // Make first run date 14 days earlier (outside 14 day window) + exploded.day_of_month = 18; + ASSERT_TRUE(base::Time::FromLocalExploded(exploded, ¤t_time)); + + SetCurrentTimeForTest(dtoi_time); + brave::BraveStatsUpdaterParams brave_stats_updater_params( + GetLocalState(), kToday, kThisWeek, kThisMonth); + SetCurrentTimeForTest(current_time); + ASSERT_EQ(brave_stats_updater_params.GetDateOfInstallationParam(), "null"); +} + // This test ensures that our weekly stats cut over on Monday TEST_F(BraveStatsUpdaterTest, IsWeeklyUpdateNeededOnMondayLastCheckedOnSunday) { base::Time::Exploded exploded; diff --git a/browser/brave_stats_updater_util.cc b/browser/brave_stats_updater_util.cc index 3ebcb06c2d6d..3cf0cf9b8001 100644 --- a/browser/brave_stats_updater_util.cc +++ b/browser/brave_stats_updater_util.cc @@ -11,6 +11,8 @@ #include "base/strings/string_piece.h" #include "base/strings/string_split.h" #include "base/strings/stringprintf.h" +#include "brave/common/pref_names.h" +#include "chrome/browser/first_run/first_run.h" namespace brave { @@ -21,6 +23,28 @@ std::string GetDateAsYMD(const base::Time& time) { exploded.day_of_month); } +base::Time GetFirstRunTime(PrefService *pref_service) { +#if defined(OS_ANDROID) + // Android doesn't use a sentinel to track first run, so we use a + // preference instead. kReferralAndroidFirstRunTimestamp is used because + // previously only referrals needed to know the first run value. + base::Time first_run_timestamp = + pref_service->GetTime(kReferralAndroidFirstRunTimestamp); + if (first_run_timestamp.is_null()) { + first_run_timestamp = base::Time::Now(); + pref_service->SetTime(kReferralAndroidFirstRunTimestamp, + first_run_timestamp); + } + return first_run_timestamp; +#else + (void)pref_service; // suppress unused warning + + // Note that CreateSentinelIfNeeded() is called in chrome_browser_main.cc, + // so this will be a non-blocking read of the cached sentinel value. + return first_run::GetFirstRunSentinelCreationTime(); +#endif // #defined(OS_ANDROID) +} + std::string GetPlatformIdentifier() { #if defined(OS_WIN) if (base::SysInfo::OperatingSystemArchitecture() == "x86") diff --git a/browser/brave_stats_updater_util.h b/browser/brave_stats_updater_util.h index d3830ee49d9e..c45d22310445 100644 --- a/browser/brave_stats_updater_util.h +++ b/browser/brave_stats_updater_util.h @@ -10,11 +10,14 @@ #include "base/time/time.h" #include "base/system/sys_info.h" +#include "components/prefs/pref_service.h" namespace brave { std::string GetDateAsYMD(const base::Time& time); +base::Time GetFirstRunTime(PrefService *pref_service); + std::string GetPlatformIdentifier(); int GetIsoWeekNumber(const base::Time& time);