Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4772)

Unified Diff: chrome/browser/ui/views/download/download_item_view.cc

Issue 2556573002: Fix SizeLabelToMinWidth() function such that no unnecessary space (Closed)
Patch Set: change to static function Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 kDangerousTextW
Jialiu Lin 2016/12/08 22:44:03 Agree, this is the only place using this const, I'
+ // 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() {

Powered by Google App Engine
This is Rietveld 408576698