DialogDelegate: begin supplanting GetDialogButtonLabel

This change introduces DialogDelegate::Params::button_labels and
DialogDelegate::set_button_label(), and makes the default behavior of
DialogDelegate::GetDialogButtonLabel() check Params::button_labels for a label
assigned to the specified button.

This change also converts a random handful of dialogs in //chrome to using this
method to demonstrate how it will eventually look. There are many, many more to
convert (~200) so subsequent CLs will be necessary.

One big open question is what to do with dialogs like
LocalCardMigrationDialogView, which change the reults of GetDialogButtonLabel
based on the dialog's internal state...

Bug: 1011446
Change-Id: Idbf9464fe32e519a2150b00a2179558cd7b78adb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1853045Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705048}

chrome/browser/ui/views/chrome_cleaner_reboot_dialog_win.cc

@@ -50,6 +50,10 @@ ChromeCleanerRebootDialog::ChromeCleanerRebootDialog(
   DCHECK(dialog_controller_);
 
   DialogDelegate::set_draggable(true);
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_OK,
+      l10n_util::GetStringUTF16(
+          IDS_CHROME_CLEANUP_REBOOT_PROMPT_RESTART_BUTTON_LABEL));
 
   set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(
       views::TEXT, views::TEXT));
@@ -92,18 +96,6 @@ views::View* ChromeCleanerRebootDialog::GetInitiallyFocusedView() {
 }
 
 // DialogDelegate overrides.
-
-base::string16 ChromeCleanerRebootDialog::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  DCHECK(button == ui::DIALOG_BUTTON_OK || button == ui::DIALOG_BUTTON_CANCEL);
-  DCHECK(dialog_controller_);
-
-  return button == ui::DIALOG_BUTTON_OK
-             ? l10n_util::GetStringUTF16(
-                   IDS_CHROME_CLEANUP_REBOOT_PROMPT_RESTART_BUTTON_LABEL)
-             : DialogDelegate::GetDialogButtonLabel(button);
-}
-
 bool ChromeCleanerRebootDialog::Accept() {
   HandleDialogInteraction(DialogInteractionResult::kAccept);
   return true;

chrome/browser/ui/views/chrome_cleaner_reboot_dialog_win.h

@@ -40,7 +40,6 @@ class ChromeCleanerRebootDialog : public views::DialogDelegateView {
   views::View* GetInitiallyFocusedView() override;
 
   // views::DialogDelegate overrides.
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   bool Accept() override;
   bool Cancel() override;
   bool Close() override;

chrome/browser/ui/views/collected_cookies_views.cc

@@ -284,11 +284,6 @@ int CollectedCookiesViews::GetDialogButtons() const {
   return ui::DIALOG_BUTTON_OK;
 }
 
-base::string16 CollectedCookiesViews::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  return l10n_util::GetStringUTF16(IDS_DONE);
-}
-
 bool CollectedCookiesViews::Accept() {
   // If the user closes our parent tab while we're still open, this method will
   // (eventually) be called in response to a WebContentsDestroyed() call from
@@ -386,6 +381,9 @@ CollectedCookiesViews::CollectedCookiesViews(content::WebContents* web_contents)
     : web_contents_(web_contents) {
   constrained_window::ShowWebModalDialogViews(this, web_contents);
   chrome::RecordDialogCreation(chrome::DialogIdentifier::COLLECTED_COOKIES);
+
+  DialogDelegate::set_button_label(ui::DIALOG_BUTTON_OK,
+                                   l10n_util::GetStringUTF16(IDS_DONE));
 }
 
 void CollectedCookiesViews::Init() {

chrome/browser/ui/views/collected_cookies_views.h

@@ -52,7 +52,6 @@ class CollectedCookiesViews
   // views::DialogDelegate:
   base::string16 GetWindowTitle() const override;
   int GetDialogButtons() const override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   bool Accept() override;
   ui::ModalType GetModalType() const override;
   bool ShouldShowCloseButton() const override;

chrome/browser/ui/views/content_setting_bubble_contents.cc

@@ -403,6 +403,17 @@ ContentSettingBubbleContents::ContentSettingBubbleContents(
       content_setting_bubble_model_(std::move(content_setting_bubble_model)) {
   chrome::RecordDialogCreation(
       chrome::DialogIdentifier::CONTENT_SETTING_CONTENTS);
+
+  // Although other code in this class treats content_setting_bubble_model_ as
+  // though it's optional, in fact it can only become null if
+  // WebContentsDestroyed() is called, which can't happen until the constructor
+  // has run - so it is never null here.
+  DCHECK(content_setting_bubble_model_);
+  const base::string16& done_text =
+      content_setting_bubble_model_->bubble_content().done_button_text;
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_OK,
+      done_text.empty() ? l10n_util::GetStringUTF16(IDS_DONE) : done_text);
 }
 
 ContentSettingBubbleContents::~ContentSettingBubbleContents() {
@@ -627,16 +638,6 @@ int ContentSettingBubbleContents::GetDialogButtons() const {
   return ui::DIALOG_BUTTON_OK;
 }
 
-base::string16 ContentSettingBubbleContents::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  if (!content_setting_bubble_model_)
-    return base::string16();
-
-  const base::string16& done_text =
-      content_setting_bubble_model_->bubble_content().done_button_text;
-  return done_text.empty() ? l10n_util::GetStringUTF16(IDS_DONE) : done_text;
-}
-
 void ContentSettingBubbleContents::StyleLearnMoreButton() {
   DCHECK(learn_more_button_);
   SkColor text_color = GetNativeTheme()->GetSystemColor(

chrome/browser/ui/views/content_setting_bubble_contents.h

@@ -71,7 +71,6 @@ class ContentSettingBubbleContents : public content::WebContentsObserver,
   bool Accept() override;
   bool Close() override;
   int GetDialogButtons() const override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   void OnThemeChanged() override;
 
  private:

chrome/browser/ui/views/external_protocol_dialog.cc

@@ -57,11 +57,6 @@ bool ExternalProtocolDialog::ShouldShowCloseButton() const {
   return false;
 }
 
-base::string16 ExternalProtocolDialog::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  return delegate_->GetDialogButtonLabel(button);
-}
-
 base::string16 ExternalProtocolDialog::GetWindowTitle() const {
   return delegate_->GetTitleText();
 }
@@ -104,6 +99,13 @@ ExternalProtocolDialog::ExternalProtocolDialog(
     WebContents* web_contents)
     : delegate_(std::move(delegate)), creation_time_(base::TimeTicks::Now()) {
   DialogDelegate::set_default_button(ui::DIALOG_BUTTON_CANCEL);
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_OK,
+      delegate_->GetDialogButtonLabel(ui::DIALOG_BUTTON_OK));
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_CANCEL,
+      delegate_->GetDialogButtonLabel(ui::DIALOG_BUTTON_CANCEL));
+
   views::MessageBoxView::InitParams params(delegate_->GetMessageText());
   message_box_view_ = new views::MessageBoxView(params);
 

chrome/browser/ui/views/external_protocol_dialog.h

@@ -36,7 +36,6 @@ class ExternalProtocolDialog : public views::DialogDelegateView {
   // views::DialogDelegateView:
   gfx::Size CalculatePreferredSize() const override;
   bool ShouldShowCloseButton() const override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   base::string16 GetWindowTitle() const override;
   bool Cancel() override;
   bool Accept() override;

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

@@ -34,6 +34,11 @@ ToolbarActionsBarBubbleViews::ToolbarActionsBarBubbleViews(
       delegate_(std::move(delegate)),
       anchored_to_action_(anchored_to_action) {
   DialogDelegate::set_default_button(delegate_->GetDefaultDialogButton());
+  DialogDelegate::set_button_label(ui::DIALOG_BUTTON_OK,
+                                   delegate_->GetActionButtonText());
+  DialogDelegate::set_button_label(ui::DIALOG_BUTTON_CANCEL,
+                                   delegate_->GetDismissButtonText());
+
   DCHECK(anchor_view);
   set_close_on_deactivate(delegate_->ShouldCloseOnDeactivate());
   chrome::RecordDialogCreation(chrome::DialogIdentifier::TOOLBAR_ACTIONS_BAR);
@@ -180,12 +185,6 @@ int ToolbarActionsBarBubbleViews::GetDialogButtons() const {
   return buttons;
 }
 
-base::string16 ToolbarActionsBarBubbleViews::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  return button == ui::DIALOG_BUTTON_OK ? delegate_->GetActionButtonText()
-                                        : delegate_->GetDismissButtonText();
-}
-
 void ToolbarActionsBarBubbleViews::ButtonPressed(views::Button* sender,
                                                  const ui::Event& event) {
   DCHECK(!delegate_notified_of_close_);

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

@@ -47,7 +47,6 @@ class ToolbarActionsBarBubbleViews : public views::BubbleDialogDelegateView,
   bool Accept() override;
   bool Close() override;
   int GetDialogButtons() const override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   void Init() override;
 
   // views::ButtonListener:

chrome/browser/ui/webui/chromeos/add_supervision/confirm_signout_dialog.cc

@@ -29,6 +29,14 @@ const int kDialogBodyTextWidth = 250;
 }  // namespace
 
 ConfirmSignoutDialog::ConfirmSignoutDialog() {
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_OK,
+      l10n_util::GetStringUTF16(
+          IDS_ADD_SUPERVISION_EXIT_DIALOG_SIGNOUT_BUTTON_LABEL));
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_CANCEL,
+      l10n_util::GetStringUTF16(
+          IDS_ADD_SUPERVISION_EXIT_DIALOG_CANCEL_BUTTON_LABEL));
   SetLayoutManager(std::make_unique<views::FillLayout>());
   SetBorder(views::CreateEmptyBorder(
       views::LayoutProvider::Get()->GetDialogInsetsForContentType(
@@ -66,16 +74,6 @@ int ConfirmSignoutDialog::GetDialogButtons() const {
          ui::DialogButton::DIALOG_BUTTON_CANCEL;
 }
 
-base::string16 ConfirmSignoutDialog::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  if (button == ui::DialogButton::DIALOG_BUTTON_OK) {
-    return l10n_util::GetStringUTF16(
-        IDS_ADD_SUPERVISION_EXIT_DIALOG_SIGNOUT_BUTTON_LABEL);
-  }
-  return l10n_util::GetStringUTF16(
-      IDS_ADD_SUPERVISION_EXIT_DIALOG_CANCEL_BUTTON_LABEL);
-}
-
 // static
 views::Widget* ConfirmSignoutDialog::current_instance_ = nullptr;
 

chrome/browser/ui/webui/chromeos/add_supervision/confirm_signout_dialog.h

@@ -29,7 +29,6 @@ class ConfirmSignoutDialog : public views::DialogDelegateView {
   // views::DialogDelegate:
   bool Accept() override;
   int GetDialogButtons() const override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
 
   static void Show();
   static bool IsShowing();

ui/base/ui_base_types.h

@@ -25,9 +25,10 @@ enum WindowShowState {
 
 // Dialog button identifiers used to specify which buttons to show the user.
 enum DialogButton {
-  DIALOG_BUTTON_NONE   = 0,
-  DIALOG_BUTTON_OK     = 1,
+  DIALOG_BUTTON_NONE = 0,
+  DIALOG_BUTTON_OK = 1,
   DIALOG_BUTTON_CANCEL = 2,
+  DIALOG_BUTTON_LAST = DIALOG_BUTTON_CANCEL,
 };
 
 // Specifies the type of modality applied to a window. Different modal

ui/views/window/dialog_delegate.cc

@@ -121,6 +121,9 @@ int DialogDelegate::GetDefaultDialogButton() const {
 
 base::string16 DialogDelegate::GetDialogButtonLabel(
     ui::DialogButton button) const {
+  if (!GetParams().button_labels[button].empty())
+    return GetParams().button_labels[button];
+
   if (button == ui::DIALOG_BUTTON_OK)
     return l10n_util::GetStringUTF16(IDS_APP_OK);
   if (button == ui::DIALOG_BUTTON_CANCEL) {

ui/views/window/dialog_delegate.h

@@ -47,6 +47,12 @@ class VIEWS_EXPORT DialogDelegate : public WidgetDelegate {
     // should use the platform-native frame, and all other dialogs should use
     // the Views-styled one.
     bool custom_frame = true;
+
+    // Text labels for the buttons on this dialog. Any button without a label
+    // here will get the default text for its type from GetDialogButtonLabel.
+    // Prefer to use this field (via set_button_label) rather than override
+    // GetDialogButtonLabel - see https://crbug.com/1011446
+    base::string16 button_labels[ui::DIALOG_BUTTON_LAST + 1];
   };
 
   DialogDelegate();
@@ -159,6 +165,10 @@ class VIEWS_EXPORT DialogDelegate : public WidgetDelegate {
   void set_use_custom_frame(bool use) { params_.custom_frame = use; }
   bool use_custom_frame() const { return params_.custom_frame; }
 
+  void set_button_label(ui::DialogButton button, base::string16 label) {
+    params_.button_labels[button] = label;
+  }
+
  protected:
   ~DialogDelegate() override;