Cache DrawStringParams in views::Label

BUG=443777
R=msw

Review URL: https://codereview.chromium.org/843503003

Cr-Commit-Position: refs/heads/master@{#310795}

ui/views/controls/label.cc

@@ -56,7 +56,7 @@ Label::~Label() {
 void Label::SetFontList(const gfx::FontList& font_list) {
   is_first_paint_text_ = true;
   font_list_ = font_list;
-  ResetCachedSize();
+  ResetLayoutCache();
   PreferredSizeChanged();
   SchedulePaint();
 }
@@ -78,7 +78,7 @@ void Label::SetTextInternal(const base::string16& text) {
     layout_text_ = text_;
   }
 
-  ResetCachedSize();
+  ResetLayoutCache();
   PreferredSizeChanged();
   SchedulePaint();
 }
@@ -113,7 +113,7 @@ void Label::SetBackgroundColor(SkColor color) {
 void Label::SetShadows(const gfx::ShadowValues& shadows) {
   is_first_paint_text_ = true;
   shadows_ = shadows;
-  text_size_valid_ = false;
+  ResetLayoutCache();
 }
 
 void Label::SetSubpixelRenderingEnabled(bool subpixel_rendering_enabled) {
@@ -148,7 +148,7 @@ void Label::SetLineHeight(int height) {
   is_first_paint_text_ = true;
   if (height != line_height_) {
     line_height_ = height;
-    ResetCachedSize();
+    ResetLayoutCache();
     PreferredSizeChanged();
     SchedulePaint();
   }
@@ -160,7 +160,7 @@ void Label::SetMultiLine(bool multi_line) {
                          elide_behavior_ == gfx::NO_ELIDE));
   if (multi_line != multi_line_) {
     multi_line_ = multi_line;
-    ResetCachedSize();
+    ResetLayoutCache();
     PreferredSizeChanged();
     SchedulePaint();
   }
@@ -178,7 +178,7 @@ void Label::SetAllowCharacterBreak(bool allow_character_break) {
   is_first_paint_text_ = true;
   if (allow_character_break != allow_character_break_) {
     allow_character_break_ = allow_character_break;
-    ResetCachedSize();
+    ResetLayoutCache();
     PreferredSizeChanged();
     SchedulePaint();
   }
@@ -190,7 +190,7 @@ void Label::SetElideBehavior(gfx::ElideBehavior elide_behavior) {
                           elide_behavior_ == gfx::NO_ELIDE));
   if (elide_behavior != elide_behavior_) {
     elide_behavior_ = elide_behavior;
-    ResetCachedSize();
+    ResetLayoutCache();
     PreferredSizeChanged();
     SchedulePaint();
   }
@@ -291,6 +291,8 @@ int Label::GetHeightForWidth(int w) const {
   int cache_width = w;
 
   int h = font_list_.GetHeight();
+  // Flags returned in the cached |DrawStringParams| has a different value
+  // from the result of |ComputeDrawStringFlags()|. The latter is needed here.
   const int flags = ComputeDrawStringFlags();
   gfx::Canvas::SizeStringInt(
       layout_text_, font_list_, &w, &h, line_height_, flags);
@@ -373,6 +375,8 @@ gfx::Size Label::GetTextSize() const {
     int h = font_list_.GetHeight();
     // For single-line strings, ignore the available width and calculate how
     // wide the text wants to be.
+    // Call |ComputeDrawStringFlags()| instead of |CalculateDrawStringParams()|
+    // here since the latter calls this function and causes infinite recursion.
     int flags = ComputeDrawStringFlags();
     if (!multi_line_)
       flags |= gfx::Canvas::NO_ELLIPSIS;
@@ -396,6 +400,7 @@ gfx::Size Label::GetTextSize() const {
 
 void Label::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   text_size_valid_ &= !multi_line_;
+  cached_draw_params_.text.clear();
 }
 
 void Label::OnPaint(gfx::Canvas* canvas) {
@@ -404,24 +409,23 @@ void Label::OnPaint(gfx::Canvas* canvas) {
   // some subclasses of Label. We do not want View's focus border painting to
   // interfere with that.
   OnPaintBorder(canvas);
+  if (layout_text_.empty())
+    return;
 
-  base::string16 paint_text;
-  gfx::Rect text_bounds;
-  int flags = 0;
-  CalculateDrawStringParams(&paint_text, &text_bounds, &flags);
+  const DrawStringParams* params = CalculateDrawStringParams();
   if (is_first_paint_text_) {
     // TODO(vadimt): Remove ScopedTracker below once crbug.com/431326 is fixed.
     tracked_objects::ScopedTracker tracking_profile(
         FROM_HERE_WITH_EXPLICIT_FUNCTION("431326 Label::PaintText first"));
 
     is_first_paint_text_ = false;
-    PaintText(canvas, paint_text, text_bounds, flags);
+    PaintText(canvas, params->text, params->bounds, params->flags);
   } else {
     // TODO(vadimt): Remove ScopedTracker below once crbug.com/431326 is fixed.
     tracked_objects::ScopedTracker tracking_profile(
         FROM_HERE_WITH_EXPLICIT_FUNCTION("431326 Label::PaintText not first"));
 
-    PaintText(canvas, paint_text, text_bounds, flags);
+    PaintText(canvas, params->text, params->bounds, params->flags);
   }
 }
 
@@ -444,7 +448,7 @@ void Label::Init(const base::string16& text, const gfx::FontList& font_list) {
   handles_tooltips_ = true;
   collapse_when_hidden_ = false;
   cached_heights_.resize(kCachedSizeLimit);
-  ResetCachedSize();
+  ResetLayoutCache();
   is_first_paint_text_ = true;
 
   SetText(text);
@@ -541,25 +545,25 @@ gfx::Rect Label::GetAvailableRect() const {
   return bounds;
 }
 
-void Label::CalculateDrawStringParams(base::string16* paint_text,
-                                      gfx::Rect* text_bounds,
-                                      int* flags) const {
-  DCHECK(paint_text && text_bounds && flags);
+const Label::DrawStringParams* Label::CalculateDrawStringParams() const {
+  if (cached_draw_params_.text.empty()) {
+    const bool forbid_ellipsis = elide_behavior_ == gfx::NO_ELIDE ||
+                                 elide_behavior_ == gfx::FADE_TAIL;
+    if (multi_line_ || forbid_ellipsis) {
+      cached_draw_params_.text = layout_text_;
+    } else {
+      cached_draw_params_.text = gfx::ElideText(layout_text_, font_list_,
+          GetAvailableRect().width(), elide_behavior_);
+    }
 
-  const bool forbid_ellipsis = elide_behavior_ == gfx::NO_ELIDE ||
-                               elide_behavior_ == gfx::FADE_TAIL;
-  if (multi_line_ || forbid_ellipsis) {
-    *paint_text = layout_text_;
-  } else {
-    *paint_text = gfx::ElideText(layout_text_, font_list_,
-                                 GetAvailableRect().width(), elide_behavior_);
+    cached_draw_params_.bounds = GetTextBounds();
+    cached_draw_params_.flags = ComputeDrawStringFlags();
+    // TODO(msw): Elide multi-line text with ElideRectangleText instead.
+    if (!multi_line_ || forbid_ellipsis)
+      cached_draw_params_.flags |= gfx::Canvas::NO_ELLIPSIS;
   }
 
-  *text_bounds = GetTextBounds();
-  *flags = ComputeDrawStringFlags();
-  // TODO(msw): Elide multi-line text with ElideRectangleText instead.
-  if (!multi_line_ || forbid_ellipsis)
-    *flags |= gfx::Canvas::NO_ELLIPSIS;
+  return &cached_draw_params_;
 }
 
 void Label::UpdateColorsFromTheme(const ui::NativeTheme* theme) {
@@ -578,7 +582,8 @@ void Label::UpdateColorsFromTheme(const ui::NativeTheme* theme) {
   RecalculateColors();
 }
 
-void Label::ResetCachedSize() {
+void Label::ResetLayoutCache() {
+  cached_draw_params_.text.clear();
   text_size_valid_ = false;
   cached_heights_cursor_ = 0;
   for (int i = 0; i < kCachedSizeLimit; ++i)

ui/views/controls/label.h

@@ -149,6 +149,14 @@ class VIEWS_EXPORT Label : public View {
   void OnNativeThemeChanged(const ui::NativeTheme* theme) override;
 
  private:
+  struct DrawStringParams {
+    DrawStringParams() : flags(0) {}
+
+    base::string16 text;
+    gfx::Rect bounds;
+    int flags;
+  };
+
   // These tests call CalculateDrawStringParams in order to verify the
   // calculations done for drawing text.
   FRIEND_TEST_ALL_PREFIXES(LabelTest, DrawSingleLineString);
@@ -173,17 +181,16 @@ class VIEWS_EXPORT Label : public View {
 
   gfx::Rect GetAvailableRect() const;
 
-  // Returns parameters to be used for the DrawString call.
-  void CalculateDrawStringParams(base::string16* paint_text,
-                                 gfx::Rect* text_bounds,
-                                 int* flags) const;
+  // Returns parameters to be used for the DrawString call. Returned value is a
+  // weak pointer, owned by and scoped to the label.
+  const DrawStringParams* CalculateDrawStringParams() const;
 
   // Updates any colors that have not been explicitly set from the theme.
   void UpdateColorsFromTheme(const ui::NativeTheme* theme);
 
-  // Resets |cached_heights_| and |cached_heights_cursor_| and mark
-  // |text_size_valid_| as false.
-  void ResetCachedSize();
+  // Resets |cached_heights_|, |cached_heights_cursor_|, |cached_draw_params_|
+  // and mark |text_size_valid_| as false.
+  void ResetLayoutCache();
 
   bool ShouldShowDefaultTooltip() const;
 
@@ -221,6 +228,9 @@ class VIEWS_EXPORT Label : public View {
   mutable std::vector<gfx::Size> cached_heights_;
   mutable int cached_heights_cursor_;
 
+  // The cached results of CalculateDrawStringParams().
+  mutable DrawStringParams cached_draw_params_;
+
   // TODO(vadimt): Remove is_first_paint_text_ before crbug.com/431326 is
   // closed.
   bool is_first_paint_text_;