Account for minimum tabstrip width in opaque browser frame.

chrome/browser/ui/views/frame/opaque_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/opaque_browser_frame_view.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/command_line.h"
@@ skipping 203 matching lines @@
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // OpaqueBrowserFrameView, BrowserNonClientFrameView implementation:
 
 gfx::Rect OpaqueBrowserFrameView::GetBoundsForTabStrip(
     views::View* tabstrip) const {
   if (!tabstrip)
     return gfx::Rect();
 
-  int tabstrip_x = browser_view()->ShouldShowAvatar() ?
-      (avatar_bounds_.right() + kAvatarRightSpacing) :
-      NonClientBorderThickness() + kTabStripIndent;
-
-  int maximized_spacing = kNewTabCaptionMaximizedSpacing;
-  int tabstrip_width =
-      (minimize_button_ ? minimize_button_->x() : width()) - tabstrip_x -
-          (frame()->IsMaximized() ?
-              maximized_spacing : kNewTabCaptionRestoredSpacing);
-  return gfx::Rect(tabstrip_x, GetHorizontalTabStripVerticalOffset(false),
-      std::max(0, tabstrip_width), tabstrip->GetPreferredSize().height());
+  gfx::Rect bounds = GetBoundsForTabStripAndAvatarArea(tabstrip);
+  if (browser_view()->ShouldShowAvatar()) {

[Peter Kasting, 2012/07/10 20:34:27]

Nit: Simpler:

  // Exclude avatar region, if present.
  if (browser_view()->ShouldShowAvatar())
    bounds.Inset(avatar_bounds_.width() + kAvatarRightSpacing, 0, 0, 0);

[Alexei Svitkine (slow), 2012/07/10 20:51:47]

That doesn't work, since that would be adding (avatar_bounds_.width() +
kAvatarRightSpacing) to the existing |x| value, instead of replacing it. It
would need to take the difference of the two if it were to pass it to Inset(),
at which point it's not really simpler.

[Peter Kasting, 2012/07/10 22:47:11]

Look carefully: I'm insetting by width(), not by right().  I'm already taking
the "difference" into account, so to speak.

+    // Adjust tab strip bounds to make room for the avatar.
+    const int right = bounds.right();
+    bounds.set_x(avatar_bounds_.right() + kAvatarRightSpacing);
+    bounds.set_width(right - bounds.x());
+  }
+  return bounds;
 }
 
 int OpaqueBrowserFrameView::GetHorizontalTabStripVerticalOffset(
     bool restored) const {
   return NonClientTopBorderHeight(restored) + ((!restored &&
       (frame()->IsMaximized() ||
       frame()->IsFullscreen())) ?
       0 : kNonClientRestoredExtraThickness);
 }
 
@@ skipping 15 matching lines @@
        (IconSize() + kTitleLogoSpacing) : 0);
 #if !defined(OS_CHROMEOS)
   if (ShouldAddDefaultCaptionButtons()) {
     min_titlebar_width +=
         minimize_button_->GetMinimumSize().width() +
         restore_button_->GetMinimumSize().width() +
         close_button_->GetMinimumSize().width();
   }
 #endif
   min_size.set_width(std::max(min_size.width(), min_titlebar_width));
+
+  // Ensure that the minimum width is enough to hold a minimum width tab strip
+  // and avatar icon at their usual insets.
+  if (browser_view()->IsTabStripVisible()) {
+    TabStrip* tabstrip = browser_view()->tabstrip();
+    const int min_tabstrip_width = tabstrip->GetMinimumSize().width();
+    const int min_tabstrip_area_width =
+        width() - GetBoundsForTabStripAndAvatarArea(tabstrip).width() +
+        min_tabstrip_width + browser_view()->GetOTRAvatarIcon().width();

[Peter Kasting, 2012/07/10 20:34:27]

Don't you also need kAvatarRightSpacing in here?

[Alexei Svitkine (slow), 2012/07/10 20:51:47]

You're right. Done.

+    min_size.set_width(std::max(min_tabstrip_area_width, min_size.width()));

[Peter Kasting, 2012/07/10 20:34:27]

Tiny nit: For consistency with the max() call above, reverse these two args.

[Alexei Svitkine (slow), 2012/07/10 20:51:47]

Done.

+  }
+
   return min_size;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // OpaqueBrowserFrameView, views::NonClientFrameView implementation:
 
 gfx::Rect OpaqueBrowserFrameView::GetBoundsForClientView() const {
   return client_view_bounds_;
 }
 
@@ skipping 280 matching lines @@
     y = unavailable_px_at_top + (NonClientTopBorderHeight(false) -
         unavailable_px_at_top - size - TitlebarBottomThickness(false) + 1) / 2;
   } else {
     // For "browser mode" windows, we use the native positioning, which is just
     // below the top frame border.
     y = frame_thickness;
   }
   return gfx::Rect(frame_thickness + kIconLeftSpacing, y, size, size);
 }
 
+gfx::Rect OpaqueBrowserFrameView::GetBoundsForTabStripAndAvatarArea(
+    views::View* tabstrip) const {
+  const int available_width =

[Peter Kasting, 2012/07/10 20:34:27]

Nit: OK to drop parens on these next two statements

[Alexei Svitkine (slow), 2012/07/10 20:51:47]

Done.

+      (minimize_button_ ? minimize_button_->x() : width());
+  const int caption_spacing = (frame()->IsMaximized() ?
+      kNewTabCaptionMaximizedSpacing : kNewTabCaptionRestoredSpacing);
+  const int tabstrip_x = NonClientBorderThickness() + kTabStripIndent;
+  const int tabstrip_width = available_width - tabstrip_x - caption_spacing;
+  return gfx::Rect(tabstrip_x, GetHorizontalTabStripVerticalOffset(false),
+                   std::max(0, tabstrip_width),

[Peter Kasting, 2012/07/10 20:34:27]

Nit: Also OK to indent 4 and combine next line with this one

[Alexei Svitkine (slow), 2012/07/10 20:51:47]

I feel that ends up being a bit too crowded in this case, so I've left it as is.

+                   tabstrip->GetPreferredSize().height());
+}
+
 void OpaqueBrowserFrameView::PaintRestoredFrameBorder(gfx::Canvas* canvas) {
   frame_background_->set_frame_color(GetFrameColor());
   frame_background_->set_theme_image(GetFrameImage());
   frame_background_->set_theme_overlay_image(GetFrameOverlayImage());
   frame_background_->set_top_area_height(GetTopAreaHeight());
 
   ui::ThemeProvider* tp = GetThemeProvider();
   frame_background_->SetSideImages(
       tp->GetImageSkiaNamed(IDR_WINDOW_LEFT_SIDE),
       tp->GetImageSkiaNamed(IDR_WINDOW_TOP_CENTER),
@@ skipping 427 matching lines @@
 
 gfx::Rect OpaqueBrowserFrameView::CalculateClientAreaBounds(int width,
                                                             int height) const {
   int top_height = NonClientTopBorderHeight(false);
   int border_thickness = NonClientBorderThickness();
   return gfx::Rect(border_thickness, top_height,
                    std::max(0, width - (2 * border_thickness)),
                    std::max(0, height - GetReservedHeight() -
                        top_height - border_thickness));
 }