Added toolbar color processing to BrowserThemePack

To fix a class of theme-related contrast bugs, BrowserThemePack now does processing for the toolbar to generate COLOR_TOOLBAR by compositing the provided image with the color it will be drawn on top of, and then samples that image into COLOR_TOOLBAR - this solves some issues with contrast checking not working properly if a toolbar image is specified alongside an explicit toolbar color that doesn't represent the color of the image.  Additionally, COLOR_FRAME (and its variants) now similarly get replaced with the values computed from their composited images regardless of whether an explicit color was provided (previously, the colors only got set as such if there was no explicit color provided).

To preserve some existing behavior, any explicit color provided for COLOR_TOOLBAR is copied into COLOR_INFOBAR (new), and COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND (existing), and several UI elements have been updated to draw using those colors instead of COLOR_TOOLBAR.

Bug: 913227
Change-Id: Idc4166f3f4e4ee75f178fddc40458b03883e85cc
Reviewed-on: https://chromium-review.googlesource.com/c/1387154
Commit-Queue: Ryan Meier <rameier@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619961}

chrome/browser/themes/browser_theme_pack.cc

@@ -58,7 +58,7 @@ constexpr int kTallestFrameHeight = kTallestTabHeight + 19;
 // theme packs that aren't int-equal to this. Increment this number if you
 // change default theme assets or if you need themes to recreate their generated
 // images (which are cached).
-const int kThemePackVersion = 61;
+const int kThemePackVersion = 62;
 
 // 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
@@ -264,6 +264,10 @@ constexpr int kNonOverwritableColorTable[] = {
     TP::COLOR_WINDOW_CONTROL_BUTTON_BACKGROUND_INACTIVE,
     TP::COLOR_WINDOW_CONTROL_BUTTON_BACKGROUND_INCOGNITO_ACTIVE,
     TP::COLOR_WINDOW_CONTROL_BUTTON_BACKGROUND_INCOGNITO_INACTIVE,
+    TP::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND,
+    TP::COLOR_INFOBAR,
+    TP::COLOR_DOWNLOAD_SHELF,
+    TP::COLOR_STATUS_BUBBLE,
 };
 constexpr size_t kNonOverwritableColorTableLength =
     base::size(kNonOverwritableColorTable);
@@ -587,17 +591,20 @@ void BrowserThemePack::SetColor(int id, SkColor color) {
   colors_[first_available_color].color = color;
 }
 
-void BrowserThemePack::ComputeColorFromImage(int color_id,
-                                             int height,
-                                             const gfx::Image& image) {
+void BrowserThemePack::SetColorIfUnspecified(int id, SkColor color) {
   SkColor temp_color;
-  if (!GetColor(color_id, &temp_color)) {
-    // Include all colors in the analysis.
-    constexpr color_utils::HSL kNoBounds = {-1, -1, -1};
-    const SkColor color = color_utils::CalculateKMeanColorOfBitmap(
-        *image.ToSkBitmap(), height, kNoBounds, kNoBounds, false);
-    SetColor(color_id, color);
-  }
+  if (!GetColor(id, &temp_color))
+    SetColor(id, color);
+}
+
+SkColor BrowserThemePack::ComputeImageColor(const gfx::Image& image,
+                                            int height) {
+  // Include all colors in the analysis.
+  constexpr color_utils::HSL kNoBounds = {-1, -1, -1};
+  const SkColor color = color_utils::CalculateKMeanColorOfBitmap(
+      *image.ToSkBitmap(), height, kNoBounds, kNoBounds, false);
+
+  return color;
 }
 
 // static
@@ -627,9 +634,14 @@ void BrowserThemePack::BuildFromExtension(
 
   pack->CropImages(&pack->images_);
 
+  // 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
+  // compositing the image).
+  pack->CreateToolbarImageAndColors(&pack->images_);
+
   // Create frame images, and generate frame colors from images where relevant.
-  // This must be done after reading colors from JSON (so they aren't
-  // overwritten).
+  // This must be done after reading colors from JSON (so they can be used for
+  // compositing the image).
   pack->CreateFrameImagesAndColors(&pack->images_);
 
   // Generate any missing frame colors.  This must be done after generating
@@ -1260,6 +1272,45 @@ void BrowserThemePack::CropImages(ImageCache* images) const {
   }
 }
 
+void BrowserThemePack::CreateToolbarImageAndColors(ImageCache* images) {
+  ImageCache temp_output;
+
+  constexpr int kSrcImageId = PRS_THEME_TOOLBAR;
+
+  const auto image_it = images->find(kSrcImageId);
+  if (image_it == images->end())
+    return;
+
+  auto image = image_it->second.AsImageSkia();
+
+  constexpr int kToolbarColorId = TP::COLOR_TOOLBAR;
+  SkColor toolbar_color;
+  // Propagate the user-specified Toolbar Color to similar elements (for
+  // backwards-compatibility with themes written before this toolbar processing
+  // was introduced).
+  if (GetColor(kToolbarColorId, &toolbar_color)) {
+    SetColor(TP::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND, toolbar_color);
+    SetColor(TP::COLOR_INFOBAR, toolbar_color);
+    SetColor(TP::COLOR_DOWNLOAD_SHELF, toolbar_color);
+    SetColor(TP::COLOR_STATUS_BUBBLE, toolbar_color);
+  }
+
+  // Generate a composite image by drawing the toolbar image on top of the
+  // specified toolbar color (if any).
+  color_utils::HSL hsl_shift{-1, -1, -1};
+  gfx::ImageSkia overlay;
+  auto source = std::make_unique<TabBackgroundImageSource>(
+      toolbar_color, image, overlay, hsl_shift, 0);
+  gfx::Size dest_size = image.size();
+
+  const gfx::Image dest_image(gfx::ImageSkia(std::move(source), dest_size));
+  temp_output[kSrcImageId] = dest_image;
+
+  SetColor(kToolbarColorId, ComputeImageColor(dest_image, dest_size.height()));
+
+  MergeImageCaches(temp_output, images);
+}
+
 void BrowserThemePack::CreateFrameImagesAndColors(ImageCache* images) {
   static constexpr struct FrameValues {
     int prs_id;
@@ -1313,8 +1364,8 @@ void BrowserThemePack::CreateFrameImagesAndColors(ImageCache* images) {
       temp_output[frame_values.prs_id] = dest_image;
 
       if (frame_values.color_id) {
-        ComputeColorFromImage(frame_values.color_id.value(),
-                              kTallestFrameHeight, dest_image);
+        SetColor(frame_values.color_id.value(),
+                 ComputeImageColor(dest_image, kTallestFrameHeight));
       }
     }
   }
@@ -1414,7 +1465,8 @@ void BrowserThemePack::GenerateWindowControlButtonColor(ImageCache* images) {
         base_color, bg_image, dest_size);
     const gfx::Image dest_image(gfx::ImageSkia(std::move(source), dest_size));
 
-    ComputeColorFromImage(bg_pair.color_id, dest_size.height(), dest_image);
+    SetColorIfUnspecified(bg_pair.color_id,
+                          ComputeImageColor(dest_image, dest_size.height()));
   }
 }
 
@@ -1489,8 +1541,8 @@ void BrowserThemePack::CreateTabBackgroundImagesAndColors(ImageCache* images) {
       const gfx::Image dest_image(gfx::ImageSkia(std::move(source), dest_size));
       temp_output[tab_id] = dest_image;
 
-      ComputeColorFromImage(kTabBackgroundMap[i].color_id, kTallestTabHeight,
-                            dest_image);
+      SetColorIfUnspecified(kTabBackgroundMap[i].color_id,
+                            ComputeImageColor(dest_image, kTallestTabHeight));
     }
   }
   MergeImageCaches(temp_output, images);

chrome/browser/themes/browser_theme_pack.h

@@ -115,9 +115,14 @@ class BrowserThemePack : public CustomThemeSupplier {
   // valid to call after BuildColorsFromJSON(), which creates |colors_|.
   void SetColor(int id, SkColor color);
 
-  // If |colors_| does not already contain an entry with identifier |id|, sets
-  // it to the dominant color of the top |height| rows of |image|.
-  void ComputeColorFromImage(int id, int height, const gfx::Image& image);
+  // If |colors_| does not already contain an entry with identifier |id|,
+  // modifies |colors_| to set the entry with identifier |id| to |color|.  If an
+  // entry for |id| already exists, does nothing.
+  // Only valid to call after BuildColorsFromJSON(), which creates |colors_|.
+  void SetColorIfUnspecified(int id, SkColor color);
+
+  // Calculates the dominant color of the top |height| rows of |image|.
+  SkColor ComputeImageColor(const gfx::Image& image, int height);
 
   // Builds a header ready to write to disk.
   void BuildHeader(const extensions::Extension* extension);
@@ -162,6 +167,10 @@ class BrowserThemePack : public CustomThemeSupplier {
   // can be of any size. Source and destination is |images|.
   void CropImages(ImageCache* images) const;
 
+  // Creates a composited toolbar image. Source and destination is |images|.
+  // Also sets toolbar color corresponding to this image.
+  void CreateToolbarImageAndColors(ImageCache* images);
+
   // Creates tinted and composited frame images. Source and destination is
   // |images|.  Also sets frame colors corresponding to these images if no
   // explicit color has been specified for these colors.

chrome/browser/themes/browser_theme_pack_unittest.cc

@@ -989,3 +989,57 @@ TEST_F(BrowserThemePackTest, TestWindowControlButtonBGColor_ButtonBGImage) {
     EXPECT_TRUE(color_utils::IsDark(control_button_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, TestToolbarColorPropagation) {
+  scoped_refptr<BrowserThemePack> pack;
+  BuildTestExtensionTheme("theme_test_toolbar_frame_images_and_colors", &pack);
+
+  SkColor infobar_color;
+  SkColor bookmark_bar_color;
+  SkColor download_shelf_color;
+  SkColor status_bubble_color;
+
+  EXPECT_TRUE(pack->GetColor(TP::COLOR_INFOBAR, &infobar_color));
+  EXPECT_TRUE(pack->GetColor(TP::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND,
+                             &bookmark_bar_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, bookmark_bar_color);
+  EXPECT_EQ(infobar_color, download_shelf_color);
+  EXPECT_EQ(infobar_color, status_bubble_color);
+}
+
+// Ensure that, given an explicit toolbar color and a toolbar image, the output
+// color in COLOR_TOOLBAR reflects the color of the image (not the explicit
+// color).
+TEST_F(BrowserThemePackTest,
+       TestToolbarColorComputedFromImageOverridesInputColor) {
+  scoped_refptr<BrowserThemePack> pack;
+  BuildTestExtensionTheme("theme_test_toolbar_frame_images_and_colors", &pack);
+
+  SkColor toolbar_color;
+  EXPECT_TRUE(pack->GetColor(TP::COLOR_TOOLBAR, &toolbar_color));
+
+  constexpr SkColor kExplicitColor = SkColorSetRGB(0, 255, 0);
+  EXPECT_NE(toolbar_color, kExplicitColor);
+}
+
+// Ensure that, given an explicit frame color and a frame image, the output
+// color in COLOR_FRAME reflects the color of the image (not the explicit
+// color).
+TEST_F(BrowserThemePackTest,
+       TestFrameColorComputedFromImageOverridesInputColor) {
+  scoped_refptr<BrowserThemePack> pack;
+  BuildTestExtensionTheme("theme_test_toolbar_frame_images_and_colors", &pack);
+
+  SkColor frame_color;
+  EXPECT_TRUE(pack->GetColor(TP::COLOR_FRAME, &frame_color));
+
+  constexpr SkColor kExplicitColor = SkColorSetRGB(255, 0, 255);
+  EXPECT_NE(frame_color, kExplicitColor);
+}

chrome/browser/themes/theme_properties.cc

@@ -42,6 +42,9 @@ base::Optional<SkColor> GetIncognitoColor(int id) {
     case ThemeProperties::COLOR_FRAME_INACTIVE:
     case ThemeProperties::COLOR_BACKGROUND_TAB_INACTIVE:
       return gfx::kGoogleGrey800;
+    case ThemeProperties::COLOR_DOWNLOAD_SHELF:
+    case ThemeProperties::COLOR_STATUS_BUBBLE:
+    case ThemeProperties::COLOR_INFOBAR:
     case ThemeProperties::COLOR_TOOLBAR:
     case ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND:
     case ThemeProperties::COLOR_NTP_BACKGROUND:
@@ -214,7 +217,10 @@ SkColor ThemeProperties::GetDefaultColor(int id, bool incognito) {
     case COLOR_FRAME_INACTIVE:
     case COLOR_BACKGROUND_TAB_INACTIVE:
       return SkColorSetRGB(0xE7, 0xEA, 0xED);
+    case COLOR_DOWNLOAD_SHELF:
+    case COLOR_INFOBAR:
     case COLOR_TOOLBAR:
+    case COLOR_STATUS_BUBBLE:
       return SK_ColorWHITE;
     case COLOR_BOOKMARK_TEXT:
     case COLOR_TAB_TEXT:

chrome/browser/themes/theme_properties.h

@@ -118,6 +118,11 @@ class ThemeProperties {
     COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND,
     COLOR_DETACHED_BOOKMARK_BAR_SEPARATOR,
 
+    // Color used for various 'shelves' and 'bars'.
+    COLOR_DOWNLOAD_SHELF,
+    COLOR_INFOBAR,
+    COLOR_STATUS_BUBBLE,
+
     // The throbber colors for tabs or anything on a toolbar (currently, only
     // the download shelf). If you're adding a throbber elsewhere, such as in
     // a dialog or bubble, you likely want

chrome/browser/themes/theme_service_browsertest.cc

@@ -18,8 +18,8 @@
 
 namespace {
 
-// The toolbar color specified in the theme.
-const SkColor kThemeToolbarColor = 0xFFCFDDC0;
+// The ntp link color specified in the theme.
+constexpr SkColor kThemeNtpLinkColor = SkColorSetRGB(36, 70, 0);
 
 bool UsingCustomTheme(const ThemeService& theme_service) {
   return !theme_service.UsingSystemTheme() &&
@@ -53,8 +53,8 @@ IN_PROC_BROWSER_TEST_F(ThemeServiceBrowserTest, PRE_ThemeDataPackInvalid) {
 
   // Test initial state.
   EXPECT_FALSE(UsingCustomTheme(*theme_service));
-  EXPECT_NE(kThemeToolbarColor,
-            theme_provider.GetColor(ThemeProperties::COLOR_TOOLBAR));
+  EXPECT_NE(kThemeNtpLinkColor,
+            theme_provider.GetColor(ThemeProperties::COLOR_NTP_LINK));
   EXPECT_EQ(base::FilePath(),
             profile->GetPrefs()->GetFilePath(prefs::kCurrentThemePackFilename));
 
@@ -66,8 +66,8 @@ IN_PROC_BROWSER_TEST_F(ThemeServiceBrowserTest, PRE_ThemeDataPackInvalid) {
 
   // Check that the theme was installed.
   EXPECT_TRUE(UsingCustomTheme(*theme_service));
-  EXPECT_EQ(kThemeToolbarColor,
-            theme_provider.GetColor(ThemeProperties::COLOR_TOOLBAR));
+  EXPECT_EQ(kThemeNtpLinkColor,
+            theme_provider.GetColor(ThemeProperties::COLOR_NTP_LINK));
   EXPECT_NE(base::FilePath(),
             profile->GetPrefs()->GetFilePath(prefs::kCurrentThemePackFilename));
 
@@ -84,8 +84,8 @@ IN_PROC_BROWSER_TEST_F(ThemeServiceBrowserTest, ThemeDataPackInvalid) {
   const ui::ThemeProvider& theme_provider =
       ThemeService::GetThemeProviderForProfile(browser()->profile());
   EXPECT_TRUE(UsingCustomTheme(*theme_service));
-  EXPECT_EQ(kThemeToolbarColor,
-            theme_provider.GetColor(ThemeProperties::COLOR_TOOLBAR));
+  EXPECT_EQ(kThemeNtpLinkColor,
+            theme_provider.GetColor(ThemeProperties::COLOR_NTP_LINK));
 }
 
 }  // namespace

chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

@@ -168,12 +168,6 @@ std::unique_ptr<views::LabelButtonBorder> CreateBookmarkButtonBorder() {
   return border;
 }
 
-SkColor GetBookmarkBarTextColor(const ui::ThemeProvider* theme_provider) {
-  return color_utils::GetColorWithMinimumContrast(
-      theme_provider->GetColor(ThemeProperties::COLOR_BOOKMARK_TEXT),
-      theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR));
-}
-
 // BookmarkButtonBase -----------------------------------------------
 
 // Base class for non-menu hosting buttons used on the bookmark bar.
@@ -993,8 +987,7 @@ void BookmarkBarView::PaintChildren(const views::PaintInfo& paint_info) {
 
     ui::PaintRecorder recorder(paint_info.context(), size());
     // TODO(sky/glen): make me pretty!
-    recorder.canvas()->FillRect(indicator_bounds,
-                                GetBookmarkBarTextColor(GetThemeProvider()));
+    recorder.canvas()->FillRect(indicator_bounds, GetBookmarkBarTextColor());
   }
 }
 
@@ -1631,7 +1624,7 @@ void BookmarkBarView::ConfigureButton(const BookmarkNode* node,
   // We don't always have a theme provider (ui tests, for example).
   const ui::ThemeProvider* const tp = GetThemeProvider();
   if (tp) {
-    SkColor color = GetBookmarkBarTextColor(tp);
+    SkColor color = GetBookmarkBarTextColor();
     button->SetEnabledTextColors(color);
     if (node->is_folder()) {
       button->SetImage(views::Button::STATE_NORMAL,
@@ -1651,9 +1644,8 @@ void BookmarkBarView::ConfigureButton(const BookmarkNode* node,
         // This favicon currently does not match the default favicon icon used
         // elsewhere in the codebase.
         // See https://crbug/814447
-        const gfx::ImageSkia icon =
-            gfx::CreateVectorIcon(kDefaultTouchFaviconIcon,
-                                  GetBookmarkBarTextColor(GetThemeProvider()));
+        const gfx::ImageSkia icon = gfx::CreateVectorIcon(
+            kDefaultTouchFaviconIcon, GetBookmarkBarTextColor());
         const gfx::ImageSkia mask =
             gfx::CreateVectorIcon(kDefaultTouchFaviconMaskIcon, SK_ColorBLACK);
         favicon = gfx::ImageSkiaOperations::CreateMaskedImage(icon, mask);
@@ -1965,7 +1957,7 @@ void BookmarkBarView::UpdateAppearanceForTheme() {
                     GetBookmarkButton(i));
   }
 
-  const SkColor color = GetBookmarkBarTextColor(theme_provider);
+  const SkColor color = GetBookmarkBarTextColor();
   other_bookmarks_button_->SetEnabledTextColors(color);
   other_bookmarks_button_->SetImage(views::Button::STATE_NORMAL,
                                     chrome::GetBookmarkFolderIcon(color));
@@ -2064,3 +2056,14 @@ int BookmarkBarView::GetIndexForButton(views::View* button) {
 
   return it - bookmark_buttons_.cbegin();
 }
+
+SkColor BookmarkBarView::GetBookmarkBarTextColor() {
+  const ui::ThemeProvider* theme_provider = GetThemeProvider();
+  int background_color_id =
+      bookmark_bar_state_ == BookmarkBar::DETACHED
+          ? ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND
+          : ThemeProperties::COLOR_TOOLBAR;
+  return color_utils::GetColorWithMinimumContrast(
+      theme_provider->GetColor(ThemeProperties::COLOR_BOOKMARK_TEXT),
+      theme_provider->GetColor(background_color_id));
+}

chrome/browser/ui/views/bookmarks/bookmark_bar_view.h

@@ -380,6 +380,9 @@ class BookmarkBarView : public views::AccessiblePaneView,
   // or -1 if |button| is not a bookmark button from this bar.
   int GetIndexForButton(views::View* button);
 
+  // Returns the color that should be used to draw text on the bookmark bar.
+  SkColor GetBookmarkBarTextColor();
+
   // Needed to react to kShowAppsShortcutInBookmarkBar changes.
   PrefChangeRegistrar profile_pref_registrar_;
 

chrome/browser/ui/views/download/download_item_view.cc

@@ -575,7 +575,7 @@ void DownloadItemView::OnPaint(gfx::Canvas* canvas) {
   // color for opaque canvases).
   canvas->DrawColor(SK_ColorBLACK);
   canvas->DrawColor(
-      GetThemeProvider()->GetColor(ThemeProperties::COLOR_TOOLBAR));
+      GetThemeProvider()->GetColor(ThemeProperties::COLOR_DOWNLOAD_SHELF));
 
   DrawStatusText(canvas);
   DrawFilename(canvas);

chrome/browser/ui/views/download/download_shelf_view.cc

@@ -156,7 +156,8 @@ void DownloadShelfView::ConfigureButtonForTheme(views::MdTextButton* button) {
     // For custom themes, we have to make up a background color for the
     // button. Use a slight tint of the shelf background.
     bg_color = color_utils::BlendTowardOppositeLuma(
-        GetThemeProvider()->GetColor(ThemeProperties::COLOR_TOOLBAR), 0x10);
+        GetThemeProvider()->GetColor(ThemeProperties::COLOR_DOWNLOAD_SHELF),
+        0x10);
   }
   button->SetBgColorOverride(bg_color);
 }
@@ -305,7 +306,7 @@ void DownloadShelfView::UpdateColorsFromTheme() {
   ConfigureButtonForTheme(show_all_view_);
 
   SetBackground(views::CreateSolidBackground(
-      GetThemeProvider()->GetColor(ThemeProperties::COLOR_TOOLBAR)));
+      GetThemeProvider()->GetColor(ThemeProperties::COLOR_DOWNLOAD_SHELF)));
 
   views::SetImageFromVectorIcon(
       close_button_, vector_icons::kCloseRoundedIcon,

chrome/browser/ui/views/infobars/infobar_view.cc

@@ -58,8 +58,7 @@ namespace {
 DEFINE_UI_CLASS_PROPERTY_KEY(LabelType, kLabelType, LabelType::kNone);
 
 // IDs of the colors to use for infobar elements.
-constexpr int kInfoBarLabelBackgroundColor =
-    ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND;
+constexpr int kInfoBarLabelBackgroundColor = ThemeProperties::COLOR_INFOBAR;
 constexpr int kInfoBarLabelTextColor = ThemeProperties::COLOR_BOOKMARK_TEXT;
 
 bool SortLabelsByDecreasingWidth(views::Label* label_1, views::Label* label_2) {

chrome/browser/ui/views/status_bubble_views.cc

@@ -478,7 +478,7 @@ void StatusBubbleViews::StatusView::OnPaint(gfx::Canvas* canvas) {
   Op(path, stroke_path, kDifference_SkPathOp, &fill_path);
   flags.setStyle(cc::PaintFlags::kFill_Style);
   const SkColor bubble_color =
-      theme_provider_->GetColor(ThemeProperties::COLOR_TOOLBAR);
+      theme_provider_->GetColor(ThemeProperties::COLOR_STATUS_BUBBLE);
   flags.setColor(bubble_color);
   canvas->sk_canvas()->drawPath(fill_path, flags);
 

chrome/browser/ui/views/toolbar/toolbar_view.cc

@@ -554,11 +554,9 @@ void ToolbarView::OnPaintBackground(gfx::Canvas* canvas) {
 
   const ui::ThemeProvider* tp = GetThemeProvider();
 
-  // Always fill the toolbar with its bg color first in case the image is
-  // transparent.
-  canvas->FillRect(GetLocalBounds(),
-                   tp->GetColor(ThemeProperties::COLOR_TOOLBAR));
-
+  // If the toolbar has a theme image, it gets composited against the toolbar
+  // background color when it's imported, so we only need to specificallh draw
+  // the background color if there is no custom image.
   if (tp->HasCustomImage(IDR_THEME_TOOLBAR)) {
     const int x_offset =
         GetMirroredX() + browser_view_->GetMirroredX() +
@@ -568,6 +566,9 @@ void ToolbarView::OnPaintBackground(gfx::Canvas* canvas) {
                          GetLayoutConstant(TABSTRIP_TOOLBAR_OVERLAP);
     canvas->TileImageInt(*tp->GetImageSkiaNamed(IDR_THEME_TOOLBAR), x_offset,
                          y_offset, 0, 0, width(), height());
+  } else {
+    canvas->FillRect(GetLocalBounds(),
+                     tp->GetColor(ThemeProperties::COLOR_TOOLBAR));
   }
 
   // Toolbar/content separator.

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

+{
+  "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuGMyBRIRAOiUgubfmS+4RHiNi/u/okGcU17xoKTXnziTiwmo7lq4V7unr3Vn4wWSG5bIG+yhQhQAG1TvfsXimBIUrrVKX4b9IADZIHR5akWgb1K4EV111PW/D8RDOwYgTPHPx5eA49TxB0Yw36DNC9LE0tPZOGgDR9hCFgaN1bcszlmJBcBmx39iGCeKPIjKB4bp5hXe33T5IUdoyuW4JHf4JDqC6unwQ1Y9nwzCRUbFis7YevbGQCgWP3I+Hp+bOo30mC2/Gq2UZfIczR4vIOrarxBV8IO4Ue4gzaDl6dGkGKnkZ3JdZKbLeRke1DmaKfjVSsTUp++X2nfMaPZphQIDAQAB",
+  "manifest_version": 2,
+  "name": "Toolbar/Frame Color + Image Test",
+  "theme": {
+    "colors": {
+      "frame": [ 255, 0, 255 ],
+      "toolbar": [ 0, 255, 0 ]
+      },
+    "images": {
+       "theme_frame": "images/theme_frame.png",
+       "theme_toolbar": "images/theme_toolbar.png"
+      }
+  },
+  "version": "0.0.1"
+}