cbuiv: migrate CryptoModulePasswordDialogView to new callbacks

This change removes CryptoModulePasswordDialogView::{Accept,Cancel} and
replaces them with explicit callbacks. It also changes the behavior
(hence the separate CL): the old code used to explicitly clear the
password entry field's text on the closure path. I can't find any good
reason why this would be done, and views::TextField has never guaranteed
that setting the text will cause the old text to be securely erased from
memory. In any case, if we want that behavior, it should happen in
//ui/views, not here.

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

chrome/browser/ui/views/crypto_module_password_dialog_view.cc

@@ -28,6 +28,13 @@ CryptoModulePasswordDialogView::CryptoModulePasswordDialogView(
   DialogDelegate::set_button_label(
       ui::DIALOG_BUTTON_OK,
       l10n_util::GetStringUTF16(IDS_CRYPTO_MODULE_AUTH_DIALOG_OK_BUTTON_LABEL));
+  DialogDelegate::set_accept_callback(base::BindOnce(
+      [](CryptoModulePasswordDialogView* dialog) {
+        dialog->callback_.Run(
+            base::UTF16ToUTF8(dialog->password_entry_->GetText()));
+      },
+      base::Unretained(this)));
+  DialogDelegate::set_cancel_callback(base::BindOnce(callback_, std::string()));
   set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(
       views::TEXT, views::CONTROL));
   Init(hostname, slot_name, reason);
@@ -52,20 +59,6 @@ base::string16 CryptoModulePasswordDialogView::GetWindowTitle() const {
   return l10n_util::GetStringUTF16(IDS_CRYPTO_MODULE_AUTH_DIALOG_TITLE);
 }
 
-bool CryptoModulePasswordDialogView::Cancel() {
-  callback_.Run(std::string());
-  const base::string16 empty;
-  password_entry_->SetText(empty);
-  return true;
-}
-
-bool CryptoModulePasswordDialogView::Accept() {
-  callback_.Run(base::UTF16ToUTF8(password_entry_->GetText()));
-  const base::string16 empty;
-  password_entry_->SetText(empty);
-  return true;
-}
-
 void CryptoModulePasswordDialogView::ContentsChanged(
     views::Textfield* sender,
     const base::string16& new_contents) {

chrome/browser/ui/views/crypto_module_password_dialog_view.h

@@ -29,17 +29,16 @@ class CryptoModulePasswordDialogView : public views::DialogDelegateView,
   ~CryptoModulePasswordDialogView() override;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(CryptoModulePasswordDialogViewTest, TestAccept);
+  FRIEND_TEST_ALL_PREFIXES(CryptoModulePasswordDialogViewTest,
+                           AcceptUsesPassword);
+  FRIEND_TEST_ALL_PREFIXES(CryptoModulePasswordDialogViewTest,
+                           CancelDoesntUsePassword);
 
   // views::WidgetDelegate:
   views::View* GetInitiallyFocusedView() override;
   ui::ModalType GetModalType() const override;
   base::string16 GetWindowTitle() const override;
 
-  // views::DialogDelegate:
-  bool Cancel() override;
-  bool Accept() override;
-
   // views::TextfieldController:
   void ContentsChanged(views::Textfield* sender,
                        const base::string16& new_contents) override;

chrome/browser/ui/views/crypto_module_password_dialog_view_unittest.cc

@@ -6,43 +6,47 @@
 
 #include <string>
 
-#include "base/bind.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
 #include "chrome/browser/ui/crypto_module_password_dialog.h"
 #include "chrome/test/views/chrome_views_test_base.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/test/views_test_base.h"
 
-class CryptoModulePasswordDialogViewTest : public ChromeViewsTestBase {
- public:
-  CryptoModulePasswordDialogViewTest() {}
-  ~CryptoModulePasswordDialogViewTest() override {}
-
-  void Capture(const std::string& text) {
-    text_ = text;
-  }
-
-  void CreateCryptoDialog(const CryptoModulePasswordCallback& callback) {
-    dialog_ = std::make_unique<CryptoModulePasswordDialogView>(
-        "slot", kCryptoModulePasswordCertEnrollment, "server", callback);
-  }
-
-  std::string text_;
-  std::unique_ptr<CryptoModulePasswordDialogView> dialog_;
-};
-
-TEST_F(CryptoModulePasswordDialogViewTest, TestAccept) {
-  CryptoModulePasswordCallback cb(
-      base::Bind(&CryptoModulePasswordDialogViewTest::Capture,
-                 base::Unretained(this)));
-  CreateCryptoDialog(cb);
-  EXPECT_EQ(dialog_->password_entry_, dialog_->GetInitiallyFocusedView());
-  EXPECT_TRUE(dialog_->GetModalType() != ui::MODAL_TYPE_NONE);
+using CryptoModulePasswordDialogViewTest = ChromeViewsTestBase;
+
+std::unique_ptr<CryptoModulePasswordDialogView> CreateCryptoDialog(
+    const CryptoModulePasswordCallback& callback) {
+  return std::make_unique<CryptoModulePasswordDialogView>(
+      "slot", kCryptoModulePasswordCertEnrollment, "server", callback);
+}
+
+TEST_F(CryptoModulePasswordDialogViewTest, AcceptUsesPassword) {
+  std::string password;
+  auto dialog = CreateCryptoDialog(base::BindLambdaForTesting(
+      [&](const std::string& text) { password = text; }));
+  EXPECT_EQ(dialog->password_entry_, dialog->GetInitiallyFocusedView());
+  EXPECT_TRUE(dialog->GetModalType() != ui::MODAL_TYPE_NONE);
+
+  const std::string kPassword = "diAl0g";
+  dialog->password_entry_->SetText(base::ASCIIToUTF16(kPassword));
+  EXPECT_TRUE(dialog->Accept());
+  EXPECT_EQ(kPassword, password);
+}
+
+TEST_F(CryptoModulePasswordDialogViewTest, CancelDoesntUsePassword) {
+  std::string password;
+  bool callback_run = false;
+  auto dialog = CreateCryptoDialog(
+      base::BindLambdaForTesting([&](const std::string& text) {
+        callback_run = true;
+        password = text;
+      }));
+
   const std::string kPassword = "diAl0g";
-  dialog_->password_entry_->SetText(base::ASCIIToUTF16(kPassword));
-  EXPECT_TRUE(dialog_->Accept());
-  EXPECT_EQ(kPassword, text_);
-  const base::string16 empty;
-  EXPECT_EQ(empty, dialog_->password_entry_->GetText());
+  dialog->password_entry_->SetText(base::ASCIIToUTF16(kPassword));
+  EXPECT_TRUE(dialog->Cancel());
+  EXPECT_TRUE(callback_run);
+  EXPECT_EQ("", password);
 }