LabelButton: cache the last computed preferred size

ui/views/controls/button/label_button.h

 // 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.
 
 #ifndef UI_VIEWS_CONTROLS_BUTTON_LABEL_BUTTON_H_
 #define UI_VIEWS_CONTROLS_BUTTON_LABEL_BUTTON_H_
 
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ skipping 47 matching lines @@
   void SetFontList(const gfx::FontList& font_list);
 
   // Set the elide behavior of this button.
   void SetElideBehavior(gfx::ElideBehavior elide_behavior);
 
   // Get or set the horizontal alignment used for the button; reversed in RTL.
   // The optional image will lead the text, unless the button is right-aligned.
   gfx::HorizontalAlignment GetHorizontalAlignment() const;
   void SetHorizontalAlignment(gfx::HorizontalAlignment alignment);
 
-  // Call set_min_size(gfx::Size()) to clear the monotonically increasing size.
-  void set_min_size(const gfx::Size& min_size) { min_size_ = min_size; }
-  void set_max_size(const gfx::Size& max_size) { max_size_ = max_size; }
+  // Call SetMinSize(gfx::Size()) to clear the monotonically increasing size.
+  void SetMinSize(const gfx::Size& min_size);
+  void SetMaxSize(const gfx::Size& min_size);

[msw, 2014/07/23 19:20:16]

nit: |max_size|

[noms (inactive), 2014/07/24 14:20:25]

Done.

 
   // Get or set the option to handle the return key; false by default.
   bool is_default() const { return is_default_; }
   void SetIsDefault(bool is_default);
 
   // Get or set the button's overall style; the default is |STYLE_TEXTBUTTON|.
   ButtonStyle style() const { return style_; }
   void SetStyle(ButtonStyle style);
 
   void SetFocusPainter(scoped_ptr<Painter> focus_painter);
@@ skipping 56 matching lines @@
   // NativeThemeDelegate:
   virtual ui::NativeTheme::Part GetThemePart() const OVERRIDE;
   virtual ui::NativeTheme::State GetThemeState(
       ui::NativeTheme::ExtraParams* params) const OVERRIDE;
   virtual const gfx::Animation* GetThemeAnimation() const OVERRIDE;
   virtual ui::NativeTheme::State GetBackgroundThemeState(
       ui::NativeTheme::ExtraParams* params) const OVERRIDE;
   virtual ui::NativeTheme::State GetForegroundThemeState(
       ui::NativeTheme::ExtraParams* params) const OVERRIDE;
 
+  // Resets |button_size_| and marks |button_size_valid_| as false.
+  void ResetCachedSize();

[sky, 2014/07/23 22:42:36]

Is it possible to override ChildPreferredSizeChanged to mark the cached state as
out of date? That way you don't need to explicitly reset this state as it's
implicitly communicated from the label.

[noms (inactive), 2014/07/24 14:20:26]

Hmm, but shouldn't actions on the LabelButton also invalidate the cached size?
For example changing the image or the border? It would definitely decrease the
calls to ResetCachedSize(), though, so I've done that.

On 2014/07/23 22:42:36, sky wrote:

[sky, 2014/07/24 15:38:05]

Yes, sorry, you may need a mix of the two. But it seems ideal to leverage
ChildPreferredSizeChanged as much as possible.

+
   // The image and label shown in the button.
   ImageView* image_;
   Label* label_;
 
   // The cached font lists in the normal and bold style.
   gfx::FontList cached_normal_font_list_;
   gfx::FontList cached_bold_font_list_;
 
   // The images and colors for each button state.
   gfx::ImageSkia button_state_images_[STATE_COUNT];
   SkColor button_state_colors_[STATE_COUNT];
 
   // Used to track whether SetTextColor() has been invoked.
   bool explicitly_set_colors_[STATE_COUNT];
 
   // |min_size_| increases monotonically with the preferred size.
   mutable gfx::Size min_size_;
   // |max_size_| may be set to clamp the preferred size.
   gfx::Size max_size_;
 
+  // Cache the last computed preferred size.
+  mutable gfx::Size button_size_;

[sky, 2014/07/23 22:42:36]

Can you name this better. It's the preferred size. Maybe cached_preferred_size_.

[noms (inactive), 2014/07/24 14:20:26]

Done.

+  mutable bool button_size_valid_;
+
   // Flag indicating default handling of the return key via an accelerator.
   // Whether or not the button appears or behaves as the default button in its
   // current context;
   bool is_default_;
 
   // The button's overall style.
   ButtonStyle style_;
 
   // True if current border was set by UpdateThemedBorder. Defaults to true.
   bool border_is_themed_border_;
 
   scoped_ptr<Painter> focus_painter_;
 
   DISALLOW_COPY_AND_ASSIGN(LabelButton);
 };
 
 }  // namespace views
 
 #endif  // UI_VIEWS_CONTROLS_BUTTON_LABEL_BUTTON_H_