Use frame color for COLOR_STATUS_BUBBLE rather than toolbar

For Autogenerated, and extension-based themes, use frame
color for COLOR_STATUS_BUBBLE rather than toolbar.

When PWAs set a theme-color, status bubble bg will
match frame.

crbug.com/1053823#c16

Bug: 1053823
Change-Id: I3a159a5c25a038ab78594c98b7208b3066a82181
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2067763
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744106}

chrome/browser/themes/browser_theme_pack.cc

@@ -69,7 +69,7 @@ constexpr int kTallestFrameHeight = kTallestTabHeight + 19;
 // change default theme assets, if you need themes to recreate their generated
 // images (which are cached), or if you changed how missing values are
 // generated.
-const int kThemePackVersion = 73;
+const int kThemePackVersion = 74;
 
 // IDs that are in the DataPack won't clash with the positive integer
 // uint16_t. kHeaderID should always have the maximum value because we want the
@@ -1054,9 +1054,9 @@ void BrowserThemePack::AddCustomThemeColorMixers(
 void BrowserThemePack::AdjustThemePack() {
   CropImages(&images_);
 
-  // Set toolbar related elements' colors (e.g. status bubble, info bar,
-  // download shelf) to toolbar color.
-  SetToolbarRelatedColors();
+  // Set frame and toolbar related elements' colors (e.g. status bubble,
+  // info bar, download shelf) to frame or toolbar color.
+  SetFrameAndToolbarRelatedColors();
 
   // Create toolbar image, and generate toolbar color from image where relevant.
   // This must be done after reading colors from JSON (so they can be used for
@@ -1470,15 +1470,16 @@ void BrowserThemePack::CropImages(ImageCache* images) const {
   }
 }
 
-void BrowserThemePack::SetToolbarRelatedColors() {
-  // Propagate the user-specified Toolbar Color to similar elements (for
-  // backwards-compatibility with themes written before this toolbar processing
-  // was introduced).
+void BrowserThemePack::SetFrameAndToolbarRelatedColors() {
+  // Propagate the user-specified Frame and Toolbar Colors to similar elements.
+  SkColor frame_color;
+  if (GetColor(TP::COLOR_FRAME, &frame_color))
+    SetColor(TP::COLOR_STATUS_BUBBLE, frame_color);
+
   SkColor toolbar_color;
   if (GetColor(TP::COLOR_TOOLBAR, &toolbar_color)) {
     SetColor(TP::COLOR_INFOBAR, toolbar_color);
     SetColor(TP::COLOR_DOWNLOAD_SHELF, toolbar_color);
-    SetColor(TP::COLOR_STATUS_BUBBLE, toolbar_color);
   }
   SkColor toolbar_button_icon_color;
   if (GetColor(TP::COLOR_TOOLBAR_BUTTON_ICON, &toolbar_button_icon_color)) {

chrome/browser/themes/browser_theme_pack.h

@@ -214,9 +214,9 @@ class BrowserThemePack : public CustomThemeSupplier {
   // can be of any size. Source and destination is |images|.
   void CropImages(ImageCache* images) const;
 
-  // Set toolbar related elements' colors (e.g. status bubble, info bar,
-  // download shelf) to toolbar color.
-  void SetToolbarRelatedColors();
+  // Set frame and toolbar related elements' colors (e.g. status bubble,
+  // info bar, download shelf) to frame or toolbar colors.
+  void SetFrameAndToolbarRelatedColors();
 
   // Creates a composited toolbar image. Source and destination is |images|.
   // Also sets toolbar color corresponding to this image.

chrome/browser/themes/browser_theme_pack_unittest.cc

@@ -939,47 +939,44 @@ TEST_F(BrowserThemePackTest, TestWindowControlButtonBGColor_ButtonBGImage) {
   }
 }
 
-// Ensure that a specified 'toolbar' color is propagated to other 'bar' and
-// 'shelf' colors (before a new color is computed from the toolbar image).
-TEST_F(BrowserThemePackTest, TestToolbarColorPropagation) {
+// Ensure that specified 'frame' and 'toolbar' colors are propagated to other
+// 'bar' and 'shelf' colors.
+TEST_F(BrowserThemePackTest, TestFrameAndToolbarColorPropagation) {
   scoped_refptr<BrowserThemePack> pack(
       new BrowserThemePack(CustomThemeSupplier::ThemeType::EXTENSION));
-  BuildTestExtensionTheme("theme_test_toolbar_frame_images_and_colors",
-                          pack.get());
+  BuildTestExtensionTheme("theme_test_toolbar_color_no_image", pack.get());
 
-  SkColor infobar_color;
-  SkColor download_shelf_color;
+  // Frame colors.
   SkColor status_bubble_color;
 
-  EXPECT_TRUE(pack->GetColor(TP::COLOR_INFOBAR, &infobar_color));
-  EXPECT_TRUE(pack->GetColor(TP::COLOR_DOWNLOAD_SHELF, &download_shelf_color));
   EXPECT_TRUE(pack->GetColor(TP::COLOR_STATUS_BUBBLE, &status_bubble_color));
 
-  constexpr SkColor kExpectedColor = SkColorSetRGB(0, 255, 0);
-  EXPECT_EQ(infobar_color, kExpectedColor);
-  EXPECT_EQ(infobar_color, download_shelf_color);
-  EXPECT_EQ(infobar_color, status_bubble_color);
-}
-
-// Ensure that a specified 'toolbar' color is propagated to other 'bar' and
-// 'shelf' colors (before a new color is computed from the toolbar image).
-TEST_F(BrowserThemePackTest, TestToolbarColorPropagationNoImage) {
-  scoped_refptr<BrowserThemePack> pack(
-      new BrowserThemePack(CustomThemeSupplier::ThemeType::EXTENSION));
-  BuildTestExtensionTheme("theme_test_toolbar_color_no_image", pack.get());
+  constexpr SkColor kExpectedFrameColor = SkColorSetRGB(255, 0, 255);
+  EXPECT_EQ(status_bubble_color, kExpectedFrameColor);
 
+  // Toolbar colors.
   SkColor infobar_color;
   SkColor download_shelf_color;
-  SkColor status_bubble_color;
 
   EXPECT_TRUE(pack->GetColor(TP::COLOR_INFOBAR, &infobar_color));
   EXPECT_TRUE(pack->GetColor(TP::COLOR_DOWNLOAD_SHELF, &download_shelf_color));
-  EXPECT_TRUE(pack->GetColor(TP::COLOR_STATUS_BUBBLE, &status_bubble_color));
 
-  constexpr SkColor kExpectedColor = SkColorSetRGB(0, 255, 0);
-  EXPECT_EQ(infobar_color, kExpectedColor);
-  EXPECT_EQ(infobar_color, download_shelf_color);
-  EXPECT_EQ(infobar_color, status_bubble_color);
+  constexpr SkColor kExpectedToolbarColor = SkColorSetRGB(0, 255, 0);
+  EXPECT_EQ(infobar_color, kExpectedToolbarColor);
+  EXPECT_EQ(download_shelf_color, kExpectedToolbarColor);
+
+  // Toolbar button icon colors.
+  SkColor toolbar_button_icon_hovered_color;
+  SkColor toolbar_button_icon_pressed_color;
+
+  EXPECT_TRUE(pack->GetColor(TP::COLOR_TOOLBAR_BUTTON_ICON_HOVERED,
+                             &toolbar_button_icon_hovered_color));
+  EXPECT_TRUE(pack->GetColor(TP::COLOR_TOOLBAR_BUTTON_ICON_PRESSED,
+                             &toolbar_button_icon_pressed_color));
+
+  constexpr SkColor kExpectedToolbarButtonIconColor = SkColorSetRGB(0, 0, 255);
+  EXPECT_EQ(toolbar_button_icon_hovered_color, kExpectedToolbarButtonIconColor);
+  EXPECT_EQ(toolbar_button_icon_pressed_color, kExpectedToolbarButtonIconColor);
 }
 
 // Ensure that, given an explicit toolbar color and a toolbar image, the output

chrome/browser/themes/theme_properties.cc

@@ -55,6 +55,7 @@ SkColor GetLightModeColor(int id) {
     case ThemeProperties::COLOR_FRAME:
     case ThemeProperties::COLOR_BACKGROUND_TAB:
     case ThemeProperties::COLOR_WINDOW_CONTROL_BUTTON_BACKGROUND_ACTIVE:
+    case ThemeProperties::COLOR_STATUS_BUBBLE:
       return SkColorSetRGB(0xDE, 0xE1, 0xE6);
     case ThemeProperties::COLOR_FRAME_INACTIVE:
     case ThemeProperties::COLOR_BACKGROUND_TAB_INACTIVE:
@@ -66,7 +67,6 @@ SkColor GetLightModeColor(int id) {
     case ThemeProperties::COLOR_DOWNLOAD_SHELF:
     case ThemeProperties::COLOR_INFOBAR:
     case ThemeProperties::COLOR_TOOLBAR:
-    case ThemeProperties::COLOR_STATUS_BUBBLE:
       return SK_ColorWHITE;
     case ThemeProperties::COLOR_HOVER_CARD_NO_PREVIEW_FOREGROUND:
       return gfx::kGoogleGrey300;

chrome/browser/ui/views/status_bubble_views.cc

@@ -407,10 +407,9 @@ void StatusBubbleViews::StatusView::SetTextLabelColors(views::Label* text) {
   SkColor bubble_color =
       theme_provider->GetColor(ThemeProperties::COLOR_STATUS_BUBBLE);
   text->SetBackgroundColor(bubble_color);
-  // Text color is the foreground tab text color at 60% alpha.
-  text->SetEnabledColor(color_utils::AlphaBlend(
-      theme_provider->GetColor(ThemeProperties::COLOR_TAB_TEXT), bubble_color,
-      0.6f));
+  // Text color is the background tab text color, adjusted if required.
+  text->SetEnabledColor(
+      theme_provider->GetColor(ThemeProperties::COLOR_BACKGROUND_TAB_TEXT));
 }
 
 const char* StatusBubbleViews::StatusView::GetClassName() const {

chrome/test/data/extensions/theme_test_toolbar_color_no_image/manifest.json

@@ -5,7 +5,8 @@
   "theme": {
     "colors": {
       "frame": [ 255, 0, 255 ],
-      "toolbar": [ 0, 255, 0 ]
+      "toolbar": [ 0, 255, 0 ],
+      "toolbar_button_icon": [ 0, 0, 255 ]
       }
   },
   "version": "0.0.1"