From 2f67c46db1e6e6c1c0bbba3f98a5b81b7356dd87 Mon Sep 17 00:00:00 2001 From: Kevin Kuehler Date: Wed, 23 Jun 2021 18:40:04 -0700 Subject: [PATCH] brave_stats: Create preference to disable ping Resolves https://github.com/brave/brave-browser/issues/16583 --- browser/brave_stats/brave_stats_updater.cc | 29 +++++++++-- .../brave_stats_updater_browsertest.cc | 52 ++++++++++++++----- common/pref_names.cc | 2 + common/pref_names.h | 1 + components/brave_stats/browser/BUILD.gn | 22 ++++---- 5 files changed, 77 insertions(+), 29 deletions(-) diff --git a/browser/brave_stats/brave_stats_updater.cc b/browser/brave_stats/brave_stats_updater.cc index 0f8b6d1a9795..0ea5a8ef1744 100644 --- a/browser/brave_stats/brave_stats_updater.cc +++ b/browser/brave_stats/brave_stats_updater.cc @@ -44,6 +44,8 @@ namespace brave_stats { namespace { +constexpr char kInvalidUrl[] = "https://no-thanks.invalid"; + BraveStatsUpdater::StatsUpdatedCallback* g_testing_stats_updated_callback = nullptr; BraveStatsUpdater::StatsUpdatedCallback* g_testing_stats_threshold_callback = @@ -173,14 +175,23 @@ bool BraveStatsUpdater::MaybeDoThresholdPing(int score) { if (HasDoneThresholdPing()) return true; + const bool reporting_enabled = + pref_service_->GetBoolean(kStatsReportingEnabled); + if (!reporting_enabled) { + if (g_testing_stats_threshold_callback) + g_testing_stats_threshold_callback->Run(GURL(kInvalidUrl)); + return false; + } + + const bool threshold_met = threshold_score_ >= kMinimumUsageThreshold; // We don't want to start the threshold ping if: // (1) The standard ping is still waiting to be sent. // (2) Stats is blocked by referral initialization or ads. // The standard usage ping will set the url and call us back. if (server_ping_startup_timer_->IsRunning() || !stats_startup_complete_) - return threshold_score_ >= kMinimumUsageThreshold; + return threshold_met; - if (threshold_score_ >= kMinimumUsageThreshold) { + if (threshold_met) { SendUserTriggeredPing(); return true; } @@ -280,6 +291,13 @@ void BraveStatsUpdater::OnServerPingTimerFired() { if (base::CompareCaseInsensitiveASCII(today_ymd, last_check_ymd) == 0) return; + const bool reporting_enabled = + pref_service_->GetBoolean(kStatsReportingEnabled); + if (!reporting_enabled) { + if (g_testing_stats_updated_callback) + g_testing_stats_updated_callback->Run(GURL(kInvalidUrl)); + return; + } SendServerPing(); } @@ -371,6 +389,7 @@ void BraveStatsUpdater::SendServerPing() { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); auto traffic_annotation = AnonymousStatsAnnotation(); auto resource_request = std::make_unique(); + auto* profile_pref_service = ProfileManager::GetPrimaryUserProfile()->GetPrefs(); auto stats_updater_params = @@ -407,8 +426,10 @@ void BraveStatsUpdater::SendUserTriggeredPing() { // so if it is empty, we have an existing user. Disable // threshold ping and don't send a request. auto threshold_query = pref_service_->GetString(kThresholdQuery); - if (threshold_query.empty()) + if (threshold_query.empty()) { DisableThresholdPing(); + return; + } resource_request->url = GURL(threshold_query); resource_request->credentials_mode = network::mojom::CredentialsMode::kOmit; @@ -435,10 +456,12 @@ void BraveStatsUpdater::SendUserTriggeredPing() { void RegisterLocalStatePrefs(PrefRegistrySimple* registry) { registry->RegisterBooleanPref(kFirstCheckMade, false); registry->RegisterBooleanPref(kThresholdCheckMade, false); + registry->RegisterBooleanPref(kStatsReportingEnabled, true); registry->RegisterStringPref(kThresholdQuery, std::string()); registry->RegisterIntegerPref(kLastCheckWOY, 0); registry->RegisterIntegerPref(kLastCheckMonth, 0); registry->RegisterStringPref(kLastCheckYMD, std::string()); registry->RegisterStringPref(kWeekOfInstallation, std::string()); } + } // namespace brave_stats diff --git a/browser/brave_stats/brave_stats_updater_browsertest.cc b/browser/brave_stats/brave_stats_updater_browsertest.cc index 4eca486ccfa6..3e64cfd2faa2 100644 --- a/browser/brave_stats/brave_stats_updater_browsertest.cc +++ b/browser/brave_stats/brave_stats_updater_browsertest.cc @@ -106,7 +106,7 @@ class BraveStatsUpdaterBrowserTest : public PlatformBrowserTest { env->SetVar("BRAVE_REFERRALS_LOCAL", "1"); // use http for local testing } - std::string GetUpdateURL() const { return update_url_; } + GURL GetUpdateURL() const { return update_url_; } void OnReferralInitialized(const std::string& referral_code) { if (wait_for_referral_initialized_loop_) { @@ -132,10 +132,7 @@ class BraveStatsUpdaterBrowserTest : public PlatformBrowserTest { } on_standard_stats_updated_ = true; - - // We get //1/usage/brave-core here, so ignore the first slash. - EXPECT_STREQ(update_url.path().c_str() + 1, "/1/usage/brave-core"); - update_url_ = update_url.spec(); + update_url_ = update_url; } void WaitForStandardStatsUpdatedCallback() { @@ -153,11 +150,7 @@ class BraveStatsUpdaterBrowserTest : public PlatformBrowserTest { } on_threshold_stats_updated_ = true; - - // We get //1/usage/brave-core-threshold here, so ignore the first slash. - EXPECT_STREQ(update_url.path().c_str() + 1, - "/1/usage/brave-core-threshold"); - update_url_ = update_url.spec(); + update_url_ = update_url; } void WaitForThresholdStatsUpdatedCallback() { @@ -169,13 +162,17 @@ class BraveStatsUpdaterBrowserTest : public PlatformBrowserTest { wait_for_threshold_stats_updated_loop_->Run(); } + void DisableStatsUsagePing() { + g_browser_process->local_state()->SetBoolean(kStatsReportingEnabled, false); + } + private: std::unique_ptr wait_for_referral_initialized_loop_; std::unique_ptr wait_for_standard_stats_updated_loop_; std::unique_ptr wait_for_threshold_stats_updated_loop_; std::string referral_code_; - std::string update_url_; + GURL update_url_; bool on_referral_initialized_ = false; bool on_standard_stats_updated_ = false; @@ -188,6 +185,9 @@ IN_PROC_BROWSER_TEST_F(BraveStatsUpdaterBrowserTest, WaitForReferralInitializeCallback(); WaitForStandardStatsUpdatedCallback(); + // We get //1/usage/brave-core here, so ignore the first slash. + EXPECT_STREQ(GetUpdateURL().path().c_str() + 1, "/1/usage/brave-core"); + // First check preference should now be true EXPECT_TRUE(g_browser_process->local_state()->GetBoolean(kFirstCheckMade)); } @@ -201,12 +201,36 @@ IN_PROC_BROWSER_TEST_F(BraveStatsUpdaterBrowserTest, WaitForReferralInitializeCallback(); WaitForThresholdStatsUpdatedCallback(); + // We get //1/usage/brave-core-threshold here, so ignore the first slash. + EXPECT_STREQ(GetUpdateURL().path().c_str() + 1, + "/1/usage/brave-core-threshold"); + // First check and Threshold check should be set. EXPECT_TRUE(g_browser_process->local_state()->GetBoolean(kFirstCheckMade)); EXPECT_TRUE( g_browser_process->local_state()->GetBoolean(kThresholdCheckMade)); } +// The stats updater should not reach the endpoint +IN_PROC_BROWSER_TEST_F(BraveStatsUpdaterBrowserTest, + StatsUpdaterUsagePingDisabledFirstCheck) { + DisableStatsUsagePing(); + + EXPECT_FALSE( + g_brave_browser_process->brave_stats_updater()->MaybeDoThresholdPing(3)); + WaitForReferralInitializeCallback(); + WaitForStandardStatsUpdatedCallback(); + WaitForThresholdStatsUpdatedCallback(); + + // Dummy URL confirms no request was triggered + EXPECT_STREQ(GetUpdateURL().host().c_str(), "no-thanks.invalid"); + + // No prefs should be updated + EXPECT_FALSE(g_browser_process->local_state()->GetBoolean(kFirstCheckMade)); + EXPECT_FALSE( + g_browser_process->local_state()->GetBoolean(kThresholdCheckMade)); +} + // Run the stats updater with no active referral and verify that the // update url specifies the default referral code IN_PROC_BROWSER_TEST_F(BraveStatsUpdaterBrowserTest, @@ -219,7 +243,7 @@ IN_PROC_BROWSER_TEST_F(BraveStatsUpdaterBrowserTest, g_browser_process->local_state()->GetBoolean(kReferralInitialization)); // Verify that update url is valid - const GURL update_url(GetUpdateURL()); + const GURL update_url = GetUpdateURL(); EXPECT_TRUE(update_url.is_valid()); // Verify that daily parameter is true @@ -246,7 +270,7 @@ IN_PROC_BROWSER_TEST_F(BraveStatsUpdaterBrowserTest, WaitForStandardStatsUpdatedCallback(); // Verify that update url is valid - const GURL update_url(GetUpdateURL()); + const GURL update_url = GetUpdateURL(); EXPECT_TRUE(update_url.is_valid()); // Verify that daily parameter is true @@ -296,7 +320,7 @@ IN_PROC_BROWSER_TEST_F(BraveStatsUpdaterReferralCodeBrowserTest, g_browser_process->local_state()->GetBoolean(kReferralInitialization)); // Verify that update url is valid - const GURL update_url(GetUpdateURL()); + const GURL update_url = GetUpdateURL(); EXPECT_TRUE(update_url.is_valid()); // Verify that daily parameter is true diff --git a/common/pref_names.cc b/common/pref_names.cc index 9a0fcf1d9fe2..14b6f75f3b12 100644 --- a/common/pref_names.cc +++ b/common/pref_names.cc @@ -19,6 +19,8 @@ const char kFirstCheckMade[] = "brave.stats.first_check_made"; // Set to true if the user met the threshold requirements and successfully // sent a ping to the stats-updater server. const char kThresholdCheckMade[] = "brave.stats.threshold_check_made"; +// Anonymous usage pings enabled +const char kStatsReportingEnabled[] = "brave.stats.reporting_enabled"; // Serialized query for to send to the stats-updater server. Needs to be saved // in the case that the user sends the standard usage ping, stops the browser, // meets the threshold requirements, and then starts the browser before the diff --git a/common/pref_names.h b/common/pref_names.h index bbb3236b88b1..b026b6c37c18 100644 --- a/common/pref_names.h +++ b/common/pref_names.h @@ -21,6 +21,7 @@ extern const char kFirstCheckMade[]; extern const char kThresholdCheckMade[]; extern const char kThresholdQuery[]; extern const char kWeekOfInstallation[]; +extern const char kStatsReportingEnabled[]; extern const char kWidevineOptedIn[]; extern const char kAskWidevineInstall[]; extern const char kUseAlternativeSearchEngineProvider[]; diff --git a/components/brave_stats/browser/BUILD.gn b/components/brave_stats/browser/BUILD.gn index bc0547dd86bf..afb7952506a5 100644 --- a/components/brave_stats/browser/BUILD.gn +++ b/components/brave_stats/browser/BUILD.gn @@ -3,17 +3,15 @@ declare_args() { } source_set("browser") { - defines = [ - "BRAVE_STATS_API_KEY=\"$brave_stats_api_key\"", - ] + defines = [ "BRAVE_STATS_API_KEY=\"$brave_stats_api_key\"" ] - sources = [ - "brave_stats_updater_util.cc", - "brave_stats_updater_util.h", - ] + sources = [ + "brave_stats_updater_util.cc", + "brave_stats_updater_util.h", + ] - deps = [ - "//base", - "//components/prefs", - ] -} \ No newline at end of file + deps = [ + "//base", + "//components/prefs", + ] +}