Reland: Replace CONTROL_BACKGROUND and DETACHED_BOOKMARK_BAR_BACKGROUND by COLOR_NTP_BACKGROUND

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

Index: chrome/browser/ui/views/frame/browser_view.cc
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index e0896b097faf8f623e0ab52116a4fa38167c3103..58a383b428a51bc6d5ceb2eabcfdff5ccb724bbf 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -222,15 +222,12 @@ void PaintDetachedBookmarkBar(gfx::Canvas* canvas,
   if (ui::MaterialDesignController::IsModeMaterial())
     fill_rect.Inset(0, 1, 0, 0);
 
-  // In detached mode, the bar is meant to overlap with |contents_container_|.
-  // The detached background color may be partially transparent, but the layer
-  // for |view| must be painted opaquely to avoid subpixel anti-aliasing
-  // artifacts, so we recreate the contents container base color here.
+  // The detached bar overlaps the |contents_container_| and so uses the same
+  // background color (the NTP background color).
   canvas->FillRect(fill_rect,
-                   tp->GetColor(ThemeProperties::COLOR_CONTROL_BACKGROUND));
-  canvas->FillRect(
-      fill_rect,
-      tp->GetColor(ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND));
+                   tp->GetColor(tp->HasCustomImage(IDR_THEME_NTP_BACKGROUND)
+                                    ? ThemeProperties::COLOR_TOOLBAR
+                                    : ThemeProperties::COLOR_NTP_BACKGROUND));

[Julien Isorce Samsung, 2016/08/16 16:01:03]

It relies on HasCustomImage to avoid regression of
https://bugs.chromium.org/p/chromium/issues/detail?id=628970. Or we can decide
to break these existing themes since it would allow also to choose a different
color than the tool bar.

[Peter Kasting, 2016/08/18 09:12:36]

Hmm.  There are arguments for both.

I'm inclined to leave this as just using COLOR_NTP_BACKGROUND and say that theme
authors can specify the color they want.  This is a breaking change for some
themes, but the CL overall already was that.

Note that this goes away if we put the NTP background image behind the detached
bar.  You talked about the NTP rendering its background using CSS.  My
contention was that I _think_ the NTP today can find out the height of the
detached bar, and if it can, the CSS for the NTP can simply be adjusted to take
that offset into account for the background image; then we can tile the top
strip of the image behind the detached bar on the browser side.

If it does _not_ know this, there are two possible solutions there, although
both of these are complicated enough that I'd do this in a different CL:
(1) Plumb that through.
(2) Crop the top off the background image when the bar is detached and make the
NTP contents redraw with the new background image.

Solution (1) is better than (2), I think, because it would be less CPU/memory
and could probably be made to match the bookmark bar attach/detach animation
better.

 
   // Draw the separators above and below bookmark bar;
   // if animating, these are fading in/out.
@@ -2074,7 +2071,7 @@ void BrowserView::InitViews() {
 
   contents_container_ = new views::View();
   contents_container_->set_background(views::Background::CreateSolidBackground(
-      GetThemeProvider()->GetColor(ThemeProperties::COLOR_CONTROL_BACKGROUND)));
+      GetThemeProvider()->GetColor(ThemeProperties::COLOR_NTP_BACKGROUND)));
   contents_container_->AddChildView(devtools_web_view_);
   contents_container_->AddChildView(contents_web_view_);
   contents_container_->SetLayoutManager(new ContentsLayoutManager(