Fix use-after-free in SettingsResetPromptDialog.

Button callbacks should clear |controller_| pointer as |controller_| deletes itself after calling Accept(), Cancel() or Close().
Otherwise SettingsResetPromptDialog dtor crashes trying to Close() already freed |controller_|.
Bug was recently introduced with https://crrev.com/c/2028171.

Change-Id: I9c9bc91f15bc639d4ebbb279091171fb07173825
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2199056Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#768757}

chrome/browser/ui/views/settings_reset_prompt_dialog.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ui/views/settings_reset_prompt_dialog.h"
 
+#include <utility>
+
 #include "chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
@@ -42,15 +44,24 @@ SettingsResetPromptDialog::SettingsResetPromptDialog(
   DialogDelegate::SetButtonLabel(
       ui::DIALOG_BUTTON_OK,
       l10n_util::GetStringUTF16(IDS_SETTINGS_RESET_PROMPT_ACCEPT_BUTTON_LABEL));
-  DialogDelegate::SetAcceptCallback(
-      base::BindOnce(&safe_browsing::SettingsResetPromptController::Accept,
-                     base::Unretained(controller_)));
-  DialogDelegate::SetCancelCallback(
-      base::BindOnce(&safe_browsing::SettingsResetPromptController::Cancel,
-                     base::Unretained(controller_)));
-  DialogDelegate::SetCloseCallback(
-      base::BindOnce(&safe_browsing::SettingsResetPromptController::Close,
-                     base::Unretained(controller_)));
+
+  // There is at most one of {Accept(), Cancel(), Close()} will be run for
+  // |controller_|. Each of them causes |controller_| deletion.
+  DialogDelegate::SetAcceptCallback(base::BindOnce(
+      [](SettingsResetPromptDialog* dialog) {
+        std::exchange(dialog->controller_, nullptr)->Accept();
+      },
+      base::Unretained(this)));
+  DialogDelegate::SetCancelCallback(base::BindOnce(
+      [](SettingsResetPromptDialog* dialog) {
+        std::exchange(dialog->controller_, nullptr)->Cancel();
+      },
+      base::Unretained(this)));
+  DialogDelegate::SetCloseCallback(base::BindOnce(
+      [](SettingsResetPromptDialog* dialog) {
+        std::exchange(dialog->controller_, nullptr)->Close();
+      },
+      base::Unretained(this)));
 
   set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(
       views::TEXT, views::TEXT));
@@ -100,8 +111,7 @@ bool SettingsResetPromptDialog::ShouldShowCloseButton() const {
 }
 
 base::string16 SettingsResetPromptDialog::GetWindowTitle() const {
-  DCHECK(controller_);
-  return controller_->GetWindowTitle();
+  return controller_ ? controller_->GetWindowTitle() : base::string16();
 }
 
 // View overrides.

chrome/browser/ui/views/settings_reset_prompt_dialog.h

@@ -45,7 +45,7 @@ class SettingsResetPromptDialog : public views::DialogDelegateView {
 
  private:
   Browser* browser_;
-  safe_browsing::SettingsResetPromptController* const controller_;
+  safe_browsing::SettingsResetPromptController* controller_;
 
   DISALLOW_COPY_AND_ASSIGN(SettingsResetPromptDialog);
 };

chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc

@@ -24,6 +24,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/test/test_browser_dialog.h"
+#include "chrome/browser/ui/views/settings_reset_prompt_dialog.h"
 #include "content/public/test/browser_test.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -171,4 +172,27 @@ IN_PROC_BROWSER_TEST_F(SettingsResetPromptDialogTest,
   ShowAndVerifyUi();
 }
 
+class SettingsResetPromptDialogCloseTest : public DialogBrowserTest {
+ public:
+  void ShowUi(const std::string& name) override {
+    auto model = std::make_unique<NiceMock<MockSettingsResetPromptModel>>(
+        browser()->profile(),
+        ModelParams{SettingType::DEFAULT_SEARCH_ENGINE, 0});
+
+    dialog_ = new SettingsResetPromptDialog(
+        new safe_browsing::SettingsResetPromptController(
+            std::move(model), std::make_unique<BrandcodedDefaultSettings>()));
+    dialog_->Show(browser());
+  }
+  void DismissUi() override { dialog_->Close(); }
+
+ private:
+  SettingsResetPromptDialog* dialog_ = nullptr;
+};
+
+IN_PROC_BROWSER_TEST_F(SettingsResetPromptDialogCloseTest,
+                       InvokeUi_DoNotCrashOnClose) {
+  ShowAndVerifyUi();
+}
+
 }  // namespace