cbui: remove more GetDialogButtonLabel overrides

This change removes overrides from:
* DesktopMediaPickerDialogView, which used static strings
* DownloadDangerPromptViews, which used static strings conditional on a
  constructor parameter
* HungRendererDialogView, which computes its strings but never updates them
  after creation
* PermissionsBubbleDialogDelegateView, which used static strings conditional on
  GetDialogButtons(), which itself returned a constant
* PasswordReuseModalWarningDialog, which used strings computed from a member
  that never changes after construction
* TabModalConfirmDialogViews, which gets its strings from a delegate but never
  updates them
* UninstallView, which used static strings

This change also removes a couple of redundant overrides of GetDialogButtons
that replicated the default, and const-ifies a couple of members where the
const-ness helps reason about code correctness.

Bug: 1011446
Change-Id: I6f89e975895c16a492cc0becad98f43a4f96e6c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1862119
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706104}

chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc

@@ -64,6 +64,9 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
     DesktopMediaPickerViews* parent,
     std::vector<std::unique_ptr<DesktopMediaList>> source_lists)
     : parent_(parent), modality_(params.modality) {
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_OK,
+      l10n_util::GetStringUTF16(IDS_DESKTOP_MEDIA_PICKER_SHARE));
   const ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
 
   SetLayoutManager(std::make_unique<views::BoxLayout>(
@@ -343,13 +346,6 @@ views::View* DesktopMediaPickerDialogView::GetInitiallyFocusedView() {
   return GetDialogClientView()->cancel_button();
 }
 
-base::string16 DesktopMediaPickerDialogView::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  return l10n_util::GetStringUTF16(button == ui::DIALOG_BUTTON_OK
-                                       ? IDS_DESKTOP_MEDIA_PICKER_SHARE
-                                       : IDS_CANCEL);
-}
-
 std::unique_ptr<views::View> DesktopMediaPickerDialogView::CreateExtraView() {
   std::unique_ptr<views::Checkbox> audio_share_checkbox;
   if (request_audio_) {

chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.h

@@ -51,7 +51,6 @@ class DesktopMediaPickerDialogView : public views::DialogDelegateView,
   base::string16 GetWindowTitle() const override;
   bool IsDialogButtonEnabled(ui::DialogButton button) const override;
   views::View* GetInitiallyFocusedView() override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   std::unique_ptr<views::View> CreateExtraView() override;
   bool Accept() override;
   bool ShouldShowCloseButton() const override;

chrome/browser/ui/views/download/download_danger_prompt_views.cc

@@ -54,7 +54,6 @@ class DownloadDangerPromptViews : public DownloadDangerPrompt,
 
   // views::DialogDelegateView:
   gfx::Size CalculatePreferredSize() const override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   base::string16 GetWindowTitle() const override;
   ui::ModalType GetModalType() const override;
   bool Cancel() override;
@@ -65,8 +64,6 @@ class DownloadDangerPromptViews : public DownloadDangerPrompt,
   void OnDownloadUpdated(download::DownloadItem* download) override;
 
  private:
-  base::string16 GetAcceptButtonTitle() const;
-  base::string16 GetCancelButtonTitle() const;
   base::string16 GetMessageBody() const;
   void RunDone(Action action);
 
@@ -75,7 +72,7 @@ class DownloadDangerPromptViews : public DownloadDangerPrompt,
   // If show_context_ is true, this is a download confirmation dialog by
   // download API, otherwise it is download recovery dialog from a regular
   // download.
-  bool show_context_;
+  const bool show_context_;
   OnDone done_;
 };
 
@@ -88,6 +85,15 @@ DownloadDangerPromptViews::DownloadDangerPromptViews(
       profile_(profile),
       show_context_(show_context),
       done_(done) {
+  // Note that this prompt is asking whether to cancel a dangerous download, so
+  // the accept path is titled "Cancel".
+  DialogDelegate::set_button_label(ui::DIALOG_BUTTON_OK,
+                                   l10n_util::GetStringUTF16(IDS_CANCEL));
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_CANCEL,
+      show_context_ ? l10n_util::GetStringUTF16(IDS_CONFIRM_DOWNLOAD)
+                    : l10n_util::GetStringUTF16(IDS_CONFIRM_DOWNLOAD_AGAIN));
+
   download_->AddObserver(this);
 
   set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(
@@ -135,20 +141,6 @@ void DownloadDangerPromptViews::InvokeActionForTesting(Action action) {
 }
 
 // views::DialogDelegate methods:
-base::string16 DownloadDangerPromptViews::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  switch (button) {
-    case ui::DIALOG_BUTTON_OK:
-      return GetAcceptButtonTitle();
-
-    case ui::DIALOG_BUTTON_CANCEL:
-      return GetCancelButtonTitle();
-
-    default:
-      return DialogDelegate::GetDialogButtonLabel(button);
-  }
-}
-
 base::string16 DownloadDangerPromptViews::GetWindowTitle() const {
   if (show_context_ || !download_)  // |download_| may be null in tests.
     return l10n_util::GetStringUTF16(IDS_CONFIRM_KEEP_DANGEROUS_DOWNLOAD_TITLE);
@@ -211,16 +203,6 @@ gfx::Size DownloadDangerPromptViews::CalculatePreferredSize() const {
   return gfx::Size(preferred_width, GetHeightForWidth(preferred_width));
 }
 
-base::string16 DownloadDangerPromptViews::GetAcceptButtonTitle() const {
-  return l10n_util::GetStringUTF16(IDS_CANCEL);
-}
-
-base::string16 DownloadDangerPromptViews::GetCancelButtonTitle() const {
-  if (show_context_)
-    return l10n_util::GetStringUTF16(IDS_CONFIRM_DOWNLOAD);
-  return l10n_util::GetStringUTF16(IDS_CONFIRM_DOWNLOAD_AGAIN);
-}
-
 base::string16 DownloadDangerPromptViews::GetMessageBody() const {
   if (show_context_) {
     switch (download_->GetDangerType()) {

chrome/browser/ui/views/hung_renderer_view.cc

@@ -369,20 +369,6 @@ void HungRendererDialogView::WindowClosing() {
   g_instance_ = nullptr;
 }
 
-int HungRendererDialogView::GetDialogButtons() const {
-  return ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL;
-}
-
-base::string16 HungRendererDialogView::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  if (button == ui::DIALOG_BUTTON_CANCEL) {
-    return l10n_util::GetPluralStringFUTF16(
-        IDS_BROWSER_HANGMONITOR_RENDERER_END,
-        hung_pages_table_model_->RowCount());
-  }
-  return l10n_util::GetStringUTF16(IDS_BROWSER_HANGMONITOR_RENDERER_WAIT);
-}
-
 bool HungRendererDialogView::Cancel() {
   auto* render_widget_host = hung_pages_table_model_->GetRenderWidgetHost();
   bool currently_unresponsive =
@@ -452,6 +438,15 @@ void HungRendererDialogView::Init() {
       hung_pages_table_model_.get(), columns, views::ICON_AND_TEXT, true);
   hung_pages_table_ = hung_pages_table.get();
 
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_CANCEL,
+      l10n_util::GetPluralStringFUTF16(IDS_BROWSER_HANGMONITOR_RENDERER_END,
+                                       hung_pages_table_model_->RowCount()));
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_OK,
+      l10n_util::GetStringUTF16(IDS_BROWSER_HANGMONITOR_RENDERER_WAIT));
+  DialogModelChanged();
+
   views::GridLayout* layout =
       SetLayoutManager(std::make_unique<views::GridLayout>());
   ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();

chrome/browser/ui/views/hung_renderer_view.h

@@ -166,8 +166,6 @@ class HungRendererDialogView : public views::DialogDelegateView,
   base::string16 GetWindowTitle() const override;
   bool ShouldShowCloseButton() const override;
   void WindowClosing() override;
-  int GetDialogButtons() const override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   bool Cancel() override;
   bool Accept() override;
   bool Close() override;

chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc

@@ -84,8 +84,6 @@ class PermissionsBubbleDialogDelegateView
   bool Accept() override;
   bool Close() override;
   void AddedToWidget() override;
-  int GetDialogButtons() const override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   void SizeToContents() override;
 
   // Repositions the bubble so it's displayed in the correct location based on
@@ -108,6 +106,11 @@ PermissionsBubbleDialogDelegateView::PermissionsBubbleDialogDelegateView(
   // measure against crbug.com/619429, permission prompts should not be accepted
   // as the default action.
   DialogDelegate::set_default_button(ui::DIALOG_BUTTON_NONE);
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_OK, l10n_util::GetStringUTF16(IDS_PERMISSION_ALLOW));
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_CANCEL, l10n_util::GetStringUTF16(IDS_PERMISSION_DENY));
+
   DCHECK(!requests.empty());
 
   set_close_on_deactivate(false);
@@ -182,21 +185,6 @@ void PermissionsBubbleDialogDelegateView::OnWidgetDestroying(
   }
 }
 
-int PermissionsBubbleDialogDelegateView::GetDialogButtons() const {
-  return ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL;
-}
-
-base::string16 PermissionsBubbleDialogDelegateView::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  if (button == ui::DIALOG_BUTTON_CANCEL)
-    return l10n_util::GetStringUTF16(IDS_PERMISSION_DENY);
-
-  // The text differs based on whether OK is the only visible button.
-  return l10n_util::GetStringUTF16(GetDialogButtons() == ui::DIALOG_BUTTON_OK
-                                       ? IDS_OK
-                                       : IDS_PERMISSION_ALLOW);
-}
-
 bool PermissionsBubbleDialogDelegateView::Cancel() {
   if (owner_)
     owner_->Deny();

chrome/browser/ui/views/safe_browsing/password_reuse_modal_warning_dialog.cc

@@ -96,6 +96,19 @@ views::Label* CreateMessageBodyLabel(base::string16 text) {
   return message_body_label;
 }
 
+base::string16 GetOkButtonLabel(
+    safe_browsing::ReusedPasswordAccountType password_type) {
+  switch (password_type.account_type()) {
+    case safe_browsing::ReusedPasswordAccountType::NON_GAIA_ENTERPRISE:
+      return l10n_util::GetStringUTF16(IDS_PAGE_INFO_CHANGE_PASSWORD_BUTTON);
+    case safe_browsing::ReusedPasswordAccountType::SAVED_PASSWORD:
+      return l10n_util::GetStringUTF16(
+          IDS_PAGE_INFO_DISMISS_PASSWORD_WARNING_BUTTON);
+    default:
+      return l10n_util::GetStringUTF16(IDS_PAGE_INFO_PROTECT_ACCOUNT_BUTTON);
+  }
+}
+
 }  // namespace
 
 namespace safe_browsing {
@@ -124,6 +137,12 @@ PasswordReuseModalWarningDialog::PasswordReuseModalWarningDialog(
       service_(service),
       url_(web_contents->GetLastCommittedURL()),
       password_type_(password_type) {
+  DialogDelegate::set_button_label(ui::DIALOG_BUTTON_OK,
+                                   GetOkButtonLabel(password_type_));
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_CANCEL,
+      l10n_util::GetStringUTF16(IDS_PAGE_INFO_IGNORE_PASSWORD_WARNING_BUTTON));
+
   // |service| maybe NULL in tests.
   if (service_)
     service_->AddObserver(this);
@@ -241,30 +260,6 @@ bool PasswordReuseModalWarningDialog::Close() {
   return true;
 }
 
-base::string16 PasswordReuseModalWarningDialog::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  switch (button) {
-    case ui::DIALOG_BUTTON_OK:
-      switch (password_type_.account_type()) {
-        case ReusedPasswordAccountType::NON_GAIA_ENTERPRISE:
-          return l10n_util::GetStringUTF16(
-              IDS_PAGE_INFO_CHANGE_PASSWORD_BUTTON);
-        case ReusedPasswordAccountType::SAVED_PASSWORD:
-          return l10n_util::GetStringUTF16(
-              IDS_PAGE_INFO_DISMISS_PASSWORD_WARNING_BUTTON);
-        default:
-          return l10n_util::GetStringUTF16(
-              IDS_PAGE_INFO_PROTECT_ACCOUNT_BUTTON);
-      }
-    case ui::DIALOG_BUTTON_CANCEL:
-      return l10n_util::GetStringUTF16(
-          IDS_PAGE_INFO_IGNORE_PASSWORD_WARNING_BUTTON);
-    default:
-      NOTREACHED();
-  }
-  return base::string16();
-}
-
 void PasswordReuseModalWarningDialog::OnGaiaPasswordChanged() {
   GetWidget()->Close();
 }

chrome/browser/ui/views/safe_browsing/password_reuse_modal_warning_dialog.h

@@ -49,7 +49,6 @@ class PasswordReuseModalWarningDialog
   bool Cancel() override;
   bool Accept() override;
   bool Close() override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
 
   // ChromePasswordProtectionService::Observer:
   void OnGaiaPasswordChanged() override;
@@ -64,7 +63,7 @@ class PasswordReuseModalWarningDialog
   OnWarningDone done_callback_;
   ChromePasswordProtectionService* service_;
   const GURL url_;
-  ReusedPasswordAccountType password_type_;
+  const ReusedPasswordAccountType password_type_;
 
   DISALLOW_COPY_AND_ASSIGN(PasswordReuseModalWarningDialog);
 };

chrome/browser/ui/views/tab_modal_confirm_dialog_views.cc

@@ -36,6 +36,11 @@ TabModalConfirmDialogViews::TabModalConfirmDialogViews(
     std::unique_ptr<TabModalConfirmDialogDelegate> delegate,
     content::WebContents* web_contents)
     : delegate_(std::move(delegate)) {
+  DialogDelegate::set_button_label(ui::DIALOG_BUTTON_OK,
+                                   delegate_->GetAcceptButtonTitle());
+  DialogDelegate::set_button_label(ui::DIALOG_BUTTON_CANCEL,
+                                   delegate_->GetCancelButtonTitle());
+
   base::Optional<int> default_button = delegate_->GetDefaultDialogButton();
   if (bool(default_button))
     DialogDelegate::set_default_button(*default_button);
@@ -89,15 +94,6 @@ base::string16 TabModalConfirmDialogViews::GetWindowTitle() const {
   return delegate_->GetTitle();
 }
 
-base::string16 TabModalConfirmDialogViews::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  if (button == ui::DIALOG_BUTTON_OK)
-    return delegate_->GetAcceptButtonTitle();
-  if (button == ui::DIALOG_BUTTON_CANCEL)
-    return delegate_->GetCancelButtonTitle();
-  return base::string16();
-}
-
 bool TabModalConfirmDialogViews::Cancel() {
   delegate_->Cancel();
   return true;

chrome/browser/ui/views/tab_modal_confirm_dialog_views.h

@@ -39,7 +39,6 @@ class TabModalConfirmDialogViews : public TabModalConfirmDialog,
   // views::DialogDelegate:
   int GetDialogButtons() const override;
   base::string16 GetWindowTitle() const override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   bool Cancel() override;
   bool Accept() override;
   bool Close() override;

chrome/browser/ui/views/uninstall_view.cc

@@ -28,6 +28,9 @@ UninstallView::UninstallView(int* user_selection,
       browsers_combo_(NULL),
       user_selection_(*user_selection),
       quit_closure_(quit_closure) {
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_OK,
+      l10n_util::GetStringUTF16(IDS_UNINSTALL_BUTTON_TEXT));
   set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(
       views::TEXT, views::TEXT));
   SetupControls();
@@ -143,14 +146,6 @@ bool UninstallView::Cancel() {
   return true;
 }
 
-base::string16 UninstallView::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  // Label the OK button 'Uninstall'; Cancel remains the same.
-  if (button == ui::DIALOG_BUTTON_OK)
-    return l10n_util::GetStringUTF16(IDS_UNINSTALL_BUTTON_TEXT);
-  return views::DialogDelegateView::GetDialogButtonLabel(button);
-}
-
 void UninstallView::ButtonPressed(views::Button* sender,
                                   const ui::Event& event) {
   if (change_default_browser_ == sender) {

chrome/browser/ui/views/uninstall_view.h

@@ -38,7 +38,6 @@ class UninstallView : public views::ButtonListener,
   // Overridden from views::DialogDelegateView:
   bool Accept() override;
   bool Cancel() override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
 
   // Overridden from views::WidgetDelegate:
   base::string16 GetWindowTitle() const override;