Reland "[ntp] Store collection ids for theme background images"

This is a reland of commit 20fca53

It adds default values for daily_refresh_enabled and collection_id to uploaded images since we now expect these for all CustomBackground structs. Also fixes the setting up of custom background in test (the main culprit).

Original change's description:
> [ntp] Store collection ids for theme background images
>
> This CL concerns the NTP handler. A follow up CL will bring the changes here to the Customize Chrome handler.
>
> Prior to this CL collection ids were only stored in user prefs ('prefs::kNtpCustomBackgroundDict') if daily refresh was turned on.
>
> Now, collection IDs will be stored anytime a theme is set.
>
> This CL obsoletes a test in ntp_custom_background_service_unittest.cc that makes sure collection ID takes priority over background URL. Before and after this CL, there are no calls to SetCustomBackground for daily refresh in the codebase where both URL and collection id are set (https://source.chromium.org/search?q=-file:out%20SetCustomBackgroundInfo%20-file:test).
>
> The changes in the CL will allow us to create a metric that stores the collection ID used by the user on NTP load.
>
> Change-Id: Ief491945ec77b6b06f56dc4ff42bad2774eb9ca4
> Bug: 1384258
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4183244
> Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Commit-Queue: Paul Adedeji <pauladedeji@google.com>
> Reviewed-by: Riley Tatum <rtatum@google.com>
> Cr-Commit-Position: refs/heads/main@{#1097166}

(cherry picked from commit 5c7d381)

Bug: 1384258
Change-Id: I101f478becf2eb412cca51c6fc919cf1eb7fc854
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4198399
Reviewed-by: Riley Tatum <rtatum@google.com>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Paul Adedeji <pauladedeji@google.com>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1097786}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4205669
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Riley Tatum <rtatum@google.com>
Cr-Commit-Position: refs/branch-heads/5563@{#57}
Cr-Branched-From: 3ac59a6-refs/heads/main@{#1097615}

chrome/browser/resources/new_tab_page/customize_backgrounds.ts

@@ -96,8 +96,7 @@ export class CustomizeBackgroundsElement extends PolymerElement {
   private getNoBackgroundClass_(): string {
     return this.theme &&
             (this.theme.backgroundImage && !this.theme.isCustomBackground ||
-             !this.theme.backgroundImage &&
-                 !this.theme.dailyRefreshCollectionId) ?
+             !this.theme.backgroundImage && !this.theme.dailyRefreshEnabled) ?
         'selected' :
         '';
   }
@@ -106,7 +105,7 @@ export class CustomizeBackgroundsElement extends PolymerElement {
     const {url} = this.images_[index].imageUrl;
     return this.theme && this.theme.backgroundImage &&
             this.theme.backgroundImage.url.url === url &&
-            !this.theme.dailyRefreshCollectionId ?
+            !this.theme.dailyRefreshEnabled ?
         'selected' :
         '';
   }
@@ -149,9 +148,11 @@ export class CustomizeBackgroundsElement extends PolymerElement {
       attributionUrl,
       imageUrl,
       previewImageUrl,
+      collectionId,
     } = image;
     this.pageHandler_.setBackgroundImage(
-        attribution1, attribution2, attributionUrl, imageUrl, previewImageUrl);
+        attribution1, attribution2, attributionUrl, imageUrl, previewImageUrl,
+        collectionId);
   }
 
   private async onSelectedCollectionChange_() {

chrome/browser/resources/new_tab_page/customize_dialog.ts

@@ -167,8 +167,8 @@ export class CustomizeDialogElement extends PolymerElement {
     if (!this.selectedCollection_) {
       return false;
     }
-    return !!this.theme &&
-        this.selectedCollection_.id === this.theme.dailyRefreshCollectionId;
+    return !!this.theme && this.theme.dailyRefreshEnabled &&
+        this.selectedCollection_!.id === this.theme.backgroundImageCollectionId;
   }
 
   private computeShowTitleNavigation_(): boolean {

chrome/browser/search/background/ntp_background_data.h

@@ -135,11 +135,14 @@ struct CustomBackground {
   // Url to learn more info about the custom background.
   GURL custom_background_attribution_action_url;
 
-  // Id of the collection being used for "daily refresh".
+  // Id of the collection being used.
   std::string collection_id;
 
   // Main color of the image.
   absl::optional<SkColor> custom_background_main_color;
+
+  // Whether daily refresh is enabled.
+  bool daily_refresh_enabled;
 };
 
 #endif  // CHROME_BROWSER_SEARCH_BACKGROUND_NTP_BACKGROUND_DATA_H_

chrome/browser/search/background/ntp_custom_background_service.cc

@@ -339,7 +339,8 @@ void NtpCustomBackgroundService::SetCustomBackgroundInfo(
 
   background_updated_timestamp_ = base::TimeTicks::Now();
 
-  if (!collection_id.empty() && is_backdrop_collection) {
+  if (!background_url.is_valid() && !collection_id.empty() &&
+      is_backdrop_collection) {
     background_service_->FetchNextCollectionImage(collection_id, absl::nullopt);
   } else if (background_url.is_valid() && is_backdrop_url) {
     if (base::FeatureList::IsEnabled(
@@ -350,7 +351,7 @@ void NtpCustomBackgroundService::SetCustomBackgroundInfo(
     }
     base::Value::Dict background_info = GetBackgroundInfoAsDict(
         background_url, attribution_line_1, attribution_line_2, action_url,
-        absl::nullopt, absl::nullopt, absl::nullopt);
+        collection_id, absl::nullopt, absl::nullopt);
     pref_service_->SetDict(prefs::kNtpCustomBackgroundDict,
                            std::move(background_info));
   } else {
@@ -417,6 +418,8 @@ NtpCustomBackgroundService::GetCustomBackground() {
     custom_background->custom_background_attribution_line_1 = std::string();
     custom_background->custom_background_attribution_line_2 = std::string();
     custom_background->custom_background_attribution_action_url = GURL();
+    custom_background->collection_id = "";
+    custom_background->daily_refresh_enabled = false;
     return custom_background;
   }
 
@@ -436,6 +439,8 @@ NtpCustomBackgroundService::GetCustomBackground() {
 
     // Set custom background information in theme info (attributions are
     // optional).
+    const base::Value* daily_refresh_timestamp =
+        background_info.Find(kNtpCustomBackgroundRefreshTimestamp);
     const base::Value* attribution_line_1 =
         background_info.Find(kNtpCustomBackgroundAttributionLine1);
     const base::Value* attribution_line_2 =
@@ -450,6 +455,8 @@ NtpCustomBackgroundService::GetCustomBackground() {
     custom_background->custom_background_url = custom_background_url;
     custom_background->is_uploaded_image = false;
     custom_background->collection_id = collection_id;
+    custom_background->daily_refresh_enabled =
+        daily_refresh_timestamp && daily_refresh_timestamp->GetInt() != 0;
     std::string custom_background_url_spec = custom_background_url.spec();
     size_t image_options_index = custom_background_url_spec.find("=");
     if (image_options_index != std::string::npos) {

chrome/browser/search/background/ntp_custom_background_service_unittest.cc

@@ -101,7 +101,8 @@ TEST_F(NtpCustomBackgroundServiceTest, SetCustomBackgroundURL) {
 
   auto custom_background = custom_background_service_->GetCustomBackground();
   EXPECT_EQ(kUrl, custom_background->custom_background_url);
-  EXPECT_EQ(false, custom_background->is_uploaded_image);
+  EXPECT_FALSE(custom_background->is_uploaded_image);
+  EXPECT_FALSE(custom_background->daily_refresh_enabled);
   EXPECT_TRUE(custom_background_service_->IsCustomBackgroundSet());
 }
 
@@ -393,6 +394,7 @@ TEST_F(NtpCustomBackgroundServiceTest, SetCustomBackgroundCollectionId) {
 
   auto custom_background = custom_background_service_->GetCustomBackground();
   EXPECT_EQ(kValidId, custom_background->collection_id);
+  EXPECT_TRUE(custom_background->daily_refresh_enabled);
   EXPECT_TRUE(custom_background_service_->IsCustomBackgroundSet());
 
   // An invalid id should clear the pref/background.
@@ -407,30 +409,6 @@ TEST_F(NtpCustomBackgroundServiceTest, SetCustomBackgroundCollectionId) {
   EXPECT_FALSE(custom_background_service_->IsCustomBackgroundSet());
 }
 
-TEST_F(NtpCustomBackgroundServiceTest,
-       CollectionIdTakePriorityOverBackgroundURL) {
-  EXPECT_CALL(observer_, OnCustomBackgroundImageUpdated).Times(1);
-  ASSERT_FALSE(custom_background_service_->IsCustomBackgroundSet());
-  const std::string kValidId("art");
-  const GURL kUrl("https://www.foo.com/");
-
-  CollectionImage image;
-  image.collection_id = kValidId;
-  image.image_url = GURL("https://www.test.com/");
-  custom_background_service_->SetNextCollectionImageForTesting(image);
-  custom_background_service_->AddValidBackdropUrlForTesting(kUrl);
-  custom_background_service_->AddValidBackdropCollectionForTesting(kValidId);
-
-  custom_background_service_->SetCustomBackgroundInfo(kUrl, GURL(), "", "",
-                                                      GURL(), kValidId);
-  task_environment_.RunUntilIdle();
-
-  auto custom_background = custom_background_service_->GetCustomBackground();
-  EXPECT_EQ(kValidId, custom_background->collection_id);
-  EXPECT_EQ("https://www.test.com/", custom_background->custom_background_url);
-  EXPECT_TRUE(custom_background_service_->IsCustomBackgroundSet());
-}
-
 TEST_F(NtpCustomBackgroundServiceTest, RefreshesBackgroundAfter24Hours) {
   EXPECT_CALL(observer_, OnCustomBackgroundImageUpdated).Times(2);
   ASSERT_FALSE(custom_background_service_->IsCustomBackgroundSet());
@@ -477,6 +455,7 @@ TEST_F(NtpCustomBackgroundServiceTest, RefreshesBackgroundAfter24Hours) {
   custom_background = custom_background_service_->GetCustomBackground();
   EXPECT_EQ(kValidId, custom_background->collection_id);
   EXPECT_EQ(kImageUrl2, custom_background->custom_background_url);
+  EXPECT_TRUE(custom_background->daily_refresh_enabled);
   EXPECT_TRUE(custom_background_service_->IsCustomBackgroundSet());
 }
 

chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom

@@ -29,6 +29,8 @@ struct CollectionImage {
   url.mojom.Url image_url;
   // URL to a preview of the image. Can point to untrusted content.
   url.mojom.Url preview_image_url;
+  // Collection id of the image;
+  string collection_id;
 };
 
 // The background image URL and styling.
@@ -55,14 +57,16 @@ struct Theme {
   skia.mojom.SkColor background_color;
   // True if the background is custom.
   bool is_custom_background;
+  // True if daily refresh is enabled.
+  bool daily_refresh_enabled;
   // True if the theme is dark (e.g. NTP background color is dark).
   bool is_dark;
   // True if the realbox icons should be themed based on the background color.
   bool theme_realbox_icons;
   // Color of Google logo. If not set show the logo multi-colored.
   skia.mojom.SkColor? logo_color;
-  // Selected collection for daily refresh.
-  string? daily_refresh_collection_id;
+  // Collection id of the background image.
+  string? background_image_collection_id;
   // The background image.
   BackgroundImage? background_image;
   // Human readable attributions of the background image.
@@ -234,7 +238,7 @@ interface PageHandler {
   // Sets the background image and notifies all NTPs of the change.
   SetBackgroundImage(string attribution_1, string attribution_2,
       url.mojom.Url attribution_url, url.mojom.Url image_url,
-      url.mojom.Url thumbnail_url);
+      url.mojom.Url thumbnail_url, string collection_id);
   // Sets collection id for daily refresh. When |collection_id| is empty, the
   // daily refresh is turned off.
   SetDailyRefreshCollectionId(string collection_id);

chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc

@@ -302,7 +302,8 @@ new_tab_page::mojom::ThemePtr MakeTheme(
         custom_background->custom_background_attribution_line_2;
     theme->background_image_attribution_url =
         custom_background->custom_background_attribution_action_url;
-    theme->daily_refresh_collection_id = custom_background->collection_id;
+    theme->background_image_collection_id = custom_background->collection_id;
+    theme->daily_refresh_enabled = custom_background->daily_refresh_enabled;
   }
 
   theme->most_visited = std::move(most_visited);
@@ -546,19 +547,18 @@ void NewTabPageHandler::SetBackgroundImage(const std::string& attribution_1,
                                            const std::string& attribution_2,
                                            const GURL& attribution_url,
                                            const GURL& image_url,
-                                           const GURL& thumbnail_url) {
-  // Populating the |collection_id| turns on refresh daily which overrides the
-  // the selected image.
+                                           const GURL& thumbnail_url,
+                                           const std::string& collection_id) {
   ntp_custom_background_service_->SetCustomBackgroundInfo(
       image_url, thumbnail_url, attribution_1, attribution_2, attribution_url,
-      /* collection_id= */ "");
+      collection_id);
   LogEvent(NTP_BACKGROUND_IMAGE_SET);
 }
 
 void NewTabPageHandler::SetDailyRefreshCollectionId(
     const std::string& collection_id) {
-  // Populating the |collection_id| turns on refresh daily which overrides the
-  // the selected image.
+  // Only populating the |collection_id| turns on refresh daily which overrides
+  // the the selected image.
   ntp_custom_background_service_->SetCustomBackgroundInfo(
       /* image_url */ GURL(), /* thumbnail_url */ GURL(),
       /* attribution_line_1= */ "", /* attribution_line_2= */ "",
@@ -1138,6 +1138,7 @@ void NewTabPageHandler::OnCollectionImagesAvailable() {
     image->attribution_url = info.attribution_action_url;
     image->image_url = info.image_url;
     image->preview_image_url = info.thumbnail_image_url;
+    image->collection_id = collection_id;
     images.push_back(std::move(image));
   }
   std::move(background_images_callback_).Run(std::move(images));

chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.h

@@ -91,7 +91,8 @@ class NewTabPageHandler : public new_tab_page::mojom::PageHandler,
                           const std::string& attribution_2,
                           const GURL& attribution_url,
                           const GURL& image_url,
-                          const GURL& thumbnail_url) override;
+                          const GURL& thumbnail_ur,
+                          const std::string& collection_id) override;
   void SetDailyRefreshCollectionId(const std::string& collection_id) override;
   void SetNoBackgroundImage() override;
   void RevertBackgroundChanges() override;

chrome/browser/ui/webui/new_tab_page/new_tab_page_handler_unittest.cc

@@ -420,7 +420,7 @@ TEST_P(NewTabPageHandlerThemeTest, SetTheme) {
   EXPECT_EQ(SkColorSetRGB(0, 0, 1), theme->background_color);
   EXPECT_FALSE(theme->is_custom_background);
   EXPECT_FALSE(theme->is_dark);
-  EXPECT_FALSE(theme->daily_refresh_collection_id.has_value());
+  EXPECT_FALSE(theme->daily_refresh_enabled);
   ASSERT_TRUE(theme->background_image);
   EXPECT_EQ("chrome-untrusted://theme/IDR_THEME_NTP_BACKGROUND?bar",
             theme->background_image->url);
@@ -447,6 +447,7 @@ TEST_P(NewTabPageHandlerThemeTest, SetTheme) {
   EXPECT_FALSE(theme->background_image_attribution_1.has_value());
   EXPECT_FALSE(theme->background_image_attribution_2.has_value());
   EXPECT_FALSE(theme->background_image_attribution_url.has_value());
+  EXPECT_FALSE(theme->background_image_collection_id.has_value());
   ASSERT_TRUE(theme->most_visited);
   EXPECT_EQ(SkColorSetRGB(0, 0, 8), theme->most_visited->background_color);
   if (RemoveScrim()) {
@@ -472,6 +473,7 @@ TEST_P(NewTabPageHandlerThemeTest, SetCustomBackground) {
   custom_background.custom_background_attribution_action_url =
       GURL("https://foo.com/action");
   custom_background.collection_id = "baz collection";
+  custom_background.daily_refresh_enabled = false;
   ON_CALL(mock_ntp_custom_background_service_, GetCustomBackground())
       .WillByDefault(testing::Return(absl::make_optional(custom_background)));
   ON_CALL(mock_theme_provider_, HasCustomImage(IDR_THEME_NTP_BACKGROUND))
@@ -497,6 +499,7 @@ TEST_P(NewTabPageHandlerThemeTest, SetCustomBackground) {
     EXPECT_FALSE(theme->background_image_attribution_1.has_value());
     EXPECT_FALSE(theme->background_image_attribution_2.has_value());
     EXPECT_FALSE(theme->background_image_attribution_url.has_value());
+    EXPECT_FALSE(theme->background_image_collection_id.has_value());
   } else {
     ASSERT_TRUE(theme);
     EXPECT_TRUE(theme->is_custom_background);
@@ -508,7 +511,8 @@ TEST_P(NewTabPageHandlerThemeTest, SetCustomBackground) {
     EXPECT_EQ("bar line", theme->background_image_attribution_2);
     EXPECT_EQ("https://foo.com/action",
               theme->background_image_attribution_url);
-    EXPECT_EQ("baz collection", theme->daily_refresh_collection_id);
+    EXPECT_FALSE(theme->daily_refresh_enabled);
+    EXPECT_EQ("baz collection", theme->background_image_collection_id);
   }
   if (RemoveScrim()) {
     EXPECT_TRUE(theme->background_image->scrim_display.has_value());
@@ -524,6 +528,36 @@ TEST_P(NewTabPageHandlerThemeTest, SetCustomBackground) {
   }
 }
 
+TEST_P(NewTabPageHandlerThemeTest, SetDailyRefresh) {
+  new_tab_page::mojom::ThemePtr theme;
+  EXPECT_CALL(mock_page_, SetTheme)
+      .Times(1)
+      .WillOnce(testing::Invoke([&theme](new_tab_page::mojom::ThemePtr arg) {
+        theme = std::move(arg);
+      }));
+  CustomBackground custom_background;
+  custom_background.daily_refresh_enabled = true;
+  custom_background.collection_id = "baz collection";
+  ON_CALL(mock_ntp_custom_background_service_, GetCustomBackground())
+      .WillByDefault(testing::Return(absl::make_optional(custom_background)));
+  ON_CALL(mock_theme_provider_, HasCustomImage(IDR_THEME_NTP_BACKGROUND))
+      .WillByDefault(testing::Return(true));
+
+  ntp_custom_background_service_observer_->OnCustomBackgroundImageUpdated();
+  mock_page_.FlushForTesting();
+
+  ASSERT_TRUE(theme);
+  if (CustomizeChromeSidePanel()) {
+    EXPECT_FALSE(theme->is_custom_background);
+    EXPECT_FALSE(theme->background_image_collection_id.has_value());
+  } else {
+    ASSERT_TRUE(theme);
+    EXPECT_TRUE(theme->is_custom_background);
+    EXPECT_TRUE(theme->daily_refresh_enabled);
+    EXPECT_EQ("baz collection", theme->background_image_collection_id);
+  }
+}
+
 INSTANTIATE_TEST_SUITE_P(All,
                          NewTabPageHandlerThemeTest,
                          ::testing::Combine(::testing::Bool(),

chrome/test/data/webui/new_tab_page/customize_backgrounds_test.ts

@@ -113,6 +113,7 @@ suite('NewTabPageCustomizeBackgroundsTest', () => {
       previewImageUrl: {url: 'https://a.com/p.png'},
       attributionUrl: {url: ''},
       attribution2: '',
+      collectionId: '',
     };
     handler.setResultFor('getBackgroundImages', Promise.resolve({
       images: [image],
@@ -144,6 +145,7 @@ suite('NewTabPageCustomizeBackgroundsTest', () => {
       previewImageUrl: {url: 'https://example.com/image.png'},
       attributionUrl: {url: ''},
       attribution2: '',
+      collectionId: '',
     };
     const customizeBackgrounds = await createCustomizeBackgrounds();
     handler.setResultFor('getBackgroundImages', Promise.resolve({
@@ -168,6 +170,7 @@ suite('NewTabPageCustomizeBackgroundsTest', () => {
       previewImageUrl: {url: 'https://example.com/image.png'},
       attributionUrl: {url: ''},
       attribution2: '',
+      collectionId: '',
     };
     const customizeBackgrounds = await createCustomizeBackgrounds();
     handler.setResultFor('getBackgroundImages', Promise.resolve({
@@ -195,6 +198,7 @@ suite('NewTabPageCustomizeBackgroundsTest', () => {
       previewImageUrl: {url: 'https://example.com/image.png'},
       attributionUrl: {url: ''},
       attribution2: '',
+      collectionId: '',
     };
     const customizeBackgrounds = await createCustomizeBackgrounds();
     handler.setResultFor('getBackgroundImages', Promise.resolve({
@@ -219,6 +223,7 @@ suite('NewTabPageCustomizeBackgroundsTest', () => {
       previewImageUrl: {url: 'https://example.com/image.png'},
       attributionUrl: {url: ''},
       attribution2: '',
+      collectionId: '',
     };
     const customizeBackgrounds = await createCustomizeBackgrounds();
     customizeBackgrounds.theme.backgroundImage =
@@ -240,6 +245,7 @@ suite('NewTabPageCustomizeBackgroundsTest', () => {
       previewImageUrl: {url: 'https://example.com/image.png'},
       attributionUrl: {url: ''},
       attribution2: '',
+      collectionId: '',
     };
     const customizeBackgrounds = await createCustomizeBackgrounds();
     handler.setResultFor('getBackgroundImages', Promise.resolve({
@@ -292,7 +298,8 @@ suite('NewTabPageCustomizeBackgroundsTest', () => {
 
     test('not selected when refresh collection set', () => {
       const theme = createTheme();
-      theme.dailyRefreshCollectionId = 'landscape';
+      theme.dailyRefreshEnabled = true;
+      theme.backgroundImageCollectionId = 'landscape';
       customizeBackgrounds.theme = theme;
       assertSetNoBackgroundImageNotCalled();
     });

chrome/test/data/webui/new_tab_page/customize_dialog_test.ts

@@ -130,7 +130,8 @@ suite('NewTabPageCustomizeDialogTest', () => {
   suite('backgrounds', () => {
     setup(() => {
       const theme = createTheme();
-      theme.dailyRefreshCollectionId = 'landscape';
+      theme.dailyRefreshEnabled = true;
+      theme.backgroundImageCollectionId = 'landscape';
       theme.backgroundImage =
           createBackgroundImage('https://example.com/image.png');
       customizeDialog.theme = theme;