Remove some pre-MD code from toolbar/tabstrip/frame.

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

Index: chrome/browser/ui/views/frame/glass_browser_frame_view.cc
diff --git a/chrome/browser/ui/views/frame/glass_browser_frame_view.cc b/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
index 939bcf2f2d28694edbed09b2f4aa46688e7cc310..a41c859554812872f5ed3e4a790c12c402f0a59f 100644
--- a/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
+++ b/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
@@ -20,7 +20,6 @@
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/grit/theme_resources.h"
 #include "skia/ext/image_operations.h"
-#include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/resource/resource_bundle_win.h"
 #include "ui/base/theme_provider.h"
 #include "ui/display/win/dpi.h"
@@ -107,10 +106,7 @@ gfx::Rect GlassBrowserFrameView::GetBoundsForTabStrip(
   // In maximized RTL windows, don't let the tabstrip overlap the caption area,
   // or the alpha-blending it does will make things like the profile switcher
   // button look glitchy.

[Peter Kasting, 2016/09/22 23:27:25]

This comment should go away, as all the code it was talking about is now gone.

[Evan Stade, 2016/09/23 00:31:07]

Done.

-  const int offset = (ui::MaterialDesignController::IsModeMaterial() ||
-                      !CaptionButtonsOnLeadingEdge() || !frame()->IsMaximized())
-                         ? GetLayoutInsets(AVATAR_ICON).right()
-                         : 0;
+  const int offset = GetLayoutInsets(AVATAR_ICON).right();
   const int x = incognito_bounds_.right() + offset;
   int end_x = width() - ClientBorderThickness(false);
   if (!CaptionButtonsOnLeadingEdge()) {
@@ -379,14 +375,9 @@ int GlassBrowserFrameView::TopAreaHeight(bool restored) const {
   // The tab top inset is equal to the height of any shadow region above the
   // tabs, plus a 1 px top stroke.  In maximized mode, we want to push the
   // shadow region off the top of the screen but leave the top stroke.
-  // Annoyingly, the pre-MD layout uses different heights for the hit-test
-  // exclusion region (which we want here, since we're trying to size the border
-  // so that the region above the tab's hit-test zone matches) versus the shadow
-  // thickness.
-  const int exclusion = GetLayoutConstant(TAB_TOP_EXCLUSION_HEIGHT);
-  return (frame()->IsMaximized() && !restored) ?
-      (top - GetLayoutInsets(TAB).top() + 1) :
-      (top + kNonClientRestoredExtraThickness - exclusion);
+  return (frame()->IsMaximized() && !restored)
+             ? (top - GetLayoutInsets(TAB).top() + 1)
+             : (top + kNonClientRestoredExtraThickness);
 }
 
 int GlassBrowserFrameView::TitlebarHeight(bool restored) const {
@@ -424,6 +415,7 @@ void GlassBrowserFrameView::PaintTitlebar(gfx::Canvas* canvas) const {
 }
 
 void GlassBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) const {
+  // TODO(estade): can this be shared with OpaqueBrowserFrameView?

[Peter Kasting, 2016/09/22 23:27:25]

Ideally, we'd share a lot more.  My goal during most of the MD refactors was to
move the glass and opaque frame implementations closer and closer together in
hopes of someday fully or partly combining the two.

[Evan Stade, 2016/09/23 00:31:07]

well, they're getting pretty close, so good job!

   gfx::Rect toolbar_bounds(browser_view()->GetToolbarBounds());
   if (toolbar_bounds.IsEmpty())
     return;
@@ -440,104 +432,45 @@ void GlassBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) const {
   const int h = toolbar_bounds.height();
   const SkColor separator_color =
       tp->GetColor(ThemeProperties::COLOR_TOOLBAR_BOTTOM_SEPARATOR);

[Peter Kasting, 2016/09/22 23:27:24]

Nit: Now that we have a lot less code, some of these are only used once and can
probably be inlined, viz. |separator_color|, |h|, |bg_y|, |bg|.

For the others, try to move them as close to first use as possible.

[Evan Stade, 2016/09/23 00:31:07]

Done. I didn't move tp, x, y, or w because the only way to move them closer is
to put them below the // Background comment, which doesn't seem more readable to
me.

-  if (ui::MaterialDesignController::IsModeMaterial()) {
-    // Background.  The top stroke is drawn above the toolbar bounds, so unlike
-    // in the non-Material Design code below, we don't need to exclude any
-    // region from having the background image drawn over it.
-    if (tp->HasCustomImage(IDR_THEME_TOOLBAR)) {
-      canvas->TileImageInt(*bg, x + GetThemeBackgroundXInset(), y - bg_y, x, y,
-                           w, h);
-    } else {
-      canvas->FillRect(toolbar_bounds,
-                       tp->GetColor(ThemeProperties::COLOR_TOOLBAR));
-    }
 
-    // Material Design has no corners to mask out.
-
-    // Top stroke.  For Material Design, the toolbar has no side strokes.
-    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, GetToolbarTopSeparatorColor(),
-                                        separator_rect, true);
-
-    // Toolbar/content separator.
-    BrowserView::Paint1pxHorizontalLine(canvas, separator_color, toolbar_bounds,
-                                        true);
+  // Background.
+  if (tp->HasCustomImage(IDR_THEME_TOOLBAR)) {
+    canvas->TileImageInt(*bg, x + GetThemeBackgroundXInset(), y - bg_y, x, y, w,
+                         h);
   } else {
-    // Background.  The top stroke is drawn using the IDR_CONTENT_TOP_XXX
-    // images, which overlay the toolbar.  The top 2 px of these images is the
-    // actual top stroke + shadow, and is partly transparent, so the toolbar
-    // background shouldn't be drawn over it.
-    const int bg_dest_y = y + kContentEdgeShadowThickness;
-    canvas->TileImageInt(*bg, x + GetThemeBackgroundXInset(), bg_dest_y - bg_y,
-                         x, bg_dest_y, w, h - kContentEdgeShadowThickness);
-
-    // On Windows 10+ where we don't draw our own window border but rather go
-    // right to the system border, the toolbar has no corners or side strokes.
-    if (base::win::GetVersion() < base::win::VERSION_WIN10) {
-      // Mask out the corners.
-      const gfx::ImageSkia* const left =
-          tp->GetImageSkiaNamed(IDR_CONTENT_TOP_LEFT_CORNER);
-      const int img_w = left->width();
-      x -= kContentEdgeShadowThickness;
-      SkPaint paint;
-      paint.setXfermodeMode(SkXfermode::kDstIn_Mode);
-      canvas->DrawImageInt(
-          *tp->GetImageSkiaNamed(IDR_CONTENT_TOP_LEFT_CORNER_MASK), 0, 0, img_w,
-          h, x, y, img_w, h, false, paint);
-      const int right_x =
-          toolbar_bounds.right() + kContentEdgeShadowThickness - img_w;
-      canvas->DrawImageInt(
-          *tp->GetImageSkiaNamed(IDR_CONTENT_TOP_RIGHT_CORNER_MASK), 0, 0,
-          img_w, h, right_x, y, img_w, h, false, paint);
-
-      // Corner and side strokes.
-      canvas->DrawImageInt(*left, 0, 0, img_w, h, x, y, img_w, h, false);
-      canvas->DrawImageInt(
-          *tp->GetImageSkiaNamed(IDR_CONTENT_TOP_RIGHT_CORNER), 0, 0, img_w, h,
-          right_x, y, img_w, h, false);
-
-      x += img_w;
-      w = right_x - x;
-    }
+    canvas->FillRect(toolbar_bounds,
+                     tp->GetColor(ThemeProperties::COLOR_TOOLBAR));
+  }
 
-    // Top stroke.
-    canvas->TileImageInt(*tp->GetImageSkiaNamed(IDR_CONTENT_TOP_CENTER), x, y,
-                         w, kContentEdgeShadowThickness);
+  // Top stroke.  The toolbar has no side strokes.

[Peter Kasting, 2016/09/22 23:27:25]

Nit: No real need for the second sentence anymore as no toolbar ever has a side
stroke in MD now.

[Evan Stade, 2016/09/23 00:31:07]

Done.

+  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, GetToolbarTopSeparatorColor(),
+                                      separator_rect, true);
 
-    // Toolbar/content separator.
-    toolbar_bounds.Inset(kClientEdgeThickness, h - kClientEdgeThickness,
-                         kClientEdgeThickness, 0);
-    canvas->FillRect(toolbar_bounds, separator_color);
-  }
+  // Toolbar/content separator.
+  BrowserView::Paint1pxHorizontalLine(canvas, separator_color, toolbar_bounds,
+                                      true);
 }
 
 void GlassBrowserFrameView::PaintClientEdge(gfx::Canvas* canvas) const {
-  // Pre-Material Design, the client edge images start below the toolbar.  In MD
-  // the client edge images start at the top of the toolbar.
+  // The client edge images start at the top of the toolbar.

[Peter Kasting, 2016/09/22 23:27:25]

Nit: Comment probably not needed anymore.

I would probably just move all the rest of these under the "// Draw the client
edge images" comment below.

[Evan Stade, 2016/09/23 00:31:07]

Done.

   gfx::Rect client_bounds = CalculateClientAreaBounds();
   const int x = client_bounds.x();
-  const bool md = ui::MaterialDesignController::IsModeMaterial();
   const gfx::Rect toolbar_bounds(browser_view()->GetToolbarBounds());

[Peter Kasting, 2016/09/22 23:27:25]

Nit: Inline into next statement

[Evan Stade, 2016/09/23 00:31:07]

Done.

-  const int y =
-      client_bounds.y() + (md ? toolbar_bounds.y() : toolbar_bounds.bottom());
+  const int y = client_bounds.y() + toolbar_bounds.y();
   const int right = client_bounds.right();
   const int bottom = std::max(y, height() - ClientBorderThickness(false));
 
-  // Draw the client edge images.  For non-MD, we fill the toolbar color
-  // underneath these images so they will lighten/darken it appropriately to
-  // create a "3D shaded" effect.  For MD, where we want a flatter appearance,
-  // we do the filling afterwards so the user sees the unmodified toolbar color.
+  // Draw the client edge images.
   const ui::ThemeProvider* tp = GetThemeProvider();
   const SkColor toolbar_color = tp->GetColor(ThemeProperties::COLOR_TOOLBAR);

[Peter Kasting, 2016/09/22 23:27:25]

Nit: Inline below.

[Evan Stade, 2016/09/23 00:31:07]

Done.

-  if (!md)
-    FillClientEdgeRects(x, y, right, bottom, toolbar_color, canvas);
-  if (!md || (base::win::GetVersion() < base::win::VERSION_WIN10)) {
+  if (base::win::GetVersion() < base::win::VERSION_WIN10) {
     const gfx::ImageSkia* const right_image =
         tp->GetImageSkiaNamed(IDR_CONTENT_RIGHT_SIDE);
     const int img_w = right_image->width();
@@ -554,8 +487,7 @@ void GlassBrowserFrameView::PaintClientEdge(gfx::Canvas* canvas) const {
     canvas->TileImageInt(*tp->GetImageSkiaNamed(IDR_CONTENT_LEFT_SIDE),
                          x - img_w, y, img_w, height);
   }
-  if (md)
-    FillClientEdgeRects(x, y, right, bottom, toolbar_color, canvas);
+  FillClientEdgeRects(x, y, right, bottom, toolbar_color, canvas);
 }
 
 void GlassBrowserFrameView::FillClientEdgeRects(int x,
@@ -599,7 +531,6 @@ void GlassBrowserFrameView::LayoutProfileSwitcher() {
 }
 
 void GlassBrowserFrameView::LayoutIncognitoIcon() {
-  const bool md = ui::MaterialDesignController::IsModeMaterial();
   const gfx::Insets insets(GetLayoutInsets(AVATAR_ICON));
   const gfx::Size size(GetIncognitoAvatarIcon().size());
   int x = ClientBorderThickness(false);
@@ -609,22 +540,13 @@ void GlassBrowserFrameView::LayoutIncognitoIcon() {
         (profile_switcher_.view() ? (profile_switcher_.view()->width() +
                                      kProfileSwitcherButtonOffset)
                                   : 0);
-  } else if (!md && !profile_indicator_icon() && IsToolbarVisible() &&
-             (base::win::GetVersion() < base::win::VERSION_WIN10)) {
-    // In non-MD before Win 10, the toolbar has a rounded corner that we don't
-    // want the tabstrip to overlap.
-    x += browser_view()->GetToolbarBounds().x() - kContentEdgeShadowThickness +
-        GetThemeProvider()->GetImageSkiaNamed(
-            IDR_CONTENT_TOP_LEFT_CORNER)->width();
   }
   const int bottom = GetTopInset(false) + browser_view()->GetTabStripHeight() -
-      insets.bottom();
-  const int y = (md || !frame()->IsMaximized())
-                    ? (bottom - size.height())
-                    : FrameTopBorderThickness(false);
+                     insets.bottom();
   incognito_bounds_.SetRect(x + (profile_indicator_icon() ? insets.left() : 0),
-                            y, profile_indicator_icon() ? size.width() : 0,
-                            bottom - y);
+                            bottom - size.height(),
+                            profile_indicator_icon() ? size.width() : 0,
+                            size.height());
   if (profile_indicator_icon())
     profile_indicator_icon()->SetBoundsRect(incognito_bounds_);
 }