Clean up frame code a bit:

* Make more functions const
* Tweak version check to "< Win 10" to match code constant names and the pattern of how we check this elsewhere in the UI code
* Rename some things from "avatar" to "incognito icon"
* Remove unnecessary member
* Replace local constants with layout constants (only in cases where the layout constants have the same values in non-MD)
* Simplify.  In a few cases this may look like a behavior change but isn't; e.g. "NonClientTopBorderHeight(false) + kTabstripTopShadowThickness" was replaced by "FrameBorderThickness(false)" at a place where the two are guaranteed to be the same value
* Prepare for further tabstrip/incognito icon positioning fixes to OpaqueBrowserFrameViewLayout::LayoutIncognitoIcon()

BUG=none
TEST=none

Review URL: https://codereview.chromium.org/1624773002

Cr-Commit-Position: refs/heads/master@{#371432}

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

@@ -303,7 +303,7 @@ int GlassBrowserFrameView::NonClientBorderThickness(bool restored) const {
   if ((frame()->IsMaximized() || frame()->IsFullscreen()) && !restored)
     return 0;
 
-  return (base::win::GetVersion() <= base::win::VERSION_WIN8_1)
+  return (base::win::GetVersion() < base::win::VERSION_WIN10)
              ? kNonClientBorderThicknessPreWin10
              : kNonClientBorderThicknessWin10;
 }
@@ -331,7 +331,7 @@ bool GlassBrowserFrameView::IsToolbarVisible() const {
       !browser_view()->toolbar()->GetPreferredSize().IsEmpty();
 }
 
-void GlassBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) {
+void GlassBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) const {
   gfx::Rect toolbar_bounds(browser_view()->GetToolbarBounds());
   if (toolbar_bounds.IsEmpty())
     return;
@@ -436,7 +436,7 @@ void GlassBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) {
   }
 }
 
-void GlassBrowserFrameView::PaintClientEdge(gfx::Canvas* canvas) {
+void GlassBrowserFrameView::PaintClientEdge(gfx::Canvas* canvas) const {
   gfx::Rect client_bounds = CalculateClientAreaBounds();
   int y = client_bounds.y();
   const bool normal_mode = browser_view()->IsTabStripVisible();

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

@@ -75,8 +75,8 @@ class GlassBrowserFrameView : public BrowserNonClientFrameView {
   bool IsToolbarVisible() const;
 
   // Paint various sub-components of this view.
-  void PaintToolbarBackground(gfx::Canvas* canvas);
-  void PaintClientEdge(gfx::Canvas* canvas);
+  void PaintToolbarBackground(gfx::Canvas* canvas) const;
+  void PaintClientEdge(gfx::Canvas* canvas) const;
   void FillClientEdgeRects(int x,
                            int y,
                            int right,

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

@@ -574,7 +574,8 @@ bool OpaqueBrowserFrameView::ShouldShowWindowTitleBar() const {
       IsMaximized());
 }
 
-void OpaqueBrowserFrameView::PaintRestoredFrameBorder(gfx::Canvas* canvas) {
+void OpaqueBrowserFrameView::PaintRestoredFrameBorder(
+    gfx::Canvas* canvas) const {
   frame_background_->set_frame_color(GetFrameColor());
   frame_background_->set_theme_image(GetFrameImage());
   frame_background_->set_theme_overlay_image(GetFrameOverlayImage());
@@ -599,7 +600,8 @@ void OpaqueBrowserFrameView::PaintRestoredFrameBorder(gfx::Canvas* canvas) {
   // all this in PaintRestoredClientEdge().
 }
 
-void OpaqueBrowserFrameView::PaintMaximizedFrameBorder(gfx::Canvas* canvas) {
+void OpaqueBrowserFrameView::PaintMaximizedFrameBorder(
+    gfx::Canvas* canvas) const {
   frame_background_->set_frame_color(GetFrameColor());
   frame_background_->set_theme_image(GetFrameImage());
   frame_background_->set_theme_overlay_image(GetFrameOverlayImage());
@@ -609,7 +611,7 @@ void OpaqueBrowserFrameView::PaintMaximizedFrameBorder(gfx::Canvas* canvas) {
   frame_background_->PaintMaximized(canvas, this);
 }
 
-void OpaqueBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) {
+void OpaqueBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) const {
   gfx::Rect toolbar_bounds(browser_view()->GetToolbarBounds());
   if (toolbar_bounds.IsEmpty())
     return;
@@ -714,7 +716,7 @@ void OpaqueBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) {
   }
 }
 
-void OpaqueBrowserFrameView::PaintClientEdge(gfx::Canvas* canvas) {
+void OpaqueBrowserFrameView::PaintClientEdge(gfx::Canvas* canvas) const {
   gfx::Rect client_bounds =
       layout_->CalculateClientAreaBounds(width(), height());
   const int x = client_bounds.x();

chrome/browser/ui/views/frame/opaque_browser_frame_view.h

@@ -140,10 +140,10 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
   // Paint various sub-components of this view.  The *FrameBorder() functions
   // also paint the background of the titlebar area, since the top frame border
   // and titlebar background are a contiguous component.
-  void PaintRestoredFrameBorder(gfx::Canvas* canvas);
-  void PaintMaximizedFrameBorder(gfx::Canvas* canvas);
-  void PaintToolbarBackground(gfx::Canvas* canvas);
-  void PaintClientEdge(gfx::Canvas* canvas);
+  void PaintRestoredFrameBorder(gfx::Canvas* canvas) const;
+  void PaintMaximizedFrameBorder(gfx::Canvas* canvas) const;
+  void PaintToolbarBackground(gfx::Canvas* canvas) const;
+  void PaintClientEdge(gfx::Canvas* canvas) const;
   void FillClientEdgeRects(int x,
                            int y,
                            int right,

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

@@ -42,14 +42,6 @@ const int kIconLeftSpacing = 2;
 // There is a 4 px gap between the icon and the title text.
 const int kIconTitleSpacing = 4;
 
-// The avatar ends 2 px above the bottom of the tabstrip (which, given the
-// way the tabstrip draws its bottom edge, will appear like a 1 px gap to the
-// user).
-const int kAvatarBottomSpacing = 2;
-
-// Space between the frame border and the edge of the avatar.
-const int kAvatarOuterSpacing = 2;
-
 // Space between the edge of the avatar and the tabstrip.
 const int kAvatarInnerSpacing = 4;
 
@@ -72,14 +64,6 @@ const int kNewTabCaptionCondensedSpacing = 16;
 // overrides the two previous constants.
 const int kNewTabNoCaptionButtonsSpacing = 5;
 
-// The top 3 px of the tabstrip is shadow; in maximized mode we push this off
-// the top of the screen so the tabs appear flush against the screen edge.
-const int kTabstripTopShadowThickness = 3;
-
-// How far to indent the tabstrip from the left side of the screen when there
-// is no avatar icon.
-const int kTabStripIndent = -6;
-
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 // Default extra space between the top of the frame and the top of the window
 // caption buttons.
@@ -132,19 +116,13 @@ void OpaqueBrowserFrameViewLayout::SetButtonOrdering(
 gfx::Rect OpaqueBrowserFrameViewLayout::GetBoundsForTabStrip(
     const gfx::Size& tabstrip_preferred_size,
     int available_width) const {
-  available_width -= trailing_button_start_;
-  available_width -= leading_button_start_;
-
-  const int caption_spacing = NewTabCaptionSpacing();
-  const int tabstrip_width = available_width - caption_spacing;
-  gfx::Rect bounds(leading_button_start_, GetTabStripInsetsTop(false),
-                   std::max(0, tabstrip_width),
+  int x = leading_button_start_ + GetLayoutInsets(AVATAR_ICON).right();
+  available_width -= x + NewTabCaptionSpacing() + trailing_button_start_;
+  gfx::Rect bounds(x, GetTabStripInsetsTop(false), std::max(0, available_width),
                    tabstrip_preferred_size.height());
 
-  int leading_tabstrip_indent = kTabStripIndent;
-  if (delegate_->ShouldShowAvatar() && !ShouldAvatarBeOnRight())
-    leading_tabstrip_indent += kAvatarInnerSpacing;
-  bounds.Inset(leading_tabstrip_indent, 0, 0, 0);
+  if (delegate_->ShouldShowAvatar() && !ShouldIncognitoIconBeOnRight())
+    bounds.Inset(kAvatarInnerSpacing, 0, 0, 0);
   return bounds;
 }
 
@@ -265,9 +243,9 @@ bool OpaqueBrowserFrameViewLayout::IsTitleBarCondensed() const {
 ///////////////////////////////////////////////////////////////////////////////
 // OpaqueBrowserFrameView, private:
 
-bool OpaqueBrowserFrameViewLayout::ShouldAvatarBeOnRight() const {
-  // The avatar should be shown either on the end of the left or the beginning
-  // of the right depending on which side has fewer buttons.
+bool OpaqueBrowserFrameViewLayout::ShouldIncognitoIconBeOnRight() const {
+  // The incognito should be shown either on the end of the left or the
+  // beginning of the right, depending on which side has fewer buttons.
   return trailing_buttons_.size() < leading_buttons_.size();
 }
 
@@ -405,41 +383,30 @@ void OpaqueBrowserFrameViewLayout::LayoutNewStyleAvatar(views::View* host) {
                                 kCaptionButtonHeight);
 }
 
-void OpaqueBrowserFrameViewLayout::LayoutAvatar(views::View* host) {
-  // Even though the avatar is used for both incognito and profiles we always
-  // use the incognito icon to layout the avatar button. The profile icon
-  // can be customized so we can't depend on its size to perform layout.
-  gfx::ImageSkia incognito_icon = delegate_->GetOTRAvatarIcon();
-
-  bool avatar_on_right = ShouldAvatarBeOnRight();
-  int avatar_bottom = GetTabStripInsetsTop(false) +
-      delegate_->GetTabStripHeight() - kAvatarBottomSpacing;
-  int avatar_restored_y = avatar_bottom - incognito_icon.height();
-  int avatar_x = avatar_on_right ?
-      host->width() - trailing_button_start_ - kAvatarOuterSpacing -
-          incognito_icon.width() :
-      leading_button_start_ + kAvatarOuterSpacing;
-  int avatar_y = IsTitleBarCondensed() ?
-      (NonClientTopBorderHeight(false) + kTabstripTopShadowThickness) :
-      avatar_restored_y;
-  avatar_bounds_.SetRect(
-      avatar_x,
-      avatar_y,
-      incognito_icon.width(),
-      delegate_->ShouldShowAvatar() ? (avatar_bottom - avatar_y) : 0);
-  if (avatar_button_) {
-    avatar_button_->SetBoundsRect(avatar_bounds_);
+void OpaqueBrowserFrameViewLayout::LayoutIncognitoIcon(views::View* host) {
+  const int old_button_size = leading_button_start_ + trailing_button_start_;
 
-    int edge_offset = kAvatarOuterSpacing + incognito_icon.width();
-    if (avatar_on_right)
-      trailing_button_start_ += edge_offset;
-    else
-      leading_button_start_ += edge_offset;
-
-    // We just add the avatar button size to the minimum size because clicking
-    // the avatar label does the same thing as clicking the avatar button.
-    minimum_size_for_buttons_ += kAvatarOuterSpacing + incognito_icon.width();
+  if (avatar_button_) {
+    const gfx::Insets insets(GetLayoutInsets(AVATAR_ICON));
+    const gfx::Size size(delegate_->GetOTRAvatarIcon().size());
+    const int incognito_width = insets.left() + size.width();
+    int x;
+    if (ShouldIncognitoIconBeOnRight()) {
+      trailing_button_start_ += incognito_width;
+      x = host->width() - trailing_button_start_;
+    } else {
+      x = leading_button_start_ + insets.left();
+      leading_button_start_ += incognito_width;
+    }
+    const int bottom = GetTabStripInsetsTop(false) +
+        delegate_->GetTabStripHeight() - insets.bottom();
+    int y = IsTitleBarCondensed() ?
+        FrameBorderThickness(false) : (bottom - size.height());
+    avatar_button_->SetBounds(x, y, size.width(), bottom - y);
   }
+
+  minimum_size_for_buttons_ +=
+      (leading_button_start_ + trailing_button_start_ - old_button_size);
 }
 
 void OpaqueBrowserFrameViewLayout::ConfigureButton(
@@ -647,7 +614,7 @@ void OpaqueBrowserFrameViewLayout::Layout(views::View* host) {
   if (delegate_->IsRegularOrGuestSession())
     LayoutNewStyleAvatar(host);
   else
-    LayoutAvatar(host);
+    LayoutIncognitoIcon(host);
 
   client_view_bounds_ = CalculateClientAreaBounds(
       host->width(), host->height());

chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h

@@ -107,9 +107,9 @@ class OpaqueBrowserFrameViewLayout : public views::LayoutManager {
     ALIGN_TRAILING
   };
 
-  // Determines whether the avatar should be shown on the right side of the tab
-  // strip (instead of the usual left).
-  bool ShouldAvatarBeOnRight() const;
+  // Determines whether the incognito icon should be shown on the right side of
+  // the tab strip (instead of the usual left).
+  bool ShouldIncognitoIconBeOnRight() const;
 
   // Determines the amount of spacing between the New Tab button and the element
   // to its immediate right.
@@ -118,7 +118,7 @@ class OpaqueBrowserFrameViewLayout : public views::LayoutManager {
   // Layout various sub-components of this view.
   void LayoutWindowControls(views::View* host);
   void LayoutTitleBar(views::View* host);
-  void LayoutAvatar(views::View* host);
+  void LayoutIncognitoIcon(views::View* host);
   void LayoutNewStyleAvatar(views::View* host);
 
   void ConfigureButton(views::View* host,
@@ -146,9 +146,6 @@ class OpaqueBrowserFrameViewLayout : public views::LayoutManager {
 
   OpaqueBrowserFrameViewLayoutDelegate* delegate_;
 
-  // The layout rect of the avatar icon, if visible.
-  gfx::Rect avatar_bounds_;
-
   // The bounds of the ClientView.
   gfx::Rect client_view_bounds_;
 

ui/views/window/frame_background.cc

@@ -51,7 +51,8 @@ void FrameBackground::SetCornerImages(const gfx::ImageSkia* top_left,
   bottom_right_corner_ = bottom_right;
 }
 
-void FrameBackground::PaintRestored(gfx::Canvas* canvas, View* view) const {
+void FrameBackground::PaintRestored(gfx::Canvas* canvas,
+                                    const View* view) const {
   // Fill with the frame color first so we have a constant background for
   // areas not covered by the theme image.
   PaintFrameColor(canvas, view);
@@ -119,7 +120,8 @@ void FrameBackground::PaintRestored(gfx::Canvas* canvas, View* view) const {
                        left_edge_->width(), left_edge_height);
 }
 
-void FrameBackground::PaintMaximized(gfx::Canvas* canvas, View* view) const {
+void FrameBackground::PaintMaximized(gfx::Canvas* canvas,
+                                     const View* view) const {
   // We will be painting from -|maximized_top_inset_| to
   // -|maximized_top_inset_| + |theme_image_|->height(). If this is less than
   // |top_area_height_|, we need to paint the frame color to fill in the area
@@ -141,7 +143,8 @@ void FrameBackground::PaintMaximized(gfx::Canvas* canvas, View* view) const {
     canvas->DrawImageInt(*theme_overlay_image_, 0, -maximized_top_inset_);
 }
 
-void FrameBackground::PaintFrameColor(gfx::Canvas* canvas, View* view) const {
+void FrameBackground::PaintFrameColor(gfx::Canvas* canvas,
+                                      const View* view) const {
   // Fill the top area.
   canvas->FillRect(gfx::Rect(0, 0, view->width(), top_area_height_),
                    frame_color_);

ui/views/window/frame_background.h

@@ -64,15 +64,15 @@ class VIEWS_EXPORT FrameBackground {
   // Paints the border for a standard, non-maximized window.  Also paints the
   // background of the title bar area, since the top frame border and the
   // title bar background are a contiguous component.
-  void PaintRestored(gfx::Canvas* canvas, View* view) const;
+  void PaintRestored(gfx::Canvas* canvas, const View* view) const;
 
   // Paints the border for a maximized window, which does not include the
   // window edges.
-  void PaintMaximized(gfx::Canvas* canvas, View* view) const;
+  void PaintMaximized(gfx::Canvas* canvas, const View* view) const;
 
  private:
   // Fills the frame area with the frame color.
-  void PaintFrameColor(gfx::Canvas* canvas, View* view) const;
+  void PaintFrameColor(gfx::Canvas* canvas, const View* view) const;
 
   SkColor frame_color_;
   const gfx::ImageSkia* theme_image_;