Handle bubble title resizing (growth) by reworking title layout

ui/views/bubble/bubble_frame_view.cc

Index: ui/views/bubble/bubble_frame_view.cc
diff --git a/ui/views/bubble/bubble_frame_view.cc b/ui/views/bubble/bubble_frame_view.cc
index 282ca89009dda9f05b08b378b7e180781b333ac9..2153d31ef8b22a23beb373db4055b0ee9a741dee 100644
--- a/ui/views/bubble/bubble_frame_view.cc
+++ b/ui/views/bubble/bubble_frame_view.cc
@@ -35,9 +35,6 @@ namespace views {
 
 namespace {
 
-// The horizontal padding between the title and the icon.
-const int kTitleHorizontalPadding = 5;
-
 // Background color of the footnote view.
 const SkColor kFootnoteBackgroundColor = SkColorSetRGB(245, 245, 245);
 
@@ -302,30 +299,24 @@ void BubbleFrameView::Layout() {
   close_position += gfx::Vector2d(-close_->width() - 7, 6);
   close_->SetPosition(close_position);
 
-  gfx::Size title_icon_size(title_icon_->GetPreferredSize());
-  gfx::Size title_label_size(title_->GetPreferredSize());
+  gfx::Size title_icon_pref_size(title_icon_->GetPreferredSize());
   int padding = 0;
-  if (title_icon_size.width() > 0 && title_label_size.width() > 0)
-    padding = kTitleHorizontalPadding;
-  const int title_height = std::max(title_icon_size.height(),
-                                    title_label_size.height());
-
-  const int title_icon_width = std::max(0, close_->x() - bounds.x());
-  title_icon_size.SetToMin(gfx::Size(title_icon_width, title_height));
-  gfx::Rect title_icon_bounds(
-      bounds.x(), bounds.y(), title_icon_size.width(), title_height);
-  title_icon_->SetBoundsRect(title_icon_bounds);
-
-  const int title_label_x = title_icon_->bounds().right() + padding;
-  const int title_label_width = std::max(0, close_->x() - title_label_x);
-  title_label_size.SetToMin(gfx::Size(title_label_width,
-                                      title_label_size.height()));
-  gfx::Rect title_label_bounds(
-      title_label_x, bounds.y(), title_label_size.width(), title_height);
-  title_->SetBoundsRect(title_label_bounds);
-
-  bounds.set_width(
-      title_icon_size.width() + title_label_size.width() + padding);
+
+  if (!title_->text().empty()) {

[msw, 2016/03/31 19:18:31]

Some WidgetDelegate may return a non-empty GetWindowTitle, but false for
ShouldShowWindowTitle, which was previously handled via
set_collapse_when_hidden(true) and pref size checking. Checking for a non-empty
string might regress layout in those cases; maybe check title_->visible() too?

[msw, 2016/03/31 21:28:27]

Ping

[Evan Stade, 2016/03/31 22:07:06]

Done. Good catch.

+    if (title_icon_pref_size.width() > 0)
+      padding = title_margins_.left();

[msw, 2016/03/31 19:18:31]

Hmm, re-using the title margins (for outside the icon and label) as the
inter-view spacing might be okay, but what is the actual px difference?
Isn't BubbleDialogDelegateView's use of kPanelHorizMargin (13) here much bigger
than the current value (5)?
The (current?) global error bubble is ugly, but how does this change make it
look? https://bugs.chromium.org/p/chromium/issues/detail?id=449737#c19

[Evan Stade, 2016/03/31 21:26:13]

This screenshot captures both visual improvements/simplifications:
https://screenshot.googleplex.com/yv3MVAqxAHB.png

+
+    const int title_label_x =
+        bounds.x() + title_icon_pref_size.width() + padding;
+    title_->SizeToFit(std::max(1, close_->x() - title_label_x));

[msw, 2016/03/31 19:18:31]

q: why 1 over 0? (for my enlightenment)

[Evan Stade, 2016/03/31 21:26:13]

because I'm afraid of crazy things like infinite loops when you try to do the
impossible (squeeze text into zero horizontal space)

+    title_->SetPosition(gfx::Point(title_label_x, bounds.y()));
+  }
+
+  const int title_height =
+      std::max(title_icon_pref_size.height(), title_->height());
+  title_icon_->SetBounds(bounds.x(), bounds.y(), title_icon_pref_size.width(),
+                         title_height);
+  bounds.set_width(title_->bounds().right() - title_icon_->x());

[msw, 2016/03/31 19:18:31]

nit: use bounds.x() instead of title_icon_->x()?

[Evan Stade, 2016/03/31 21:26:13]

Done.

   bounds.set_height(title_height);
 
   if (footnote_container_) {
@@ -508,7 +499,7 @@ gfx::Size BubbleFrameView::GetSizeForClientSize(
   gfx::Size title_icon_size = title_icon_->GetPreferredSize();
   gfx::Size title_label_size = title_->GetPreferredSize();
   if (title_icon_size.width() > 0 && title_label_size.width() > 0)
-    title_bar_width += kTitleHorizontalPadding;
+    title_bar_width += title_margins_.left();
   title_bar_width += title_icon_size.width();
   if (close_->visible())
     title_bar_width += close_->width() + 1;