Use standard icon/title in global error bubble

chrome/browser/ui/views/global_error_bubble_view.cc

Index: chrome/browser/ui/views/global_error_bubble_view.cc
diff --git a/chrome/browser/ui/views/global_error_bubble_view.cc b/chrome/browser/ui/views/global_error_bubble_view.cc
index b4eee9fe2c08966e578db2612a0a4d25061ec5ef..5ed12b94c71bee1f4c5d3604a6debb96a6f41fbe 100644
--- a/chrome/browser/ui/views/global_error_bubble_view.cc
+++ b/chrome/browser/ui/views/global_error_bubble_view.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/ui/views/toolbar/wrench_toolbar_button.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image.h"
+#include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
@@ -31,9 +32,6 @@ enum {
 
 const int kMaxBubbleViewWidth = 262;
 
-// The horizontal padding between the title and the icon.
-const int kTitleHorizontalPadding = 5;
-
 // The vertical inset of the wrench bubble anchor from the wrench menu button.
 const int kAnchorVerticalInset = 5;
 
@@ -73,18 +71,6 @@ GlobalErrorBubbleView::GlobalErrorBubbleView(
   set_anchor_view_insets(
       gfx::Insets(kAnchorVerticalInset, 0, kAnchorVerticalInset, 0));
 
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  gfx::Image image = error_->GetBubbleViewIcon();
-  CHECK(!image.IsEmpty());
-  scoped_ptr<views::ImageView> image_view(new views::ImageView());
-  image_view->SetImage(image.ToImageSkia());
-
-  base::string16 title_string(error_->GetBubbleViewTitle());
-  scoped_ptr<views::Label> title_label(new views::Label(title_string));
-  title_label->SetMultiLine(true);
-  title_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  title_label->SetFontList(rb.GetFontList(ui::ResourceBundle::MediumFont));
-
   std::vector<base::string16> message_strings(error_->GetBubbleViewMessages());
   std::vector<views::Label*> message_labels;
   for (size_t i = 0; i < message_strings.size(); ++i) {
@@ -114,24 +100,17 @@ GlobalErrorBubbleView::GlobalErrorBubbleView(
 
   views::GridLayout* layout = new views::GridLayout(this);
   SetLayoutManager(layout);
-  layout->SetInsets(kBubblePadding, kBubblePadding,
-                    kBubblePadding, kBubblePadding);
 
-  // Top row, icon and title.
+  // BubbleFrameView adds enough padding below title, no top padding needed.
+  layout->SetInsets(0, kBubblePadding, kBubblePadding, kBubblePadding);
+
+  // First row, message labels.
   views::ColumnSet* cs = layout->AddColumnSet(0);
-  cs->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER,
-                0, views::GridLayout::USE_PREF, 0, 0);
-  cs->AddPaddingColumn(0, kTitleHorizontalPadding);
   cs->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL,
                 1, views::GridLayout::USE_PREF, 0, 0);
 
-  // Middle rows, message labels.
+  // Second row, accept and cancel button.
   cs = layout->AddColumnSet(1);
-  cs->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL,
-                1, views::GridLayout::USE_PREF, 0, 0);
-
-  // Bottom row, accept and cancel button.
-  cs = layout->AddColumnSet(2);
   cs->AddPaddingColumn(1, views::kRelatedControlHorizontalSpacing);
   cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::LEADING,
                 0, views::GridLayout::USE_PREF, 0, 0);
@@ -141,20 +120,15 @@ GlobalErrorBubbleView::GlobalErrorBubbleView(
                   0, views::GridLayout::USE_PREF, 0, 0);
   }
 
-  layout->StartRow(1, 0);
-  layout->AddView(image_view.release());
-  layout->AddView(title_label.release());
-  layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
-
   for (size_t i = 0; i < message_labels.size(); ++i) {
-    layout->StartRow(1, 1);
+    layout->StartRow(1, 0);
     layout->AddView(message_labels[i]);
     if (i < message_labels.size() - 1)
       layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
   }
   layout->AddPaddingRow(0, views::kLabelToControlVerticalSpacing);
 
-  layout->StartRow(0, 2);
+  layout->StartRow(0, 1);
   layout->AddView(accept_button.release());
   if (cancel_button.get())
     layout->AddView(cancel_button.release());
@@ -193,6 +167,24 @@ void GlobalErrorBubbleView::WindowClosing() {
     error_->BubbleViewDidClose(browser_);
 }
 
+bool GlobalErrorBubbleView::ShouldShowWindowTitle() const {
+  return true;

[msw, 2015/02/19 23:56:14]

You shouldn't need to override this, WidgetDelegateView already returns true.
(unless you want to do something like [D]CHECK(!GetWindowTitle().empty()))

[xiaoling, 2015/02/20 19:43:21]

Done.

+}
+
+base::string16 GlobalErrorBubbleView::GetWindowTitle() const {
+  return error_->GetBubbleViewTitle();
+}
+
+bool GlobalErrorBubbleView::ShouldShowWindowIcon() const {
+  return true;
+}
+
+gfx::ImageSkia GlobalErrorBubbleView::GetWindowIcon() {
+  gfx::Image image = error_->GetBubbleViewIcon();
+  CHECK(!image.IsEmpty());

[msw, 2015/02/19 23:56:13]

Do all global error bubbles really have valid icons? Maybe DCHECK?

[xiaoling, 2015/02/20 19:43:21]

Done.

+  return *image.ToImageSkia();
+}
+
 bool GlobalErrorBubbleView::ShouldShowCloseButton() const {
   return error_ && error_->ShouldShowCloseButton();
 }