Stop LabelButton's minimum size from increasing to its preferred size.

When a LabelButton changes its label from a longer to a shorter string
it would not decrease in size. This patch removes that code and cleans
up the tests and related code.

Bug: 790808
Change-Id: I6fce4b3ecf3335c2a9d31bc2a3ac73e18f1e92a8
Reviewed-on: https://chromium-review.googlesource.com/798323
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521021}

chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

@@ -1726,7 +1726,6 @@ void BookmarkBarView::ConfigureButton(const BookmarkNode* node,
     }
   }
 
-  button->SetMinSize(gfx::Size());
   button->set_context_menu_controller(this);
   button->set_drag_controller(this);
   if (node->is_url()) {

chrome/browser/ui/views/page_info/permission_selector_row.cc

@@ -400,14 +400,12 @@ void PermissionSelectorRow::PermissionChanged(
 
   // Update the menu button text to reflect the new setting.
   if (menu_button_) {
-    menu_button_->SetText(PageInfoUI::PermissionActionToUIString(
-        profile_, permission.type, permission.setting,
-        permission.default_setting, content_settings::SETTING_SOURCE_USER));
-    menu_button_->SizeToPreferredSize();
     // Re-layout will be done at the |PageInfoBubbleView| level, since
     // that view may need to resize itself to accomodate the new sizes of its
     // contents.
-    menu_button_->InvalidateLayout();
+    menu_button_->SetText(PageInfoUI::PermissionActionToUIString(
+        profile_, permission.type, permission.setting,
+        permission.default_setting, content_settings::SETTING_SOURCE_USER));
   } else if (combobox_) {
     bool use_default = permission.setting == CONTENT_SETTING_DEFAULT;
     combobox_->UpdateSelectedIndex(use_default);

chrome/browser/ui/views/profiles/avatar_button.cc

@@ -449,9 +449,6 @@ void AvatarButton::Update() {
           : gfx::ShadowValues(
                 10, gfx::ShadowValue(gfx::Vector2d(), 2.0f, SK_ColorDKGRAY)));
 
-  // We want the button to resize if the new text is shorter.
-  SetMinSize(gfx::Size());
-
   if (use_generic_button) {
     SetImage(views::Button::STATE_NORMAL, generic_avatar_);
   } else if (error_controller_.HasAvatarError()) {

chrome/browser/ui/views/translate/translate_bubble_view.cc

@@ -971,28 +971,15 @@ void TranslateBubbleView::SwitchToErrorView(
 }
 
 void TranslateBubbleView::UpdateAdvancedView() {
-  DCHECK(source_language_combobox_);
-  DCHECK(target_language_combobox_);
-  DCHECK(advanced_done_button_);
-
-  base::string16 source_language_name =
-      model_->GetLanguageNameAt(model_->GetOriginalLanguageIndex());
-  base::string16 target_language_name =
-      model_->GetLanguageNameAt(model_->GetTargetLanguageIndex());
-
   // "Always translate" checkbox doesn't exist in an incognito window.
   if (advanced_always_translate_checkbox_) {
     advanced_always_translate_checkbox_->SetText(
         l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ALWAYS));
   }
 
-  base::string16 label;
-  if (model_->IsPageTranslatedInCurrentLanguages())
-    label = l10n_util::GetStringUTF16(IDS_DONE);
-  else
-    label = l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ACCEPT);
-  advanced_done_button_->SetText(label);
-  advanced_done_button_->SizeToPreferredSize();
-  if (advanced_view_)
-    advanced_view_->Layout();
+  DCHECK(advanced_done_button_);
+  advanced_done_button_->SetText(
+      l10n_util::GetStringUTF16(model_->IsPageTranslatedInCurrentLanguages()
+                                    ? IDS_DONE
+                                    : IDS_TRANSLATE_BUBBLE_ACCEPT));
 }

ui/views/controls/button/label_button.cc

@@ -207,9 +207,7 @@ gfx::Size LabelButton::CalculatePreferredSize() const {
   size.set_height(std::max(preferred_label_size.height() + GetInsets().height(),
                            size.height()));
 
-  // Increase the minimum size monotonically with the preferred size.
   size.SetToMax(min_size_);
-  min_size_ = size;
 
   // Clamp size to max size (if valid).
   if (max_size_.width() > 0)

ui/views/controls/button/label_button.h

@@ -69,7 +69,6 @@ class VIEWS_EXPORT LabelButton : public Button, public NativeThemeDelegate {
   // optional image will lead the text, unless the button is right-aligned.
   void SetHorizontalAlignment(gfx::HorizontalAlignment alignment);
 
-  // Call SetMinSize(gfx::Size()) to clear the monotonically increasing size.
   void SetMinSize(const gfx::Size& min_size);
   void SetMaxSize(const gfx::Size& max_size);
 
@@ -83,9 +82,7 @@ class VIEWS_EXPORT LabelButton : public Button, public NativeThemeDelegate {
   ButtonStyle style() const { return style_; }
   void SetStyleDeprecated(ButtonStyle style);
 
-  // Sets the spacing between the image and the text. Shrinking the spacing
-  // will not shrink the overall button size, as it is monotonically increasing.
-  // Call SetMinSize(gfx::Size()) to clear the size if needed.
+  // Sets the spacing between the image and the text.
   void SetImageLabelSpacing(int spacing);
 
   // Creates the default border for this button. This can be overridden by
@@ -214,9 +211,8 @@ class VIEWS_EXPORT LabelButton : public Button, public NativeThemeDelegate {
   // Used to track whether SetTextColor() has been invoked.
   std::array<bool, STATE_COUNT> explicitly_set_colors_;
 
-  // |min_size_| increases monotonically with the preferred size.
-  mutable gfx::Size min_size_;
-  // |max_size_| may be set to clamp the preferred size.
+  // |min_size_| and |max_size_| may be set to clamp the preferred size.
+  gfx::Size min_size_;
   gfx::Size max_size_;
 
   // Cache the last computed preferred size.

ui/views/controls/button/label_button_unittest.cc

@@ -154,7 +154,6 @@ TEST_F(LabelButtonTest, Label) {
   const int short_text_width = gfx::GetStringWidth(short_text, font_list);
   const int long_text_width = gfx::GetStringWidth(long_text, font_list);
 
-  // The width increases monotonically with string size (it does not shrink).
   EXPECT_LT(button_->GetPreferredSize().width(), short_text_width);
   button_->SetText(short_text);
   EXPECT_GT(button_->GetPreferredSize().height(), font_list.GetHeight());
@@ -163,16 +162,20 @@ TEST_F(LabelButtonTest, Label) {
   button_->SetText(long_text);
   EXPECT_GT(button_->GetPreferredSize().width(), long_text_width);
   button_->SetText(short_text);
-  EXPECT_GT(button_->GetPreferredSize().width(), long_text_width);
+  EXPECT_GT(button_->GetPreferredSize().width(), short_text_width);
+  EXPECT_LT(button_->GetPreferredSize().width(), long_text_width);
 
   // Clamp the size to a maximum value.
-  button_->SetMaxSize(gfx::Size(long_text_width, 1));
-  EXPECT_EQ(button_->GetPreferredSize(), gfx::Size(long_text_width, 1));
+  button_->SetText(long_text);
+  button_->SetMaxSize(gfx::Size(short_text_width, 1));
+  EXPECT_EQ(button_->GetPreferredSize(), gfx::Size(short_text_width, 1));
 
-  // Clear the monotonically increasing minimum size.
-  button_->SetMinSize(gfx::Size());
-  EXPECT_GT(button_->GetPreferredSize().width(), short_text_width);
-  EXPECT_LT(button_->GetPreferredSize().width(), long_text_width);
+  // Clamp the size to a minimum value.
+  button_->SetText(short_text);
+  button_->SetMaxSize(gfx::Size());
+  button_->SetMinSize(gfx::Size(long_text_width, font_list.GetHeight() * 2));
+  EXPECT_EQ(button_->GetPreferredSize(),
+            gfx::Size(long_text_width, font_list.GetHeight() * 2));
 }
 
 // Test behavior of View::GetAccessibleNodeData() for buttons when setting a
@@ -212,7 +215,6 @@ TEST_F(LabelButtonTest, Image) {
   const gfx::ImageSkia small_image = CreateTestImage(small_size, small_size);
   const gfx::ImageSkia large_image = CreateTestImage(large_size, large_size);
 
-  // The width increases monotonically with image size (it does not shrink).
   EXPECT_LT(button_->GetPreferredSize().width(), small_size);
   EXPECT_LT(button_->GetPreferredSize().height(), small_size);
   button_->SetImage(Button::STATE_NORMAL, small_image);
@@ -224,17 +226,21 @@ TEST_F(LabelButtonTest, Image) {
   EXPECT_GT(button_->GetPreferredSize().width(), large_size);
   EXPECT_GT(button_->GetPreferredSize().height(), large_size);
   button_->SetImage(Button::STATE_NORMAL, small_image);
-  EXPECT_GT(button_->GetPreferredSize().width(), large_size);
-  EXPECT_GT(button_->GetPreferredSize().height(), large_size);
+  EXPECT_GT(button_->GetPreferredSize().width(), small_size);
+  EXPECT_GT(button_->GetPreferredSize().height(), small_size);
+  EXPECT_LT(button_->GetPreferredSize().width(), large_size);
+  EXPECT_LT(button_->GetPreferredSize().height(), large_size);
 
   // Clamp the size to a maximum value.
+  button_->SetImage(Button::STATE_NORMAL, large_image);
   button_->SetMaxSize(gfx::Size(large_size, 1));
   EXPECT_EQ(button_->GetPreferredSize(), gfx::Size(large_size, 1));
 
-  // Clear the monotonically increasing minimum size.
-  button_->SetMinSize(gfx::Size());
-  EXPECT_GT(button_->GetPreferredSize().width(), small_size);
-  EXPECT_LT(button_->GetPreferredSize().width(), large_size);
+  // Clamp the size to a minimum value.
+  button_->SetImage(Button::STATE_NORMAL, small_image);
+  button_->SetMaxSize(gfx::Size());
+  button_->SetMinSize(gfx::Size(large_size, large_size));
+  EXPECT_EQ(button_->GetPreferredSize(), gfx::Size(large_size, large_size));
 }
 
 TEST_F(LabelButtonTest, LabelAndImage) {
@@ -246,7 +252,6 @@ TEST_F(LabelButtonTest, LabelAndImage) {
   const gfx::ImageSkia image = CreateTestImage(image_size, image_size);
   ASSERT_LT(font_list.GetHeight(), image_size);
 
-  // The width increases monotonically with content size (it does not shrink).
   EXPECT_LT(button_->GetPreferredSize().width(), text_width);
   EXPECT_LT(button_->GetPreferredSize().width(), image_size);
   EXPECT_LT(button_->GetPreferredSize().height(), image_size);
@@ -279,21 +284,24 @@ TEST_F(LabelButtonTest, LabelAndImage) {
   EXPECT_LT(button_->label()->bounds().right(), button_->image()->bounds().x());
 
   button_->SetText(base::string16());
-  EXPECT_GT(button_->GetPreferredSize().width(), text_width + image_size);
+  EXPECT_LT(button_->GetPreferredSize().width(), text_width + image_size);
+  EXPECT_GT(button_->GetPreferredSize().width(), image_size);
   EXPECT_GT(button_->GetPreferredSize().height(), image_size);
   button_->SetImage(Button::STATE_NORMAL, gfx::ImageSkia());
-  EXPECT_GT(button_->GetPreferredSize().width(), text_width + image_size);
-  EXPECT_GT(button_->GetPreferredSize().height(), image_size);
+  EXPECT_LT(button_->GetPreferredSize().width(), image_size);
+  EXPECT_LT(button_->GetPreferredSize().height(), image_size);
 
-  // Clamp the size to a maximum value.
-  button_->SetMaxSize(gfx::Size(image_size, 1));
-  EXPECT_EQ(button_->GetPreferredSize(), gfx::Size(image_size, 1));
+  // Clamp the size to a minimum value.
+  button_->SetText(text);
+  button_->SetImage(Button::STATE_NORMAL, image);
+  button_->SetMinSize(gfx::Size((text_width + image_size) * 2, image_size * 2));
+  EXPECT_EQ(button_->GetPreferredSize().width(), (text_width + image_size) * 2);
+  EXPECT_EQ(button_->GetPreferredSize().height(), image_size * 2);
 
-  // Clear the monotonically increasing minimum size.
+  // Clamp the size to a maximum value.
   button_->SetMinSize(gfx::Size());
-  EXPECT_LT(button_->GetPreferredSize().width(), text_width);
-  EXPECT_LT(button_->GetPreferredSize().width(), image_size);
-  EXPECT_LT(button_->GetPreferredSize().height(), image_size);
+  button_->SetMaxSize(gfx::Size(1, 1));
+  EXPECT_EQ(button_->GetPreferredSize(), gfx::Size(1, 1));
 }
 
 // This test was added because GetHeightForWidth and GetPreferredSize were
@@ -326,10 +334,6 @@ TEST_F(LabelButtonTest, GetHeightForWidthConsistentWithGetPreferredSize) {
     EXPECT_EQ(std::max(image_size, font_height) + button_->GetInsets().height(),
               preferred_button_size.height());
 
-    // Clear min size, this ensures that GetHeightForWidth() is consistent on
-    // its own and not because min_size_ is set to the preferred size.
-    button_->SetMinSize(gfx::Size());
-
     // Make sure this preferred height is consistent with GetHeightForWidth().
     EXPECT_EQ(preferred_button_size.height(),
               button_->GetHeightForWidth(preferred_button_size.width()));
@@ -399,7 +403,6 @@ TEST_F(LabelButtonTest, ChangeTextSize) {
 
   // The button and the label view return to its original size when the original
   // text is restored.
-  button_->SetMinSize(gfx::Size());
   button_->SetText(text);
   EXPECT_EQ(original_label_width, button_->label()->bounds().width());
   EXPECT_EQ(original_width, button_->GetPreferredSize().width());
@@ -418,7 +421,6 @@ TEST_F(LabelButtonTest, ChangeLabelImageSpacing) {
   EXPECT_GT(button_->GetPreferredSize().width(), original_width);
 
   // The button shrinks if the original spacing is restored.
-  button_->SetMinSize(gfx::Size());
   button_->SetImageLabelSpacing(kOriginalSpacing);
   EXPECT_EQ(original_width, button_->GetPreferredSize().width());
 }

ui/views/examples/button_example.cc

@@ -113,8 +113,6 @@ void ButtonExample::LabelButtonPressed(LabelButton* label_button,
     }
   } else if (event.IsAltDown()) {
     label_button->SetIsDefault(!label_button->is_default());
-  } else {
-    label_button->SetMinSize(gfx::Size());
   }
   example_view()->GetLayoutManager()->Layout(example_view());
 }