Skip to content

Commit

Permalink
Merge pull request #19823 from brave/ntp-si-initial-count-update
Browse files Browse the repository at this point in the history
NTP SI: update default counts and reset to initial counts on SI data update, browser restart and every 24hr
  • Loading branch information
petemill committed Sep 27, 2023
2 parents 941913c + 22c14c5 commit 3c5e87d
Show file tree
Hide file tree
Showing 10 changed files with 142 additions and 22 deletions.
5 changes: 5 additions & 0 deletions browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions chromium_src/chrome/browser/prefs/browser_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -229,6 +230,10 @@ void MigrateObsoleteProfilePrefs(Profile* profile) {
}
#endif

// Added 2023-09
ntp_background_images::ViewCounterService::MigrateObsoleteProfilePrefs(
profile->GetPrefs());

// END_MIGRATE_OBSOLETE_PROFILE_PREFS
}

Expand Down
4 changes: 2 additions & 2 deletions components/ntp_background_images/browser/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<int> kInitialCountToBrandedWallpaper{
&kBraveNTPBrandedWallpaper, "initial_count_to_branded_wallpaper", 1};

// Show branded wallpaper every nth new tab page.
constexpr base::FeatureParam<int> kCountToBrandedWallpaper{
&kBraveNTPBrandedWallpaper, "count_to_branded_wallpaper", 3};
&kBraveNTPBrandedWallpaper, "count_to_branded_wallpaper", 2};

} // namespace features
} // namespace ntp_background_images
Expand Down
37 changes: 32 additions & 5 deletions components/ntp_background_images/browser/view_counter_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 <algorithm>

#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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
Expand All @@ -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
13 changes: 12 additions & 1 deletion components/ntp_background_images/browser/view_counter_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include "base/gtest_prod_util.h"
#include "base/memory/raw_ptr.h"
#include "base/timer/timer.h"

class PrefService;

Expand Down Expand Up @@ -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);
Expand All @@ -69,15 +77,18 @@ class ViewCounterModel {

void RegisterPageViewForBackgroundImages();

void OnTimerCountsResetExpired();

// For NTP SI.
raw_ptr<PrefService> 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<size_t> campaigns_total_branded_image_count_;
std::vector<size_t> campaigns_current_branded_image_index_;
base::RepeatingTimer timer_counts_reset_;

// For NTP BI.
int current_wallpaper_image_index_ = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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<base::test::FeatureRefAndParams> 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) {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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());

Expand Down
22 changes: 17 additions & 5 deletions components/ntp_background_images/browser/view_counter_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion components/ntp_background_images/common/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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[] =
Expand Down
1 change: 0 additions & 1 deletion components/ntp_background_images/common/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand Down

0 comments on commit 3c5e87d

Please sign in to comment.