Fixed clipping of text in notification toasts.

This involved changing BoundedLabel to make its GetPreferredSize()
method take a line limit argument, then modifying NotificationView to
use that argument to get its correct preferred height.

BUG=230448
R=mukai@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@194058 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 284 insertions, 282 deletions

diff --git a/ui/message_center/views/bounded_label.cc b/ui/message_center/views/bounded_label.cc
index 4018c0d..f9deb2c 100644
--- a/ui/message_center/views/bounded_label.cc
+++ b/ui/message_center/views/bounded_label.cc
@@ -24,30 +24,25 @@ namespace message_center {
 // InnerBoundedLabel ///////////////////////////////////////////////////////////
 
 // InnerBoundedLabel is a views::Label subclass that does all of the work for
-// BoundedLabel. It is kept private to BoundedLabel to prevent outside code from
-// calling a number of views::Label methods like SetFont() that would break
-// BoundedLabel caching but can't be fixed because they're not virtual.
+// BoundedLabel. It is kept private to prevent outside code from calling a
+// number of views::Label methods like setFont() that break BoundedLabel's
+// caching but can't be overridden.
 //
 // TODO(dharcourt): Move the line limiting functionality to views::Label to make
 // this unnecessary.
 
 class InnerBoundedLabel : public views::Label {
  public:
-  InnerBoundedLabel(const BoundedLabel& owner, size_t line_limit);
+  InnerBoundedLabel(const BoundedLabel& owner);
   virtual ~InnerBoundedLabel();
 
-  void SetLineLimit(size_t limit);
-  size_t GetLinesForWidth(int width);
-  size_t GetPreferredLines();
-  size_t GetActualLines();
+  void SetNativeTheme(const ui::NativeTheme* theme);
 
-  void ChangeNativeTheme(const ui::NativeTheme* theme);
+  // Pass in a -1 width to use the preferred width, a -1 limit to skip limits.
+  int GetLinesForWidthAndLimit(int width, int limit);
+  gfx::Size GetSizeForWidthAndLines(int width, int lines);
 
-  std::vector<string16> GetWrappedText(int width, size_t line_limit);
-
-  // Overridden from views::Label.
-  virtual gfx::Size GetPreferredSize() OVERRIDE;
-  virtual int GetHeightForWidth(int width) OVERRIDE;
+  std::vector<string16> GetWrappedText(int width, int lines);
 
  protected:
   // Overridden from views::Label.
@@ -55,107 +50,112 @@ class InnerBoundedLabel : public views::Label {
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
 
  private:
-  gfx::Insets GetOwnerInsets();
-  size_t GetPreferredLinesForWidth(int width);
-  gfx::Size GetSizeForWidth(int width);
   int GetTextFlags();
 
   void ClearCaches();
-  size_t GetCachedLinesForWidth(int width);
-  void SetCachedLinesForWidth(int width, size_t lines);
-  gfx::Size GetCachedSizeForWidth(int width);
-  void SetCachedSizeForWidth(int width, gfx::Size size);
+  int GetCachedLines(int width);
+  void SetCachedLines(int width, int lines);
+  gfx::Size GetCachedSize(const std::pair<int, int>& width_and_lines);
+  void SetCachedSize(std::pair<int, int> width_and_lines, gfx::Size size);
 
   const BoundedLabel* owner_;  // Weak reference.
-  size_t line_limit_;
-  std::map<int,size_t> lines_cache_;
+  string16 wrapped_text_;
+  int wrapped_text_width_;
+  int wrapped_text_lines_;
+  std::map<int, int> lines_cache_;
   std::list<int> lines_widths_;  // Most recently used in front.
-  std::map<int,gfx::Size> size_cache_;
-  std::list<int> size_widths_;  // Most recently used in front.
+  std::map<std::pair<int, int>, gfx::Size> size_cache_;
+  std::list<std::pair<int, int> > size_widths_and_lines_;  // Recent in front.
 
   DISALLOW_COPY_AND_ASSIGN(InnerBoundedLabel);
 };
 
-InnerBoundedLabel::InnerBoundedLabel(const BoundedLabel& owner,
-                                     size_t line_limit) {
+InnerBoundedLabel::InnerBoundedLabel(const BoundedLabel& owner)
+    : owner_(&owner),
+      wrapped_text_width_(0),
+      wrapped_text_lines_(0) {
   SetMultiLine(true);
   SetAllowCharacterBreak(true);
   SetElideBehavior(views::Label::ELIDE_AT_END);
   SetHorizontalAlignment(gfx::ALIGN_LEFT);
   set_collapse_when_hidden(true);
-  owner_ = &owner;
-  line_limit_ = line_limit;
 }
 
 InnerBoundedLabel::~InnerBoundedLabel() {
 }
 
-void InnerBoundedLabel::SetLineLimit(size_t limit) {
-  if (limit != line_limit_) {
-    std::swap(limit, line_limit_);
-    if (GetPreferredLines() > line_limit_ || GetPreferredLines() > limit) {
-      ClearCaches();
-      PreferredSizeChanged();
-    }
-  }
-}
-
-size_t InnerBoundedLabel::GetLinesForWidth(int width) {
-  return std::min(GetPreferredLinesForWidth(width), line_limit_);
-}
-
-size_t InnerBoundedLabel::GetPreferredLines() {
-  return GetPreferredLinesForWidth(width());
+void InnerBoundedLabel::SetNativeTheme(const ui::NativeTheme* theme) {
+  ClearCaches();
+  OnNativeThemeChanged(theme);
 }
 
-size_t InnerBoundedLabel::GetActualLines() {
-  return std::min(GetPreferredLinesForWidth(width()), line_limit_);
+int InnerBoundedLabel::GetLinesForWidthAndLimit(int width, int limit) {
+  if (width == 0 || limit == 0)
+    return 0;
+  int lines = GetCachedLines(width);
+  if (lines == std::numeric_limits<int>::max()) {
+    int text_width = std::max(width - owner_->GetInsets().width(), 0);
+    lines = GetWrappedText(text_width, lines).size();
+    SetCachedLines(width, lines);
+  }
+  return (limit < 0 || lines <= limit) ? lines : limit;
 }
 
-void InnerBoundedLabel::ChangeNativeTheme(const ui::NativeTheme* theme) {
-  ClearCaches();
-  OnNativeThemeChanged(theme);
+gfx::Size InnerBoundedLabel::GetSizeForWidthAndLines(int width, int lines) {
+  if (width == 0 || lines == 0)
+    return gfx::Size();
+  std::pair<int, int> key(width, lines);
+  gfx::Size size = GetCachedSize(key);
+  if (size.height() == std::numeric_limits<int>::max()) {
+    gfx::Insets insets = owner_->GetInsets();
+    int text_width = (width < 0) ? std::numeric_limits<int>::max() :
+                                   std::max(width - insets.width(), 0);
+    int text_height = std::numeric_limits<int>::max();
+    std::vector<string16> wrapped = GetWrappedText(text_width, lines);
+    gfx::Canvas::SizeStringInt(JoinString(wrapped, '\n'), font(),
+                               &text_width, &text_height, GetTextFlags());
+    size.set_width(text_width + insets.width());
+    size.set_height(text_height + insets.height());
+    SetCachedSize(key, size);
+  }
+  return size;
 }
 
-std::vector<string16> InnerBoundedLabel::GetWrappedText(int width,
-                                                        size_t line_limit) {
+std::vector<string16> InnerBoundedLabel::GetWrappedText(int width, int lines) {
   // Short circuit simple case.
-  if (line_limit == 0)
+  if (width == 0 || lines == 0)
     return std::vector<string16>();
 
-  // Restrict line_limit to ensure (line_limit + 1) * line_height <= INT_MAX.
-  int line_height = std::max(font().GetHeight(), 2);  // At least 2 pixels.
-  unsigned int max_limit = std::numeric_limits<int>::max() / line_height - 1;
-  line_limit = (line_limit <= max_limit) ? line_limit : max_limit;
+  // Restrict line limit to ensure (lines + 1) * line_height <= INT_MAX and
+  // use it to calculate a reasonable text height.
+  int height = std::numeric_limits<int>::max();
+  if (lines > 0) {
+    int line_height = std::max(font().GetHeight(), 2);  // At least 2 pixels.
+    int max_lines = std::numeric_limits<int>::max() / line_height - 1;
+    lines = std::min(lines, max_lines);
+    height = (lines + 1) * line_height;
+  }
 
-  // Split, using ui::IGNORE_LONG_WORDS instead of ui::WRAP_LONG_WORDS to
+  // Wrap, using ui::IGNORE_LONG_WORDS instead of ui::WRAP_LONG_WORDS to
   // avoid an infinite loop in ui::ElideRectangleText() for small widths.
-  std::vector<string16> lines;
-  int height = static_cast<int>((line_limit + 1) * line_height);
-  ui::ElideRectangleText(text(), font(), width, height,
-                         ui::IGNORE_LONG_WORDS, &lines);
+  std::vector<string16> wrapped;
+  ui::ElideRectangleText(text(), font(),
+                         (width < 0) ? std::numeric_limits<int>::max() : width,
+                         height, ui::IGNORE_LONG_WORDS, &wrapped);
 
   // Elide if necessary.
-  if (lines.size() > line_limit) {
+  if (lines > 0 && wrapped.size() > static_cast<unsigned int>(lines)) {
     // Add an ellipsis to the last line. If this ellipsis makes the last line
     // too wide, that line will be further elided by the ui::ElideText below,
     // so for example "ABC" could become "ABC..." and then "AB...".
-    string16 last = lines[line_limit - 1] + UTF8ToUTF16(ui::kEllipsis);
-    if (font().GetStringWidth(last) > width)
+    string16 last = wrapped[lines - 1] + UTF8ToUTF16(ui::kEllipsis);
+    if (width > 0 && font().GetStringWidth(last) > width)
       last = ui::ElideText(last, font(), width, ui::ELIDE_AT_END);
-    lines.resize(line_limit - 1);
-    lines.push_back(last);
+    wrapped.resize(lines - 1);
+    wrapped.push_back(last);
   }
 
-  return lines;
-}
-
-gfx::Size InnerBoundedLabel::GetPreferredSize() {
-  return GetSizeForWidth(std::numeric_limits<int>::max());
-}
-
-int InnerBoundedLabel::GetHeightForWidth(int width) {
-  return GetSizeForWidth(width).height();
+  return wrapped;
 }
 
 void InnerBoundedLabel::OnBoundsChanged(const gfx::Rect& previous_bounds) {
@@ -167,46 +167,18 @@ void InnerBoundedLabel::OnPaint(gfx::Canvas* canvas) {
   views::Label::OnPaintBackground(canvas);
   views::Label::OnPaintFocusBorder(canvas);
   views::Label::OnPaintBorder(canvas);
-  int height = GetSizeForWidth(width()).height() - GetOwnerInsets().height();
+  int lines = owner_->line_limit();
+  int height = GetSizeForWidthAndLines(width(), lines).height();
   if (height > 0) {
-    gfx::Rect bounds = GetLocalBounds();
-    bounds.Inset(GetOwnerInsets());
-    bounds.set_y(bounds.y() + (bounds.height() - height) / 2);
-    bounds.set_height(height);
-    std::vector<string16> text = GetWrappedText(bounds.width(), line_limit_);
-    PaintText(canvas, JoinString(text, '\n'), bounds, GetTextFlags());
-  }
-}
-
-gfx::Insets InnerBoundedLabel::GetOwnerInsets() {
-  return owner_->GetInsets();
-}
-
-size_t InnerBoundedLabel::GetPreferredLinesForWidth(int width) {
-  size_t lines = GetCachedLinesForWidth(width);
-  if (lines == std::numeric_limits<size_t>::max()) {
-    int content_width = std::max(width - GetOwnerInsets().width(), 0);
-    lines = GetWrappedText(content_width, lines).size();
-    SetCachedLinesForWidth(width, lines);
-  }
-  return lines;
-}
-
-gfx::Size InnerBoundedLabel::GetSizeForWidth(int width) {
-  gfx::Size size = GetCachedSizeForWidth(width);
-  if (size.height() == std::numeric_limits<int>::max()) {
-    int text_width = std::max(width - GetOwnerInsets().width(), 0);
-    int text_height = 0;
-    if (line_limit_ > 0) {
-      std::vector<string16> text = GetWrappedText(text_width, line_limit_);
-      gfx::Canvas::SizeStringInt(JoinString(text, '\n'), font(),
-                                 &text_width, &text_height, GetTextFlags());
+    gfx::Rect bounds(width(), height);
+    bounds.Inset(owner_->GetInsets());
+    if (bounds.width() != wrapped_text_width_ || lines != wrapped_text_lines_) {
+      wrapped_text_ = JoinString(GetWrappedText(bounds.width(), lines), '\n');
+      wrapped_text_width_ = bounds.width();
+      wrapped_text_lines_ = lines;
     }
-    size.set_width(text_width + GetOwnerInsets().width());
-    size.set_height(text_height + GetOwnerInsets().height());
-    SetCachedSizeForWidth(width, size);
+    PaintText(canvas, wrapped_text_, bounds, GetTextFlags());
   }
-  return size;
 }
 
 int InnerBoundedLabel::GetTextFlags() {
@@ -230,23 +202,26 @@ int InnerBoundedLabel::GetTextFlags() {
 }
 
 void InnerBoundedLabel::ClearCaches() {
+  wrapped_text_width_ = 0;
+  wrapped_text_lines_ = 0;
   lines_cache_.clear();
   lines_widths_.clear();
   size_cache_.clear();
-  size_widths_.clear();
+  size_widths_and_lines_.clear();
 }
 
-size_t InnerBoundedLabel::GetCachedLinesForWidth(int width) {
-  size_t lines = std::numeric_limits<size_t>::max();
-  if (lines_cache_.find(width) != lines_cache_.end()) {
-    lines = lines_cache_[width];
+int InnerBoundedLabel::GetCachedLines(int width) {
+  int lines = std::numeric_limits<int>::max();
+  std::map<int, int>::const_iterator found;
+  if ((found = lines_cache_.find(width)) != lines_cache_.end()) {
+    lines = found->second;
     lines_widths_.remove(width);
     lines_widths_.push_front(width);
   }
   return lines;
 }
 
-void InnerBoundedLabel::SetCachedLinesForWidth(int width, size_t lines) {
+void InnerBoundedLabel::SetCachedLines(int width, int lines) {
   if (lines_cache_.size() >= kPreferredLinesCacheSize) {
     lines_cache_.erase(lines_widths_.back());
     lines_widths_.pop_back();
@@ -255,62 +230,61 @@ void InnerBoundedLabel::SetCachedLinesForWidth(int width, size_t lines) {
   lines_widths_.push_front(width);
 }
 
-gfx::Size InnerBoundedLabel::GetCachedSizeForWidth(int width) {
-  gfx::Size size(width, std::numeric_limits<int>::max());
-  if (size_cache_.find(width) != size_cache_.end()) {
-    size = size_cache_[width];
-    size_widths_.remove(width);
-    size_widths_.push_front(width);
+gfx::Size InnerBoundedLabel::GetCachedSize(
+    const std::pair<int, int>& width_and_lines) {
+  gfx::Size size(width_and_lines.first, std::numeric_limits<int>::max());
+  std::map<std::pair<int, int>, gfx::Size>::const_iterator found;
+  if ((found = size_cache_.find(width_and_lines)) != size_cache_.end()) {
+    size = found->second;
+    size_widths_and_lines_.remove(width_and_lines);
+    size_widths_and_lines_.push_front(width_and_lines);
   }
   return size;
 }
 
-void InnerBoundedLabel::SetCachedSizeForWidth(int width, gfx::Size size) {
+void InnerBoundedLabel::SetCachedSize(std::pair<int, int> width_and_lines,
+                                      gfx::Size size) {
   if (size_cache_.size() >= kPreferredLinesCacheSize) {
-    size_cache_.erase(size_widths_.back());
-    size_widths_.pop_back();
+    size_cache_.erase(size_widths_and_lines_.back());
+    size_widths_and_lines_.pop_back();
   }
-  size_cache_[width] = size;
-  size_widths_.push_front(width);
+  size_cache_[width_and_lines] = size;
+  size_widths_and_lines_.push_front(width_and_lines);
 }
 
 // BoundedLabel ///////////////////////////////////////////////////////////
 
-BoundedLabel::BoundedLabel(const string16& text,
-                           gfx::Font font,
-                           size_t line_limit) {
-  label_.reset(new InnerBoundedLabel(*this, line_limit));
+BoundedLabel::BoundedLabel(const string16& text, gfx::Font font)
+    : line_limit_(-1) {
+  label_.reset(new InnerBoundedLabel(*this));
   label_->SetFont(font);
   label_->SetText(text);
 }
 
-BoundedLabel::BoundedLabel(const string16& text, size_t line_limit) {
-  label_.reset(new InnerBoundedLabel(*this, line_limit));
+BoundedLabel::BoundedLabel(const string16& text) {
+  label_.reset(new InnerBoundedLabel(*this));
   label_->SetText(text);
 }
 
 BoundedLabel::~BoundedLabel() {
 }
 
-void BoundedLabel::SetLineLimit(size_t lines) {
-  label_->SetLineLimit(lines);
-}
-
-size_t BoundedLabel::GetLinesForWidth(int width) {
-  return visible() ? label_->GetLinesForWidth(width) : 0;
+void BoundedLabel::SetColors(SkColor textColor, SkColor backgroundColor) {
+  label_->SetEnabledColor(textColor);
+  label_->SetBackgroundColor(backgroundColor);
 }
 
-size_t BoundedLabel::GetPreferredLines() {
-  return visible() ? label_->GetPreferredLines() : 0;
+void BoundedLabel::SetLineLimit(int lines) {
+  line_limit_ = std::max(lines, -1);
 }
 
-size_t BoundedLabel::GetActualLines() {
-  return visible() ? label_->GetActualLines() : 0;
+int BoundedLabel::GetLinesForWidthAndLimit(int width, int limit) {
+  return visible() ? label_->GetLinesForWidthAndLimit(width, limit) : 0;
 }
 
-void BoundedLabel::SetColors(SkColor textColor, SkColor backgroundColor) {
-  label_->SetEnabledColor(textColor);
-  label_->SetBackgroundColor(backgroundColor);
+gfx::Size BoundedLabel::GetSizeForWidthAndLines(int width, int lines) {
+  return visible() ?
+         label_->GetSizeForWidthAndLines(width, lines) : gfx::Size();
 }
 
 int BoundedLabel::GetBaseline() const {
@@ -318,11 +292,12 @@ int BoundedLabel::GetBaseline() const {
 }
 
 gfx::Size BoundedLabel::GetPreferredSize() {
-  return visible() ? label_->GetPreferredSize() : gfx::Size();
+  return visible() ? label_->GetSizeForWidthAndLines(-1, -1) : gfx::Size();
 }
 
-int BoundedLabel::GetHeightForWidth(int weight) {
-  return visible() ? label_->GetHeightForWidth(weight) : 0;
+int BoundedLabel::GetHeightForWidth(int width) {
+  return visible() ?
+         label_->GetSizeForWidthAndLines(width, line_limit_).height() : 0;
 }
 
 void BoundedLabel::Paint(gfx::Canvas* canvas) {
@@ -344,11 +319,11 @@ void BoundedLabel::OnBoundsChanged(const gfx::Rect& previous_bounds) {
 }
 
 void BoundedLabel::OnNativeThemeChanged(const ui::NativeTheme* theme) {
-  label_->ChangeNativeTheme(theme);
+  label_->SetNativeTheme(theme);
 }
 
-string16 BoundedLabel::GetWrappedTextForTest(int width, size_t line_limit) {
-  return JoinString(label_->GetWrappedText(width, line_limit), '\n');
+string16 BoundedLabel::GetWrappedTextForTest(int width, int lines) {
+  return JoinString(label_->GetWrappedText(width, lines), '\n');
 }
 
 }  // namespace message_center
diff --git a/ui/message_center/views/bounded_label.h b/ui/message_center/views/bounded_label.h
index a159a49..4018680 100644
--- a/ui/message_center/views/bounded_label.h
+++ b/ui/message_center/views/bounded_label.h
@@ -14,37 +14,42 @@
 #include "ui/views/view.h"
 
 namespace gfx {
-
 class Font;
-
-}  // namespace gfx
+}
 
 namespace message_center {
 
 class InnerBoundedLabel;
 
+namespace test {
+class BoundedLabelTest;
+}
+
 // BoundedLabels display left aligned text up to a maximum number of lines, with
 // ellipsis at the end of the last line for any omitted text. BoundedLabel is a
 // direct subclass of views::Views rather than a subclass of views::Label
-// because of limitations in views::Label's implementation. See the description
-// of InnerBoundedLabel in the .cc file for details.
+// to avoid exposing some of views::Label's methods that can't be made to work
+// with BoundedLabel. See the description of InnerBoundedLabel in the
+// bounded_label.cc file for details.
 class MESSAGE_CENTER_EXPORT BoundedLabel : public views::View {
  public:
-  BoundedLabel(const string16& text, gfx::Font font, size_t line_limit);
-  BoundedLabel(const string16& text, size_t line_limit);
+  BoundedLabel(const string16& text, gfx::Font font);
+  BoundedLabel(const string16& text);
   virtual ~BoundedLabel();
 
-  void SetLineLimit(size_t lines);
-  size_t GetLinesForWidth(int width);
-  size_t GetPreferredLines();
-  size_t GetActualLines();
-
   void SetColors(SkColor textColor, SkColor backgroundColor);
+  void SetLineLimit(int lines);  // Pass in -1 for no limit.
+
+  int line_limit() const { return line_limit_; }
+
+  // Pass in a -1 width to use the preferred width, a -1 limit to skip limits.
+  int GetLinesForWidthAndLimit(int width, int limit);
+  gfx::Size GetSizeForWidthAndLines(int width, int lines);
 
   // Overridden from views::View.
   virtual int GetBaseline() const OVERRIDE;
   virtual gfx::Size GetPreferredSize() OVERRIDE;
-  virtual int GetHeightForWidth(int w) OVERRIDE;
+  virtual int GetHeightForWidth(int width) OVERRIDE;
   virtual void Paint(gfx::Canvas* canvas) OVERRIDE;
   virtual bool HitTestRect(const gfx::Rect& rect) const OVERRIDE;
   virtual void GetAccessibleState(ui::AccessibleViewState* state) OVERRIDE;
@@ -55,11 +60,12 @@ class MESSAGE_CENTER_EXPORT BoundedLabel : public views::View {
   virtual void OnNativeThemeChanged(const ui::NativeTheme* theme) OVERRIDE;
 
  private:
-  friend class BoundedLabelTest;
+  friend class test::BoundedLabelTest;
 
-  string16 GetWrappedTextForTest(int width, size_t line_limit);
+  string16 GetWrappedTextForTest(int width, int lines);
 
   scoped_ptr<InnerBoundedLabel> label_;
+  int line_limit_;
 
   DISALLOW_COPY_AND_ASSIGN(BoundedLabel);
 };
diff --git a/ui/message_center/views/bounded_label_unittest.cc b/ui/message_center/views/bounded_label_unittest.cc
index af44f66..f9656f4 100644
--- a/ui/message_center/views/bounded_label_unittest.cc
+++ b/ui/message_center/views/bounded_label_unittest.cc
@@ -15,18 +15,31 @@
 
 namespace message_center {
 
-/* Test fixture declaration ***************************************************/
+namespace test {
+
+/* Test fixture ***************************************************************/
 
 class BoundedLabelTest : public testing::Test {
  public:
-  BoundedLabelTest();
-  virtual ~BoundedLabelTest();
+  BoundedLabelTest() {
+    digit_pixels_ = font_.GetStringWidth(UTF8ToUTF16("0"));
+    space_pixels_ = font_.GetStringWidth(UTF8ToUTF16(" "));
+    ellipsis_pixels_ = font_.GetStringWidth(UTF8ToUTF16("\xE2\x80\xA6"));
+  }
+
+  virtual ~BoundedLabelTest() {}
 
   // Replaces all occurences of three periods ("...") in the specified string
   // with an ellipses character (UTF8 "\xE2\x80\xA6") and returns a string16
   // with the results. This allows test strings to be specified as ASCII const
   // char* strings, making tests more readable and easier to write.
-  string16 ToString(const char* string);
+  string16 ToString(const char* string) {
+    const string16 periods = UTF8ToUTF16("...");
+    const string16 ellipses = UTF8ToUTF16("\xE2\x80\xA6");
+    string16 result = UTF8ToUTF16(string);
+    ReplaceSubstringsAfterOffset(&result, 0, periods, ellipses);
+    return result;
+  }
 
   // Converts the specified elision width to pixels. To make tests somewhat
   // independent of the fonts of the platform on which they're run, the elision
@@ -36,18 +49,32 @@ class BoundedLabelTest : public testing::Test {
   // test plaform. It is assumed that all digits have the same width in that
   // font, that this width is greater than the width of spaces, and that the
   // width of 3 digits is greater than the width of ellipses.
-  int ToPixels(int width);
+  int ToPixels(int width) {
+    return digit_pixels_ * width / 100 +
+           space_pixels_ * (width % 100) / 10 +
+           ellipsis_pixels_ * (width % 10);
+  }
 
   // Exercise BounderLabel::GetWrappedText() using the fixture's test label.
-  string16 GetWrappedText(int width);
+  string16 GetWrappedText(int width) {
+    return label_->GetWrappedTextForTest(width, lines_);
+  }
 
-  // Exercise BounderLabel::GetPreferredLines() using the fixture's test label.
-  int GetPreferredLines(int width);
+  // Exercise BounderLabel::GetLinesForWidthAndLimit() using the test label.
+  int GetLinesForWidth(int width) {
+    label_->SetBounds(0, 0, width, font_.GetHeight() * lines_);
+    return label_->GetLinesForWidthAndLimit(width, lines_);
+  }
 
  protected:
   // Creates a label to test with. Returns this fixture, which can be used to
   // test the newly created label using the exercise methods above.
-  BoundedLabelTest& Label(string16 text, size_t lines);
+  BoundedLabelTest& Label(string16 text, int lines) {
+    lines_ = lines;
+    label_.reset(new BoundedLabel(text, font_));
+    label_->SetLineLimit(lines_);
+    return *this;
+  }
 
  private:
   gfx::Font font_;  // The default font, which will be used for tests.
@@ -55,58 +82,18 @@ class BoundedLabelTest : public testing::Test {
   int space_pixels_;
   int ellipsis_pixels_;
   scoped_ptr<BoundedLabel> label_;
-  size_t max_lines_;
+  int lines_;
 };
 
-/* Test fixture definition ****************************************************/
-
-BoundedLabelTest::BoundedLabelTest() {
-  digit_pixels_ = font_.GetStringWidth(UTF8ToUTF16("0"));
-  space_pixels_ = font_.GetStringWidth(UTF8ToUTF16(" "));
-  ellipsis_pixels_ = font_.GetStringWidth(UTF8ToUTF16("\xE2\x80\xA6"));
-}
-
-BoundedLabelTest::~BoundedLabelTest() {
-}
-
-string16 BoundedLabelTest::ToString(const char* string) {
-  const string16 periods = UTF8ToUTF16("...");
-  const string16 ellipses = UTF8ToUTF16("\xE2\x80\xA6");
-  string16 result = UTF8ToUTF16(string);
-  ReplaceSubstringsAfterOffset(&result, 0, periods, ellipses);
-  return result;
-}
-
-int BoundedLabelTest::ToPixels(int width) {
-  return digit_pixels_ * width / 100 +
-         space_pixels_ * (width % 100) / 10 +
-         ellipsis_pixels_ * (width % 10);
-}
-
-string16 BoundedLabelTest::GetWrappedText(int width) {
-  return label_->GetWrappedTextForTest(width, max_lines_);
-}
-
-int BoundedLabelTest::GetPreferredLines(int width) {
-  label_->SetBounds(0, 0, width, font_.GetHeight() * max_lines_);
-  return label_->GetPreferredLines();
-}
-
-BoundedLabelTest& BoundedLabelTest::Label(string16 text, size_t lines) {
-  max_lines_ = lines;
-  label_.reset(new BoundedLabel(text, font_, max_lines_));
-  return *this;
-}
-
-/* Test macro definitions *****************************************************/
+/* Test macro *****************************************************************/
 
 #define TEST_WRAP(expected, text, width, lines) \
   EXPECT_EQ(ToString(expected), \
             Label(ToString(text), lines).GetWrappedText(ToPixels(width)))
 
-#define TEST_PREFERRED_LINES(expected, text, width, lines) \
+#define TEST_LINES(expected, text, width, lines) \
   EXPECT_EQ(expected, \
-            Label(ToString(text), lines).GetPreferredLines(ToPixels(width)))
+            Label(ToString(text), lines).GetLinesForWidth(ToPixels(width)))
 
 /* Elision tests **************************************************************/
 
@@ -115,7 +102,7 @@ TEST_F(BoundedLabelTest, ElisionTest) {
   TEST_WRAP("123", "123", 301, 1);
   TEST_WRAP("123", "123", 301, 2);
   TEST_WRAP("123", "123", 301, 3);
-  TEST_WRAP("123\n456", "123 456", 0, 2);  // 301, 2);
+  TEST_WRAP("123\n456", "123 456", 301, 2);
   TEST_WRAP("123\n456", "123 456", 301, 3);
   TEST_WRAP("123\n456\n789", "123 456 789", 301, 3);
 
@@ -186,37 +173,42 @@ TEST_F(BoundedLabelTest, ElisionTest) {
 
   // TODO(dharcourt): Add test cases for:
   // - Empty and very large strings
-  // - Zero and very large max lines values
+  // - Zero, very large, and negative line limit values
   // - Other input boundary conditions
   // TODO(dharcourt): Add some randomly generated fuzz test cases.
 
 }
 
-/* GetPreferredLinesTest ******************************************************/
+/* GetLinesTest ***************************************************************/
 
-TEST_F(BoundedLabelTest, GetPreferredLinesTest) {
-  // Zero, small, and negative width values should yield one word per line.
-  TEST_PREFERRED_LINES(2, "123 456", 0, 1);
-  TEST_PREFERRED_LINES(2, "123 456", 1, 1);
-  TEST_PREFERRED_LINES(2, "123 456", 2, 1);
-  TEST_PREFERRED_LINES(2, "123 456", 3, 1);
-  TEST_PREFERRED_LINES(2, "123 456", -1, 1);
-  TEST_PREFERRED_LINES(2, "123 456", -2, 1);
-  TEST_PREFERRED_LINES(2, "123 456", std::numeric_limits<int>::min(), 1);
+TEST_F(BoundedLabelTest, GetLinesTest) {
+  // Zero and negative width values should yield zero lines.
+  TEST_LINES(0, "123 456", 0, 1);
+  TEST_LINES(0, "123 456", -1, 1);
+  TEST_LINES(0, "123 456", -2, 1);
+  TEST_LINES(0, "123 456", std::numeric_limits<int>::min(), 1);
+
+  // Small width values should yield one word per line.
+  TEST_LINES(1, "123 456", 1, 1);
+  TEST_LINES(2, "123 456", 1, 2);
+  TEST_LINES(1, "123 456", 2, 1);
+  TEST_LINES(2, "123 456", 2, 2);
+  TEST_LINES(1, "123 456", 3, 1);
+  TEST_LINES(2, "123 456", 3, 2);
 
   // Large width values should yield all words on one line.
-  TEST_PREFERRED_LINES(1, "123 456", 610, 1);
-  TEST_PREFERRED_LINES(1, "123 456", std::numeric_limits<int>::max(), 1);
+  TEST_LINES(1, "123 456", 610, 1);
+  TEST_LINES(1, "123 456", std::numeric_limits<int>::max(), 1);
 }
 
 /* Other tests ****************************************************************/
 
 // TODO(dharcourt): Add test cases to verify that:
-// - SetMaxLines() affects GetActualLines(), GetHeightForWidth(), GetTextSize(),
-//   and GetWrappedText() return values but not GetPreferredLines() or
-//   GetTextSize() ones.
+// - SetMaxLines() affects the return values of some methods but not others.
 // - Bound changes affects GetPreferredLines(), GetTextSize(), and
 //   GetWrappedText() return values.
 // - GetTextFlags are as expected.
 
+}  // namespace test
+
 }  // namespace message_center
diff --git a/ui/message_center/views/message_center_view_unittest.cc b/ui/message_center/views/message_center_view_unittest.cc
index f8dffc5..ba8ce45 100644
--- a/ui/message_center/views/message_center_view_unittest.cc
+++ b/ui/message_center/views/message_center_view_unittest.cc
@@ -186,15 +186,15 @@ TEST_F(MessageCenterViewTest, CallTest) {
   // Exercise (with size values that just need to be large enough).
   GetMessageCenterView()->SetBounds(0, 0, 100, 100);
 
-  // Verify that this didn't generate more than 1 Layout() call per descendant
-  // NotificationView or more than a total of 14 GetPreferredSize() and
-  // GetHeightForWidth() calls per descendant NotificationView. 14 is a very
+  // Verify that this didn't generate more than 2 Layout() call per descendant
+  // NotificationView or more than a total of 20 GetPreferredSize() and
+  // GetHeightForWidth() calls per descendant NotificationView. 20 is a very
   // large number corresponding to the current reality. That number will be
   // ratcheted down over time as the code improves.
-  EXPECT_LE(GetCallCount(LAYOUT), GetNotificationCount());
+  EXPECT_LE(GetCallCount(LAYOUT), GetNotificationCount() * 2);
   EXPECT_LE(GetCallCount(GET_PREFERRED_SIZE) +
             GetCallCount(GET_HEIGHT_FOR_WIDTH),
-            GetNotificationCount() * 14);
+            GetNotificationCount() * 20);
 }
 
 }  // namespace message_center
diff --git a/ui/message_center/views/notification_view.cc b/ui/message_center/views/notification_view.cc
index 42bbdd5..597ac96 100644
--- a/ui/message_center/views/notification_view.cc
+++ b/ui/message_center/views/notification_view.cc
@@ -46,9 +46,9 @@ const int kButtonIconToTitlePadding = 16;
 const int kButtonTitleTopPadding = 0;
 
 // Line limits.
-const size_t kTitleLineLimit = 3;
-const size_t kMessageCollapsedLineLimit = 3;
-const size_t kMessageExpandedLineLimit = 7;
+const int kTitleLineLimit = 3;
+const int kMessageCollapsedLineLimit = 3;
+const int kMessageExpandedLineLimit = 7;
 
 // Character limits: Displayed text will be subject to the line limits above,
 // but we also remove trailing characters from text to reduce processing cost.
@@ -386,19 +386,19 @@ NotificationView::NotificationView(const Notification& notification,
   if (!notification.title().empty()) {
     title_view_ = new BoundedLabel(
         ui::TruncateString(notification.title(), kTitleCharacterLimit),
-        views::Label().font().DeriveFont(2), kTitleLineLimit);
+        views::Label().font().DeriveFont(2));
+    title_view_->SetLineLimit(kTitleLineLimit);
     title_view_->SetColors(message_center::kRegularTextColor,
                            kRegularTextBackgroundColor);
     title_view_->set_border(MakeTextBorder(3, 0));
     top_view_->AddChildView(title_view_);
   }
 
-  // Create the message view if appropriate. Its line limit is given a bogus
-  // value here, it will be reset in Layout() after this view's width is set.
+  // Create the message view if appropriate.
   message_view_ = NULL;
   if (!notification.message().empty()) {
     message_view_ = new BoundedLabel(
-        ui::TruncateString(notification.message(), kMessageCharacterLimit), 0);
+        ui::TruncateString(notification.message(), kMessageCharacterLimit));
     message_view_->SetVisible(!is_expanded() || !notification.items().size());
     message_view_->SetColors(kDimTextColor, kDimTextBackgroundColor);
     message_view_->set_border(MakeTextBorder(4, 1));
@@ -483,10 +483,26 @@ gfx::Size NotificationView::GetPreferredSize() {
 }
 
 int NotificationView::GetHeightForWidth(int width) {
-  gfx::Insets insets = GetInsets();
-  int top_height = top_view_->GetHeightForWidth(width - insets.width());
-  int bottom_height = bottom_view_->GetHeightForWidth(width - insets.width());
-  return std::max(top_height, kIconSize) + bottom_height + insets.height();
+  // Get the height assuming no line limit changes.
+  int content_width = width - GetInsets().width();
+  int top_height = top_view_->GetHeightForWidth(content_width);
+  int bottom_height = bottom_view_->GetHeightForWidth(content_width);
+  int total_height = std::max(top_height, kIconSize) +
+                     bottom_height + GetInsets().height();
+
+  // Fix for http://crbug.com/230448: Adjust the height when the message_view's
+  // line limit would be different for the specified width than it currently is.
+  // TODO(dharcourt): Avoid BoxLayout and directly compute the correct height.
+  if (message_view_ && title_view_) {
+    int used_limit = message_view_->line_limit();
+    int correct_limit = GetMessageLineLimit(width);
+    if (used_limit != correct_limit) {
+      total_height -= GetMessageHeight(content_width, used_limit);
+      total_height += GetMessageHeight(content_width, correct_limit);
+    }
+  }
+
+  return total_height;
 }
 
 void NotificationView::Layout() {
@@ -496,7 +512,7 @@ void NotificationView::Layout() {
 
   // Before any resizing, set or adjust the number of message lines.
   if (message_view_)
-    message_view_->SetLineLimit(GetMessageLineLimit());
+    message_view_->SetLineLimit(GetMessageLineLimit(width()));
 
   // Background.
   background_view_->SetBounds(insets.left(), insets.top(),
@@ -521,14 +537,11 @@ void NotificationView::Layout() {
                             close_size.width(), close_size.height());
 
   // Expand button.
-  if (!IsExpansionNeeded()) {
-    expand_button()->SetVisible(false);
-  } else {
-    gfx::Size expand_size(expand_button()->GetPreferredSize());
-    int expand_y = bottom_y - expand_size.height();
-    expand_button()->SetBounds(content_right - expand_size.width(), expand_y,
-                               expand_size.width(), expand_size.height());
-  }
+  gfx::Size expand_size(expand_button()->GetPreferredSize());
+  int expand_y = bottom_y - expand_size.height();
+  expand_button()->SetVisible(IsExpansionNeeded(width()));
+  expand_button()->SetBounds(content_right - expand_size.width(), expand_y,
+                             expand_size.width(), expand_size.height());
 }
 
 void NotificationView::ButtonPressed(views::Button* sender,
@@ -548,8 +561,6 @@ void NotificationView::ButtonPressed(views::Button* sender,
   if (sender == expand_button()) {
     if (message_view_ && item_views_.size())
       message_view_->SetVisible(false);
-    else if (message_view_)
-      message_view_->SetLineLimit(GetMessageLineLimit());
     for (size_t i = 0; i < item_views_.size(); ++i)
       item_views_[i]->SetVisible(true);
     if (image_view_)
@@ -560,28 +571,43 @@ void NotificationView::ButtonPressed(views::Button* sender,
   }
 }
 
-bool NotificationView::IsExpansionNeeded() {
-  if (is_expanded())
-    return false;
-  if (image_view_ || item_views_.size())
-    return true;
-  size_t title_lines = title_view_ ? title_view_->GetActualLines() : 0;
-  size_t message_lines = message_view_ ? message_view_->GetPreferredLines() : 0;
-  return (title_lines + message_lines > kMessageCollapsedLineLimit);
+bool NotificationView::IsExpansionNeeded(int width) {
+  return (!is_expanded() &&
+              (image_view_ ||
+                  item_views_.size() ||
+                  IsMessageExpansionNeeded(width)));
+}
+
+bool NotificationView::IsMessageExpansionNeeded(int width) {
+  int current = GetMessageLines(width, GetMessageLineLimit(width));
+  int expanded = GetMessageLines(width, kMessageExpandedLineLimit);
+  return current < expanded;
 }
 
-size_t NotificationView::GetMessageLineLimit() {
-  // Return limit for expanded notifications, except for image notifications
-  // that always have collapsed messages to leave room for the image.
+int NotificationView::GetMessageLineLimit(int width) {
+  // Expanded notifications get a larger limit, except for image notifications,
+  // whose images must be kept flush against their icons.
   if (is_expanded() && !image_view_)
     return kMessageExpandedLineLimit;
 
   // If there's a title ensure title + message lines <= collapsed line limit.
-  if (title_view_)
-    return kMessageCollapsedLineLimit - title_view_->GetLinesForWidth(width());
+  if (title_view_) {
+    int title_lines = title_view_->GetLinesForWidthAndLimit(width, -1);
+    return std::max(kMessageCollapsedLineLimit - title_lines, 0);
+  }
 
   // If there's no title we get an extra line because message lines are shorter.
   return kMessageCollapsedLineLimit + 1;
 }
 
+int NotificationView::GetMessageLines(int width, int limit) {
+  return message_view_ ?
+         message_view_->GetLinesForWidthAndLimit(width, limit) : 0;
+}
+
+int NotificationView::GetMessageHeight(int width, int limit) {
+  return message_view_ ?
+         message_view_->GetSizeForWidthAndLines(width, limit).height() : 0;
+}
+
 }  // namespace message_center
diff --git a/ui/message_center/views/notification_view.h b/ui/message_center/views/notification_view.h
index 773673b..8afc4b8 100644
--- a/ui/message_center/views/notification_view.h
+++ b/ui/message_center/views/notification_view.h
@@ -46,8 +46,11 @@ class MESSAGE_CENTER_EXPORT NotificationView : public MessageView {
                    bool expanded);
 
  private:
-  bool IsExpansionNeeded();
-  size_t GetMessageLineLimit();
+  bool IsExpansionNeeded(int width);
+  bool IsMessageExpansionNeeded(int width);
+  int GetMessageLineLimit(int width);
+  int GetMessageLines(int width, int limit);
+  int GetMessageHeight(int width, int limit);
 
   // Weak references to NotificationView descendants owned by their parents.
   views::View* background_view_;