Convert ToolbarActionsBarBubbleViews to BubbleDialogDelegate

chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc

Index: chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc
diff --git a/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc b/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc
index 8c6cc2cee86b4e4623ac1dc63472e832c19b8087..150aadd4b92fa55e2ef68b7c50ea0a8c1b9512c3 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc
@@ -9,19 +9,14 @@
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/label.h"
-#include "ui/views/layout/grid_layout.h"
-#include "ui/views/layout/layout_constants.h"
+#include "ui/views/layout/fill_layout.h"
 
 ToolbarActionsBarBubbleViews::ToolbarActionsBarBubbleViews(
     views::View* anchor_view,
     std::unique_ptr<ToolbarActionsBarBubbleDelegate> delegate)
-    : views::BubbleDelegateView(anchor_view, views::BubbleBorder::TOP_RIGHT),
-      delegate_(std::move(delegate)),
-      heading_label_(nullptr),
-      content_label_(nullptr),
-      dismiss_button_(nullptr),
-      action_button_(nullptr),
-      acknowledged_(false) {}
+    : views::BubbleDialogDelegateView(anchor_view,
+                                      views::BubbleBorder::TOP_RIGHT),
+      delegate_(std::move(delegate)) {}
 
 ToolbarActionsBarBubbleViews::~ToolbarActionsBarBubbleViews() {}
 
@@ -30,84 +25,45 @@ void ToolbarActionsBarBubbleViews::Show() {
   GetWidget()->Show();
 }
 
-void ToolbarActionsBarBubbleViews::Init() {
-  views::GridLayout* layout = new views::GridLayout(this);
-  layout->SetInsets(views::kPanelVertMargin, views::kPanelHorizMargin,
-                    views::kPanelVertMargin, views::kPanelHorizMargin);
-  SetLayoutManager(layout);
-
-  enum ColumnSetId {
-    HEADER_AND_BODY_COLUMN_SET = 0,
-    BUTTON_STRIP_COLUMN_SET,
-  };
-
-  views::ColumnSet* header_and_body_cs =
-      layout->AddColumnSet(HEADER_AND_BODY_COLUMN_SET);
-  header_and_body_cs->AddColumn(views::GridLayout::TRAILING,
-                                views::GridLayout::LEADING, 1,
-                                views::GridLayout::USE_PREF, 0, 0);
-  views::ColumnSet* buttons_cs = layout->AddColumnSet(BUTTON_STRIP_COLUMN_SET);
-  buttons_cs->AddPaddingColumn(1, 0);
-  buttons_cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::LEADING,
-                        0, views::GridLayout::USE_PREF, 0, 0);
-  buttons_cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
-  buttons_cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::LEADING,
-                        0, views::GridLayout::USE_PREF, 0, 0);
-
-  // Add a header.
-  layout->StartRow(0, HEADER_AND_BODY_COLUMN_SET);
-  heading_label_ = new views::Label(delegate_->GetHeadingText());
-  heading_label_->SetFontList(
-      ui::ResourceBundle::GetSharedInstance().GetFontList(
-          ui::ResourceBundle::MediumFont));
-  heading_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  layout->AddView(heading_label_);
-  layout->AddPaddingRow(0, views::kLabelToControlVerticalSpacing);
-  int width = views::Widget::GetLocalizedContentsWidth(
-      IDS_EXTENSION_TOOLBAR_REDESIGN_NOTIFICATION_BUBBLE_WIDTH_CHARS);
+base::string16 ToolbarActionsBarBubbleViews::GetWindowTitle() const {
+  return delegate_->GetHeadingText();
+}
 
-  // Add the content string.
-  layout->StartRow(0, HEADER_AND_BODY_COLUMN_SET);
-  content_label_ = new views::Label(delegate_->GetBodyText());
-  content_label_->SetMultiLine(true);
-  content_label_->SizeToFit(width);
-  content_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  layout->AddView(content_label_);
-  layout->AddPaddingRow(0, views::kLabelToControlVerticalSpacing);
+bool ToolbarActionsBarBubbleViews::Cancel() {
+  // Technically we don't know if it's an explicit user action or deactivation,
+  // but this reason isn't used for anything currently.

[Devlin, 2016/04/12 23:01:05]

This will be used after https://codereview.chromium.org/1881773002/.  I'd like
to see if we can get the close reason to be correct in this patch so that we
don't lose functionality that will be needed for combining with
ExtensionMessageBubble.

[Evan Stade, 2016/04/12 23:33:58]

Done.

+  delegate_->OnBubbleClosed(
+      ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION);
+  return true;
+}
 
-  layout->StartRow(0, BUTTON_STRIP_COLUMN_SET);
-  base::string16 dismiss_button_text = delegate_->GetDismissButtonText();
-  if (!dismiss_button_text.empty()) {
-    dismiss_button_ = new views::LabelButton(this, dismiss_button_text);
-    dismiss_button_->SetStyle(views::Button::STYLE_BUTTON);
-    layout->AddView(dismiss_button_, 1, 1, views::GridLayout::TRAILING,
-                    views::GridLayout::FILL);
-  } else {
-    layout->SkipColumns(1);
-  }
+bool ToolbarActionsBarBubbleViews::Accept() {
+  delegate_->OnBubbleClosed(ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE);
+  return true;
+}
 
-  action_button_ =
-      new views::LabelButton(this, delegate_->GetActionButtonText());
-  action_button_->SetStyle(views::Button::STYLE_BUTTON);
-  layout->AddView(action_button_, 1, 1, views::GridLayout::TRAILING,
-                  views::GridLayout::FILL);
+int ToolbarActionsBarBubbleViews::GetDialogButtons() const {
+  int buttons = ui::DIALOG_BUTTON_OK;
+  if (!delegate_->GetDismissButtonText().empty())
+    buttons |= ui::DIALOG_BUTTON_CANCEL;
+  return buttons;
 }
 
-void ToolbarActionsBarBubbleViews::OnWidgetDestroying(views::Widget* widget) {
-  BubbleDelegateView::OnWidgetDestroying(widget);
-  if (!acknowledged_) {
-    ToolbarActionsBarBubbleDelegate::CloseAction close_action =
-        close_reason() == CloseReason::DEACTIVATION
-            ? ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_DEACTIVATION
-            : ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION;
-    delegate_->OnBubbleClosed(close_action);
-    acknowledged_ = true;
-  }
+base::string16 ToolbarActionsBarBubbleViews::GetDialogButtonLabel(
+    ui::DialogButton button) const {
+  return button == ui::DIALOG_BUTTON_OK ? delegate_->GetActionButtonText()

[Devlin, 2016/04/12 23:01:05]

optional nit: I'd slightly prefer a switch here so that we can NOTREACHED on
ui::DIALOG_BUTTON_NONE and DCHECK(!delegate_->GetDismissButtonText().empty()) on
ui::DIALOG_BUTTON_CANCEL, and if we ever add something like a
ui::DIALOG_BUTTON_LEARN_MORE or lump close buttons in there or anything, we get
a compile error.

[Evan Stade, 2016/04/12 23:33:58]

I see what you're saying, but this pattern is pretty standard across dialogs.

+                                        : delegate_->GetDismissButtonText();
 }
 
-void ToolbarActionsBarBubbleViews::ButtonPressed(views::Button* sender,
-                                                 const ui::Event& event) {
-  delegate_->OnBubbleClosed(ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE);
-  acknowledged_ = true;
-  GetWidget()->Close();
+void ToolbarActionsBarBubbleViews::Init() {
+  SetLayoutManager(new views::FillLayout());
+
+  int width = views::Widget::GetLocalizedContentsWidth(
+      IDS_EXTENSION_TOOLBAR_REDESIGN_NOTIFICATION_BUBBLE_WIDTH_CHARS);
+  // Add the content string.
+  views::Label* content_label = new views::Label(delegate_->GetBodyText());
+  content_label->SetMultiLine(true);
+  content_label->SizeToFit(width);
+  content_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+  AddChildView(content_label);
 }