cbuiv: migrate a few more Accept/Cancel overrides

This change:
1) Migrates ExtensionInstallDialogView to OnDialog* methods and removes
   the handled_result_ instance var, which is redundant with the
   presence of the callback
2) Migrates ExtensionUninstallDialogDelegateView to lambdas
3) Migrates ImportLockDialogView to lambdas with a close callback
4) Migrates PaymentRequestDialogView to OnDialog* methods
5) Migrates PlatformKeysCertificateSelector to a lambda
6) Migrates WebAppUninstallDialogDelegateView to OnDialog* methods
7) Migrates AppModalDialogViewViews to lambdas with a close callback

For each dialog I attempted a manual inspection to figure out whether a
close callback is needed or not.

Bug: 1011446
Change-Id: I038371177d932546afe556f9f0665f841bb95e7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135711Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757957}

chrome/browser/ui/views/extensions/extension_install_dialog_view.cc

@@ -200,7 +200,6 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
       prompt_(std::move(prompt)),
       title_(prompt_->GetDialogTitle()),
       scroll_view_(nullptr),
-      handled_result_(false),
       install_button_enabled_(false),
       withhold_permissions_checkbox_(nullptr) {
   DCHECK(prompt_->extension());
@@ -223,6 +222,10 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
 
   DialogDelegate::SetDefaultButton(default_button);
   DialogDelegate::SetButtons(buttons);
+  DialogDelegate::SetAcceptCallback(base::BindOnce(
+      &ExtensionInstallDialogView::OnDialogAccepted, base::Unretained(this)));
+  DialogDelegate::SetCancelCallback(base::BindOnce(
+      &ExtensionInstallDialogView::OnDialogCanceled, base::Unretained(this)));
   DialogDelegate::set_draggable(true);
   if (prompt_->has_webstore_data()) {
     auto store_link = std::make_unique<views::Link>(
@@ -249,10 +252,8 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
 }
 
 ExtensionInstallDialogView::~ExtensionInstallDialogView() {
-  if (!handled_result_ && !done_callback_.is_null()) {
-    std::move(done_callback_)
-        .Run(ExtensionInstallPrompt::Result::USER_CANCELED);
-  }
+  if (done_callback_)
+    OnDialogCanceled();
 }
 
 void ExtensionInstallDialogView::SetInstallButtonDelayForTesting(
@@ -366,20 +367,16 @@ void ExtensionInstallDialogView::AddedToWidget() {
   GetBubbleFrameView()->SetTitleView(std::move(title_container));
 }
 
-bool ExtensionInstallDialogView::Cancel() {
-  if (handled_result_)
-    return true;
+void ExtensionInstallDialogView::OnDialogCanceled() {
+  DCHECK(done_callback_);
 
-  handled_result_ = true;
   UpdateInstallResultHistogram(false);
   std::move(done_callback_).Run(ExtensionInstallPrompt::Result::USER_CANCELED);
-  return true;
 }
 
-bool ExtensionInstallDialogView::Accept() {
-  DCHECK(!handled_result_);
+void ExtensionInstallDialogView::OnDialogAccepted() {
+  DCHECK(done_callback_);
 
-  handled_result_ = true;
   UpdateInstallResultHistogram(true);
   // If the prompt had a checkbox element and it was checked we send that along
   // as the result, otherwise we just send a normal accepted result.
@@ -389,7 +386,6 @@ bool ExtensionInstallDialogView::Accept() {
           ? ExtensionInstallPrompt::Result::ACCEPTED_AND_OPTION_CHECKED
           : ExtensionInstallPrompt::Result::ACCEPTED;
   std::move(done_callback_).Run(result);
-  return true;
 }
 
 bool ExtensionInstallDialogView::IsDialogButtonEnabled(

chrome/browser/ui/views/extensions/extension_install_dialog_view.h

@@ -58,8 +58,6 @@ class ExtensionInstallDialogView
   gfx::Size CalculatePreferredSize() const override;
   void VisibilityChanged(views::View* starting_from, bool is_visible) override;
   void AddedToWidget() override;
-  bool Cancel() override;
-  bool Accept() override;
   bool IsDialogButtonEnabled(ui::DialogButton button) const override;
   bool ShouldShowCloseButton() const override;
 
@@ -77,6 +75,8 @@ class ExtensionInstallDialogView
   ui::ModalType GetModalType() const override;
 
   void LinkClicked();
+  void OnDialogCanceled();
+  void OnDialogAccepted();
 
   // Creates the contents area that contains permissions and other extension
   // info.
@@ -105,10 +105,6 @@ class ExtensionInstallDialogView
   // collapsible/expandable sections).
   views::ScrollView* scroll_view_;
 
-  // Set to true once the user's selection has been received and the callback
-  // has been run.
-  bool handled_result_;
-
   // Used to record time between dialog creation and acceptance, cancellation,
   // or dismissal.
   base::Optional<base::ElapsedTimer> install_result_timer_;

chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc

@@ -90,8 +90,6 @@ class ExtensionUninstallDialogDelegateView
   const char* GetClassName() const override;
 
   // views::DialogDelegateView:
-  bool Accept() override;
-  bool Cancel() override;
   gfx::Size CalculatePreferredSize() const override;
 
   // views::WidgetDelegate:
@@ -211,6 +209,21 @@ ExtensionUninstallDialogDelegateView::ExtensionUninstallDialogDelegateView(
       ui::DIALOG_BUTTON_OK,
       l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_UNINSTALL_BUTTON));
 
+  DialogDelegate::SetAcceptCallback(base::BindOnce(
+      [](ExtensionUninstallDialogDelegateView* view) {
+        if (view->dialog_) {
+          view->dialog_->DialogAccepted(view->checkbox_ &&
+                                        view->checkbox_->GetChecked());
+        }
+      },
+      base::Unretained(this)));
+  DialogDelegate::SetCancelCallback(base::BindOnce(
+      [](ExtensionUninstallDialogDelegateView* view) {
+        if (view->dialog_)
+          view->dialog_->DialogCanceled();
+      },
+      base::Unretained(this)));
+
   ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
   SetLayoutManager(std::make_unique<views::BoxLayout>(
       views::BoxLayout::Orientation::kVertical, gfx::Insets(),
@@ -271,18 +284,6 @@ const char* ExtensionUninstallDialogDelegateView::GetClassName() const {
   return "ExtensionUninstallDialogDelegateView";
 }
 
-bool ExtensionUninstallDialogDelegateView::Accept() {
-  if (dialog_)
-    dialog_->DialogAccepted(checkbox_ && checkbox_->GetChecked());
-  return true;
-}
-
-bool ExtensionUninstallDialogDelegateView::Cancel() {
-  if (dialog_)
-    dialog_->DialogCanceled();
-  return true;
-}
-
 gfx::Size ExtensionUninstallDialogDelegateView::CalculatePreferredSize() const {
   const int width = ChromeLayoutProvider::Get()->GetDistanceMetric(
                         is_bubble_ ? DISTANCE_BUBBLE_PREFERRED_WIDTH

chrome/browser/ui/views/importer/import_lock_dialog_view.cc

@@ -49,6 +49,17 @@ ImportLockDialogView::ImportLockDialogView(
   DialogDelegate::SetButtonLabel(
       ui::DIALOG_BUTTON_OK, l10n_util::GetStringUTF16(IDS_IMPORTER_LOCK_OK));
 
+  auto done_callback = [](ImportLockDialogView* dialog, bool accepted) {
+    if (dialog->callback_) {
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE, base::BindOnce(dialog->callback_, accepted));
+    }
+  };
+  DialogDelegate::SetAcceptCallback(
+      base::BindOnce(done_callback, base::Unretained(this), true));
+  DialogDelegate::SetCancelCallback(
+      base::BindOnce(done_callback, base::Unretained(this), false));
+
   SetLayoutManager(std::make_unique<views::FillLayout>());
   views::Label* description_label =
       new views::Label(l10n_util::GetStringUTF16(IDS_IMPORTER_LOCK_TEXT));
@@ -74,22 +85,6 @@ base::string16 ImportLockDialogView::GetWindowTitle() const {
   return l10n_util::GetStringUTF16(IDS_IMPORTER_LOCK_TITLE);
 }
 
-bool ImportLockDialogView::Accept() {
-  if (callback_) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(callback_, true));
-  }
-  return true;
-}
-
-bool ImportLockDialogView::Cancel() {
-  if (callback_) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(callback_, false));
-  }
-  return true;
-}
-
 bool ImportLockDialogView::ShouldShowCloseButton() const {
   return false;
 }

chrome/browser/ui/views/importer/import_lock_dialog_view.h

@@ -26,8 +26,6 @@ class ImportLockDialogView : public views::DialogDelegateView {
 
   // views::DialogDelegate:
   base::string16 GetWindowTitle() const override;
-  bool Accept() override;
-  bool Cancel() override;
 
   // views::WidgetDelegate:
   bool ShouldShowCloseButton() const override;

chrome/browser/ui/views/payments/payment_request_dialog_view.cc

@@ -65,6 +65,9 @@ PaymentRequestDialogView::PaymentRequestDialogView(
 
   DialogDelegate::SetButtons(ui::DIALOG_BUTTON_NONE);
 
+  DialogDelegate::SetCloseCallback(base::BindOnce(
+      &PaymentRequestDialogView::OnDialogClosed, base::Unretained(this)));
+
   request->spec()->AddObserver(this);
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
@@ -112,7 +115,7 @@ views::View* PaymentRequestDialogView::GetInitiallyFocusedView() {
   return view_stack_.get();
 }
 
-bool PaymentRequestDialogView::Cancel() {
+void PaymentRequestDialogView::OnDialogClosed() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   // Called when the widget is about to close. We send a message to the
   // PaymentRequest object to signal user cancellation.
@@ -128,7 +131,6 @@ bool PaymentRequestDialogView::Cancel() {
   view_stack_.reset();
   controller_map_.clear();
   request_->UserCancelled();
-  return true;
 }
 
 bool PaymentRequestDialogView::ShouldShowCloseButton() const {

chrome/browser/ui/views/payments/payment_request_dialog_view.h

@@ -101,7 +101,6 @@ class PaymentRequestDialogView : public views::DialogDelegateView,
   views::View* GetInitiallyFocusedView() override;
 
   // views::DialogDelegate:
-  bool Cancel() override;
   bool ShouldShowCloseButton() const override;
 
   // payments::PaymentRequestDialog:
@@ -185,6 +184,7 @@ class PaymentRequestDialogView : public views::DialogDelegateView,
   void OnDialogOpened();
   void ShowInitialPaymentSheet();
   void SetupSpinnerOverlay();
+  void OnDialogClosed();
 
   // views::View
   gfx::Size CalculatePreferredSize() const override;

chrome/browser/ui/views/platform_keys_certificate_selector_chromeos.cc

@@ -63,6 +63,11 @@ PlatformKeysCertificateSelector::PlatformKeysCertificateSelector(
       extension_name_(extension_name),
       callback_(callback) {
   DCHECK(!callback_.is_null());
+  DialogDelegate::SetCancelCallback(base::BindOnce(
+      [](PlatformKeysCertificateSelector* dialog) {
+        std::move(dialog->callback_).Run(nullptr);
+      },
+      base::Unretained(this)));
   chrome::RecordDialogCreation(
       chrome::DialogIdentifier::PLATFORM_KEYS_CERTIFICATE_SELECTOR);
 }
@@ -90,12 +95,6 @@ void PlatformKeysCertificateSelector::Init() {
   CertificateSelector::InitWithText(std::move(label));
 }
 
-bool PlatformKeysCertificateSelector::Cancel() {
-  DCHECK(!callback_.is_null());
-  std::move(callback_).Run(nullptr);
-  return true;
-}
-
 void PlatformKeysCertificateSelector::AcceptCertificate(
     std::unique_ptr<net::ClientCertIdentity> identity) {
   DCHECK(!callback_.is_null());

chrome/browser/ui/views/platform_keys_certificate_selector_chromeos.h

@@ -36,7 +36,6 @@ class PlatformKeysCertificateSelector : public chrome::CertificateSelector {
   void Init();
 
   // chrome::CertificateSelector:
-  bool Cancel() override;
   void AcceptCertificate(
       std::unique_ptr<net::ClientCertIdentity> identity) override;
 

chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_view.cc

@@ -68,6 +68,12 @@ WebAppUninstallDialogDelegateView::WebAppUninstallDialogDelegateView(
   DialogDelegate::SetButtonLabel(
       ui::DIALOG_BUTTON_OK,
       l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_UNINSTALL_BUTTON));
+  DialogDelegate::SetAcceptCallback(
+      base::BindOnce(&WebAppUninstallDialogDelegateView::OnDialogAccepted,
+                     base::Unretained(this)));
+  DialogDelegate::SetCancelCallback(
+      base::BindOnce(&WebAppUninstallDialogDelegateView::OnDialogCanceled,
+                     base::Unretained(this)));
 
   ChromeLayoutProvider* layout_provider = ChromeLayoutProvider::Get();
   SetLayoutManager(std::make_unique<views::BoxLayout>(
@@ -100,25 +106,20 @@ WebAppUninstallDialogDelegateView::~WebAppUninstallDialogDelegateView() {
     dialog_->CallCallback(/*uninstalled=*/false);
 }
 
-bool WebAppUninstallDialogDelegateView::Accept() {
+void WebAppUninstallDialogDelegateView::OnDialogAccepted() {
   if (!dialog_)
-    return true;
+    return;
 
   bool uninstalled = Uninstall();
   if (checkbox_->GetChecked())
     ClearWebAppSiteData();
 
-  dialog_->CallCallback(uninstalled);
-  dialog_ = nullptr;
-  return true;
+  std::exchange(dialog_, nullptr)->CallCallback(uninstalled);
 }
 
-bool WebAppUninstallDialogDelegateView::Cancel() {
-  if (dialog_) {
-    dialog_->CallCallback(/*uninstalled=*/false);
-    dialog_ = nullptr;
-  }
-  return true;
+void WebAppUninstallDialogDelegateView::OnDialogCanceled() {
+  if (dialog_)
+    std::exchange(dialog_, nullptr)->CallCallback(/*uninstalled=*/false);
 }
 
 gfx::Size WebAppUninstallDialogDelegateView::CalculatePreferredSize() const {

chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_view.h

@@ -49,8 +49,6 @@ class WebAppUninstallDialogDelegateView : public views::DialogDelegateView {
 
  private:
   // views::DialogDelegateView:
-  bool Accept() override;
-  bool Cancel() override;
   gfx::Size CalculatePreferredSize() const override;
 
   // views::WidgetDelegate:
@@ -65,6 +63,9 @@ class WebAppUninstallDialogDelegateView : public views::DialogDelegateView {
   void ClearWebAppSiteData();
   void ProcessAutoConfirmValue();
 
+  void OnDialogAccepted();
+  void OnDialogCanceled();
+
   WebAppUninstallDialogViews* dialog_;
   base::string16 app_name_;
 

components/javascript_dialogs/views/app_modal_dialog_view_views.cc

@@ -44,6 +44,21 @@ AppModalDialogViewViews::AppModalDialogViewViews(
               content::JAVASCRIPT_DIALOG_TYPE_ALERT
           ? ui::DIALOG_BUTTON_OK
           : (ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL));
+  DialogDelegate::SetAcceptCallback(base::BindOnce(
+      [](AppModalDialogViewViews* dialog) {
+        dialog->controller_->OnAccept(
+            dialog->message_box_view_->GetInputText(),
+            dialog->message_box_view_->IsCheckBoxSelected());
+      },
+      base::Unretained(this)));
+  auto cancel_callback = [](AppModalDialogViewViews* dialog) {
+    dialog->controller_->OnCancel(
+        dialog->message_box_view_->IsCheckBoxSelected());
+  };
+  DialogDelegate::SetCancelCallback(
+      base::BindOnce(cancel_callback, base::Unretained(this)));
+  DialogDelegate::SetCloseCallback(
+      base::BindOnce(cancel_callback, base::Unretained(this)));
 
   if (controller_->is_before_unload_dialog()) {
     DialogDelegate::SetButtonLabel(
@@ -96,17 +111,6 @@ void AppModalDialogViewViews::DeleteDelegate() {
   delete this;
 }
 
-bool AppModalDialogViewViews::Cancel() {
-  controller_->OnCancel(message_box_view_->IsCheckBoxSelected());
-  return true;
-}
-
-bool AppModalDialogViewViews::Accept() {
-  controller_->OnAccept(message_box_view_->GetInputText(),
-                        message_box_view_->IsCheckBoxSelected());
-  return true;
-}
-
 ui::ModalType AppModalDialogViewViews::GetModalType() const {
   return ui::MODAL_TYPE_SYSTEM;
 }

components/javascript_dialogs/views/app_modal_dialog_view_views.h

@@ -36,8 +36,6 @@ class AppModalDialogViewViews : public AppModalDialogView,
   // views::DialogDelegate:
   base::string16 GetWindowTitle() const override;
   void DeleteDelegate() override;
-  bool Cancel() override;
-  bool Accept() override;
   ui::ModalType GetModalType() const override;
   views::View* GetContentsView() override;
   views::View* GetInitiallyFocusedView() override;