Make sure to fill canvas opaquely when painting bookmark bar background.

chrome/browser/ui/views/frame/browser_view.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/frame/browser_view.h"
 
 #include <stdint.h>
 #include <algorithm>
 #include <utility>
 
@@ skipping 205 matching lines @@
   int thickness = views::NonClientFrameView::kClientEdgeThickness;
   int y = at_top ? 0 : (view->height() - thickness);
   canvas->FillRect(gfx::Rect(0, y, view->width(), thickness), color);
 }
 
 // TODO(kuan): These functions are temporarily for the bookmark bar while its
 // detached state is at the top of the page;  it'll be moved to float on the
 // content page in the very near future, at which time, these local functions
 // will be removed.
 void PaintDetachedBookmarkBar(gfx::Canvas* canvas,
-                              BookmarkBarView* view,
-                              Profile* profile) {
+                              BookmarkBarView* view) {
   // Paint background for detached state; if animating, this is fade in/out.
-  const ui::ThemeProvider& tp =
-      ThemeService::GetThemeProviderForProfile(profile);
+  const ui::ThemeProvider* tp = view->GetThemeProvider();
   canvas->DrawColor(
-      tp.GetColor(ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND));
+      tp->GetColor(ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND));
+
   // Draw the separators above and below bookmark bar;
   // if animating, these are fading in/out.
   SkColor separator_color =
-      tp.GetColor(ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_SEPARATOR);
+      tp->GetColor(ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_SEPARATOR);
 
   if (ui::MaterialDesignController::IsModeMaterial()) {
     BrowserView::Paint1pxHorizontalLine(
         canvas, separator_color,
         gfx::Rect(0, 0, view->width(),
                   views::NonClientFrameView::kClientEdgeThickness),
         true);
   } else {
     PaintHorizontalBorder(canvas, view, true, separator_color);
   }
 
   // For the bottom separator, increase the luminance. Either double it or halve
   // the distance to 1.0, whichever is less of a difference.
   color_utils::HSL hsl;
   color_utils::SkColorToHSL(separator_color, &hsl);
   hsl.l = std::min((hsl.l + 1) / 2, hsl.l * 2);
   BrowserView::Paint1pxHorizontalLine(
       canvas, color_utils::HSLToSkColor(hsl, SK_AlphaOPAQUE),
       view->GetLocalBounds(), true);
 }
 
 // Paints the background (including the theme image behind content area) for
 // the Bookmarks Bar when it is attached to the Toolbar into |bounds|.
 // |background_origin| is the origin to use for painting the theme image.
 void PaintBackgroundAttachedMode(gfx::Canvas* canvas,
                                  const ui::ThemeProvider* theme_provider,
                                  const gfx::Rect& bounds,
                                  const gfx::Point& background_origin) {
-  canvas->FillRect(bounds,
-                   theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR));
+  canvas->DrawColor(theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR));
 
   // Always tile the background image in pre-MD. In MD, only tile if there's a
   // non-default image.
   // TODO(estade): remove IDR_THEME_TOOLBAR when MD is default.
   if (theme_provider->HasCustomImage(IDR_THEME_TOOLBAR) ||
       !ui::MaterialDesignController::IsModeMaterial()) {
     canvas->TileImageInt(*theme_provider->GetImageSkiaNamed(IDR_THEME_TOOLBAR),
                          background_origin.x(),
                          background_origin.y(),
                          bounds.x(),
@@ skipping 140 matching lines @@
     BrowserView* browser_view,
     BookmarkBarView* bookmark_bar_view,
     Browser* browser)
     : browser_view_(browser_view),
       bookmark_bar_view_(bookmark_bar_view),
       browser_(browser) {
 }
 
 void BookmarkBarViewBackground::Paint(gfx::Canvas* canvas,
                                       views::View* view) const {
+  // Start with a frame-colored canvas in case the attached/detached colors
+  // aren't opaque.
+  canvas->DrawColor(SK_ColorBLACK);
+  canvas->DrawColor(
+      view->GetThemeProvider()->GetColor(ThemeProperties::COLOR_FRAME));

[sky, 2016/04/07 18:23:45]

Shouldn't this route through BrowserNonClientFrameView::GetFrameColor? In fact
perhaps there should be a called on BrowserNonClientFrameView for this painting,
eg BrowserNonClientFrameView::PaintBackground(...).

[Evan Stade, 2016/04/07 19:30:24]

This comment led me to realize I was wrong about the frame being the background,
because it wouldn't make sense for the bar to change color based on the frame's
activation state. So then I found this[1] and I was able to confirm that if I
changed[2] to something weird like SK_ColorGREEN then I could see the green
shining through on this theme with the old, non-layered version of the bookmark
bar.

Thus the new patch starts by painting COLOR_CONTROL_BACKGROUND. (Getting a hold
of contents_container_ from here to call ->background()->Paint() on it is a bit
of a hassle.)

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

+
   int toolbar_overlap = bookmark_bar_view_->GetToolbarOverlap();
 
   SkAlpha detached_alpha = static_cast<SkAlpha>(
       bookmark_bar_view_->size_animation().CurrentValueBetween(0xff, 0));
   if (detached_alpha != 0xff) {
     PaintAttachedBookmarkBar(canvas, bookmark_bar_view_, browser_view_,
                              toolbar_overlap);
   }
 
   if (!bookmark_bar_view_->IsDetached() || detached_alpha == 0)
     return;
 
   // While animating, set opacity to cross-fade between attached and detached
   // backgrounds including their respective separators.
   canvas->SaveLayerAlpha(detached_alpha);
-  PaintDetachedBookmarkBar(canvas, bookmark_bar_view_, browser_->profile());
+  PaintDetachedBookmarkBar(canvas, bookmark_bar_view_);
   canvas->Restore();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserView, public:
 
 // static
 const char BrowserView::kViewClassName[] = "BrowserView";
 
 BrowserView::BrowserView()
@@ skipping 2214 matching lines @@
 }
 
 extensions::ActiveTabPermissionGranter*
 BrowserView::GetActiveTabPermissionGranter() {
   content::WebContents* web_contents = GetActiveWebContents();
   if (!web_contents)
     return nullptr;
   return extensions::TabHelper::FromWebContents(web_contents)
       ->active_tab_permission_granter();
 }