diff --git a/browser/brave_profile_prefs.cc b/browser/brave_profile_prefs.cc index 5bfbefae5b85..a17b4a6000f7 100644 --- a/browser/brave_profile_prefs.cc +++ b/browser/brave_profile_prefs.cc @@ -36,6 +36,7 @@ #include "brave/components/de_amp/common/pref_names.h" #include "brave/components/debounce/browser/debounce_service.h" #include "brave/components/ipfs/buildflags/buildflags.h" +#include "brave/components/ntp_background_images/browser/view_counter_service.h" #include "brave/components/ntp_background_images/buildflags/buildflags.h" #include "brave/components/omnibox/browser/brave_omnibox_prefs.h" #include "brave/components/request_otr/common/buildflags/buildflags.h" @@ -270,6 +271,10 @@ void RegisterProfilePrefsForMigration( registry->RegisterListPref(pref); } #endif + + // Added 2023-09 + ntp_background_images::ViewCounterService::RegisterProfilePrefsForMigration( + registry); } void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { diff --git a/chromium_src/chrome/browser/prefs/browser_prefs.cc b/chromium_src/chrome/browser/prefs/browser_prefs.cc index fcf45023326a..74cceff341d1 100644 --- a/chromium_src/chrome/browser/prefs/browser_prefs.cc +++ b/chromium_src/chrome/browser/prefs/browser_prefs.cc @@ -17,6 +17,7 @@ #include "brave/components/brave_wallet/browser/keyring_service.h" #include "brave/components/constants/pref_names.h" #include "brave/components/decentralized_dns/core/utils.h" +#include "brave/components/ntp_background_images/browser/view_counter_service.h" #include "brave/components/ntp_background_images/buildflags/buildflags.h" #include "brave/components/omnibox/browser/brave_omnibox_prefs.h" #include "brave/components/tor/buildflags/buildflags.h" @@ -229,6 +230,10 @@ void MigrateObsoleteProfilePrefs(Profile* profile) { } #endif + // Added 2023-09 + ntp_background_images::ViewCounterService::MigrateObsoleteProfilePrefs( + profile->GetPrefs()); + // END_MIGRATE_OBSOLETE_PROFILE_PREFS } diff --git a/components/ntp_background_images/browser/features.h b/components/ntp_background_images/browser/features.h index cfadf1df8fdc..f24da985dadf 100644 --- a/components/ntp_background_images/browser/features.h +++ b/components/ntp_background_images/browser/features.h @@ -18,13 +18,13 @@ BASE_DECLARE_FEATURE(kBraveNTPSuperReferralWallpaper); BASE_DECLARE_FEATURE(kBraveNTPBrandedWallpaper); -// Show initial branded wallpaper after nth new tab page for fresh installs. +// Show initial branded wallpaper after nth new tab page for fresh opens. constexpr base::FeatureParam kInitialCountToBrandedWallpaper{ &kBraveNTPBrandedWallpaper, "initial_count_to_branded_wallpaper", 1}; // Show branded wallpaper every nth new tab page. constexpr base::FeatureParam kCountToBrandedWallpaper{ - &kBraveNTPBrandedWallpaper, "count_to_branded_wallpaper", 3}; + &kBraveNTPBrandedWallpaper, "count_to_branded_wallpaper", 2}; } // namespace features } // namespace ntp_background_images diff --git a/components/ntp_background_images/browser/view_counter_model.cc b/components/ntp_background_images/browser/view_counter_model.cc index cef294292bf7..bbf29e5f83c6 100644 --- a/components/ntp_background_images/browser/view_counter_model.cc +++ b/components/ntp_background_images/browser/view_counter_model.cc @@ -4,21 +4,34 @@ // you can obtain one at http://mozilla.org/MPL/2.0/. #include "brave/components/ntp_background_images/browser/view_counter_model.h" +#include #include "base/check.h" #include "base/logging.h" #include "base/rand_util.h" +#include "base/time/time.h" #include "brave/components/ntp_background_images/browser/features.h" #include "brave/components/ntp_background_images/common/pref_names.h" #include "components/prefs/pref_service.h" namespace ntp_background_images { +namespace { + +constexpr base::TimeDelta kCountsResetTimeDelay = base::Days(1); + +} // namespace + ViewCounterModel::ViewCounterModel(PrefService* prefs) : prefs_(prefs) { CHECK(prefs); - count_to_branded_wallpaper_ = - prefs->GetInteger(prefs::kCountToBrandedWallpaper); + // When browser is restarted we reset to "initial" count. This will also get + // set again in the Reset() function, called e.g. when component is updated. + count_to_branded_wallpaper_ = features::kInitialCountToBrandedWallpaper.Get(); + + // We also reset when a specific amount of time is elapsed when in SI mode + timer_counts_reset_.Start(FROM_HERE, kCountsResetTimeDelay, this, + &ViewCounterModel::OnTimerCountsResetExpired); } ViewCounterModel::~ViewCounterModel() = default; @@ -103,9 +116,6 @@ void ViewCounterModel::RegisterPageViewForBrandedImages() { // Randomize campaign index for next time. current_campaign_index_ = base::RandInt(0, total_campaign_count_ - 1); } - - prefs_->SetInteger(prefs::kCountToBrandedWallpaper, - count_to_branded_wallpaper_); } void ViewCounterModel::RegisterPageViewForBackgroundImages() { @@ -144,6 +154,16 @@ void ViewCounterModel::IncreaseBackgroundWallpaperImageIndex() { current_wallpaper_image_index_ %= total_image_count_; } +void ViewCounterModel::MaybeResetBrandedWallpaperCount() { + // Set count so that user is more likely to see new branded data at least once + // Only reset count for SI images + if (!always_show_branded_wallpaper_ && show_branded_wallpaper_) { + count_to_branded_wallpaper_ = + std::min(count_to_branded_wallpaper_, + features::kInitialCountToBrandedWallpaper.Get()); + } +} + void ViewCounterModel::Reset() { current_wallpaper_image_index_ = 0; total_image_count_ = 0; @@ -152,6 +172,13 @@ void ViewCounterModel::Reset() { total_campaign_count_ = 0; campaigns_total_branded_image_count_.clear(); campaigns_current_branded_image_index_.clear(); + MaybeResetBrandedWallpaperCount(); + // Restart timer with same parameters as set during this class' constructor + timer_counts_reset_.Reset(); +} + +void ViewCounterModel::OnTimerCountsResetExpired() { + MaybeResetBrandedWallpaperCount(); } } // namespace ntp_background_images diff --git a/components/ntp_background_images/browser/view_counter_model.h b/components/ntp_background_images/browser/view_counter_model.h index 0cc94a86b692..eedd70274dfb 100644 --- a/components/ntp_background_images/browser/view_counter_model.h +++ b/components/ntp_background_images/browser/view_counter_model.h @@ -11,6 +11,7 @@ #include "base/gtest_prod_util.h" #include "base/memory/raw_ptr.h" +#include "base/timer/timer.h" class PrefService; @@ -46,12 +47,19 @@ class ViewCounterModel { bool ShouldShowBrandedWallpaper() const; void RegisterPageView(); + void MaybeResetBrandedWallpaperCount(); void Reset(); void IncreaseBackgroundWallpaperImageIndex(); private: friend class NTPBackgroundImagesViewCounterTest; FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest, NTPSponsoredImagesTest); + FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest, + NTPSponsoredImagesCountResetTest); + FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest, + NTPSponsoredImagesCountResetMinTest); + FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest, + NTPSponsoredImagesCountResetTimerTest); FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest, NTPSponsoredImagesCountToBrandedWallpaperTest); FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest, NTPBackgroundImagesTest); @@ -69,15 +77,18 @@ class ViewCounterModel { void RegisterPageViewForBackgroundImages(); + void OnTimerCountsResetExpired(); + // For NTP SI. raw_ptr prefs_ = nullptr; - int count_to_branded_wallpaper_ = 1; + int count_to_branded_wallpaper_ = 0; bool always_show_branded_wallpaper_ = false; bool show_branded_wallpaper_ = true; size_t current_campaign_index_ = 0; size_t total_campaign_count_ = 0; std::vector campaigns_total_branded_image_count_; std::vector campaigns_current_branded_image_index_; + base::RepeatingTimer timer_counts_reset_; // For NTP BI. int current_wallpaper_image_index_ = 0; diff --git a/components/ntp_background_images/browser/view_counter_model_unittest.cc b/components/ntp_background_images/browser/view_counter_model_unittest.cc index ecfa706cf888..e4b2bcc0b9fd 100644 --- a/components/ntp_background_images/browser/view_counter_model_unittest.cc +++ b/components/ntp_background_images/browser/view_counter_model_unittest.cc @@ -4,10 +4,13 @@ // you can obtain one at http://mozilla.org/MPL/2.0/. #include "brave/components/ntp_background_images/browser/view_counter_model.h" +#include "base/test/scoped_feature_list.h" +#include "base/time/time.h" #include "brave/components/ntp_background_images/browser/features.h" #include "brave/components/ntp_background_images/browser/view_counter_service.h" #include "brave/components/ntp_background_images/common/pref_names.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 { @@ -19,22 +22,29 @@ namespace ntp_background_images { class ViewCounterModelTest : public testing::Test { public: - ViewCounterModelTest() = default; + ViewCounterModelTest() + : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {} ~ViewCounterModelTest() override = default; void SetUp() override { auto* registry = prefs()->registry(); ViewCounterService::RegisterProfilePrefs(registry); + + base::FieldTrialParams parameters; + std::vector enabled_features; + parameters[features::kInitialCountToBrandedWallpaper.name] = "1"; + parameters[features::kCountToBrandedWallpaper.name] = "3"; + enabled_features.emplace_back(features::kBraveNTPBrandedWallpaper, + parameters); + feature_list_.InitWithFeaturesAndParameters(enabled_features, {}); } sync_preferences::TestingPrefServiceSyncable* prefs() { return &prefs_; } - void SetCountToBrandedWallPaper(int count) { - prefs()->SetInteger(prefs::kCountToBrandedWallpaper, count); - } - protected: + content::BrowserTaskEnvironment task_environment_; sync_preferences::TestingPrefServiceSyncable prefs_; + base::test::ScopedFeatureList feature_list_; }; TEST_F(ViewCounterModelTest, NTPSponsoredImagesTest) { @@ -82,9 +92,8 @@ TEST_F(ViewCounterModelTest, NTPSponsoredImagesTest) { } TEST_F(ViewCounterModelTest, NTPSponsoredImagesCountToBrandedWallpaperTest) { - SetCountToBrandedWallPaper(1); - ViewCounterModel model(prefs()); + model.count_to_branded_wallpaper_ = 1; model.SetCampaignsTotalBrandedImageCount(kTestCampaignsTotalImageCount); @@ -118,6 +127,56 @@ TEST_F(ViewCounterModelTest, NTPSponsoredImagesCountToBrandedWallpaperTest) { model.RegisterPageView(); } +TEST_F(ViewCounterModelTest, NTPSponsoredImagesCountResetTest) { + ViewCounterModel model(prefs()); + model.SetCampaignsTotalBrandedImageCount(kTestCampaignsTotalImageCount); + + // Verify param value for initial count was used + EXPECT_EQ(model.count_to_branded_wallpaper_, 1); + model.RegisterPageView(); + EXPECT_TRUE(model.ShouldShowBrandedWallpaper()); + model.RegisterPageView(); + EXPECT_FALSE(model.ShouldShowBrandedWallpaper()); + EXPECT_EQ(model.count_to_branded_wallpaper_, 3); + + // We expect to be reset to initial count when source data updates (which + // calls Reset). + model.Reset(); + EXPECT_EQ(model.count_to_branded_wallpaper_, 1); +} + +TEST_F(ViewCounterModelTest, NTPSponsoredImagesCountResetMinTest) { + ViewCounterModel model(prefs()); + model.SetCampaignsTotalBrandedImageCount(kTestCampaignsTotalImageCount); + + // Verify param value for initial count was used + EXPECT_EQ(model.count_to_branded_wallpaper_, 1); + model.RegisterPageView(); + EXPECT_TRUE(model.ShouldShowBrandedWallpaper()); + EXPECT_EQ(model.count_to_branded_wallpaper_, 0); + + // We expect to be reset to initial count only if count_to_branded_wallpaper_ + // is higher than initial count. + model.Reset(); + EXPECT_TRUE(model.ShouldShowBrandedWallpaper()); + EXPECT_EQ(model.count_to_branded_wallpaper_, 0); +} + +TEST_F(ViewCounterModelTest, NTPSponsoredImagesCountResetTimerTest) { + ViewCounterModel model(prefs()); + model.SetCampaignsTotalBrandedImageCount(kTestCampaignsTotalImageCount); + + // Verify param value for initial count was used + EXPECT_EQ(model.count_to_branded_wallpaper_, 1); + model.RegisterPageView(); + EXPECT_TRUE(model.ShouldShowBrandedWallpaper()); + model.RegisterPageView(); + EXPECT_FALSE(model.ShouldShowBrandedWallpaper()); + EXPECT_EQ(model.count_to_branded_wallpaper_, 3); + task_environment_.FastForwardBy(base::Days(1)); + EXPECT_EQ(model.count_to_branded_wallpaper_, 1); +} + TEST_F(ViewCounterModelTest, NTPBackgroundImagesTest) { ViewCounterModel model(prefs()); diff --git a/components/ntp_background_images/browser/view_counter_service.cc b/components/ntp_background_images/browser/view_counter_service.cc index be5f38617b8f..15532e7528d9 100644 --- a/components/ntp_background_images/browser/view_counter_service.cc +++ b/components/ntp_background_images/browser/view_counter_service.cc @@ -48,6 +48,10 @@ constexpr char kSponsoredNewTabsHistogramName[] = "Brave.NTP.SponsoredNewTabsCreated"; constexpr int kSponsoredNewTabsBuckets[] = {0, 10, 20, 30, 40, 50}; +// Obsolete pref +constexpr char kObsoleteCountToBrandedWallpaperPref[] = + "brave.count_to_branded_wallpaper"; + } // namespace namespace ntp_background_images { @@ -60,11 +64,8 @@ void ViewCounterService::RegisterLocalStatePrefs(PrefRegistrySimple* registry) { void ViewCounterService::RegisterProfilePrefs( user_prefs::PrefRegistrySyncable* registry) { - registry->RegisterBooleanPref( - prefs::kBrandedWallpaperNotificationDismissed, false); - registry->RegisterIntegerPref( - prefs::kCountToBrandedWallpaper, - features::kInitialCountToBrandedWallpaper.Get()); + registry->RegisterBooleanPref(prefs::kBrandedWallpaperNotificationDismissed, + false); registry->RegisterBooleanPref( prefs::kNewTabPageShowSponsoredImagesBackgroundImage, true); // Integer type is used because this pref is used by radio button group in @@ -75,6 +76,17 @@ void ViewCounterService::RegisterProfilePrefs( prefs::kNewTabPageShowBackgroundImage, true); } +void ViewCounterService::RegisterProfilePrefsForMigration( + user_prefs::PrefRegistrySyncable* registry) { + // Added 09/2023 + registry->RegisterIntegerPref(kObsoleteCountToBrandedWallpaperPref, 0); +} + +void ViewCounterService::MigrateObsoleteProfilePrefs(PrefService* prefs) { + // Added 09/2023 + prefs->ClearPref(kObsoleteCountToBrandedWallpaperPref); +} + ViewCounterService::ViewCounterService( NTPBackgroundImagesService* service, BraveNTPCustomBackgroundService* custom_service, diff --git a/components/ntp_background_images/browser/view_counter_service.h b/components/ntp_background_images/browser/view_counter_service.h index 52f8c3bddba6..cde48a024c91 100644 --- a/components/ntp_background_images/browser/view_counter_service.h +++ b/components/ntp_background_images/browser/view_counter_service.h @@ -63,6 +63,9 @@ class ViewCounterService : public KeyedService, static void RegisterLocalStatePrefs(PrefRegistrySimple* registry); static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); + static void RegisterProfilePrefsForMigration( + user_prefs::PrefRegistrySyncable* registry); + static void MigrateObsoleteProfilePrefs(PrefService* prefs); // Lets the counter know that a New Tab Page view has occured. // This should always be called as it will evaluate whether the user has diff --git a/components/ntp_background_images/common/pref_names.cc b/components/ntp_background_images/common/pref_names.cc index 6ac63121bff8..9ed2a37ace5b 100644 --- a/components/ntp_background_images/common/pref_names.cc +++ b/components/ntp_background_images/common/pref_names.cc @@ -10,7 +10,6 @@ namespace prefs { const char kBrandedWallpaperNotificationDismissed[] = "brave.branded_wallpaper_notification_dismissed"; -const char kCountToBrandedWallpaper[] = "brave.count_to_branded_wallpaper"; const char kNewTabPageShowSponsoredImagesBackgroundImage[] = "brave.new_tab_page.show_branded_background_image"; const char kNewTabPageSuperReferralThemesOption[] = diff --git a/components/ntp_background_images/common/pref_names.h b/components/ntp_background_images/common/pref_names.h index d42ddf4f0fef..0af9364ce106 100644 --- a/components/ntp_background_images/common/pref_names.h +++ b/components/ntp_background_images/common/pref_names.h @@ -13,7 +13,6 @@ namespace prefs { // The one is sponsored images wallpaper and the other is super referral // wallpaper. extern const char kNewTabPageShowSponsoredImagesBackgroundImage[]; -extern const char kCountToBrandedWallpaper[]; extern const char kNewTabPageSuperReferralThemesOption[]; extern const char kBrandedWallpaperNotificationDismissed[]; extern const char kNewTabPageShowBackgroundImage[];