[Extensions Toolbar] Protect against crazy bounds

chrome/browser/ui/views/toolbar/toolbar_view.cc

Index: chrome/browser/ui/views/toolbar/toolbar_view.cc
diff --git a/chrome/browser/ui/views/toolbar/toolbar_view.cc b/chrome/browser/ui/views/toolbar/toolbar_view.cc
index 5f1b2a608f0a7b92ac4182509a32659576a85043..21dea04eabd9baad1baa53a9309ca6aa13a61adb 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_view.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_view.cc
@@ -604,21 +604,30 @@ void ToolbarView::Layout() {
                    theme_provider->GetDisplayProperty(
                        ThemeProperties::PROPERTY_TOOLBAR_VIEW_STANDARD_SPACING);
 
-  int browser_actions_width = browser_actions_->GetPreferredSize().width();
+  int browser_actions_desired_width =
+      browser_actions_->GetPreferredSize().width();
   int app_menu_width = app_menu_->GetPreferredSize().width();
   const int right_edge_spacing = theme_provider->GetDisplayProperty(
       ThemeProperties::PROPERTY_TOOLBAR_VIEW_RIGHT_EDGE_SPACING);
   const int location_bar_right_padding = theme_provider->GetDisplayProperty(
       ThemeProperties::PROPERTY_TOOLBAR_VIEW_LOCATION_BAR_RIGHT_PADDING);
+
+  // Don't allow the omnibox to shrink to the point of non-existence.
+  int kMinimumLocationBarWidth = 100;

[Peter Kasting, 2015/09/15 00:13:38]

(1) You should probably BUG=58915 in your CL description
(2) Use GetMinimumSize() on various elements (e.g. location_bar_) rather than
hardcoding values here

[Devlin, 2015/09/15 18:22:39]

1) Wow, that's an old one.  Good find.
2) Done.

   int available_width = std::max(
-      0, width() - right_edge_spacing - app_menu_width - browser_actions_width -
-             (browser_actions_width > 0 ? element_padding : 0) -
-             location_bar_right_padding - next_element_x);
+      0, width() - right_edge_spacing - app_menu_width -
+             (browser_actions_desired_width > 0 ? element_padding : 0) -
+             location_bar_right_padding - next_element_x -
+             kMinimumLocationBarWidth);
+  int browser_actions_width =
+      std::min(available_width, browser_actions_desired_width);
+  available_width -= browser_actions_width;

[Peter Kasting, 2015/09/15 00:13:39]

This gives the browser actions area as much space as it wants up to the limit of
making the location bar really small.  Maybe if people haven't manually sized
this area we should instead limit the space this takes up to e.g. half the
toolbar width or something?  After all, even if you have a minimum width for the
location bar to avoid glitchy-looking UI, it's still not going to be much fun to
use.

[Devlin, 2015/09/15 18:22:39]

Hmm... I'd like to avoid this for now.  For one thing, it's more consistent with
current behavior, and I'd be worried that it'd be hard to find the right balance
for a minimum without a lot of experimentation, and would probably require a
little UI review, which would slow this down a lot.

More importantly, though, most of our users have relatively few extensions (over
90% have 3 or fewer), so the number of users for whom this would have an effect
is pretty small, and is also (probably?) more likely to be the power users that
already know they can resize it.  I'd like to avoid any "implicit" resizing,
unless absolutely necessary for usability (as in the case of the omnibox
shrinking to nothingness).

+  int location_bar_width = kMinimumLocationBarWidth + available_width;

[Peter Kasting, 2015/09/15 00:13:39]

This statement seems wrong.  Why are we adding in kMinimumLocationBarWidth?

[Devlin, 2015/09/15 18:22:39]

Above, we subtract out the minimum location bar width from the available width,
so the "leftover" width is the available width + the minimum location bar width.
 I've added a comment to explain this a bit more.

 
   int location_height = location_bar_->GetPreferredSize().height();
   int location_y = (height() - location_height + 1) / 2;
   location_bar_->SetBounds(next_element_x, location_y,
-                           std::max(available_width, 0), location_height);
+                           location_bar_width, location_height);
   next_element_x = location_bar_->bounds().right() + location_bar_right_padding;
 
   browser_actions_->SetBounds(