LabelButton: cache the last computed preferred size

ui/views/controls/button/label_button.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 "ui/views/controls/button/label_button.h"
 
 #include "base/logging.h"
 #include "grit/ui_resources.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/animation/throb_animation.h"
@@ skipping 62 matching lines @@
 
 const gfx::ImageSkia& LabelButton::GetImage(ButtonState for_state) {
   if (for_state != STATE_NORMAL && button_state_images_[for_state].isNull())
     return button_state_images_[STATE_NORMAL];
   return button_state_images_[for_state];
 }
 
 void LabelButton::SetImage(ButtonState for_state, const gfx::ImageSkia& image) {
   button_state_images_[for_state] = image;
   UpdateImage();
+  ResetCachedSize();
 }
 
 const base::string16& LabelButton::GetText() const {
   return label_->text();
 }
 
 void LabelButton::SetText(const base::string16& text) {
   SetAccessibleName(text);
   label_->SetText(text);
+  ResetCachedSize();
 }
 
 void LabelButton::SetTextColor(ButtonState for_state, SkColor color) {
   button_state_colors_[for_state] = color;
   if (for_state == STATE_DISABLED)
     label_->SetDisabledColor(color);
   else if (for_state == state())
     label_->SetEnabledColor(color);
   explicitly_set_colors_[for_state] = true;
 }
 
 void LabelButton::SetTextShadows(const gfx::ShadowValues& shadows) {
   label_->set_shadows(shadows);
+  ResetCachedSize();
 }
 
 void LabelButton::SetTextSubpixelRenderingEnabled(bool enabled) {
   label_->set_subpixel_rendering_enabled(enabled);
 }
 
 bool LabelButton::GetTextMultiLine() const {
   return label_->is_multi_line();
 }
 
 void LabelButton::SetTextMultiLine(bool text_multi_line) {
   label_->SetMultiLine(text_multi_line);
+  ResetCachedSize();
 }
 
 const gfx::FontList& LabelButton::GetFontList() const {
   return label_->font_list();
 }
 
 void LabelButton::SetFontList(const gfx::FontList& font_list) {
   cached_normal_font_list_ = font_list;
   cached_bold_font_list_ = font_list.DeriveWithStyle(
       font_list.GetFontStyle() | gfx::Font::BOLD);
 
   // STYLE_BUTTON uses bold text to indicate default buttons.
   label_->SetFontList(
       style_ == STYLE_BUTTON && is_default_ ?
       cached_bold_font_list_ : cached_normal_font_list_);
+  ResetCachedSize();
 }
 
 void LabelButton::SetElideBehavior(gfx::ElideBehavior elide_behavior) {
   label_->SetElideBehavior(elide_behavior);
+  ResetCachedSize();
 }
 
 gfx::HorizontalAlignment LabelButton::GetHorizontalAlignment() const {
   return label_->GetHorizontalAlignment();
 }
 
 void LabelButton::SetHorizontalAlignment(gfx::HorizontalAlignment alignment) {
   label_->SetHorizontalAlignment(alignment);
   InvalidateLayout();
 }
@@ skipping 33 matching lines @@
     set_min_size(gfx::Size(70, 33));
 
   OnNativeThemeChanged(GetNativeTheme());
 }
 
 void LabelButton::SetFocusPainter(scoped_ptr<Painter> focus_painter) {
   focus_painter_ = focus_painter.Pass();
 }
 
 gfx::Size LabelButton::GetPreferredSize() const {
-  // Use a temporary label copy for sizing to avoid calculation side-effects.
-  Label label(GetText(), cached_normal_font_list_);
-  label.set_shadows(label_->shadows());
-  label.SetMultiLine(GetTextMultiLine());
+  if (!button_size_valid_) {
+    // Use a temporary label copy for sizing to avoid calculation side-effects.
+    Label label(GetText(), cached_normal_font_list_);
+    label.set_shadows(label_->shadows());
+    label.SetMultiLine(GetTextMultiLine());
 
-  if (style() == STYLE_BUTTON) {
-    // Some text appears wider when rendered normally than when rendered bold.
-    // Accommodate the widest, as buttons may show bold and shouldn't resize.
-    const int current_width = label.GetPreferredSize().width();
-    label.SetFontList(cached_bold_font_list_);
-    if (label.GetPreferredSize().width() < current_width)
-      label.SetFontList(cached_normal_font_list_);
+    if (style() == STYLE_BUTTON) {
+      // Some text appears wider when rendered normally than when rendered bold.
+      // Accommodate the widest, as buttons may show bold and shouldn't resize.
+      const int current_width = label.GetPreferredSize().width();
+      label.SetFontList(cached_bold_font_list_);
+      if (label.GetPreferredSize().width() < current_width)
+        label.SetFontList(cached_normal_font_list_);
+    }
+
+    // Resize multi-line labels given the current limited available width.

[Evan Stade, 2014/07/15 19:10:06]

by the way, I don't think the code should be resizing the label in
GetPreferredSize() --- that should be part of Layout. Also, calls to width()
here are suspect because the preferred size shouldn't depend on the current
size.

[msw, 2014/07/15 22:51:43]

This is addressed in my WIP https://codereview.chromium.org/23228004/

[noms (inactive), 2014/07/21 15:40:43]

Done.

+    const gfx::Size image_size(image_->GetPreferredSize());
+    const int image_width = image_size.width();
+    if (GetTextMultiLine() && (width() > image_width + kSpacing))
+      label.SizeToFit(width() - image_width - (image_width > 0 ? kSpacing : 0));
+
+    // Calculate the required size.
+    gfx::Size size(label.GetPreferredSize());
+    if (image_width > 0 && size.width() > 0)
+      size.Enlarge(kSpacing, 0);
+    size.SetToMax(gfx::Size(0, image_size.height()));
+    const gfx::Insets insets(GetInsets());
+    size.Enlarge(image_size.width() + insets.width(), insets.height());
+
+    // Make the size at least as large as the minimum size needed by the border.
+    size.SetToMax(border() ? border()->GetMinimumSize() : gfx::Size());
+
+    // Increase the minimum size monotonically with the preferred size.
+    size.SetToMax(min_size_);
+    min_size_ = size;
+
+    // Return the largest known size clamped to the maximum size (if valid).
+    if (max_size_.width() > 0)
+      size.set_width(std::min(max_size_.width(), size.width()));
+    if (max_size_.height() > 0)
+      size.set_height(std::min(max_size_.height(), size.height()));
+
+    // Cache this computed size, as recomputing it is an expensive operation.
+    button_size_valid_ = true;
+    button_size_ = size;
   }
-
-  // Resize multi-line labels given the current limited available width.
-  const gfx::Size image_size(image_->GetPreferredSize());
-  const int image_width = image_size.width();
-  if (GetTextMultiLine() && (width() > image_width + kSpacing))
-    label.SizeToFit(width() - image_width - (image_width > 0 ? kSpacing : 0));
-
-  // Calculate the required size.
-  gfx::Size size(label.GetPreferredSize());
-  if (image_width > 0 && size.width() > 0)
-    size.Enlarge(kSpacing, 0);
-  size.SetToMax(gfx::Size(0, image_size.height()));
-  const gfx::Insets insets(GetInsets());
-  size.Enlarge(image_size.width() + insets.width(), insets.height());
-
-  // Make the size at least as large as the minimum size needed by the border.
-  size.SetToMax(border() ? border()->GetMinimumSize() : gfx::Size());
-
-  // Increase the minimum size monotonically with the preferred size.
-  size.SetToMax(min_size_);
-  min_size_ = size;
-
-  // Return the largest known size clamped to the maximum size (if valid).
-  if (max_size_.width() > 0)
-    size.set_width(std::min(max_size_.width(), size.width()));
-  if (max_size_.height() > 0)
-    size.set_height(std::min(max_size_.height(), size.height()));
-  return size;
+  return button_size_;
 }
 
 void LabelButton::Layout() {
   gfx::HorizontalAlignment adjusted_alignment = GetHorizontalAlignment();
   if (base::i18n::IsRTL() && adjusted_alignment != gfx::ALIGN_CENTER)
     adjusted_alignment = (adjusted_alignment == gfx::ALIGN_LEFT) ?
         gfx::ALIGN_RIGHT : gfx::ALIGN_LEFT;
 
   gfx::Rect child_area(GetChildAreaBounds());
   child_area.Inset(GetInsets());
@@ skipping 205 matching lines @@
   GetExtraParams(params);
   return ui::NativeTheme::kNormal;
 }
 
 ui::NativeTheme::State LabelButton::GetForegroundThemeState(
     ui::NativeTheme::ExtraParams* params) const {
   GetExtraParams(params);
   return ui::NativeTheme::kHovered;
 }
 
+void LabelButton::ResetCachedSize() {
+  button_size_valid_ = false;
+  button_size_= gfx::Size();
+}
+
 }  // namespace views