Refactored GlassBrowserFrameView and BrowserDesktopTreeHostWin.

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

 // Copyright (c) 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/glass_browser_frame_view.h"
 
 #include <utility>
 
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/windows_version.h"
@@ skipping 24 matching lines @@
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/resources/grit/views_resources.h"
 #include "ui/views/win/hwnd_util.h"
 #include "ui/views/window/client_view.h"
 
 HICON GlassBrowserFrameView::throbber_icons_[
     GlassBrowserFrameView::kThrobberIconCount];
 
 namespace {
-// Size of client edge drawn inside the outer frame borders.
-const int kNonClientBorderThicknessPreWin10 = 3;
-const int kNonClientBorderThicknessWin10 = 1;
+// Thickness of the border in the client area that separates it from the
+// non-client area. Not to be confused with kClientEdgeThickness, which is the
+// thickness of the border between the web content and our frame border.

[Peter Kasting, 2016/04/15 00:20:52]

Is the 1 DP "client edge thickness" at least _included_ in this value?  Because
it doesn't seem (from my memory of the layout here) that we draw a separate 1 DP
for that and 1 or 3 DP for this adjacent to each other.

If so, the comment should probably make that more clear...

[Bret, 2016/04/15 23:07:08]

It does. Comment edited.

+const int kClientBorderThicknessPreWin10 = 3;
+const int kClientBorderThicknessWin10 = 1;
 // Besides the frame border, there's empty space atop the window in restored
 // mode, to use to drag the window around.
 const int kNonClientRestoredExtraThickness = 11;
-// In the window corners, the resize areas don't actually expand bigger, but the
-// 16 px at the end of the top and bottom edges triggers diagonal resizing.
+// In the window corners the resize area is not actually bigger, but the 16
+// pixels at the end of the top edges trigger diagonal resizing.

[Peter Kasting, 2016/04/15 00:20:52]

Why did you remove "and bottom"?  We still say "edges", and we still talk about
this applying to the bottom edge down where this constant is used.

[Bret, 2016/04/15 23:07:08]

Oops, I actually thought it wasn't used at the bottom edge but I double-checked
and you're right.

 const int kResizeCornerWidth = 16;
-// How far the new avatar button is from the left of the minimize button.
-const int kNewAvatarButtonOffset = 5;
+// How far the profile switcher button is from the left of the minimize button.
+const int kProfileSwitcherButtonOffset = 5;
 // The content edge images have a shadow built into them.
 const int kContentEdgeShadowThickness = 2;
 // In restored mode, the New Tab button isn't at the same height as the caption
 // buttons, but the space will look cluttered if it actually slides under them,
 // so we stop it when the gap between the two is down to 5 px.
 const int kNewTabCaptionRestoredSpacing = 5;
 // In maximized mode, where the New Tab button and the caption buttons are at
 // similar vertical coordinates, we need to reserve a larger, 16 px gap to avoid
 // looking too cluttered.
 const int kNewTabCaptionMaximizedSpacing = 16;
+// Height of the profile switcher button. Same as the height of the Windows 7/8
+// caption buttons but doesn't change for 10 where the caption buttons are much
+// bigger.
+const int kProfileSwitcherButtonHeightRestored = 20;
+const int kProfileSwitcherButtonHeightMaximized = 19;
 
 // Converts the |image| to a Windows icon and returns the corresponding HICON
 // handle. |image| is resized to desired |width| and |height| if needed.
 base::win::ScopedHICON CreateHICONFromSkBitmapSizedTo(
     const gfx::ImageSkia& image,
     int width,
     int height) {
   return IconUtil::CreateHICONFromSkBitmap(
       width == image.width() && height == image.height()
           ? *image.bitmap()
@@ skipping 19 matching lines @@
 
 GlassBrowserFrameView::~GlassBrowserFrameView() {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // GlassBrowserFrameView, BrowserNonClientFrameView implementation:
 
 gfx::Rect GlassBrowserFrameView::GetBoundsForTabStrip(
     views::View* tabstrip) const {
   // In maximized RTL windows, don't let the tabstrip overlap the caption area,
-  // or the alpha-blending it does will make things like the new avatar button
-  // look glitchy.
-  const int offset =
-    (ui::MaterialDesignController::IsModeMaterial() || !base::i18n::IsRTL() ||
-     !frame()->IsMaximized()) ?
-        GetLayoutInsets(AVATAR_ICON).right() : 0;
+  // or the alpha-blending it does will make things like the profile switcher
+  // button look glitchy.
+  const int offset = (ui::MaterialDesignController::IsModeMaterial() ||
+                      !CaptionButtonsOnLeadingEdge() || !frame()->IsMaximized())
+                         ? GetLayoutInsets(AVATAR_ICON).right()
+                         : 0;
   const int x = incognito_bounds_.right() + offset;
-  int end_x = width() - NonClientBorderThickness(false);
-  if (!base::i18n::IsRTL()) {
+  int end_x = width() - ClientBorderThickness(false);
+  if (!CaptionButtonsOnLeadingEdge()) {
     end_x = std::min(frame()->GetMinimizeButtonOffset(), end_x) -
         (frame()->IsMaximized() ?
             kNewTabCaptionMaximizedSpacing : kNewTabCaptionRestoredSpacing);
 
-    // The new avatar button is optionally displayed to the left of the
+    // The profile switcher button is optionally displayed to the left of the
     // minimize button.
     if (profile_switcher_.view()) {
       const int old_end_x = end_x;
-      end_x -= profile_switcher_.view()->width() + kNewAvatarButtonOffset;
+      end_x -= profile_switcher_.view()->width() + kProfileSwitcherButtonOffset;
 
       // In non-maximized mode, allow the new tab button to slide completely
-      // under the avatar button.
+      // under the profile switcher button.
       if (!frame()->IsMaximized()) {
         end_x = std::min(end_x + GetLayoutSize(NEW_TAB_BUTTON).width() +
                              kNewTabCaptionRestoredSpacing,
                          old_end_x);
       }
     }
   }
-  return gfx::Rect(x, NonClientTopBorderHeight(false), std::max(0, end_x - x),
+  return gfx::Rect(x, NonClientTopHeight(false), std::max(0, end_x - x),
                    tabstrip->GetPreferredSize().height());
 }
 
 int GlassBrowserFrameView::GetTopInset(bool restored) const {
   return GetClientAreaInsets(restored).top();
 }
 
 int GlassBrowserFrameView::GetThemeBackgroundXInset() const {
   return 0;
 }
@@ skipping 65 matching lines @@
 }
 
 int GlassBrowserFrameView::NonClientHitTest(const gfx::Point& point) {
   // If the browser isn't in normal mode, we haven't customized the frame, so
   // Windows can figure this out.  If the point isn't within our bounds, then
   // it's in the native portion of the frame, so again Windows can figure it
   // out.
   if (!browser_view()->IsBrowserTypeNormal() || !bounds().Contains(point))
     return HTNOWHERE;
 
-  // See if the point is within the incognito icon or the new avatar menu.
+  // See if the point is within the incognito icon or the profile switcher menu.
   if ((avatar_button() &&
        avatar_button()->GetMirroredBounds().Contains(point)) ||
       (profile_switcher_.view() &&
        profile_switcher_.view()->GetMirroredBounds().Contains(point)))
     return HTCLIENT;
 
   int frame_component = frame()->client_view()->NonClientHitTest(point);
 
   // See if we're in the sysmenu region.  We still have to check the tabstrip
   // first so that clicks in a tab don't get treated as sysmenu clicks.
-  int nonclient_border_thickness = NonClientBorderThickness(false);
-  if (gfx::Rect(nonclient_border_thickness,
+  int client_border_thickness = ClientBorderThickness(false);
+  if (gfx::Rect(client_border_thickness,
                 gfx::win::GetSystemMetricsInDIP(SM_CYSIZEFRAME),
                 gfx::win::GetSystemMetricsInDIP(SM_CXSMICON),
-                gfx::win::GetSystemMetricsInDIP(SM_CYSMICON)).Contains(point))
+                gfx::win::GetSystemMetricsInDIP(SM_CYSMICON))
+          .Contains(point))
     return (frame_component == HTCLIENT) ? HTCLIENT : HTSYSMENU;
 
   if (frame_component != HTNOWHERE)
     return frame_component;
 
-  int frame_top_border_height = FrameTopBorderHeight(false);
+  int top_border_thickness = NonClientTopBorderThickness(false);
   // We want the resize corner behavior to apply to the kResizeCornerWidth
   // pixels at each end of the top and bottom edges.  Because |point|'s x
   // coordinate is based on the DWM-inset portion of the window (so, it's 0 at
   // the first pixel inside the left DWM margin), we need to subtract the DWM
   // margin thickness, which we calculate as the total frame border thickness
   // minus the nonclient border thickness.
-  const int dwm_margin = FrameBorderThickness() - nonclient_border_thickness;
-  int window_component = GetHTComponentForFrame(point, frame_top_border_height,
-      nonclient_border_thickness, frame_top_border_height,
-      kResizeCornerWidth - dwm_margin, frame()->widget_delegate()->CanResize());
+  const int dwm_margin = NonClientBorderThickness() - client_border_thickness;
+  int window_component = GetHTComponentForFrame(
+      point, top_border_thickness, client_border_thickness,
+      top_border_thickness, kResizeCornerWidth - dwm_margin,
+      frame()->widget_delegate()->CanResize());
   // Fall back to the caption if no other component matches.
   return (window_component == HTNOWHERE) ? HTCAPTION : window_component;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // GlassBrowserFrameView, views::View overrides:
 
 void GlassBrowserFrameView::OnPaint(gfx::Canvas* canvas) {
   if (!browser_view()->IsTabStripVisible())
     return;
   if (IsToolbarVisible())
     PaintToolbarBackground(canvas);
   if (!frame()->IsMaximized())
     PaintClientEdge(canvas);
 }
 
 void GlassBrowserFrameView::Layout() {
   if (browser_view()->IsRegularOrGuestSession())
-    LayoutNewStyleAvatar();
+    LayoutProfileSwitcher();
   LayoutIncognitoIcon();
   LayoutClientView();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // GlassBrowserFrameView, protected:
 
 // BrowserNonClientFrameView:
 void GlassBrowserFrameView::UpdateAvatar() {
   if (browser_view()->IsRegularOrGuestSession())
     profile_switcher_.Update(AvatarButtonStyle::NATIVE);
   else
     UpdateOldAvatarButton();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // GlassBrowserFrameView, private:
 
 // views::NonClientFrameView:
 bool GlassBrowserFrameView::DoesIntersectRect(const views::View* target,
                                               const gfx::Rect& rect) const {
   CHECK_EQ(target, this);
   bool hit_incognito_icon = avatar_button() &&
       avatar_button()->GetMirroredBounds().Intersects(rect);
-  bool hit_new_avatar_button =
+  bool hit_profile_switcher_button =
       profile_switcher_.view() &&
       profile_switcher_.view()->GetMirroredBounds().Intersects(rect);
-  return hit_incognito_icon || hit_new_avatar_button ||
+  return hit_incognito_icon || hit_profile_switcher_button ||
          !frame()->client_view()->bounds().Intersects(rect);
 }
 
-int GlassBrowserFrameView::FrameBorderThickness() const {
+int GlassBrowserFrameView::ClientBorderThickness(bool restored) const {
+  if ((frame()->IsMaximized() || frame()->IsFullscreen()) && !restored)
+    return 0;
+
+  return (base::win::GetVersion() < base::win::VERSION_WIN10)
+             ? kClientBorderThicknessPreWin10
+             : kClientBorderThicknessWin10;
+}
+
+int GlassBrowserFrameView::NonClientBorderThickness() const {
   return (frame()->IsMaximized() || frame()->IsFullscreen()) ?
       0 : gfx::win::GetSystemMetricsInDIP(SM_CXSIZEFRAME);
 }
 
-int GlassBrowserFrameView::FrameTopBorderHeight(bool restored) const {
-  // We'd like to use FrameBorderThickness() here, but the maximized Aero glass
-  // frame has a 0 frame border around most edges and a CYSIZEFRAME-thick border
-  // at the top (see AeroGlassFrame::OnGetMinMaxInfo()).
+int GlassBrowserFrameView::NonClientTopBorderThickness(bool restored) const {
+  // Distinct from NonClientBorderThickness() because Windows gives maximized
+  // windows an offscreen CYSIZEFRAME-thick region around the edges. The
+  // left/right/bottom edges don't worry about this because we cancel them out
+  // in BrowserDesktopWindowTreeHostWin::GetClientAreaInsets() so the offscreen
+  // area is non-client as far as Windows is concerned. However because we want
+  // to push away the top part of the glass's gradient in Win7 we set the top
+  // client inset to 0. Thus we must compensate here to avoid having UI elements
+  // drift off the top of the screen.
   return (frame()->IsFullscreen() && !restored) ?
       0 : gfx::win::GetSystemMetricsInDIP(SM_CYSIZEFRAME);
 }
 
-int GlassBrowserFrameView::NonClientBorderThickness(bool restored) const {
-  if ((frame()->IsMaximized() || frame()->IsFullscreen()) && !restored)
-    return 0;
-
-  return (base::win::GetVersion() < base::win::VERSION_WIN10)
-             ? kNonClientBorderThicknessPreWin10
-             : kNonClientBorderThicknessWin10;
-}
-
-int GlassBrowserFrameView::NonClientTopBorderHeight(bool restored) const {
+int GlassBrowserFrameView::NonClientTopHeight(bool restored) const {
   if (frame()->IsFullscreen() && !restored)
     return 0;
 
-  const int top = FrameTopBorderHeight(restored);
+  const int top = NonClientTopBorderThickness(restored);
   // 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);
 }
 
+int GlassBrowserFrameView::CaptionButtonY() const {
+  // Windows draws its caption buttons at the top of the screen for maximized
+  // windows, and 1 px from the top of the window when restored.
+  return frame()->IsMaximized() ? NonClientTopBorderThickness(false) : 1;
+}
+
 bool GlassBrowserFrameView::IsToolbarVisible() const {
   return browser_view()->IsToolbarVisible() &&
       !browser_view()->toolbar()->GetPreferredSize().IsEmpty();
 }
 
+bool GlassBrowserFrameView::CaptionButtonsOnLeadingEdge() const {
+  // Windows always draws the buttons on the right, even in RTL mode.

[Peter Kasting, 2016/04/15 00:20:51]

Nit: Maybe add more detail, e.g.:

http://crbug.com/560619 - Because we don't set WS_EX_LAYOUTRTL (which would
conflict with Chrome's own RTL layout logic), Windows always draws the caption
buttons on the right, even when we want to be RTL.  TODO(bsep): Fix this by
drawing the caption buttons ourselves.

[Bret, 2016/04/15 23:07:08]

Okay I like your comment but since this situation will probably partially
persist as long as we support Windows 7 I don't want to tether myself to a TODO
like that.

+  return base::i18n::IsRTL();
+}
+
 void GlassBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) const {
   gfx::Rect toolbar_bounds(browser_view()->GetToolbarBounds());
   if (toolbar_bounds.IsEmpty())
     return;
   gfx::Point toolbar_origin(toolbar_bounds.origin());
   ConvertPointToTarget(browser_view(), this, &toolbar_origin);
   toolbar_bounds.set_origin(toolbar_origin);
 
   const ui::ThemeProvider* tp = GetThemeProvider();
   const gfx::ImageSkia* const bg = tp->GetImageSkiaNamed(IDR_THEME_TOOLBAR);
@@ skipping 86 matching lines @@
   // Pre-Material Design, the client edge images start below the toolbar.  In MD
   // the client edge images start at the top of the toolbar.
   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());
   const int y =
       client_bounds.y() + (md ? toolbar_bounds.y() : toolbar_bounds.bottom());
   const int w = client_bounds.width();
   const int right = client_bounds.right();
-  const int bottom = std::max(y, height() - NonClientBorderThickness(false));
+  const int bottom = std::max(y, height() - ClientBorderThickness(false));
   const int height = bottom - y;
 
   // 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.
   const ui::ThemeProvider* tp = GetThemeProvider();
   const SkColor toolbar_color = tp->GetColor(ThemeProperties::COLOR_TOOLBAR);
   if (!md)
     FillClientEdgeRects(x, y, right, bottom, toolbar_color, canvas);
@@ skipping 22 matching lines @@
                                                 gfx::Canvas* canvas) const {
   gfx::Rect side(x - kClientEdgeThickness, y, kClientEdgeThickness,
                  bottom + kClientEdgeThickness - y);
   canvas->FillRect(side, color);
   canvas->FillRect(gfx::Rect(x, bottom, right - x, kClientEdgeThickness),
                    color);
   side.set_x(right);
   canvas->FillRect(side, color);
 }
 
-void GlassBrowserFrameView::LayoutNewStyleAvatar() {
+void GlassBrowserFrameView::LayoutProfileSwitcher() {
   DCHECK(browser_view()->IsRegularOrGuestSession());
   if (!profile_switcher_.view())
     return;
 
   gfx::Size label_size = profile_switcher_.view()->GetPreferredSize();
 
   int button_x = frame()->GetMinimizeButtonOffset() -
-      kNewAvatarButtonOffset - label_size.width();
-  if (base::i18n::IsRTL())
+                 kProfileSwitcherButtonOffset - label_size.width();
+  if (CaptionButtonsOnLeadingEdge())
     button_x = width() - frame()->GetMinimizeButtonOffset() +
-        kNewAvatarButtonOffset;
+               kProfileSwitcherButtonOffset;
 
-  // The caption button position and size is confusing.  In maximized mode, the
-  // caption buttons are SM_CYMENUSIZE pixels high and are placed
-  // FrameTopBorderHeight() pixels from the top of the window; all those top
-  // border pixels are offscreen, so this result in caption buttons flush with
-  // the top of the screen.  In restored mode, the caption buttons are first
-  // placed just below a 2 px border at the top of the window (which is the
-  // first two pixels' worth of FrameTopBorderHeight()), then extended upwards
-  // one extra pixel to overlap part of this border.
-  //
-  // To match both of these, we size the button as if it's always the extra one
-  // pixel in height, then we place it at the correct position in restored mode,
-  // or one pixel above the top of the screen in maximized mode.
-  int button_y = frame()->IsMaximized() ? (FrameTopBorderHeight(false) - 1) : 1;
-  profile_switcher_.view()->SetBounds(
-      button_x, button_y, label_size.width(),
-      gfx::win::GetSystemMetricsInDIP(SM_CYMENUSIZE) + 1);
+  const int button_y = CaptionButtonY();
+  const int button_h = frame()->IsMaximized()
+                           ? kProfileSwitcherButtonHeightMaximized
+                           : kProfileSwitcherButtonHeightRestored;
+  profile_switcher_.view()->SetBounds(button_x, button_y, label_size.width(),
+                                      button_h);
 }
 
 void GlassBrowserFrameView::LayoutIncognitoIcon() {
   const bool md = ui::MaterialDesignController::IsModeMaterial();
   const gfx::Insets insets(GetLayoutInsets(AVATAR_ICON));
   const gfx::Size size(GetOTRAvatarIcon().size());
-  int x = NonClientBorderThickness(false);
+  int x = ClientBorderThickness(false);
   // In RTL, the icon needs to start after the caption buttons.
-  if (base::i18n::IsRTL()) {
+  if (CaptionButtonsOnLeadingEdge()) {
     x = width() - frame()->GetMinimizeButtonOffset() +
-        (profile_switcher_.view()
-             ? (profile_switcher_.view()->width() + kNewAvatarButtonOffset)
-             : 0);
+        (profile_switcher_.view() ? (profile_switcher_.view()->width() +
+                                     kProfileSwitcherButtonOffset)
+                                  : 0);
   } else if (!md && !avatar_button() && 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()) : FrameTopBorderHeight(false);
+  const int y = (md || !frame()->IsMaximized())
+                    ? (bottom - size.height())
+                    : NonClientTopBorderThickness(false);
   incognito_bounds_.SetRect(x + (avatar_button() ? insets.left() : 0), y,
                             avatar_button() ? size.width() : 0, bottom - y);
   if (avatar_button())
     avatar_button()->SetBoundsRect(incognito_bounds_);
 }
 
 void GlassBrowserFrameView::LayoutClientView() {
   client_view_bounds_ = CalculateClientAreaBounds();
 }
 
 gfx::Insets GlassBrowserFrameView::GetClientAreaInsets(bool restored) const {
   if (!browser_view()->IsTabStripVisible())
     return gfx::Insets();
 
-  const int top_height = NonClientTopBorderHeight(restored);
-  const int border_thickness = NonClientBorderThickness(restored);
+  const int top_height = NonClientTopHeight(restored);
+  const int border_thickness = ClientBorderThickness(restored);
   return gfx::Insets(top_height,
                      border_thickness,
                      border_thickness,
                      border_thickness);
 }
 
 gfx::Rect GlassBrowserFrameView::CalculateClientAreaBounds() const {
   gfx::Rect bounds(GetLocalBounds());
   bounds.Inset(GetClientAreaInsets(false));
   return bounds;
@@ skipping 73 matching lines @@
   static bool initialized = false;
   if (!initialized) {
     for (int i = 0; i < kThrobberIconCount; ++i) {
       throbber_icons_[i] =
           ui::LoadThemeIconFromResourcesDataDLL(IDI_THROBBER_01 + i);
       DCHECK(throbber_icons_[i]);
     }
     initialized = true;
   }
 }