From 51fef070e78e896e85161026c027f22ce68dae6d Mon Sep 17 00:00:00 2001 From: Darnell Andries Date: Mon, 25 Nov 2024 20:43:51 -0800 Subject: [PATCH] Add VPN widget metrics --- .../brave_new_tab_page_handler.cc | 8 ++ .../new_tab_page/brave_new_tab_page_handler.h | 1 + .../brave_new_tab_ui/async/brave_vpn.ts | 3 + .../brave_new_tab_ui/brave_new_tab_page.mojom | 1 + components/brave_vpn/browser/BUILD.gn | 10 +- .../brave_vpn/browser/brave_vpn_metrics.cc | 115 ++++++++++++++++++ .../brave_vpn/browser/brave_vpn_metrics.h | 57 +++++++++ .../browser/brave_vpn_metrics_unittest.cc | 115 ++++++++++++++++++ .../brave_vpn/browser/brave_vpn_service.cc | 65 +--------- .../brave_vpn/browser/brave_vpn_service.h | 17 +-- .../browser/brave_vpn_service_unittest.cc | 52 -------- .../brave_vpn/common/brave_vpn_utils.cc | 1 + components/brave_vpn/common/pref_names.h | 2 + components/p3a/metric_names.h | 4 + 14 files changed, 325 insertions(+), 126 deletions(-) create mode 100644 components/brave_vpn/browser/brave_vpn_metrics.cc create mode 100644 components/brave_vpn/browser/brave_vpn_metrics.h create mode 100644 components/brave_vpn/browser/brave_vpn_metrics_unittest.cc diff --git a/browser/ui/webui/new_tab_page/brave_new_tab_page_handler.cc b/browser/ui/webui/new_tab_page/brave_new_tab_page_handler.cc index d92a89084c51..bd2eb016dd80 100644 --- a/browser/ui/webui/new_tab_page/brave_new_tab_page_handler.cc +++ b/browser/ui/webui/new_tab_page/brave_new_tab_page_handler.cc @@ -353,6 +353,14 @@ void BraveNewTabPageHandler::OpenVPNAccountPage() { #endif } +void BraveNewTabPageHandler::ReportVPNWidgetUsage() { +#if BUILDFLAG(ENABLE_BRAVE_VPN) + auto* vpn_service = + brave_vpn::BraveVpnServiceFactory::GetForProfile(profile_); + vpn_service->brave_vpn_metrics()->RecordWidgetUsage(true); +#endif +} + bool BraveNewTabPageHandler::IsCustomBackgroundImageEnabled() const { if (profile_->GetPrefs()->IsManagedPreference(GetThemePrefNameInMigration( ThemePrefInMigration::kNtpCustomBackgroundDict))) { diff --git a/browser/ui/webui/new_tab_page/brave_new_tab_page_handler.h b/browser/ui/webui/new_tab_page/brave_new_tab_page_handler.h index 388bdca47279..c26997d8bf5b 100644 --- a/browser/ui/webui/new_tab_page/brave_new_tab_page_handler.h +++ b/browser/ui/webui/new_tab_page/brave_new_tab_page_handler.h @@ -77,6 +77,7 @@ class BraveNewTabPageHandler : public brave_new_tab_page::mojom::PageHandler, void RefreshVPNState() override; void LaunchVPNPanel() override; void OpenVPNAccountPage() override; + void ReportVPNWidgetUsage() override; // Observe BraveNTPCustomBackgroundService. void OnBackgroundUpdated(); diff --git a/components/brave_new_tab_ui/async/brave_vpn.ts b/components/brave_new_tab_ui/async/brave_vpn.ts index 065de1b2f6f2..f9d54de6612e 100644 --- a/components/brave_new_tab_ui/async/brave_vpn.ts +++ b/components/brave_new_tab_ui/async/brave_vpn.ts @@ -39,10 +39,12 @@ handler.on(Actions.initialize.getType(), handler.on(Actions.launchVPNPanel.getType(), async (store) => { getNTPBrowserAPI().pageHandler.launchVPNPanel() + getNTPBrowserAPI().pageHandler.reportVPNWidgetUsage() }) handler.on(Actions.openVPNAccountPage.getType(), async (store) => { getNTPBrowserAPI().pageHandler.openVPNAccountPage() + getNTPBrowserAPI().pageHandler.reportVPNWidgetUsage() }) handler.on(Actions.toggleConnection.getType(), async (store) => { @@ -56,6 +58,7 @@ handler.on(Actions.toggleConnection.getType(), async (store) => { } else { getVPNServiceHandler().connect() } + getNTPBrowserAPI().pageHandler.reportVPNWidgetUsage() }) handler.on( diff --git a/components/brave_new_tab_ui/brave_new_tab_page.mojom b/components/brave_new_tab_ui/brave_new_tab_page.mojom index 6a2169791a4d..fd3fa0c5294b 100644 --- a/components/brave_new_tab_ui/brave_new_tab_page.mojom +++ b/components/brave_new_tab_ui/brave_new_tab_page.mojom @@ -86,6 +86,7 @@ interface PageHandler { RefreshVPNState(); LaunchVPNPanel(); OpenVPNAccountPage(); + ReportVPNWidgetUsage(); }; // WebUI-side handler for requests from the browser. diff --git a/components/brave_vpn/browser/BUILD.gn b/components/brave_vpn/browser/BUILD.gn index f78fde79075c..0eb69426561b 100644 --- a/components/brave_vpn/browser/BUILD.gn +++ b/components/brave_vpn/browser/BUILD.gn @@ -9,6 +9,8 @@ assert(enable_brave_vpn) static_library("browser") { sources = [ + "brave_vpn_metrics.cc", + "brave_vpn_metrics.h", "brave_vpn_service.cc", "brave_vpn_service.h", "brave_vpn_service_delegate.h", @@ -26,10 +28,12 @@ static_library("browser") { "//base", "//brave/components/brave_vpn/common", "//brave/components/brave_vpn/common/mojom", + "//brave/components/constants", "//brave/components/p3a_utils", "//brave/components/resources:strings", "//brave/components/skus/browser", "//brave/components/skus/common:mojom", + "//brave/components/time_period_storage", "//brave/components/version_info", "//build:buildflag_header_h", "//components/keyed_service/core", @@ -45,7 +49,10 @@ static_library("browser") { source_set("unit_tests") { testonly = true - sources = [ "brave_vpn_service_unittest.cc" ] + sources = [ + "brave_vpn_metrics_unittest.cc", + "brave_vpn_service_unittest.cc", + ] deps = [ ":browser", @@ -57,6 +64,7 @@ source_set("unit_tests") { "//brave/components/brave_vpn/browser/connection/wireguard/credentials:unit_tests", "//brave/components/brave_vpn/common", "//brave/components/brave_vpn/common/mojom", + "//brave/components/constants", "//brave/components/skus/browser", "//brave/components/skus/common", "//brave/components/skus/common:mojom", diff --git a/components/brave_vpn/browser/brave_vpn_metrics.cc b/components/brave_vpn/browser/brave_vpn_metrics.cc new file mode 100644 index 000000000000..52c4fa0d4116 --- /dev/null +++ b/components/brave_vpn/browser/brave_vpn_metrics.cc @@ -0,0 +1,115 @@ +/* Copyright (c) 2024 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#include "brave/components/brave_vpn/browser/brave_vpn_metrics.h" + +#include "base/metrics/histogram_macros.h" +#include "brave/components/brave_vpn/common/brave_vpn_constants.h" +#include "brave/components/brave_vpn/common/pref_names.h" +#include "brave/components/constants/pref_names.h" +#include "brave/components/p3a_utils/bucket.h" +#include "brave/components/p3a_utils/feature_usage.h" +#include "components/prefs/pref_change_registrar.h" +#include "components/prefs/pref_service.h" + +namespace brave_vpn { + +namespace { + +constexpr int kWidgetUsageBuckets[] = {1, 10, 20}; + +} // namespace + +BraveVpnMetrics::BraveVpnMetrics(PrefService* local_state, + PrefService* profile_prefs) + : local_state_(local_state), + profile_prefs_(profile_prefs), + widget_usage_storage_(local_state_, + prefs::kBraveVPNWidgetUsageWeeklyStorage) { + pref_change_registrar_.Init(profile_prefs); + pref_change_registrar_.Add( + kNewTabPageShowBraveVPN, + base::BindRepeating(&BraveVpnMetrics::HandleShowWidgetChange, + base::Unretained(this))); + RecordAllMetrics(false); +} + +BraveVpnMetrics::~BraveVpnMetrics() = default; + +void BraveVpnMetrics::RecordAllMetrics(bool new_usage) { + if (new_usage) { + p3a_utils::RecordFeatureUsage(local_state_, prefs::kBraveVPNFirstUseTime, + prefs::kBraveVPNLastUseTime); + } + p3a_utils::RecordFeatureNewUserReturning( + local_state_, prefs::kBraveVPNFirstUseTime, prefs::kBraveVPNLastUseTime, + prefs::kBraveVPNUsedSecondDay, kNewUserReturningHistogramName); + p3a_utils::RecordFeatureDaysInMonthUsed( + local_state_, new_usage, prefs::kBraveVPNLastUseTime, + prefs::kBraveVPNDaysInMonthUsed, kDaysInMonthUsedHistogramName); + p3a_utils::RecordFeatureLastUsageTimeMetric( + local_state_, prefs::kBraveVPNLastUseTime, kLastUsageTimeHistogramName); + RecordWidgetUsage(false); + report_timer_.Start(FROM_HERE, + base::Time::Now() + base::Hours(kP3AIntervalHours), + base::BindOnce(&BraveVpnMetrics::RecordAllMetrics, + base::Unretained(this), false)); +} + +#if BUILDFLAG(IS_ANDROID) +void BraveVpnMetrics::RecordAndroidBackgroundP3A(int64_t session_start_time_ms, + int64_t session_end_time_ms) { + if (session_start_time_ms < 0 || session_end_time_ms < 0) { + RecordAllMetrics(false); + return; + } + base::Time session_start_time = + base::Time::FromMillisecondsSinceUnixEpoch( + static_cast(session_start_time_ms)) + .LocalMidnight(); + base::Time session_end_time = base::Time::FromMillisecondsSinceUnixEpoch( + static_cast(session_end_time_ms)) + .LocalMidnight(); + for (base::Time day = session_start_time; day <= session_end_time; + day += base::Days(1)) { + bool is_last_day = day == session_end_time; + // Call functions for each day in the last session to ensure + // p3a_util functions produce the correct result + p3a_utils::RecordFeatureUsage(local_state_, prefs::kBraveVPNFirstUseTime, + prefs::kBraveVPNLastUseTime, day); + p3a_utils::RecordFeatureNewUserReturning( + local_state_, prefs::kBraveVPNFirstUseTime, prefs::kBraveVPNLastUseTime, + prefs::kBraveVPNUsedSecondDay, kNewUserReturningHistogramName, + is_last_day); + p3a_utils::RecordFeatureDaysInMonthUsed( + local_state_, day, prefs::kBraveVPNLastUseTime, + prefs::kBraveVPNDaysInMonthUsed, kDaysInMonthUsedHistogramName, + is_last_day); + } + p3a_utils::RecordFeatureLastUsageTimeMetric( + local_state_, prefs::kBraveVPNLastUseTime, kLastUsageTimeHistogramName); +} +#endif + +void BraveVpnMetrics::RecordWidgetUsage(bool new_usage) { + if (new_usage) { + widget_usage_storage_.AddDelta(1u); + } + auto total = widget_usage_storage_.GetWeeklySum(); + if (total == 0) { + return; + } + p3a_utils::RecordToHistogramBucket(kWidgetUsageHistogramName, + kWidgetUsageBuckets, total); +} + +void BraveVpnMetrics::HandleShowWidgetChange() { + if (profile_prefs_->GetBoolean(kNewTabPageShowBraveVPN)) { + return; + } + UMA_HISTOGRAM_BOOLEAN(kHideWidgetHistogramName, true); +} + +} // namespace brave_vpn diff --git a/components/brave_vpn/browser/brave_vpn_metrics.h b/components/brave_vpn/browser/brave_vpn_metrics.h new file mode 100644 index 000000000000..cde25f15eac0 --- /dev/null +++ b/components/brave_vpn/browser/brave_vpn_metrics.h @@ -0,0 +1,57 @@ +/* Copyright (c) 2024 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_COMPONENTS_BRAVE_VPN_BROWSER_BRAVE_VPN_METRICS_H_ +#define BRAVE_COMPONENTS_BRAVE_VPN_BROWSER_BRAVE_VPN_METRICS_H_ + +#include "base/timer/wall_clock_timer.h" +#include "brave/components/time_period_storage/weekly_storage.h" +#include "components/prefs/pref_change_registrar.h" + +class PrefService; + +namespace brave_vpn { + +inline constexpr char kNewUserReturningHistogramName[] = + "Brave.VPN.NewUserReturning"; +inline constexpr char kDaysInMonthUsedHistogramName[] = + "Brave.VPN.DaysInMonthUsed"; +inline constexpr char kLastUsageTimeHistogramName[] = "Brave.VPN.LastUsageTime"; +inline constexpr char kWidgetUsageHistogramName[] = "Brave.VPN.WidgetUsage"; +inline constexpr char kHideWidgetHistogramName[] = "Brave.VPN.HideWidget"; + +class BraveVpnMetrics { + public: + BraveVpnMetrics(PrefService* local_prefs, PrefService* profile_prefs); + ~BraveVpnMetrics(); + + BraveVpnMetrics(const BraveVpnMetrics&) = delete; + BraveVpnMetrics& operator=(const BraveVpnMetrics&) = delete; + + // new_usage should be set to true if a new VPN connection was just + // established. + void RecordAllMetrics(bool new_usage); + +#if BUILDFLAG(IS_ANDROID) + void RecordAndroidBackgroundP3A(int64_t session_start_time_ms, + int64_t session_end_time_ms); +#endif + + void RecordWidgetUsage(bool new_usage); + + private: + void HandleShowWidgetChange(); + + raw_ptr local_state_; + raw_ptr profile_prefs_; + PrefChangeRegistrar pref_change_registrar_; + + WeeklyStorage widget_usage_storage_; + base::WallClockTimer report_timer_; +}; + +} // namespace brave_vpn + +#endif // BRAVE_COMPONENTS_BRAVE_VPN_BROWSER_BRAVE_VPN_METRICS_H_ diff --git a/components/brave_vpn/browser/brave_vpn_metrics_unittest.cc b/components/brave_vpn/browser/brave_vpn_metrics_unittest.cc new file mode 100644 index 000000000000..3c4c38ebd501 --- /dev/null +++ b/components/brave_vpn/browser/brave_vpn_metrics_unittest.cc @@ -0,0 +1,115 @@ +/* Copyright (c) 2024 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#include "brave/components/brave_vpn/browser/brave_vpn_metrics.h" + +#include + +#include "base/test/metrics/histogram_tester.h" +#include "brave/components/brave_vpn/common/brave_vpn_utils.h" +#include "brave/components/constants/pref_names.h" +#include "components/prefs/testing_pref_service.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" +#include "content/public/test/browser_task_environment.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace brave_vpn { + +class BraveVpnMetricsTest : public testing::Test { + public: + BraveVpnMetricsTest() + : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {} + + void SetUp() override { + base::Time future_mock_time; + if (base::Time::FromString("2023-01-04", &future_mock_time)) { + task_environment_.AdvanceClock(future_mock_time - base::Time::Now()); + } + profile_prefs_.registry()->RegisterBooleanPref(kNewTabPageShowBraveVPN, + true); + brave_vpn::RegisterLocalStatePrefs(local_state_.registry()); + metrics_ = + std::make_unique(&local_state_, &profile_prefs_); + } + + protected: + content::BrowserTaskEnvironment task_environment_; + TestingPrefServiceSimple local_state_; + sync_preferences::TestingPrefServiceSyncable profile_prefs_; + std::unique_ptr metrics_; + base::HistogramTester histogram_tester_; +}; + +TEST_F(BraveVpnMetricsTest, NewUserReturningMetric) { + metrics_->RecordAllMetrics(false); + histogram_tester_.ExpectBucketCount(kNewUserReturningHistogramName, 0, 2); + + task_environment_.FastForwardBy(base::Days(1)); + metrics_->RecordAllMetrics(true); + histogram_tester_.ExpectBucketCount(kNewUserReturningHistogramName, 2, 1); + + task_environment_.FastForwardBy(base::Days(1)); + metrics_->RecordAllMetrics(true); + histogram_tester_.ExpectBucketCount(kNewUserReturningHistogramName, 3, 1); + + task_environment_.FastForwardBy(base::Days(6)); + histogram_tester_.ExpectBucketCount(kNewUserReturningHistogramName, 1, 1); +} + +TEST_F(BraveVpnMetricsTest, DaysInMonthUsedMetric) { + metrics_->RecordAllMetrics(false); + histogram_tester_.ExpectTotalCount(kDaysInMonthUsedHistogramName, 0); + + metrics_->RecordAllMetrics(true); + histogram_tester_.ExpectBucketCount(kDaysInMonthUsedHistogramName, 1, 1); + + task_environment_.FastForwardBy(base::Days(1)); + metrics_->RecordAllMetrics(true); + histogram_tester_.ExpectBucketCount(kDaysInMonthUsedHistogramName, 2, 1); + task_environment_.FastForwardBy(base::Days(1)); + histogram_tester_.ExpectBucketCount(kDaysInMonthUsedHistogramName, 2, 2); + + metrics_->RecordAllMetrics(true); + task_environment_.FastForwardBy(base::Days(30)); + histogram_tester_.ExpectBucketCount(kDaysInMonthUsedHistogramName, 0, 1); +} + +TEST_F(BraveVpnMetricsTest, LastUsageTimeMetric) { + histogram_tester_.ExpectTotalCount(kLastUsageTimeHistogramName, 0); + + metrics_->RecordAllMetrics(true); + histogram_tester_.ExpectBucketCount(kLastUsageTimeHistogramName, 1, 1); + + task_environment_.AdvanceClock(base::Days(10)); + metrics_->RecordAllMetrics(true); + histogram_tester_.ExpectBucketCount(kLastUsageTimeHistogramName, 1, 2); + task_environment_.AdvanceClock(base::Days(10)); + metrics_->RecordAllMetrics(false); + histogram_tester_.ExpectBucketCount(kLastUsageTimeHistogramName, 2, 1); +} + +TEST_F(BraveVpnMetricsTest, WidgetUsageAndHideMetrics) { + histogram_tester_.ExpectTotalCount(kWidgetUsageHistogramName, 0); + histogram_tester_.ExpectTotalCount(kHideWidgetHistogramName, 0); + + // Test widget usage recording + metrics_->RecordWidgetUsage(true); + histogram_tester_.ExpectUniqueSample(kWidgetUsageHistogramName, 0, 1); + + // Record multiple usages + metrics_->RecordWidgetUsage(true); + metrics_->RecordWidgetUsage(true); + histogram_tester_.ExpectBucketCount(kWidgetUsageHistogramName, 1, 2); + + profile_prefs_.SetBoolean(kNewTabPageShowBraveVPN, true); + histogram_tester_.ExpectTotalCount(kHideWidgetHistogramName, 0); + // Test hide widget metric + profile_prefs_.SetBoolean(kNewTabPageShowBraveVPN, false); + histogram_tester_.ExpectUniqueSample(kHideWidgetHistogramName, true, 1); + + histogram_tester_.ExpectTotalCount(kWidgetUsageHistogramName, 3); +} + +} // namespace brave_vpn diff --git a/components/brave_vpn/browser/brave_vpn_service.cc b/components/brave_vpn/browser/brave_vpn_service.cc index eb1397d8375b..3be1a6cb1f1e 100644 --- a/components/brave_vpn/browser/brave_vpn_service.cc +++ b/components/brave_vpn/browser/brave_vpn_service.cc @@ -25,7 +25,6 @@ #include "brave/components/brave_vpn/common/brave_vpn_constants.h" #include "brave/components/brave_vpn/common/brave_vpn_utils.h" #include "brave/components/brave_vpn/common/pref_names.h" -#include "brave/components/p3a_utils/feature_usage.h" #include "brave/components/skus/browser/skus_utils.h" #include "brave/components/version_info/version_info.h" #include "components/grit/brave_components_strings.h" @@ -52,7 +51,8 @@ BraveVpnService::BraveVpnService( : local_prefs_(local_prefs), profile_prefs_(profile_prefs), skus_service_getter_(skus_service_getter), - api_request_(new BraveVpnAPIRequest(url_loader_factory)) { + api_request_(new BraveVpnAPIRequest(url_loader_factory)), + brave_vpn_metrics_(local_prefs, profile_prefs) { DCHECK(IsBraveVPNFeatureEnabled()); #if !BUILDFLAG(IS_ANDROID) DCHECK(connection_manager); @@ -67,7 +67,6 @@ BraveVpnService::BraveVpnService( #endif // !BUILDFLAG(IS_ANDROID) CheckInitialState(); - InitP3A(); } BraveVpnService::~BraveVpnService() = default; @@ -165,7 +164,7 @@ void BraveVpnService::OnConnectionStateChanged(mojom::ConnectionState state) { delegate_->ShowBraveVpnStatusTrayIcon(); } #endif - RecordP3A(true); + brave_vpn_metrics_.RecordAllMetrics(true); } for (const auto& obs : observers_) { @@ -826,67 +825,14 @@ void BraveVpnService::RefreshSubscriberCredential() { ReloadPurchasedState(); } -// TODO(simonhong): Should move p3a to BraveVPNConnectionManager? -void BraveVpnService::InitP3A() { - p3a_timer_.Start(FROM_HERE, base::Hours(kP3AIntervalHours), this, - &BraveVpnService::OnP3AInterval); - RecordP3A(false); -} - -void BraveVpnService::RecordP3A(bool new_usage) { - if (new_usage) { - p3a_utils::RecordFeatureUsage(local_prefs_, prefs::kBraveVPNFirstUseTime, - prefs::kBraveVPNLastUseTime); - } - p3a_utils::RecordFeatureNewUserReturning( - local_prefs_, prefs::kBraveVPNFirstUseTime, prefs::kBraveVPNLastUseTime, - prefs::kBraveVPNUsedSecondDay, kNewUserReturningHistogramName); - p3a_utils::RecordFeatureDaysInMonthUsed( - local_prefs_, new_usage, prefs::kBraveVPNLastUseTime, - prefs::kBraveVPNDaysInMonthUsed, kDaysInMonthUsedHistogramName); - p3a_utils::RecordFeatureLastUsageTimeMetric( - local_prefs_, prefs::kBraveVPNLastUseTime, kLastUsageTimeHistogramName); -} - #if BUILDFLAG(IS_ANDROID) void BraveVpnService::RecordAndroidBackgroundP3A(int64_t session_start_time_ms, int64_t session_end_time_ms) { - if (session_start_time_ms < 0 || session_end_time_ms < 0) { - RecordP3A(false); - return; - } - base::Time session_start_time = - base::Time::FromMillisecondsSinceUnixEpoch( - static_cast(session_start_time_ms)) - .LocalMidnight(); - base::Time session_end_time = base::Time::FromMillisecondsSinceUnixEpoch( - static_cast(session_end_time_ms)) - .LocalMidnight(); - for (base::Time day = session_start_time; day <= session_end_time; - day += base::Days(1)) { - bool is_last_day = day == session_end_time; - // Call functions for each day in the last session to ensure - // p3a_util functions produce the correct result - p3a_utils::RecordFeatureUsage(local_prefs_, prefs::kBraveVPNFirstUseTime, - prefs::kBraveVPNLastUseTime, day); - p3a_utils::RecordFeatureNewUserReturning( - local_prefs_, prefs::kBraveVPNFirstUseTime, prefs::kBraveVPNLastUseTime, - prefs::kBraveVPNUsedSecondDay, kNewUserReturningHistogramName, - is_last_day); - p3a_utils::RecordFeatureDaysInMonthUsed( - local_prefs_, day, prefs::kBraveVPNLastUseTime, - prefs::kBraveVPNDaysInMonthUsed, kDaysInMonthUsedHistogramName, - is_last_day); - } - p3a_utils::RecordFeatureLastUsageTimeMetric( - local_prefs_, prefs::kBraveVPNLastUseTime, kLastUsageTimeHistogramName); + brave_vpn_metrics_.RecordAndroidBackgroundP3A(session_start_time_ms, + session_end_time_ms); } #endif -void BraveVpnService::OnP3AInterval() { - RecordP3A(false); -} - void BraveVpnService::SetPurchasedState( const std::string& env, PurchasedState state, @@ -939,7 +885,6 @@ void BraveVpnService::Shutdown() { skus_service_.reset(); observers_.Clear(); api_request_.reset(); - p3a_timer_.Stop(); subs_cred_refresh_timer_.Stop(); #if !BUILDFLAG(IS_ANDROID) diff --git a/components/brave_vpn/browser/brave_vpn_service.h b/components/brave_vpn/browser/brave_vpn_service.h index b1884b5f2cf5..906b6e1bed09 100644 --- a/components/brave_vpn/browser/brave_vpn_service.h +++ b/components/brave_vpn/browser/brave_vpn_service.h @@ -20,6 +20,7 @@ #include "base/timer/timer.h" #include "base/values.h" #include "brave/components/brave_vpn/browser/api/brave_vpn_api_request.h" +#include "brave/components/brave_vpn/browser/brave_vpn_metrics.h" #include "brave/components/brave_vpn/browser/brave_vpn_service_delegate.h" #include "brave/components/brave_vpn/browser/connection/brave_vpn_connection_manager.h" #include "brave/components/brave_vpn/common/brave_vpn_data_types.h" @@ -51,12 +52,6 @@ namespace brave_vpn { class BraveVPNServiceDelegate; -inline constexpr char kNewUserReturningHistogramName[] = - "Brave.VPN.NewUserReturning"; -inline constexpr char kDaysInMonthUsedHistogramName[] = - "Brave.VPN.DaysInMonthUsed"; -inline constexpr char kLastUsageTimeHistogramName[] = "Brave.VPN.LastUsageTime"; - // This class is used by desktop and android. // However, it includes desktop specific impls and it's hidden // by IS_ANDROID ifdef. @@ -168,14 +163,13 @@ class BraveVpnService : delegate_ = std::move(delegate); } - // new_usage should be set to true if a new VPN connection was just - // established. - void RecordP3A(bool new_usage); #if BUILDFLAG(IS_ANDROID) void RecordAndroidBackgroundP3A(int64_t session_start_time_ms, int64_t session_end_time_ms); #endif + BraveVpnMetrics* brave_vpn_metrics() { return &brave_vpn_metrics_; } + private: friend class BraveVPNServiceTest; friend class BraveVpnButtonUnitTest; @@ -204,9 +198,6 @@ class BraveVpnService : // KeyedService overrides: void Shutdown() override; - void InitP3A(); - void OnP3AInterval(); - mojom::PurchasedInfo GetPurchasedInfoSync() const; void SetPurchasedState( const std::string& env, @@ -251,8 +242,8 @@ class BraveVpnService : mojo::RemoteSet observers_; std::unique_ptr api_request_; std::unique_ptr delegate_; - base::RepeatingTimer p3a_timer_; base::OneShotTimer subs_cred_refresh_timer_; + BraveVpnMetrics brave_vpn_metrics_; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/components/brave_vpn/browser/brave_vpn_service_unittest.cc b/components/brave_vpn/browser/brave_vpn_service_unittest.cc index 33bb9ae18f14..7588b4dc643b 100644 --- a/components/brave_vpn/browser/brave_vpn_service_unittest.cc +++ b/components/brave_vpn/browser/brave_vpn_service_unittest.cc @@ -17,7 +17,6 @@ #include "base/memory/scoped_refptr.h" #include "base/run_loop.h" #include "base/strings/string_number_conversions.h" -#include "base/test/metrics/histogram_tester.h" #include "base/test/scoped_feature_list.h" #include "brave/components/brave_vpn/browser/api/brave_vpn_api_request.h" #include "brave/components/brave_vpn/browser/brave_vpn_service_helper.h" @@ -390,8 +389,6 @@ class BraveVPNServiceTest : public testing::Test { connection_manager_->connection_api_impl_.get()); } #endif - void RecordP3A(bool new_usage) { service_->RecordP3A(new_usage); } - std::string GetCurrentEnvironment() { return service_->GetCurrentEnvironment(); } @@ -569,7 +566,6 @@ class BraveVPNServiceTest : public testing::Test { data_decoder::test::InProcessDataDecoder in_process_data_decoder_; network::TestURLLoaderFactory url_loader_factory_; scoped_refptr shared_url_loader_factory_; - base::HistogramTester histogram_tester_; }; TEST_F(BraveVPNServiceTest, ResponseSanitizingTest) { @@ -1122,52 +1118,4 @@ TEST_F(BraveVPNServiceTest, LoadPurchasedStateForAnotherEnv) { EXPECT_EQ(GetCurrentEnvironment(), skus::kEnvStaging); } -TEST_F(BraveVPNServiceTest, NewUserReturningMetric) { - RecordP3A(false); - histogram_tester_.ExpectBucketCount(kNewUserReturningHistogramName, 0, 2); - - task_environment_.FastForwardBy(base::Days(1)); - RecordP3A(true); - histogram_tester_.ExpectBucketCount(kNewUserReturningHistogramName, 2, 1); - - task_environment_.FastForwardBy(base::Days(1)); - RecordP3A(true); - histogram_tester_.ExpectBucketCount(kNewUserReturningHistogramName, 3, 1); - - task_environment_.FastForwardBy(base::Days(6)); - histogram_tester_.ExpectBucketCount(kNewUserReturningHistogramName, 1, 1); -} - -TEST_F(BraveVPNServiceTest, DaysInMonthUsedMetric) { - RecordP3A(false); - histogram_tester_.ExpectTotalCount(kDaysInMonthUsedHistogramName, 0); - - RecordP3A(true); - histogram_tester_.ExpectBucketCount(kDaysInMonthUsedHistogramName, 1, 1); - - task_environment_.FastForwardBy(base::Days(1)); - RecordP3A(true); - histogram_tester_.ExpectBucketCount(kDaysInMonthUsedHistogramName, 2, 1); - task_environment_.FastForwardBy(base::Days(1)); - histogram_tester_.ExpectBucketCount(kDaysInMonthUsedHistogramName, 2, 2); - - RecordP3A(true); - task_environment_.FastForwardBy(base::Days(30)); - histogram_tester_.ExpectBucketCount(kDaysInMonthUsedHistogramName, 0, 1); -} - -TEST_F(BraveVPNServiceTest, LastUsageTimeMetric) { - histogram_tester_.ExpectTotalCount(kLastUsageTimeHistogramName, 0); - - RecordP3A(true); - histogram_tester_.ExpectBucketCount(kLastUsageTimeHistogramName, 1, 1); - - task_environment_.AdvanceClock(base::Days(10)); - RecordP3A(true); - histogram_tester_.ExpectBucketCount(kLastUsageTimeHistogramName, 1, 2); - task_environment_.AdvanceClock(base::Days(10)); - RecordP3A(false); - histogram_tester_.ExpectBucketCount(kLastUsageTimeHistogramName, 2, 1); -} - } // namespace brave_vpn diff --git a/components/brave_vpn/common/brave_vpn_utils.cc b/components/brave_vpn/common/brave_vpn_utils.cc index a4086f35eea0..09228efe8455 100644 --- a/components/brave_vpn/common/brave_vpn_utils.cc +++ b/components/brave_vpn/common/brave_vpn_utils.cc @@ -52,6 +52,7 @@ void RegisterVPNLocalStatePrefs(PrefRegistrySimple* registry) { #if BUILDFLAG(IS_MAC) registry->RegisterBooleanPref(prefs::kBraveVPNOnDemandEnabled, false); #endif + registry->RegisterListPref(prefs::kBraveVPNWidgetUsageWeeklyStorage); } // Region name map between v1 and v2. diff --git a/components/brave_vpn/common/pref_names.h b/components/brave_vpn/common/pref_names.h index 2947761fd8a8..bac7359625f8 100644 --- a/components/brave_vpn/common/pref_names.h +++ b/components/brave_vpn/common/pref_names.h @@ -85,6 +85,8 @@ inline constexpr char kBraveVPNUsedSecondDay[] = "brave.brave_vpn.used_second_day"; inline constexpr char kBraveVPNDaysInMonthUsed[] = "brave.brave_vpn.days_in_month_used"; +inline constexpr char kBraveVPNWidgetUsageWeeklyStorage[] = + "brave.brave_vpn.widget_usage"; } // namespace prefs } // namespace brave_vpn diff --git a/components/p3a/metric_names.h b/components/p3a/metric_names.h index 08a0099aad4a..74a6631f7ff3 100644 --- a/components/p3a/metric_names.h +++ b/components/p3a/metric_names.h @@ -209,8 +209,10 @@ inline constexpr auto kCollectedTypicalHistograms = "Brave.Toolbar.MenuDismissRate", "Brave.Toolbar.MenuOpens", "Brave.VPN.DaysInMonthUsed", + "Brave.VPN.HideWidget", "Brave.VPN.LastUsageTime", "Brave.VPN.NewUserReturning", + "Brave.VPN.WidgetUsage", "Brave.VerticalTabs.GroupTabs", "Brave.VerticalTabs.OpenTabs", "Brave.VerticalTabs.PinnedTabs", @@ -358,6 +360,8 @@ inline constexpr auto kEphemeralHistograms = "Brave.Today.UsageMonthly", "Brave.Today.WeeklyTotalCardClicks", "Brave.Uptime.BrowserOpenTime.2", + "Brave.VPN.HideWidget", + "Brave.VPN.WidgetUsage", "Brave.VerticalTabs.GroupTabs", "Brave.VerticalTabs.OpenTabs", "Brave.VerticalTabs.PinnedTabs",