Account for minimum tabstrip width in opaque browser frame.

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

Index: chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
===================================================================
--- chrome/browser/ui/views/frame/opaque_browser_frame_view.cc	(revision 145906)
+++ chrome/browser/ui/views/frame/opaque_browser_frame_view.cc	(working copy)
@@ -221,17 +221,14 @@
   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(
@@ -267,6 +264,18 @@
   }
 #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;
 }
 
@@ -567,6 +576,19 @@
   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());