Fixed NTP shows same background images

fix brave/brave-browser#32359 Sometimes we could meet the condition that ViewCounterModel::count_to_branded_wallpaper_ is zero and no SI campaign. When this happens, we didn't increase background wallpaper index. This caused showing same background image till new campaign starts. We should increase background images index in this situation also.
brave · Aug 28, 2023 · 08d4e29 · 08d4e29
1 parent 13fcadb
commit 08d4e29
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 4 deletions.
diff --git a/components/ntp_background_images/browser/view_counter_model.cc b/components/ntp_background_images/browser/view_counter_model.cc
@@ -118,8 +118,10 @@ void ViewCounterModel::RegisterPageViewForBackgroundImages() {
     return;
 
   // Don't count when SI will be visible.
-  if (show_branded_wallpaper_ && count_to_branded_wallpaper_ == 0)
+  if (show_branded_wallpaper_ && total_campaign_count_ != 0 &&
+      count_to_branded_wallpaper_ == 0) {
     return;
+  }
 
   // Increase background image index
   current_wallpaper_image_index_++;

diff --git a/components/ntp_background_images/browser/view_counter_model.h b/components/ntp_background_images/browser/view_counter_model.h
@@ -55,7 +55,10 @@ class ViewCounterModel {
   FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest,
                            NTPSponsoredImagesCountToBrandedWallpaperTest);
   FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest, NTPBackgroundImagesTest);
-  FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest, NTPBackgroundImagesOnlyTest);
+  FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest,
+                           NTPBackgroundImagesWithSIDisabledTest);
+  FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest,
+                           NTPBackgroundImagesWithEmptyCampaignTest);
   FRIEND_TEST_ALL_PREFIXES(ViewCounterModelTest,
                            NTPFailedToLoadSponsoredImagesTest);
   FRIEND_TEST_ALL_PREFIXES(NTPBackgroundImagesViewCounterTest, ModelTest);

diff --git a/components/ntp_background_images/browser/view_counter_model_unittest.cc b/components/ntp_background_images/browser/view_counter_model_unittest.cc
@@ -144,8 +144,8 @@ TEST_F(ViewCounterModelTest, NTPBackgroundImagesTest) {
   }
 }
 
-// Test for background images only case (SI not active)
-TEST_F(ViewCounterModelTest, NTPBackgroundImagesOnlyTest) {
+// Test for background images only case (SI option is disabled)
+TEST_F(ViewCounterModelTest, NTPBackgroundImagesWithSIDisabledTest) {
   ViewCounterModel model(prefs());
 
   model.set_total_image_count(kTestImageCount);
@@ -177,6 +177,25 @@ TEST_F(ViewCounterModelTest, NTPBackgroundImagesOnlyTest) {
   }
 }
 
+// Test for background images only case (SI option is enabled but no campaign)
+TEST_F(ViewCounterModelTest, NTPBackgroundImagesWithEmptyCampaignTest) {
+  ViewCounterModel model(prefs());
+
+  // Check background wallpaper index is properly updated when SI option is
+  // enabled but there is no campaign.
+  model.Reset();
+  model.set_total_image_count(kTestImageCount);
+  model.set_show_branded_wallpaper(true);
+  model.count_to_branded_wallpaper_ = 0;
+
+  constexpr int kTestPageViewCount = 30;
+  for (int i = 0; i < kTestPageViewCount; ++i) {
+    EXPECT_EQ(i % model.total_image_count_,
+              model.current_wallpaper_image_index());
+    model.RegisterPageView();
+  }
+}
+
 TEST_F(ViewCounterModelTest, NTPSuperReferralTest) {
   ViewCounterModel model(prefs());