Commit 2944cf12 authored by Vasilii Sukhanov's avatar Vasilii Sukhanov Committed by Commit Bot

Move PresaveGeneratedPassword from PasswordManagerDriver to PasswordManagerClient.

The UI needs the new password value. Thus, the method is needed in ChromePasswordManagerClient.
Also it doesn't seem right to scatter all the generation events across two interfaces while autofill::mojom::PasswordManagerClient solely concentrates on generation.

Bug: 851021
Change-Id: I6a6dcc59a934202bb0e2e336ff1324da131c6dfd
Reviewed-on: https://chromium-review.googlesource.com/1104339Reviewed-by: default avatarDominic Battré <battre@chromium.org>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Reviewed-by: default avatarVaclav Brozek <vabr@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569224}
parent 17766b88
......@@ -725,6 +725,16 @@ void ChromePasswordManagerClient::PasswordGenerationRejectedByTyping() {
HidePasswordGenerationPopup();
}
void ChromePasswordManagerClient::PresaveGeneratedPassword(
const autofill::PasswordForm& password_form) {
password_manager_.OnPresaveGeneratedPassword(password_form);
}
void ChromePasswordManagerClient::PasswordNoLongerGenerated(
const autofill::PasswordForm& password_form) {
password_manager_.OnPasswordNoLongerGenerated(password_form);
}
const GURL& ChromePasswordManagerClient::GetMainFrameURL() const {
return web_contents()->GetVisibleURL();
}
......
......@@ -125,6 +125,10 @@ class ChromePasswordManagerClient
const autofill::PasswordForm& form) override;
void GenerationAvailableForForm(const autofill::PasswordForm& form) override;
void PasswordGenerationRejectedByTyping() override;
void PresaveGeneratedPassword(
const autofill::PasswordForm& password_form) override;
void PasswordNoLongerGenerated(
const autofill::PasswordForm& password_form) override;
void HidePasswordGenerationPopup();
......
......@@ -41,20 +41,6 @@ void FakeContentPasswordManagerDriver::SameDocumentNavigation(
password_form_same_document_navigation_ = password_form;
}
void FakeContentPasswordManagerDriver::PresaveGeneratedPassword(
const autofill::PasswordForm& password_form) {
called_presave_generated_password_ = true;
password_is_generated_ = true;
EXPECT_EQ(autofill::PasswordForm::TYPE_GENERATED, password_form.type);
}
void FakeContentPasswordManagerDriver::PasswordNoLongerGenerated(
const autofill::PasswordForm& password_form) {
called_password_no_longer_generated_ = true;
password_is_generated_ = false;
EXPECT_EQ(autofill::PasswordForm::TYPE_GENERATED, password_form.type);
}
void FakeContentPasswordManagerDriver::ShowPasswordSuggestions(
int key,
base::i18n::TextDirection text_direction,
......@@ -98,7 +84,6 @@ void FakeContentPasswordManagerDriver::CheckSafeBrowsingReputation(
void FakeContentPasswordManagerDriver::ShowManualFallbackForSaving(
const autofill::PasswordForm& password_form) {
called_show_manual_fallback_for_saving_count_++;
last_fallback_for_saving_was_for_generated_password_ = password_is_generated_;
}
void FakeContentPasswordManagerDriver::HideManualFallbackForSaving() {
......
......@@ -122,22 +122,6 @@ class FakeContentPasswordManagerDriver
save_generation_field_ = base::nullopt;
}
bool called_password_no_longer_generated() const {
return called_password_no_longer_generated_;
}
void reset_called_password_no_longer_generated() {
called_password_no_longer_generated_ = false;
}
bool called_presave_generated_password() const {
return called_presave_generated_password_;
}
void reset_called_presave_generated_password() {
called_presave_generated_password_ = false;
}
int called_check_safe_browsing_reputation_cnt() const {
return called_check_safe_browsing_reputation_cnt_;
}
......@@ -146,10 +130,6 @@ class FakeContentPasswordManagerDriver
return called_show_manual_fallback_for_saving_count_;
}
bool last_fallback_for_saving_was_for_generated_password() const {
return last_fallback_for_saving_was_for_generated_password_;
}
bool called_manual_fallback_suggestion() {
return called_manual_fallback_suggestion_;
}
......@@ -169,12 +149,6 @@ class FakeContentPasswordManagerDriver
void SameDocumentNavigation(
const autofill::PasswordForm& password_form) override;
void PresaveGeneratedPassword(
const autofill::PasswordForm& password_form) override;
void PasswordNoLongerGenerated(
const autofill::PasswordForm& password_form) override;
void ShowPasswordSuggestions(int key,
base::i18n::TextDirection text_direction,
const base::string16& typed_username,
......@@ -234,12 +208,6 @@ class FakeContentPasswordManagerDriver
bool called_save_generation_field_ = false;
// Records data received via SaveGenerationFieldDetectedByClassifier() call.
base::Optional<base::string16> save_generation_field_;
// Records whether PresaveGeneratedPassword() gets called.
bool called_presave_generated_password_ = false;
// Records whether PasswordNoLongerGenerated() gets called.
bool called_password_no_longer_generated_ = false;
// True iff the current password is generated.
bool password_is_generated_ = false;
// Records number of times CheckSafeBrowsingReputation() gets called.
int called_check_safe_browsing_reputation_cnt_ = 0;
......@@ -247,8 +215,6 @@ class FakeContentPasswordManagerDriver
// Records the number of request to show manual fallback for password saving.
// If it is zero, the fallback is not available.
int called_show_manual_fallback_for_saving_count_ = 0;
// True if the last request of saving fallback was for a generated password.
bool last_fallback_for_saving_was_for_generated_password_ = false;
mojo::BindingSet<autofill::mojom::PasswordManagerDriver> bindings_;
};
......
......@@ -14,6 +14,7 @@
#include "components/autofill/core/common/password_form.h"
#include "components/autofill/core/common/password_generation_util.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "testing/gmock/include/gmock/gmock.h"
class FakePasswordManagerClient
: public autofill::mojom::PasswordManagerClient {
......@@ -59,6 +60,13 @@ class FakePasswordManagerClient
called_password_generation_rejected_by_typing_ = false;
}
// TODO(crbug.com/851021): move all the methods to GMock.
// autofill::mojom::PasswordManagerClient:
MOCK_METHOD1(PresaveGeneratedPassword,
void(const autofill::PasswordForm& password_form));
MOCK_METHOD1(PasswordNoLongerGenerated,
void(const autofill::PasswordForm& password_form));
private:
// autofill::mojom::PasswordManagerClient:
void AutomaticGenerationStatusChanged(
......@@ -91,6 +99,8 @@ class FakePasswordManagerClient
bool called_password_generation_rejected_by_typing_ = false;
mojo::AssociatedBinding<autofill::mojom::PasswordManagerClient> binding_;
DISALLOW_COPY_AND_ASSIGN(FakePasswordManagerClient);
};
#endif // CHROME_RENDERER_AUTOFILL_FAKE_PASSWORD_MANAGER_CLIENT_H_
......@@ -2363,6 +2363,9 @@ TEST_F(PasswordAutofillAgentTest,
GetMainFrame()->GetDocument(), 0, 2);
base::string16 password = base::ASCIIToUTF16("NewPass22");
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)));
password_generation_->GeneratedPasswordAccepted(password);
SaveAndSubmitForm();
......@@ -2377,12 +2380,15 @@ TEST_F(PasswordAutofillAgentTest,
SetAccountCreationFormsDetectedMessage(password_generation_,
GetMainFrame()->GetDocument(), 0, 2);
base::string16 password = base::ASCIIToUTF16("NewPass22");
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)));
password_generation_->GeneratedPasswordAccepted(password);
// A user fills username and password, the generated value is gone.
EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_));
SimulateOnFillPasswordForm(fill_data_);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(fake_driver_.called_password_no_longer_generated());
// The password field shoudln't reveal the value on focusing.
WebDocument document = GetMainFrame()->GetDocument();
......@@ -3280,11 +3286,10 @@ TEST_F(PasswordAutofillAgentTest, NoForm_MultipleAJAXEventsWithoutSubmission) {
}
TEST_F(PasswordAutofillAgentTest, ManualFallbackForSaving) {
EXPECT_CALL(fake_pw_client_, PresaveGeneratedPassword(testing::_)).Times(0);
// The users enters a username. No password - no fallback.
SimulateUsernameTyping(kUsernameName);
EXPECT_EQ(0, fake_driver_.called_show_manual_fallback_for_saving_count());
EXPECT_FALSE(
fake_driver_.last_fallback_for_saving_was_for_generated_password());
// The user enters a password.
SimulatePasswordTyping(kPasswordName);
......
......@@ -397,6 +397,9 @@ TEST_F(PasswordGenerationAgentTest, FillTest) {
EXPECT_TRUE(second_password_element.Value().IsNull());
base::string16 password = base::ASCIIToUTF16("random_password");
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)));
password_generation_->GeneratedPasswordAccepted(password);
// Password fields are filled out and set as being autofilled.
......@@ -436,6 +439,9 @@ TEST_F(PasswordGenerationAgentTest, EditingTest) {
WebInputElement second_password_element = element.To<WebInputElement>();
base::string16 password = base::ASCIIToUTF16("random_password");
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)));
password_generation_->GeneratedPasswordAccepted(password);
// Passwords start out the same.
......@@ -444,16 +450,18 @@ TEST_F(PasswordGenerationAgentTest, EditingTest) {
// After editing the first field they are still the same.
std::string edited_password_ascii = "edited_password";
base::string16 edited_password = base::ASCIIToUTF16(edited_password_ascii);
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, edited_password)));
SimulateUserInputChangeForElement(&first_password_element,
edited_password_ascii);
base::string16 edited_password = base::ASCIIToUTF16(edited_password_ascii);
EXPECT_EQ(edited_password, first_password_element.Value().Utf16());
EXPECT_EQ(edited_password, second_password_element.Value().Utf16());
fake_driver_.reset_called_password_no_longer_generated();
// Verify that password mirroring works correctly even when the password
// is deleted.
EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_));
SimulateUserInputChangeForElement(&first_password_element, std::string());
EXPECT_EQ(base::string16(), first_password_element.Value().Utf16());
EXPECT_EQ(base::string16(), second_password_element.Value().Utf16());
......@@ -461,7 +469,6 @@ TEST_F(PasswordGenerationAgentTest, EditingTest) {
// Should have notified the browser that the password is no longer generated
// and trigger generation again.
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(fake_driver_.called_password_no_longer_generated());
EXPECT_TRUE(GetCalledAutomaticGenerationStatusChangedTrue());
}
......@@ -742,35 +749,34 @@ TEST_F(PasswordGenerationAgentTest, PresavingGeneratedPassword) {
ExpectManualGenerationAvailable(test_case.generation_element, true);
base::string16 password = base::ASCIIToUTF16("random_password");
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)));
password_generation_->GeneratedPasswordAccepted(password);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(fake_driver_.called_presave_generated_password());
fake_driver_.reset_called_presave_generated_password();
FocusField(test_case.generation_element);
EXPECT_CALL(fake_pw_client_, PresaveGeneratedPassword(testing::_));
SimulateUserTypingASCIICharacter('a', true);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(fake_driver_.called_presave_generated_password());
fake_driver_.reset_called_presave_generated_password();
FocusField("username");
EXPECT_CALL(fake_pw_client_, PresaveGeneratedPassword(testing::_));
SimulateUserTypingASCIICharacter('X', true);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(fake_driver_.called_presave_generated_password());
fake_driver_.reset_called_presave_generated_password();
FocusField(test_case.generation_element);
EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_));
for (size_t i = 0; i < password.length(); ++i)
SimulateUserTypingASCIICharacter(ui::VKEY_BACK, false);
SimulateUserTypingASCIICharacter(ui::VKEY_BACK, true);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(fake_driver_.called_password_no_longer_generated());
fake_driver_.reset_called_password_no_longer_generated();
EXPECT_CALL(fake_pw_client_, PresaveGeneratedPassword(testing::_)).Times(0);
FocusField("username");
SimulateUserTypingASCIICharacter('Y', true);
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(fake_driver_.called_presave_generated_password());
testing::Mock::VerifyAndClearExpectations(&fake_pw_client_);
}
}
......@@ -779,18 +785,22 @@ TEST_F(PasswordGenerationAgentTest, FallbackForSaving) {
SelectGenerationFallbackInContextMenu("first_password");
ExpectManualGenerationAvailable("first_password", true);
EXPECT_EQ(0, fake_driver_.called_show_manual_fallback_for_saving_count());
password_generation_->GeneratedPasswordAccepted(
base::ASCIIToUTF16("random_password"));
base::string16 password = base::ASCIIToUTF16("random_password");
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)))
.WillOnce(testing::InvokeWithoutArgs([this]() {
// Make sure that generation event was propagated to the browser before
// the fallback showing. Otherwise, the fallback for saving provides a
// save bubble instead of a confirmation bubble.
EXPECT_EQ(0,
fake_driver_.called_show_manual_fallback_for_saving_count());
}));
password_generation_->GeneratedPasswordAccepted(password);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(fake_driver_.called_presave_generated_password());
// Two fallback requests are expected because generation changes either new
// password and confirmation fields.
EXPECT_EQ(2, fake_driver_.called_show_manual_fallback_for_saving_count());
// Make sure that generation event was propagated to the browser before the
// fallback showing. Otherwise, the fallback for saving provides a save bubble
// instead of a confirmation bubble.
EXPECT_TRUE(
fake_driver_.last_fallback_for_saving_was_for_generated_password());
}
TEST_F(PasswordGenerationAgentTest, FormClassifierVotesSignupForm) {
......@@ -848,6 +858,9 @@ TEST_F(PasswordGenerationAgentTest, ConfirmationFieldVoteFromServer) {
WebInputElement confirmation_password_element = element.To<WebInputElement>();
base::string16 password = base::ASCIIToUTF16("random_password");
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)));
password_generation_->GeneratedPasswordAccepted(password);
EXPECT_EQ(password, generation_element.Value().Utf16());
// Check that the generated password was copied according to the server's
......@@ -868,7 +881,11 @@ TEST_F(PasswordGenerationAgentTest, RevealPassword) {
const char* kTextFieldId = "username";
ExpectAutomaticGenerationAvailable(kGenerationElementId, true);
password_generation_->GeneratedPasswordAccepted(base::ASCIIToUTF16("pwd"));
base::string16 password = base::ASCIIToUTF16("pwd");
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)));
password_generation_->GeneratedPasswordAccepted(password);
const bool kFalseTrue[] = {false, true};
for (bool clickOnInputField : kFalseTrue) {
......@@ -899,12 +916,16 @@ TEST_F(PasswordGenerationAgentTest, JavascriptClearedTheField) {
const char kGenerationElementId[] = "first_password";
ExpectAutomaticGenerationAvailable(kGenerationElementId, true);
password_generation_->GeneratedPasswordAccepted(base::ASCIIToUTF16("pwd"));
base::string16 password = base::ASCIIToUTF16("pwd");
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)));
password_generation_->GeneratedPasswordAccepted(password);
EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_));
ExecuteJavaScriptForTests(
"document.getElementById('first_password').value = '';");
FocusField(kGenerationElementId);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(fake_driver_.called_password_no_longer_generated());
}
TEST_F(PasswordGenerationAgentTest, GenerationFallbackTest) {
......@@ -940,8 +961,8 @@ TEST_F(PasswordGenerationAgentTest, AutofillToGenerationField) {
const WebInputElement input_element = element.To<WebInputElement>();
// Since password isn't generated (just suitable field was detected),
// |OnFieldAutofilled| wouldn't trigger any actions.
EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_)).Times(0);
password_generation_->OnFieldAutofilled(input_element);
EXPECT_FALSE(fake_driver_.called_password_no_longer_generated());
}
TEST_F(PasswordGenerationAgentTestForHtmlAnnotation, AnnotateForm) {
......
......@@ -129,13 +129,6 @@ interface PasswordManagerDriver {
ShowManualFallbackSuggestion(mojo_base.mojom.TextDirection text_direction,
gfx.mojom.RectF bounds);
// Instructs the browser to presave the form with generated password.
PresaveGeneratedPassword(PasswordForm password_form);
// Instructs the browser that form no longer contains a generated password and
// the presaved form should be removed.
PasswordNoLongerGenerated(PasswordForm password_form);
// Sends the outcome of HTML parsing based form classifier that detects the
// forms where password generation should be available.
SaveGenerationFieldDetectedByClassifier(
......@@ -176,4 +169,11 @@ interface PasswordManagerClient {
// by the user typing more characters than the maximum offer size into the
// password field.
PasswordGenerationRejectedByTyping();
// Instructs the browser to presave the form with generated password.
PresaveGeneratedPassword(PasswordForm password_form);
// Instructs the browser that form no longer contains a generated password and
// the presaved form should be removed.
PasswordNoLongerGenerated(PasswordForm password_form);
};
......@@ -390,7 +390,7 @@ void PasswordGenerationAgent::GeneratedPasswordAccepted(
}
std::unique_ptr<PasswordForm> presaved_form(CreatePasswordFormToPresave());
if (presaved_form)
GetPasswordManagerDriver()->PresaveGeneratedPassword(*presaved_form);
GetPasswordManagerClient()->PresaveGeneratedPassword(*presaved_form);
// Call UpdateStateForTextChange after the corresponding PasswordFormManager
// is notified that the password was generated.
......@@ -591,7 +591,7 @@ bool PasswordGenerationAgent::TextDidChangeInTextField(
std::unique_ptr<PasswordForm> presaved_form(
CreatePasswordFormToPresave());
if (presaved_form)
GetPasswordManagerDriver()->PresaveGeneratedPassword(*presaved_form);
GetPasswordManagerClient()->PresaveGeneratedPassword(*presaved_form);
}
return false;
}
......@@ -611,7 +611,7 @@ bool PasswordGenerationAgent::TextDidChangeInTextField(
&generation_form_data_->password_elements);
std::unique_ptr<PasswordForm> presaved_form(CreatePasswordFormToPresave());
if (presaved_form) {
GetPasswordManagerDriver()->PresaveGeneratedPassword(*presaved_form);
GetPasswordManagerClient()->PresaveGeneratedPassword(*presaved_form);
}
} else if (element.Value().length() > kMaximumOfferSize) {
// User has rejected the feature and has started typing a password.
......@@ -695,7 +695,7 @@ void PasswordGenerationAgent::PasswordNoLongerGenerated() {
&generation_element_, &generation_form_data_->password_elements);
std::unique_ptr<PasswordForm> presaved_form(CreatePasswordFormToPresave());
if (presaved_form)
GetPasswordManagerDriver()->PasswordNoLongerGenerated(*presaved_form);
GetPasswordManagerClient()->PasswordNoLongerGenerated(*presaved_form);
}
void PasswordGenerationAgent::UserTriggeredGeneratePassword() {
......
......@@ -58,12 +58,6 @@ class FakeContentPasswordManagerDriver : public mojom::PasswordManagerDriver {
void SameDocumentNavigation(
const autofill::PasswordForm& password_form) override {}
void PresaveGeneratedPassword(
const autofill::PasswordForm& password_form) override {}
void PasswordNoLongerGenerated(
const autofill::PasswordForm& password_form) override {}
void ShowPasswordSuggestions(int key,
base::i18n::TextDirection text_direction,
const base::string16& typed_username,
......
......@@ -259,24 +259,6 @@ void ContentPasswordManagerDriver::SameDocumentNavigation(
GetPasswordManager()->OnSameDocumentNavigation(this, password_form);
}
void ContentPasswordManagerDriver::PresaveGeneratedPassword(
const autofill::PasswordForm& password_form) {
if (!CheckChildProcessSecurityPolicy(
password_form.origin,
BadMessageReason::CPMD_BAD_ORIGIN_PRESAVE_GENERATED_PASSWORD))
return;
GetPasswordManager()->OnPresaveGeneratedPassword(password_form);
}
void ContentPasswordManagerDriver::PasswordNoLongerGenerated(
const autofill::PasswordForm& password_form) {
if (!CheckChildProcessSecurityPolicy(
password_form.origin,
BadMessageReason::CPMD_BAD_ORIGIN_PASSWORD_NO_LONGER_GENERATED))
return;
GetPasswordManager()->OnPasswordNoLongerGenerated(password_form);
}
void ContentPasswordManagerDriver::SaveGenerationFieldDetectedByClassifier(
const autofill::PasswordForm& password_form,
const base::string16& generation_field) {
......
......@@ -97,10 +97,6 @@ class ContentPasswordManagerDriver
const autofill::PasswordForm& password_form) override;
void SameDocumentNavigation(
const autofill::PasswordForm& password_form) override;
void PresaveGeneratedPassword(
const autofill::PasswordForm& password_form) override;
void PasswordNoLongerGenerated(
const autofill::PasswordForm& password_form) override;
void ShowPasswordSuggestions(int key,
base::i18n::TextDirection text_direction,
const base::string16& typed_username,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment