Remove DialogDelegate::OnClosed() which is redundant with

chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/password_manager/chrome_password_manager_client.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ skipping 182 matching lines @@
 
   SetupChooseCredentials(std::move(local_credentials),
                          ScopedVector<autofill::PasswordForm>(), origin);
   ASSERT_TRUE(controller()->current_account_chooser());
   AccountChooserDialogView* dialog = controller()->current_account_chooser();
   EXPECT_CALL(*this,
               OnChooseCredential(Field(
                   &password_manager::CredentialInfo::type,
                   password_manager::CredentialType::CREDENTIAL_TYPE_EMPTY)));
   EXPECT_CALL(*controller(), OnDialogClosed());
-  EXPECT_TRUE(dialog->Close());
+  dialog->GetWidget()->Close();
 
   EXPECT_FALSE(controller()->current_autosignin_prompt());
 }
 
 IN_PROC_BROWSER_TEST_F(
     PasswordDialogViewTest,
     PopupAccountChooserWithMultipleCredentialsReturnNonEmpty) {
   GURL origin("https://example.com");
   ScopedVector<autofill::PasswordForm> local_credentials;
   autofill::PasswordForm form;
@@ skipping 40 matching lines @@
   SetupChooseCredentials(std::move(local_credentials),
                          ScopedVector<autofill::PasswordForm>(), origin);
 
   EXPECT_TRUE(controller()->current_account_chooser());
   AccountChooserDialogView* dialog = controller()->current_account_chooser();
   EXPECT_CALL(*this,
               OnChooseCredential(Field(
                   &password_manager::CredentialInfo::type,
                   password_manager::CredentialType::CREDENTIAL_TYPE_EMPTY)));
   EXPECT_CALL(*controller(), OnDialogClosed());
-  EXPECT_TRUE(dialog->Close());
+  dialog->GetWidget()->Close();
   EXPECT_FALSE(controller()->current_autosignin_prompt());
 }
 
 IN_PROC_BROWSER_TEST_F(PasswordDialogViewTest,
                        PopupAccountChooserWithSingleCredentialReturnNonEmpty) {
   GURL origin("https://example.com");
   ScopedVector<autofill::PasswordForm> local_credentials;
   autofill::PasswordForm form;
   form.origin = origin;
   form.display_name = base::ASCIIToUTF16("Peter");
@@ skipping 10 matching lines @@
   EXPECT_CALL(*this,
               OnChooseCredential(Field(
                   &password_manager::CredentialInfo::type,
                   password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD)));
   EXPECT_TRUE(
       password_bubble_experiment::ShouldShowAutoSignInPromptFirstRunExperience(
           browser()->profile()->GetPrefs()));
   controller()->ChooseCredential(
       form, password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD);
 
-  EXPECT_CALL(*controller(), OnDialogClosed());

[vasilii, 2016/02/11 10:37:48]

It should be called when the autosignin dialog is destroyed.

[Evan Stade, 2016/02/11 20:02:11]

well, this is kinda janky. This wasn't being called on destruction which I think
was an oversight, although it doesn't even seem very important to call it in
this case. The fix is to call it when the controller is destroyed before the
dialog is closed. I had to make the change in the prod class as well as the test
class because virtual fns don't work in destructors.

[vasilii, 2016/02/12 12:57:09]

Please don't change the production code.
The method was called before your CL as the test passed. I wonder why it's not
called on dialog destruction anymore.

   EXPECT_TRUE(controller()->current_autosignin_prompt());
 }
 
 IN_PROC_BROWSER_TEST_F(PasswordDialogViewTest,
                        PopupAccountChooserWithDisabledAutoSignin) {
   EXPECT_TRUE(
       password_bubble_experiment::ShouldShowAutoSignInPromptFirstRunExperience(
           browser()->profile()->GetPrefs()));
   GURL origin("https://example.com");
   ScopedVector<autofill::PasswordForm> local_credentials;
@@ skipping 70 matching lines @@
   ASSERT_TRUE(controller()->current_autosignin_prompt());
   EXPECT_EQ(password_manager::ui::INACTIVE_STATE, controller()->GetState());
   AutoSigninFirstRunDialogView* dialog =
       controller()->current_autosignin_prompt();
   // This is the way how ESC is processed. It's important to reproduce it
   // because of double AutoSigninFirstRunDialogView::OnClosed call due to a bug
   // http://crbug.com/583330.
   ui::Accelerator esc(ui::VKEY_ESCAPE, 0);
   EXPECT_CALL(*controller(), OnDialogClosed());
   EXPECT_TRUE(dialog->GetWidget()->client_view()->AcceleratorPressed(esc));
+  content::RunAllPendingInMessageLoop();

[vasilii, 2016/02/11 10:37:48]

Is it now async?
You can drop http://crbug.com/583330 reference if you close it with this CL.

[Evan Stade, 2016/02/11 20:02:11]

Done.

   testing::Mock::VerifyAndClearExpectations(controller());
   EXPECT_TRUE(
       password_bubble_experiment::ShouldShowAutoSignInPromptFirstRunExperience(
           browser()->profile()->GetPrefs()));
 }
 
 IN_PROC_BROWSER_TEST_F(PasswordDialogViewTest,
                        PopupAutoSigninPromptAfterBlockedZeroclick) {
   EXPECT_TRUE(
       password_bubble_experiment::ShouldShowAutoSignInPromptFirstRunExperience(
@@ skipping 30 matching lines @@
   client()->NotifyUserAutoSigninBlockedOnFirstRun(std::move(blocked_form));
   client()->NotifySuccessfulLoginWithExistingPassword(form);
   ASSERT_FALSE(controller()->current_autosignin_prompt());
   browser()->profile()->GetPrefs()->SetBoolean(
       password_manager::prefs::kCredentialsEnableAutosignin, true);
 
   // Successful login with the same form after block will prompt:
   blocked_form.reset(new autofill::PasswordForm(form));
   client()->NotifyUserAutoSigninBlockedOnFirstRun(std::move(blocked_form));
   client()->NotifySuccessfulLoginWithExistingPassword(form);
-  EXPECT_CALL(*controller(), OnDialogClosed());

[vasilii, 2016/02/11 10:37:48]

Same as above

[Evan Stade, 2016/02/11 20:02:11]

Done.

   ASSERT_TRUE(controller()->current_autosignin_prompt());
 }
 
 }  // namespace