Fixed StyledLabel size caching

ui/views/controls/styled_label.cc

Index: ui/views/controls/styled_label.cc
diff --git a/ui/views/controls/styled_label.cc b/ui/views/controls/styled_label.cc
index 700e62a392115963ae77adb8abb4a2fb60aa0205..b8f41f2b2205b7ba20499012dadb08e4926f1d4f 100644
--- a/ui/views/controls/styled_label.cc
+++ b/ui/views/controls/styled_label.cc
@@ -97,7 +97,8 @@ StyledLabel::StyledLabel(const base::string16& text,
     : specified_line_height_(0),
       listener_(listener),
       displayed_on_background_color_set_(false),
-      auto_color_readability_enabled_(true) {
+      auto_color_readability_enabled_(true),
+      layout_built_for_width_(0) {
   base::TrimWhitespace(text, base::TRIM_TRAILING, &text_);
 }
 
@@ -140,8 +141,17 @@ void StyledLabel::SetLineHeight(int line_height) {
 }
 
 void StyledLabel::SetDisplayedOnBackgroundColor(SkColor color) {
+  if (displayed_on_background_color_ == color)
+    return;
+
   displayed_on_background_color_ = color;
   displayed_on_background_color_set_ = true;
+
+  for (int i = 0, count = child_count(); i < count; ++i) {
+    DCHECK(!strcmp(child_at(i)->GetClassName(), Label::kViewClassName) ||

[sky, 2014/12/12 16:01:10]

Use == for comparison.

[tfarina, 2014/12/12 16:05:07]

Can we? GetClassName() returns const char* not std::string.

[sky, 2014/12/12 16:21:17]

Yes. The expectation is GetClassName() returns a constant value that never
changes. See other examples.

[tfarina, 2014/12/12 16:50:48]

My comment was more: In C when comparing strings (pointer to chars) you use
strcmp. I doubt comparing with == will yield to correct results. Isn't that case
in C++?

[sadrul, 2014/12/12 17:19:31]

== wouldn't work correctly for string comparison. But since GetClassName() is
expected to return a constant value, comparing the two pointers (instead of the
two strings) would be sufficient.

[edjomin, 2014/12/15 07:37:21]

Done.

+           !strcmp(child_at(i)->GetClassName(), Link::kViewClassName));
+    static_cast<Label*>(child_at(i))->SetBackgroundColor(color);
+  }
 }
 
 gfx::Insets StyledLabel::GetInsets() const {
@@ -164,19 +174,18 @@ gfx::Insets StyledLabel::GetInsets() const {
 }
 
 int StyledLabel::GetHeightForWidth(int w) const {
-  if (w != calculated_size_.width()) {
-    // TODO(erg): Munge the const-ness of the style label. CalculateAndDoLayout
-    // doesn't actually make any changes to member variables when |dry_run| is
-    // set to true. In general, the mutating and non-mutating parts shouldn't
-    // be in the same codepath.
-    calculated_size_ =
-        const_cast<StyledLabel*>(this)->CalculateAndDoLayout(w, true);
-  }
+  // TODO(erg): Munge the const-ness of the style label. CalculateAndDoLayout
+  // doesn't actually make any changes to member variables when |dry_run| is
+  // set to true. In general, the mutating and non-mutating parts shouldn't
+  // be in the same codepath.
+  calculated_size_ =
+      const_cast<StyledLabel*>(this)->CalculateAndDoLayout(w, true);
   return calculated_size_.height();
 }
 
 void StyledLabel::Layout() {
   calculated_size_ = CalculateAndDoLayout(GetLocalBounds().width(), false);
+  layout_built_for_width_ = calculated_size_.width();
 }
 
 void StyledLabel::PreferredSizeChanged() {
@@ -190,12 +199,16 @@ void StyledLabel::LinkClicked(Link* source, int event_flags) {
 }
 
 gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
+  width -= GetInsets().width();
+  if (width == calculated_size_.width() &&
+      (dry_run || layout_built_for_width_ == width))
+    return calculated_size_;
+
   if (!dry_run) {
     RemoveAllChildViews(true);
     link_targets_.clear();
   }
 
-  width -= GetInsets().width();
   if (width <= 0 || text_.empty())
     return gfx::Size();