Fix SizeLabelToMinWidth() function such that no unnecessary space

chrome/browser/ui/views/download/download_item_view.cc

Index: chrome/browser/ui/views/download/download_item_view.cc
diff --git a/chrome/browser/ui/views/download/download_item_view.cc b/chrome/browser/ui/views/download/download_item_view.cc
index 99b5344d90e8e6a49eaaed4000c68bd01650753a..b13dda407ae909a9e8076ea62a0e2cf11dd29cc6 100644
--- a/chrome/browser/ui/views/download/download_item_view.cc
+++ b/chrome/browser/ui/views/download/download_item_view.cc
@@ -1016,13 +1016,27 @@ void DownloadItemView::SizeLabelToMinWidth() {
   if (dangerous_download_label_sized_)
     return;
 
-  base::string16 label_text = dangerous_download_label_->text();
+  dangerous_download_label_->SetSize(
+      AdjustTextAndGetSize(dangerous_download_label_));
+  dangerous_download_label_sized_ = true;
+}
+
+// static
+gfx::Size DownloadItemView::AdjustTextAndGetSize(views::Label* label) {
+  gfx::Size size = label->GetPreferredSize();
+
+  // If the label is already narrower than kDangerousTextWidth, we don't need to

[Peter Kasting, 2016/12/08 21:53:52]

Nit: Can we move the definition of kDangerousTextWidth to here?  It's not
immediately obvious otherwise it's really local to this conditional in this
function.

(This came up as I tried to figure out a better header comment for this
function.)

[Jialiu Lin, 2016/12/08 22:44:03]

Agree, this is the only place using this const, I'll remove it altogether. 

+  // linebreak it, as it will fit on a single line.
+  if (size.width() <= kDangerousTextWidth)
+    return size;
+
+  base::string16 label_text = label->text();
   base::TrimWhitespace(label_text, base::TRIM_ALL, &label_text);
   DCHECK_EQ(base::string16::npos, label_text.find('\n'));
 
   // Make the label big so that GetPreferredSize() is not constrained by the
   // current width.
-  dangerous_download_label_->SetBounds(0, 0, 1000, 1000);
+  label->SetBounds(0, 0, 1000, 1000);
 
   // Use a const string from here. BreakIterator requies that text.data() not
   // change during its lifetime.
@@ -1035,16 +1049,11 @@ void DownloadItemView::SizeLabelToMinWidth() {
   bool status = iter.Init();
   DCHECK(status);
 
-  base::string16 prev_text = original_text;
-  gfx::Size size = dangerous_download_label_->GetPreferredSize();
-  int min_width = size.width();
-
   // Go through the string and try each line break (starting with no line break)
-  // searching for the optimal line break position.  Stop if we find one that
-  // yields one that is less than kDangerousTextWidth wide.  This is to prevent
-  // a short string (e.g.: "This file is malicious") from being broken up
-  // unnecessarily.
-  while (iter.Advance() && min_width > kDangerousTextWidth) {
+  // searching for the optimal line break position. Stop if we find one that
+  // yields minimum label width.
+  base::string16 prev_text = original_text;
+  for (gfx::Size min_width_size = size; iter.Advance(); min_width_size = size) {
     size_t pos = iter.pos();
     if (pos >= original_text.length())
       break;
@@ -1057,21 +1066,17 @@ void DownloadItemView::SizeLabelToMinWidth() {
       current_text.replace(pos - 1, 1, 1, base::char16('\n'));
     else
       current_text.insert(pos, 1, base::char16('\n'));
-    dangerous_download_label_->SetText(current_text);
-    size = dangerous_download_label_->GetPreferredSize();
+    label->SetText(current_text);
+    size = label->GetPreferredSize();
 
     // If the width is growing again, it means we passed the optimal width spot.
-    if (size.width() > min_width) {
-      dangerous_download_label_->SetText(prev_text);
-      break;
-    } else {
-      min_width = size.width();
+    if (size.width() > min_width_size.width()) {
+      label->SetText(prev_text);
+      return min_width_size;
     }
     prev_text = current_text;
   }
-
-  dangerous_download_label_->SetSize(size);
-  dangerous_download_label_sized_ = true;
+  return size;
 }
 
 void DownloadItemView::Reenable() {