ash: fix feature disable dialog close behavior

As part of the DialogDelegate close refactor I changed the default
behavior of dialogs that have both cancel and accept callbacks to
require opting in to receiving a callback on close as well. This dialog
is one of the few that were both depending on this behavior and had no
test coverage for that dependency.

This change:
1) Opts that dialog into receiving a callback on close, using the
   pattern called "pattern 2" from r743269
2) Adds test coverage for the behavior here

Bug: 1056116
Change-Id: I3b4a30c6015bf76534b6b083586a13fdc55bac2a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2100727Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Jenny Zhang <jennyz@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750563}

ash/autoclick/autoclick_unittest.cc

@@ -1076,7 +1076,7 @@ TEST_F(AutoclickTest, ConfirmationDialogShownWhenDisablingFeature) {
   Shell::Get()->accessibility_controller()->SetAutoclickEnabled(false);
   AccessibilityFeatureDisableDialog* dialog =
       GetAutoclickController()->GetDisableDialogForTesting();
-  EXPECT_TRUE(dialog);
+  ASSERT_TRUE(dialog);
 
   // Canceling the dialog will cause the feature to continue to be enabled.
   dialog->CancelDialog();
@@ -1085,11 +1085,21 @@ TEST_F(AutoclickTest, ConfirmationDialogShownWhenDisablingFeature) {
   EXPECT_TRUE(Shell::Get()->accessibility_controller()->autoclick_enabled());
   EXPECT_TRUE(GetAutoclickController()->IsEnabled());
 
+  // Disable it again and close the dialog; the feature stays enabled.
+  Shell::Get()->accessibility_controller()->SetAutoclickEnabled(false);
+  dialog = GetAutoclickController()->GetDisableDialogForTesting();
+  ASSERT_TRUE(dialog);
+  dialog->GetWidget()->Close();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(GetAutoclickController()->GetDisableDialogForTesting());
+  EXPECT_TRUE(Shell::Get()->accessibility_controller()->autoclick_enabled());
+  EXPECT_TRUE(GetAutoclickController()->IsEnabled());
+
   // Try to disable it again, and this time accept the dialog to actually
   // disable the feature.
   Shell::Get()->accessibility_controller()->SetAutoclickEnabled(false);
   dialog = GetAutoclickController()->GetDisableDialogForTesting();
-  EXPECT_TRUE(dialog);
+  ASSERT_TRUE(dialog);
   dialog->AcceptDialog();
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(GetAutoclickController()->GetDisableDialogForTesting());

ash/system/accessibility/accessibility_feature_disable_dialog.cc

@@ -26,11 +26,19 @@ AccessibilityFeatureDisableDialog::AccessibilityFeatureDisableDialog(
     int dialog_text_id,
     base::OnceClosure on_accept_callback,
     base::OnceClosure on_cancel_callback)
-    : window_title_(l10n_util::GetStringUTF16(window_title_text_id)) {
+    : window_title_(l10n_util::GetStringUTF16(window_title_text_id)),
+      on_cancel_callback_(std::move(on_cancel_callback)) {
   DialogDelegate::SetButtonLabel(
       ui::DIALOG_BUTTON_OK, l10n_util::GetStringUTF16(IDS_ASH_YES_BUTTON));
   DialogDelegate::SetAcceptCallback(std::move(on_accept_callback));
-  DialogDelegate::SetCancelCallback(std::move(on_cancel_callback));
+
+  auto on_cancel = [](AccessibilityFeatureDisableDialog* dialog) {
+    std::move(dialog->on_cancel_callback_).Run();
+  };
+  DialogDelegate::SetCancelCallback(
+      base::BindOnce(on_cancel, base::Unretained(this)));
+  DialogDelegate::SetCloseCallback(
+      base::BindOnce(on_cancel, base::Unretained(this)));
 
   SetLayoutManager(std::make_unique<views::FillLayout>());
   SetBorder(views::CreateEmptyBorder(

ash/system/accessibility/accessibility_feature_disable_dialog.h

@@ -37,6 +37,8 @@ class AccessibilityFeatureDisableDialog : public views::DialogDelegateView {
  private:
   const base::string16 window_title_;
 
+  base::OnceClosure on_cancel_callback_;
+
   base::WeakPtrFactory<AccessibilityFeatureDisableDialog> weak_ptr_factory_{
       this};