Reland: Replace CONTROL_BACKGROUND and DETACHED_BOOKMARK_BAR_BACKGROUND by COLOR_NTP_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 <memory>
@@ skipping 204 matching lines @@
                               BookmarkBarView* view) {
   // Paint background for detached state; if animating, this is fade in/out.
   const ui::ThemeProvider* tp = view->GetThemeProvider();
   gfx::Rect fill_rect = view->GetLocalBounds();
   // In MD, we have to not color the top 1dp, because that should be painted by
   // the toolbar. We will, however, paint the 1px separator at the bottom of the
   // first dp. See crbug.com/610359
   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.
   SkColor separator_color =
       tp->GetColor(ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_SEPARATOR);
 
   if (ui::MaterialDesignController::IsModeMaterial()) {
     BrowserView::Paint1pxHorizontalLine(
         canvas, separator_color,
         gfx::Rect(0, 0, view->width(),
@@ skipping 1823 matching lines @@
 
   web_contents_close_handler_.reset(
       new WebContentsCloseHandler(contents_web_view_));
 
   devtools_web_view_ = new views::WebView(browser_->profile());
   devtools_web_view_->set_id(VIEW_ID_DEV_TOOLS_DOCKED);
   devtools_web_view_->SetVisible(false);
 
   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(
       devtools_web_view_, contents_web_view_));
   AddChildView(contents_container_);
   set_contents_view(contents_container_);
 
   // Top container holds tab strip and toolbar and lives at the front of the
   // view hierarchy.
   top_container_ = new TopContainerView(this);
@@ skipping 574 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();
 }