Implement MD specs for non-tabbed windows in Ash

chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.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_non_client_frame_view_ash.h"
 
 #include <algorithm>
 
 #include "ash/ash_layout_constants.h"
 #include "ash/frame/caption_buttons/frame_caption_button_container_view.h"
@@ skipping 11 matching lines @@
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/frame/browser_frame.h"
 #include "chrome/browser/ui/views/frame/browser_header_painter_ash.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
 #include "chrome/browser/ui/views/frame/web_app_left_header_view_ash.h"
 #include "chrome/browser/ui/views/profiles/avatar_menu_button.h"
 #include "chrome/browser/ui/views/tab_icon_view.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
+#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "components/signin/core/common/profile_management_switches.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/extension_registry.h"
 #include "grit/theme_resources.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/window.h"
 #include "ui/base/hit_test.h"
 #include "ui/base/layout.h"
@@ skipping 140 matching lines @@
     return ((frame()->IsMaximized() || frame()->IsFullscreen()) && !restored) ?
         kTabstripTopSpacingShort : kTabstripTopSpacingTall;
   }
 
   if (UsePackagedAppHeaderStyle() || UseWebAppHeaderStyle())
     return header_painter_->GetHeaderHeight();
 
   int caption_buttons_bottom = caption_button_container_->bounds().bottom();
 
   // The toolbar partially overlaps the caption buttons.
-  if (browser_view()->IsToolbarVisible())
-    return caption_buttons_bottom - kContentShadowHeight;
+  if (IsToolbarVisible())
+    return caption_buttons_bottom;
 
   return caption_buttons_bottom + kClientEdgeThickness;
 }
 
 int BrowserNonClientFrameViewAsh::GetThemeBackgroundXInset() const {
   return ash::HeaderPainterUtil::GetThemeBackgroundXInset();
 }
 
 void BrowserNonClientFrameViewAsh::UpdateThrobber(bool running) {
   if (window_icon_)
@@ skipping 95 matching lines @@
     return;
   }
 
   caption_button_container_->SetPaintAsActive(ShouldPaintAsActive());
   if (web_app_left_header_view_)
     web_app_left_header_view_->SetPaintAsActive(ShouldPaintAsActive());
 
   ash::HeaderPainter::Mode header_mode = ShouldPaintAsActive() ?
       ash::HeaderPainter::MODE_ACTIVE : ash::HeaderPainter::MODE_INACTIVE;
   header_painter_->PaintHeader(canvas, header_mode);
-  if (browser_view()->IsToolbarVisible())
+
+  if (IsToolbarVisible())
     PaintToolbarBackground(canvas);
-  else if (!UsePackagedAppHeaderStyle() && !UseWebAppHeaderStyle())
-    PaintContentEdge(canvas);
+  PaintContentEdge(canvas);
 }
 
 void BrowserNonClientFrameViewAsh::Layout() {
   // The header must be laid out before computing |painted_height| because the
   // computation of |painted_height| for app and popup windows depends on the
   // position of the window controls.
   header_painter_->LayoutHeader();
 
   int painted_height = GetTopInset(false);
   if (browser_view()->IsTabStripVisible()) {
     painted_height += browser_view()->tabstrip()->GetPreferredSize().height();
-  } else if (browser_view()->IsToolbarVisible()) {
+  } else if (IsToolbarVisible()) {
     // Paint the header so that it overlaps with the top few pixels of the
     // toolbar because the top few pixels of the toolbar are not opaque.
     const int kToolbarTopEdgeExclusion = 2;
     painted_height += kToolbarTopEdgeExclusion;
   }
   header_painter_->SetHeaderHeightForPainting(painted_height);
 
   if (avatar_button()) {
     LayoutAvatar();
     header_painter_->UpdateLeftViewXInset(avatar_button()->bounds().right());
@@ skipping 199 matching lines @@
 }
 
 void BrowserNonClientFrameViewAsh::PaintImmersiveLightbarStyleHeader(
     gfx::Canvas* canvas) {
   // The light bar header is not themed because theming it does not look good.
   canvas->FillRect(
       gfx::Rect(width(), header_painter_->GetHeaderHeightForPainting()),
       SK_ColorBLACK);
 }
 
+bool BrowserNonClientFrameViewAsh::IsToolbarVisible() const {
+  return browser_view()->IsToolbarVisible() &&
+      !browser_view()->toolbar()->GetPreferredSize().IsEmpty();
+}
+
 void BrowserNonClientFrameViewAsh::PaintToolbarBackground(gfx::Canvas* canvas) {
   gfx::Rect toolbar_bounds(browser_view()->GetToolbarBounds());
   if (toolbar_bounds.IsEmpty())
     return;
   gfx::Point toolbar_origin(toolbar_bounds.origin());
   View::ConvertPointToTarget(browser_view(), this, &toolbar_origin);
   toolbar_bounds.set_origin(toolbar_origin);
+  const int h = toolbar_bounds.height();
+  const bool md = ui::MaterialDesignController::IsModeMaterial();
+  const ui::ThemeProvider* tp = GetThemeProvider();
+  const SkColor separator_color =
+      tp->GetColor(ThemeProperties::COLOR_TOOLBAR_BOTTOM_SEPARATOR);
 
-  int x = toolbar_bounds.x();
-  int w = toolbar_bounds.width();
-  int y = toolbar_bounds.y();
-  int h = toolbar_bounds.height();
-  const ui::ThemeProvider* tp = GetThemeProvider();
+  if (browser_view()->IsTabStripVisible()) {
+    gfx::ImageSkia* bg = tp->GetImageSkiaNamed(IDR_THEME_TOOLBAR);
+    int x = toolbar_bounds.x();
+    const int y = toolbar_bounds.y();
+    const int bg_y =
+        GetTopInset(false) + Tab::GetYInsetForActiveTabBackground();
+    const int w = toolbar_bounds.width();
 
-  if (ui::MaterialDesignController::IsModeMaterial()) {
-    if (tp->HasCustomImage(IDR_THEME_TOOLBAR)) {
-      // Paint the main toolbar image.  Since this image is also used to draw
-      // the tab background, we must use the tab strip offset to compute the
-      // image source y position.  If you have to debug this code use an image
-      // editor to paint a diagonal line through the toolbar image and ensure it
-      // lines up across the tab and toolbar.
-      gfx::ImageSkia* theme_toolbar = tp->GetImageSkiaNamed(IDR_THEME_TOOLBAR);
-      canvas->TileImageInt(*theme_toolbar, x + GetThemeBackgroundXInset(),
-                           y - GetTopInset(false), x, y, w,
-                           theme_toolbar->height());
-    } else {
-      canvas->FillRect(toolbar_bounds,
-                       tp->GetColor(ThemeProperties::COLOR_TOOLBAR));
-    }
+    if (md) {
+      if (tp->HasCustomImage(IDR_THEME_TOOLBAR)) {
+        // Paint the main toolbar image.  Since this image is also used to draw
+        // the tab background, we must use the tab strip offset to compute the
+        // image source y position.  If you have to debug this code use an image
+        // editor to paint a diagonal line through the toolbar image and ensure
+        // it lines up across the tab and toolbar.
+        canvas->TileImageInt(*bg, x + GetThemeBackgroundXInset(), y - bg_y, x,
+                             y, w, h);
+      } else {
+        canvas->FillRect(toolbar_bounds,
+                         tp->GetColor(ThemeProperties::COLOR_TOOLBAR));
+      }
 
-    // Draw the separator line atop the toolbar, on the left and right of the
-    // tabstrip.
-    // TODO(tdanderson): Draw the separator line for non-tabbed windows.
-    if (browser_view()->IsTabStripVisible()) {
+      // Separator line above the toolbar, which coincides with the bottom edge
+      // of the tab strip.
       gfx::Rect separator_rect(x, y, w, 0);
       gfx::ScopedCanvas scoped_canvas(canvas);
       gfx::Rect tabstrip_bounds(
           GetBoundsForTabStrip(browser_view()->tabstrip()));
       tabstrip_bounds.set_x(GetMirroredXForRect(tabstrip_bounds));
       canvas->sk_canvas()->clipRect(gfx::RectToSkRect(tabstrip_bounds),
                                     SkRegion::kDifference_Op);
       separator_rect.set_y(tabstrip_bounds.bottom());
       BrowserView::Paint1pxHorizontalLine(
           canvas, tp->GetColor(ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR),
           separator_rect, true);
+    } else {
+      // Gross hack: We split the toolbar images into two pieces, since
+      // sometimes (popup mode) the toolbar isn't tall enough to show the whole
+      // image.  The split happens between the top shadow section and the bottom
+      // gradient section so that we never break the gradient.
+      // NOTE(pkotwicz): If the computation for |bottom_y| is changed, Layout()
+      // must be changed as well.
+      int split_point = kFrameShadowThickness * 2;
+      int bottom_y = y + split_point;
+      int bottom_edge_height = h - split_point;
+
+      canvas->FillRect(gfx::Rect(x, bottom_y, w, bottom_edge_height),
+                       tp->GetColor(ThemeProperties::COLOR_TOOLBAR));
+
+      // Paint the main toolbar image.  Since this image is also used to draw
+      // the tab background, we must use the tab strip offset to compute the
+      // image source y position.  If you have to debug this code use an image
+      // editor to paint a diagonal line through the toolbar image and ensure it
+      // lines up across the tab and toolbar.
+      canvas->TileImageInt(
+          *bg,
+          x + GetThemeBackgroundXInset(),
+          bottom_y - bg_y,
+          x, bottom_y,
+          w, bg->height());
+
+      // The pre-material design content area line has a shadow that extends a
+      // couple of pixels above the toolbar bounds.
+      const int kContentShadowHeight = 2;
+      gfx::ImageSkia* toolbar_top =
+          tp->GetImageSkiaNamed(IDR_TOOLBAR_SHADE_TOP);
+      canvas->TileImageInt(*toolbar_top,
+                           0, 0,
+                           x, y - kContentShadowHeight,
+                           w, split_point + kContentShadowHeight + 1);
+
+      // Draw the "lightening" shade line around the edges of the toolbar.
+      gfx::ImageSkia* toolbar_left =
+          tp->GetImageSkiaNamed(IDR_TOOLBAR_SHADE_LEFT);
+      canvas->TileImageInt(*toolbar_left,
+                           0, 0,
+                           x + kClientEdgeThickness,
+                           y + kClientEdgeThickness + kContentShadowHeight,
+                           toolbar_left->width(), bg->height());
+      gfx::ImageSkia* toolbar_right =
+          tp->GetImageSkiaNamed(IDR_TOOLBAR_SHADE_RIGHT);
+      canvas->TileImageInt(*toolbar_right,
+                           0, 0,
+                           w - toolbar_right->width() - 2*kClientEdgeThickness,
+                           y + kClientEdgeThickness + kContentShadowHeight,
+                           toolbar_right->width(), bg->height());
     }
+  }
 
-    // Draw the content/toolbar separator.
-    toolbar_bounds.Inset(kClientEdgeThickness, 0);
-    BrowserView::Paint1pxHorizontalLine(
-        canvas,
-        tp->GetColor(ThemeProperties::COLOR_TOOLBAR_BOTTOM_SEPARATOR),
-        toolbar_bounds, true);
+  // Draw the toolbar/content separator.
+  toolbar_bounds.Inset(kClientEdgeThickness, h - kClientEdgeThickness,
+                       kClientEdgeThickness, 0);
+  if (md) {
+    canvas->FillRect(toolbar_bounds, SK_ColorWHITE);

[tdanderson, 2016/01/29 22:28:43]

Without line 656, at 2x device scale factor we will have a transparent row of
pixels between the toolbar and the separator line. By filling in with white here
it gives the illusion that the bottom of the toolbar is actually flush with the
separator line.

[Peter Kasting, 2016/01/30 02:53:37]

This is actually a bug on non-Ash as well; I must have regressed this.  Let me
fix this for other platforms, which should likely fix for Ash as well.

[tdanderson, 2016/02/01 22:17:51]

Thanks, I've added a TODO in the next patch set.

+    BrowserView::Paint1pxHorizontalLine(canvas, separator_color,
+                                        toolbar_bounds, true);
   } else {
-    // Gross hack: We split the toolbar images into two pieces, since sometimes
-    // (popup mode) the toolbar isn't tall enough to show the whole image.  The
-    // split happens between the top shadow section and the bottom gradient
-    // section so that we never break the gradient.
-    // NOTE(pkotwicz): If the computation for |bottom_y| is changed, Layout()
-    // must be changed as well.
-    int split_point = kFrameShadowThickness * 2;
-    int bottom_y = y + split_point;
-    int bottom_edge_height = h - split_point;
-
-    canvas->FillRect(gfx::Rect(x, bottom_y, w, bottom_edge_height),
-                     tp->GetColor(ThemeProperties::COLOR_TOOLBAR));
-
-    // Paint the main toolbar image.  Since this image is also used to draw the
-    // tab background, we must use the tab strip offset to compute the image
-    // source y position.  If you have to debug this code use an image editor
-    // to paint a diagonal line through the toolbar image and ensure it lines up
-    // across the tab and toolbar.
-    gfx::ImageSkia* theme_toolbar = tp->GetImageSkiaNamed(IDR_THEME_TOOLBAR);
-    canvas->TileImageInt(
-        *theme_toolbar,
-        x + GetThemeBackgroundXInset(),
-        bottom_y - GetTopInset(false),
-        x, bottom_y,
-        w, theme_toolbar->height());
-
-    // The pre-material design content area line has a shadow that extends a
-    // couple of pixels above the toolbar bounds.
-    const int kContentShadowHeight = 2;
-    gfx::ImageSkia* toolbar_top = tp->GetImageSkiaNamed(IDR_TOOLBAR_SHADE_TOP);
-    canvas->TileImageInt(*toolbar_top,
-                         0, 0,
-                         x, y - kContentShadowHeight,
-                         w, split_point + kContentShadowHeight + 1);
-
-    // Draw the "lightening" shade line around the edges of the toolbar.
-    gfx::ImageSkia* toolbar_left =
-        tp->GetImageSkiaNamed(IDR_TOOLBAR_SHADE_LEFT);
-    canvas->TileImageInt(*toolbar_left,
-                         0, 0,
-                         x + kClientEdgeThickness,
-                         y + kClientEdgeThickness + kContentShadowHeight,
-                         toolbar_left->width(), theme_toolbar->height());
-    gfx::ImageSkia* toolbar_right =
-        tp->GetImageSkiaNamed(IDR_TOOLBAR_SHADE_RIGHT);
-    canvas->TileImageInt(*toolbar_right,
-                         0, 0,
-                         w - toolbar_right->width() - 2 * kClientEdgeThickness,
-                         y + kClientEdgeThickness + kContentShadowHeight,
-                         toolbar_right->width(), theme_toolbar->height());
-
-    // Draw the content/toolbar separator.
-    canvas->FillRect(
-        gfx::Rect(x + kClientEdgeThickness,
-                  toolbar_bounds.bottom() - kClientEdgeThickness,
-                  w - (2 * kClientEdgeThickness), kClientEdgeThickness),
-        tp->GetColor(ThemeProperties::COLOR_TOOLBAR_BOTTOM_SEPARATOR));
+    canvas->FillRect(toolbar_bounds, separator_color);
   }
 }
 
 void BrowserNonClientFrameViewAsh::PaintContentEdge(gfx::Canvas* canvas) {
-  DCHECK(!UsePackagedAppHeaderStyle() && !UseWebAppHeaderStyle());
-  canvas->FillRect(
-      gfx::Rect(0, caption_button_container_->bounds().bottom(), width(),
-                kClientEdgeThickness),
-      GetThemeProvider()->GetColor(
-          ThemeProperties::COLOR_TOOLBAR_BOTTOM_SEPARATOR));
+  if (browser_view()->IsTabStripVisible())
+    return;
+
+  gfx::Rect separator_rect(
+      0, caption_button_container_->bounds().bottom(), width(), 0);
+  BrowserView::Paint1pxHorizontalLine(

[tdanderson, 2016/01/29 22:28:43]

DefaultHeaderPainter::PaintTitleBar() already paints this separator line
(DefaultHeaderPainter is used when UsePackagedAppHeaderStyle() ||
UseWebAppHeaderStyle() is true). So line 670 looks to be redundant in such
cases, but without it I am sometimes noticing incomplete rendering of the line
(until I force a repaint by maximizing/unmaximizing or resizing the window).

[Peter Kasting, 2016/01/30 02:53:37]

I would try to figure out why this isn't the case before your change, where that
was the only thing to render this line and there was no redundancy.

[tdanderson, 2016/02/01 22:17:51]

Fixed this in the next patch set.

+      canvas, GetThemeProvider()->GetColor(
+          ThemeProperties::COLOR_TOOLBAR_BOTTOM_SEPARATOR),
+      separator_rect, true);
 }