[cc-side-panel] Replace default color with main color if it exists

Adds a separate <customize-chrome-color> for the extracted / main color of the background image, which is shown in place of the default color if a main color for the background image exists. The main color circles is selected whenever the currently set theme matches the extracted color. To that end, this CL refactors the selection logic somewhat to better guarantee at most one selected color circle. + Makes the main color optional in the background image data to robustly identify the case where there is no main color. + Sets main color to null if extracted color feature is disabled so that we can disable the feature later on if need be. https://screenshot.googleplex.com/57TvkNaruwuuoTF Fixed: 1401018 Change-Id: I9cadd272146081c044e484bb0f024e08d1f23c62 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4112661 Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org> Reviewed-by: Alex Gough <ajgo@chromium.org> Cr-Commit-Position: refs/heads/main@{#1084962}
chromium · Dec 19, 2022 · 44145ef · 44145ef
1 parent d2e82e2
commit 44145ef
Show file tree

Hide file tree

Showing 13 changed files with 207 additions and 47 deletions.
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
@@ -6773,6 +6773,9 @@ Keep your key file in a safe place. You will need it to create new versions of y
       <message name="IDS_NTP_CUSTOMIZE_DEFAULT_LABEL" desc="The label for the Default theme in the customization menu on the New Tab Page">
         Default color
       </message>
+      <message name="IDS_NTP_CUSTOMIZE_MAIN_COLOR_LABEL" desc="The label for the main color theme in the customization menu on the New Tab Page">
+        Color based on theme
+      </message>
       <message name="IDS_NTP_COLORS_WARM_GREY" desc="A color option in the customization menu on the New Tab Page.">
         Warm grey
       </message>

diff --git a/chrome/app/generated_resources_grd/IDS_NTP_CUSTOMIZE_MAIN_COLOR_LABEL.png.sha1 b/chrome/app/generated_resources_grd/IDS_NTP_CUSTOMIZE_MAIN_COLOR_LABEL.png.sha1
@@ -0,0 +1 @@
+2047f3cd7a6cc0b347ba3ac3bd59dfdd4f586780
diff --git a/chrome/browser/resources/side_panel/customize_chrome/colors.html b/chrome/browser/resources/side_panel/customize_chrome/colors.html
@@ -40,20 +40,35 @@
 <!-- TODO(crbug.com/1395210): Make grid adaptive. -->
 <cr-grid columns="4" role="radiogroup"
     aria-label="$i18n{colorsContainerLabel}">
-  <!-- TODO(crbug.com/1401018): Add extracted color. -->
-  <customize-chrome-color
-      id="defaultColor"
-      background-color="[[defaultColor_.background]]"
-      foreground-color="[[defaultColor_.foreground]]"
-      background-color-hidden="[[themeHasBackgroundImage_(theme_)]]"
-      title="$i18n{defaultColorName}"
-      aria-label="$i18n{defaultColorName}"
-      role="radio"
-      checked="[[isDefaultColorSelected_]]"
-      aria-checked$="[[isDefaultColorSelected_]]"
-      tabindex$="[[tabIndex_(isDefaultColorSelected_)]]"
-      on-click="onDefaultColorClick_">
-  </customize-chrome-color>
+  <template is="dom-if" if="[[!themeHasMainColor_(theme_)]]" restamp>
+    <customize-chrome-color
+        id="defaultColor"
+        background-color="[[defaultColor_.background]]"
+        foreground-color="[[defaultColor_.foreground]]"
+        background-color-hidden="[[themeHasBackgroundImage_(theme_)]]"
+        title="$i18n{defaultColorName}"
+        aria-label="$i18n{defaultColorName}"
+        role="radio"
+        checked="[[isDefaultColorSelected_]]"
+        aria-checked$="[[isDefaultColorSelected_]]"
+        tabindex$="[[tabIndex_(isDefaultColorSelected_)]]"
+        on-click="onDefaultColorClick_">
+    </customize-chrome-color>
+  </template>
+  <template is="dom-if" if="[[themeHasMainColor_(theme_)]]" restamp>
+    <customize-chrome-color
+        id="mainColor"
+        foreground-color="[[mainColor_]]"
+        background-color-hidden
+        title="$i18n{mainColorName}"
+        aria-label="$i18n{mainColorName}"
+        role="radio"
+        checked="[[isMainColorSelected_]]"
+        aria-checked$="[[isMainColorSelected_]]"
+        tabindex$="[[tabIndex_(isMainColorSelected_)]]"
+        on-click="onMainColorClick_">
+    </customize-chrome-color>
+  </template>
   <template id="chromeColors" is="dom-repeat" items="[[colors_]]">
     <customize-chrome-color
         class="chrome-color"
@@ -63,10 +78,10 @@
         title="[[item.name]]"
         aria-label$="[[item.name]]"
         role="radio"
-        checked="[[isChromeColorSelected_(item.foreground, theme_, colors_)]]"
+        checked="[[isChromeColorSelected_(item.foreground, selectedColor_)]]"
         aria-checked$=
-            "[[isChromeColorSelected_(item.foreground, theme_, colors_)]]"
-        tabindex$="[[chromeColorTabIndex_(item.foreground, theme_, colors_)]]"
+            "[[isChromeColorSelected_(item.foreground, selectedColor_)]]"
+        tabindex$="[[chromeColorTabIndex_(item.foreground, selectedColor_)]]"
         on-click="onChromeColorClick_">
     </customize-chrome-color>
   </template>

diff --git a/chrome/browser/resources/side_panel/customize_chrome/colors.ts b/chrome/browser/resources/side_panel/customize_chrome/colors.ts
@@ -29,15 +29,21 @@ export const DARK_DEFAULT_COLOR: Color = {
   foreground: {value: 0xff202124},
 };
 
-function isChromeColorSelected(
-    theme: Theme|undefined, colors: ChromeColor[]|undefined, color: SkColor) {
-  return !!theme && !!colors && !!theme.foregroundColor &&
-      theme.foregroundColor.value === color.value;
+enum ColorType {
+  NONE,
+  DEFAULT,
+  MAIN,
+  CHROME,
+  CUSTOM,
+}
+
+interface SelectedColor {
+  type: ColorType;
+  chromeColor?: SkColor;
 }
 
 export interface ColorsElement {
   $: {
-    defaultColor: ColorElement,
     chromeColors: DomRepeat,
     customColorContainer: HTMLElement,
     customColor: ColorElement,
@@ -61,15 +67,27 @@ export class ColorsElement extends PolymerElement {
         type: Object,
         computed: 'computeDefaultColor_(theme_)',
       },
+      mainColor_: {
+        type: Object,
+        computed: 'computeMainColor_(theme_)',
+      },
       colors_: Array,
       theme_: Object,
+      selectedColor_: {
+        type: Object,
+        computed: 'computeSelectedColor_(theme_, colors_)',
+      },
       isDefaultColorSelected_: {
         type: Object,
-        computed: 'computeIsDefaultColorSelected_(theme_)',
+        computed: 'computeIsDefaultColorSelected_(selectedColor_)',
+      },
+      isMainColorSelected_: {
+        type: Object,
+        computed: 'computeIsMainColorSelected_(selectedColor_)',
       },
       isCustomColorSelected_: {
         type: Object,
-        computed: 'computeIsCustomColorSelected_(theme_, color_)',
+        computed: 'computeIsCustomColorSelected_(selectedColor_)',
       },
       customColor_: {
         type: Object,
@@ -89,6 +107,7 @@ export class ColorsElement extends PolymerElement {
 
   private colors_: ChromeColor[];
   private theme_: Theme;
+  private selectedColor_: SelectedColor;
   private isCustomColorSelected_: boolean;
   private customColor_: Color;
   private setThemeListenerId_: number|null = null;
@@ -122,37 +141,80 @@ export class ColorsElement extends PolymerElement {
                                         LIGHT_DEFAULT_COLOR;
   }
 
+  private computeMainColor_(): SkColor|undefined {
+    return this.theme_ && this.theme_.backgroundImage &&
+        this.theme_.backgroundImage.mainColor;
+  }
+
+  private computeSelectedColor_(): SelectedColor {
+    if (!this.colors_ || !this.theme_) {
+      return {type: ColorType.NONE};
+    }
+    if (!this.theme_.foregroundColor) {
+      return {type: ColorType.DEFAULT};
+    }
+    if (this.theme_.backgroundImage && this.theme_.backgroundImage.mainColor &&
+        this.theme_.backgroundImage.mainColor!.value ===
+            this.theme_.foregroundColor.value) {
+      return {type: ColorType.MAIN};
+    }
+    if (this.colors_.find(
+            (color: ChromeColor) => color.foreground.value ===
+                this.theme_.foregroundColor!.value)) {
+      return {
+        type: ColorType.CHROME,
+        chromeColor: this.theme_.foregroundColor!,
+      };
+    }
+    return {type: ColorType.CUSTOM};
+  }
+
   private computeIsDefaultColorSelected_(): boolean {
-    return this.theme_ && !this.theme_.foregroundColor;
+    return this.selectedColor_.type === ColorType.DEFAULT;
+  }
+
+  private computeIsMainColorSelected_(): boolean {
+    return this.selectedColor_.type === ColorType.MAIN;
   }
 
   private computeIsCustomColorSelected_(): boolean {
-    return !!this.colors_ && !!this.theme_ && !!this.theme_.foregroundColor &&
-        !this.colors_.find(
-            (color: ChromeColor) =>
-                color.foreground.value === this.theme_.foregroundColor!.value);
+    return this.selectedColor_.type === ColorType.CUSTOM;
   }
 
   private isChromeColorSelected_(color: SkColor): boolean {
-    return isChromeColorSelected(this.theme_, this.colors_, color);
+    return this.selectedColor_.type === ColorType.CHROME &&
+        this.selectedColor_.chromeColor!.value === color.value;
   }
 
   private tabIndex_(selected: boolean): string {
     return selected ? '0' : '-1';
   }
 
   private chromeColorTabIndex_(color: SkColor): string {
-    return isChromeColorSelected(this.theme_, this.colors_, color) ? '0' : '-1';
+    return this.selectedColor_.type === ColorType.CHROME &&
+            this.selectedColor_.chromeColor!.value === color.value ?
+        '0' :
+        '-1';
   }
 
   private themeHasBackgroundImage_(): boolean {
     return !!this.theme_ && !!this.theme_.backgroundImage;
   }
 
+  private themeHasMainColor_(): boolean {
+    return !!this.theme_ && !!this.theme_.backgroundImage &&
+        !!this.theme_.backgroundImage.mainColor;
+  }
+
   private onDefaultColorClick_() {
     CustomizeChromeApiProxy.getInstance().handler.setDefaultColor();
   }
 
+  private onMainColorClick_() {
+    CustomizeChromeApiProxy.getInstance().handler.setForegroundColor(
+        this.theme_!.backgroundImage!.mainColor!);
+  }
+
   private onChromeColorClick_(e: Event) {
     CustomizeChromeApiProxy.getInstance().handler.setForegroundColor(
         this.$.chromeColors.itemForElement(e.target as HTMLElement).foreground);

diff --git a/chrome/browser/search/background/ntp_background_data.h b/chrome/browser/search/background/ntp_background_data.h
@@ -8,6 +8,7 @@
 #include <string>
 
 #include "chrome/browser/search/background/ntp_background.pb.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "url/gurl.h"
 
@@ -135,7 +136,7 @@ struct CustomBackground {
   std::string collection_id;
 
   // Main color of the image.
-  SkColor custom_background_main_color;
+  absl::optional<SkColor> custom_background_main_color;
 };
 
 #endif  // CHROME_BROWSER_SEARCH_BACKGROUND_NTP_BACKGROUND_DATA_H_
diff --git a/chrome/browser/search/background/ntp_custom_background_service.cc b/chrome/browser/search/background/ntp_custom_background_service.cc
@@ -430,7 +430,10 @@ NtpCustomBackgroundService::GetCustomBackground() {
     const base::Value* attribution_action_url =
         background_info.Find(kNtpCustomBackgroundAttributionActionURL);
     const base::Value* color =
-        background_info.Find(kNtpCustomBackgroundMainColor);
+        base::FeatureList::IsEnabled(
+            ntp_features::kCustomizeChromeColorExtraction)
+            ? background_info.Find(kNtpCustomBackgroundMainColor)
+            : nullptr;
     custom_background->custom_background_url = custom_background_url;
     custom_background->is_uploaded_image = false;
     custom_background->collection_id = collection_id;

diff --git a/chrome/browser/search/background/ntp_custom_background_service_unittest.cc b/chrome/browser/search/background/ntp_custom_background_service_unittest.cc
@@ -25,6 +25,7 @@
 #include "content/public/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/image/image.h"
 #include "url/gurl.h"
@@ -577,8 +578,9 @@ TEST_F(NtpCustomBackgroundServiceTest, TestUpdateCustomBackgroundColor) {
   task_environment_.RunUntilIdle();
   auto custom_background = custom_background_service_->GetCustomBackground();
   auto custom_background_main_color =
-      custom_background ? custom_background->custom_background_main_color : 0;
-  EXPECT_NE(SK_ColorRED, custom_background_main_color);
+      custom_background ? custom_background->custom_background_main_color
+                        : SK_ColorWHITE;
+  EXPECT_NE(SK_ColorRED, custom_background_main_color.value_or(SK_ColorWHITE));
 
   const GURL kUrl("https://www.foo.com");
   const GURL kThumbnailUrl("https://www.thumbnail.com");
@@ -597,12 +599,16 @@ TEST_F(NtpCustomBackgroundServiceTest, TestUpdateCustomBackgroundColor) {
       GURL("different_url"), image, image_fetcher::RequestMetadata());
   task_environment_.RunUntilIdle();
   custom_background = custom_background_service_->GetCustomBackground();
-  EXPECT_NE(SK_ColorRED, custom_background->custom_background_main_color);
+  EXPECT_NE(
+      SK_ColorRED,
+      custom_background->custom_background_main_color.value_or(SK_ColorWHITE));
 
   // Background color should update.
   custom_background_service_->UpdateCustomBackgroundColorAsync(
       kUrl, image, image_fetcher::RequestMetadata());
   task_environment_.RunUntilIdle();
   custom_background = custom_background_service_->GetCustomBackground();
-  EXPECT_EQ(SK_ColorRED, custom_background->custom_background_main_color);
+  EXPECT_EQ(
+      SK_ColorRED,
+      custom_background->custom_background_main_color.value_or(SK_ColorWHITE));
 }
diff --git a/chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome.mojom b/chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome.mojom
@@ -15,6 +15,8 @@ struct BackgroundImage {
   bool is_uploaded_image;
   // Title of the background image.
   string title;
+  // The main color of the background image.
+  skia.mojom.SkColor? main_color;
 };
 
 // A generic theme.

diff --git a/chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome_page_handler.cc b/chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome_page_handler.cc
@@ -151,6 +151,10 @@ void CustomizeChromePageHandler::UpdateTheme() {
     background_image->is_uploaded_image = custom_background->is_uploaded_image;
     background_image->title =
         custom_background->custom_background_attribution_line_1;
+    if (custom_background->custom_background_main_color.has_value()) {
+      background_image->main_color =
+          *custom_background->custom_background_main_color;
+    }
   } else {
     background_image = nullptr;
   }

diff --git a/...me/browser/ui/webui/side_panel/customize_chrome/customize_chrome_page_handler_unittest.cc b/...me/browser/ui/webui/side_panel/customize_chrome/customize_chrome_page_handler_unittest.cc
@@ -370,6 +370,7 @@ TEST_P(CustomizeChromePageHandlerSetThemeTest, SetTheme) {
   custom_background.custom_background_url = GURL("https://foo.com/img.png");
   custom_background.custom_background_attribution_line_1 = "foo line";
   custom_background.is_uploaded_image = false;
+  custom_background.custom_background_main_color = SK_ColorGREEN;
   ON_CALL(mock_ntp_custom_background_service_, GetCustomBackground())
       .WillByDefault(testing::Return(absl::make_optional(custom_background)));
   ON_CALL(mock_theme_service(), UsingDefaultTheme())
@@ -384,7 +385,9 @@ TEST_P(CustomizeChromePageHandlerSetThemeTest, SetTheme) {
   ASSERT_TRUE(theme);
   ASSERT_TRUE(theme->background_image);
   EXPECT_EQ("https://foo.com/img.png", theme->background_image->url);
-  ASSERT_FALSE(theme->background_image->is_uploaded_image);
+  EXPECT_FALSE(theme->background_image->is_uploaded_image);
+  EXPECT_EQ(SK_ColorGREEN,
+            theme->background_image->main_color.value_or(SK_ColorWHITE));
   EXPECT_EQ("foo line", theme->background_image->title);
   EXPECT_TRUE(theme->system_dark_mode);
   EXPECT_EQ(

diff --git a/chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome_ui.cc b/chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome_ui.cc
@@ -36,6 +36,7 @@ CustomizeChromeUI::CustomizeChromeUI(content::WebUI* web_ui)
       {"customizeThisPage", IDS_NTP_CUSTOM_BG_CUSTOMIZE_NTP_LABEL},
       {"appearanceHeader", IDS_NTP_CUSTOMIZE_APPEARANCE_LABEL},
       {"defaultColorName", IDS_NTP_CUSTOMIZE_DEFAULT_LABEL},
+      {"mainColorName", IDS_NTP_CUSTOMIZE_MAIN_COLOR_LABEL},
       {"mostVisited", IDS_NTP_CUSTOMIZE_MOST_VISITED_LABEL},
       {"myShortcuts", IDS_NTP_CUSTOMIZE_MY_SHORTCUTS_LABEL},
       {"shortcutsCurated", IDS_NTP_CUSTOMIZE_MY_SHORTCUTS_DESC},