Commit acf9b5dc authored by Vasilii Sukhanov's avatar Vasilii Sukhanov Committed by Commit Bot

Ignore blur events caused by the password generation.

When PasswordGenerationAgent copies the password value from the password field
to "Confirm Password" filed, a blur event is generated. It hides the currently
open generation prompt. Those events are just side-effects of how the class
works and should be ignored.

Bug: 899756,850079
Change-Id: Ia5d4f0184900fda97118252c48209ffcf1ce3444
Reviewed-on: https://chromium-review.googlesource.com/c/1327205
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarSebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606863}
parent 5657f328
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
FakePasswordManagerClient::FakePasswordManagerClient() : binding_(this) {} FakePasswordManagerClient::FakePasswordManagerClient() : binding_(this) {}
FakePasswordManagerClient::~FakePasswordManagerClient() {} FakePasswordManagerClient::~FakePasswordManagerClient() = default;
void FakePasswordManagerClient::BindRequest( void FakePasswordManagerClient::BindRequest(
autofill::mojom::PasswordManagerClientAssociatedRequest request) { autofill::mojom::PasswordManagerClientAssociatedRequest request) {
...@@ -33,10 +33,6 @@ void FakePasswordManagerClient::ShowManualPasswordGenerationPopup( ...@@ -33,10 +33,6 @@ void FakePasswordManagerClient::ShowManualPasswordGenerationPopup(
called_show_manual_pw_generation_popup_ = true; called_show_manual_pw_generation_popup_ = true;
} }
void FakePasswordManagerClient::ShowPasswordEditingPopup(
const gfx::RectF& bounds,
const autofill::PasswordForm& form) {}
void FakePasswordManagerClient::GenerationAvailableForForm( void FakePasswordManagerClient::GenerationAvailableForForm(
const autofill::PasswordForm& form) { const autofill::PasswordForm& form) {
called_generation_available_for_form_ = true; called_generation_available_for_form_ = true;
......
...@@ -66,6 +66,9 @@ class FakePasswordManagerClient ...@@ -66,6 +66,9 @@ class FakePasswordManagerClient
void(const autofill::PasswordForm& password_form)); void(const autofill::PasswordForm& password_form));
MOCK_METHOD1(PasswordNoLongerGenerated, MOCK_METHOD1(PasswordNoLongerGenerated,
void(const autofill::PasswordForm& password_form)); void(const autofill::PasswordForm& password_form));
MOCK_METHOD2(ShowPasswordEditingPopup,
void(const gfx::RectF& bounds,
const autofill::PasswordForm& form));
private: private:
// autofill::mojom::PasswordManagerClient: // autofill::mojom::PasswordManagerClient:
...@@ -79,9 +82,6 @@ class FakePasswordManagerClient ...@@ -79,9 +82,6 @@ class FakePasswordManagerClient
const autofill::password_generation::PasswordGenerationUIData& ui_data) const autofill::password_generation::PasswordGenerationUIData& ui_data)
override; override;
void ShowPasswordEditingPopup(const gfx::RectF& bounds,
const autofill::PasswordForm& form) override;
void GenerationAvailableForForm(const autofill::PasswordForm& form) override; void GenerationAvailableForForm(const autofill::PasswordForm& form) override;
void PasswordGenerationRejectedByTyping() override; void PasswordGenerationRejectedByTyping() override;
......
...@@ -39,6 +39,7 @@ using blink::WebElement; ...@@ -39,6 +39,7 @@ using blink::WebElement;
using blink::WebInputElement; using blink::WebInputElement;
using blink::WebNode; using blink::WebNode;
using blink::WebString; using blink::WebString;
using testing::_;
namespace autofill { namespace autofill {
...@@ -456,6 +457,7 @@ TEST_F(PasswordGenerationAgentTest, EditingTest) { ...@@ -456,6 +457,7 @@ TEST_F(PasswordGenerationAgentTest, EditingTest) {
EXPECT_CALL(fake_pw_client_, EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field( PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, edited_password))); &autofill::PasswordForm::password_value, edited_password)));
EXPECT_CALL(fake_pw_client_, ShowPasswordEditingPopup(_, _));
SimulateUserInputChangeForElement(&first_password_element, SimulateUserInputChangeForElement(&first_password_element,
edited_password_ascii); edited_password_ascii);
EXPECT_EQ(edited_password, first_password_element.Value().Utf16()); EXPECT_EQ(edited_password, first_password_element.Value().Utf16());
...@@ -478,6 +480,58 @@ TEST_F(PasswordGenerationAgentTest, EditingTest) { ...@@ -478,6 +480,58 @@ TEST_F(PasswordGenerationAgentTest, EditingTest) {
EXPECT_TRUE(GetCalledAutomaticGenerationStatusChangedTrue()); EXPECT_TRUE(GetCalledAutomaticGenerationStatusChangedTrue());
} }
TEST_F(PasswordGenerationAgentTest, EditingEventsTest) {
LoadHTMLWithUserGesture(kAccountCreationFormHTML);
SetNotBlacklistedMessage(password_generation_, kAccountCreationFormHTML);
SetAccountCreationFormsDetectedMessage(password_generation_,
GetMainFrame()->GetDocument(), 0, 1);
// Generate password.
FocusField("first_password");
base::string16 password = base::ASCIIToUTF16("random_password");
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)));
password_generation_->GeneratedPasswordAccepted(password);
fake_pw_client_.Flush();
fake_pw_client_.reset_called_automatic_generation_status_changed_true();
testing::Mock::VerifyAndClearExpectations(&fake_pw_client_);
// Start removing characters one by one and observe the events sent to the
// browser.
EXPECT_CALL(fake_pw_client_, ShowPasswordEditingPopup(_, _));
FocusField("first_password");
fake_pw_client_.Flush();
testing::Mock::VerifyAndClearExpectations(&fake_pw_client_);
size_t max_chars_to_delete_before_editing =
password.length() -
PasswordGenerationAgent::kMinimumLengthForEditedPassword;
for (size_t i = 0; i < max_chars_to_delete_before_editing; ++i) {
password.erase(password.end() - 1);
EXPECT_CALL(fake_pw_client_,
PresaveGeneratedPassword(testing::Field(
&autofill::PasswordForm::password_value, password)));
SimulateUserTypingASCIICharacter(ui::VKEY_BACK, true);
fake_pw_client_.Flush();
fake_driver_.Flush();
EXPECT_TRUE(fake_driver_.last_focused_element_was_fillable());
EXPECT_TRUE(fake_driver_.last_focused_input_was_password());
testing::Mock::VerifyAndClearExpectations(&fake_pw_client_);
}
// Delete one more character and move back to the generation state.
EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(_));
SimulateUserTypingASCIICharacter(ui::VKEY_BACK, true);
fake_pw_client_.Flush();
// The remaining characters no longer count as a generated password, so
// generation should be offered again.
EXPECT_TRUE(GetCalledAutomaticGenerationStatusChangedTrue());
// Last focused element shouldn't change while editing.
fake_driver_.Flush();
EXPECT_TRUE(fake_driver_.last_focused_element_was_fillable());
EXPECT_TRUE(fake_driver_.last_focused_input_was_password());
}
TEST_F(PasswordGenerationAgentTest, BlacklistedTest) { TEST_F(PasswordGenerationAgentTest, BlacklistedTest) {
// Did not receive not blacklisted message. Don't show password generation // Did not receive not blacklisted message. Don't show password generation
// icon. // icon.
...@@ -605,6 +659,7 @@ TEST_F(PasswordGenerationAgentTest, MinimumLengthForEditedPassword) { ...@@ -605,6 +659,7 @@ TEST_F(PasswordGenerationAgentTest, MinimumLengthForEditedPassword) {
testing::Mock::VerifyAndClearExpectations(&fake_pw_client_); testing::Mock::VerifyAndClearExpectations(&fake_pw_client_);
// Delete most of the password. // Delete most of the password.
EXPECT_CALL(fake_pw_client_, ShowPasswordEditingPopup(_, _));
FocusField("first_password"); FocusField("first_password");
size_t max_chars_to_delete = size_t max_chars_to_delete =
password.length() - password.length() -
...@@ -828,6 +883,7 @@ TEST_F(PasswordGenerationAgentTest, PresavingGeneratedPassword) { ...@@ -828,6 +883,7 @@ TEST_F(PasswordGenerationAgentTest, PresavingGeneratedPassword) {
password_generation_->GeneratedPasswordAccepted(password); password_generation_->GeneratedPasswordAccepted(password);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_CALL(fake_pw_client_, ShowPasswordEditingPopup(_, _));
FocusField(test_case.generation_element); FocusField(test_case.generation_element);
EXPECT_CALL(fake_pw_client_, PresaveGeneratedPassword(testing::_)); EXPECT_CALL(fake_pw_client_, PresaveGeneratedPassword(testing::_));
SimulateUserTypingASCIICharacter('a', true); SimulateUserTypingASCIICharacter('a', true);
...@@ -838,6 +894,7 @@ TEST_F(PasswordGenerationAgentTest, PresavingGeneratedPassword) { ...@@ -838,6 +894,7 @@ TEST_F(PasswordGenerationAgentTest, PresavingGeneratedPassword) {
SimulateUserTypingASCIICharacter('X', true); SimulateUserTypingASCIICharacter('X', true);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_CALL(fake_pw_client_, ShowPasswordEditingPopup(_, _));
FocusField(test_case.generation_element); FocusField(test_case.generation_element);
EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_)); EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_));
for (size_t i = 0; i < password.length(); ++i) for (size_t i = 0; i < password.length(); ++i)
...@@ -950,7 +1007,9 @@ TEST_F(PasswordGenerationAgentTest, RevealPassword) { ...@@ -950,7 +1007,9 @@ TEST_F(PasswordGenerationAgentTest, RevealPassword) {
for (bool clickOnInputField : kFalseTrue) { for (bool clickOnInputField : kFalseTrue) {
SCOPED_TRACE(testing::Message("clickOnInputField = ") << clickOnInputField); SCOPED_TRACE(testing::Message("clickOnInputField = ") << clickOnInputField);
// Click on the generation field to reveal the password value. // Click on the generation field to reveal the password value.
EXPECT_CALL(fake_pw_client_, ShowPasswordEditingPopup(_, _));
FocusField(kGenerationElementId); FocusField(kGenerationElementId);
fake_pw_client_.Flush();
WebDocument document = GetMainFrame()->GetDocument(); WebDocument document = GetMainFrame()->GetDocument();
blink::WebElement element = document.GetElementById( blink::WebElement element = document.GetElementById(
...@@ -1130,6 +1189,7 @@ TEST_F(PasswordGenerationAgentTest, PasswordUnmaskedUntilCompleteDeletion) { ...@@ -1130,6 +1189,7 @@ TEST_F(PasswordGenerationAgentTest, PasswordUnmaskedUntilCompleteDeletion) {
// Delete characters of the generated password until only // Delete characters of the generated password until only
// |kMinimumLengthForEditedPassword| - 1 chars remain. // |kMinimumLengthForEditedPassword| - 1 chars remain.
fake_pw_client_.reset_called_automatic_generation_status_changed_true(); fake_pw_client_.reset_called_automatic_generation_status_changed_true();
EXPECT_CALL(fake_pw_client_, ShowPasswordEditingPopup(_, _));
FocusField(kGenerationElementId); FocusField(kGenerationElementId);
EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_)); EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_));
size_t max_chars_to_delete = size_t max_chars_to_delete =
...@@ -1180,6 +1240,7 @@ TEST_F(PasswordGenerationAgentTest, ShortPasswordMaskedAfterChangingFocus) { ...@@ -1180,6 +1240,7 @@ TEST_F(PasswordGenerationAgentTest, ShortPasswordMaskedAfterChangingFocus) {
// Delete characters of the generated password until only // Delete characters of the generated password until only
// |kMinimumLengthForEditedPassword| - 1 chars remain. // |kMinimumLengthForEditedPassword| - 1 chars remain.
fake_pw_client_.reset_called_automatic_generation_status_changed_true(); fake_pw_client_.reset_called_automatic_generation_status_changed_true();
EXPECT_CALL(fake_pw_client_, ShowPasswordEditingPopup(_, _));
FocusField(kGenerationElementId); FocusField(kGenerationElementId);
EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_)); EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_));
size_t max_chars_to_delete = size_t max_chars_to_delete =
......
...@@ -311,6 +311,12 @@ void AutofillAgent::Shutdown() { ...@@ -311,6 +311,12 @@ void AutofillAgent::Shutdown() {
} }
void AutofillAgent::TextFieldDidEndEditing(const WebInputElement& element) { void AutofillAgent::TextFieldDidEndEditing(const WebInputElement& element) {
// Sometimes "blur" events are side effects of the password generation
// handling the page. They should not affect any UI in the browser.
if (password_generation_agent_ &&
password_generation_agent_->ShouldIgnoreBlur()) {
return;
}
GetAutofillDriver()->DidEndTextFieldEditing(); GetAutofillDriver()->DidEndTextFieldEditing();
password_autofill_agent_->DidEndTextFieldEditing(); password_autofill_agent_->DidEndTextFieldEditing();
if (password_generation_agent_) if (password_generation_agent_)
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <memory> #include <memory>
#include <utility> #include <utility>
#include "base/auto_reset.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/stl_util.h" #include "base/stl_util.h"
...@@ -237,6 +238,11 @@ struct PasswordGenerationAgent::GenerationItemInfo { ...@@ -237,6 +238,11 @@ struct PasswordGenerationAgent::GenerationItemInfo {
// interesting. // interesting.
bool editing_popup_shown_ = false; bool editing_popup_shown_ = false;
// True when PasswordGenerationAgent updates other password fields on the page
// due to the generated password being edited. It's used to suppress the fake
// blur events coming from there.
bool updating_other_password_fileds_ = false;
DISALLOW_COPY_AND_ASSIGN(GenerationItemInfo); DISALLOW_COPY_AND_ASSIGN(GenerationItemInfo);
}; };
...@@ -337,6 +343,11 @@ void PasswordGenerationAgent::OnFieldAutofilled( ...@@ -337,6 +343,11 @@ void PasswordGenerationAgent::OnFieldAutofilled(
} }
} }
bool PasswordGenerationAgent::ShouldIgnoreBlur() const {
return current_generation_item_ &&
current_generation_item_->updating_other_password_fileds_;
}
void PasswordGenerationAgent::FindPossibleGenerationForm() { void PasswordGenerationAgent::FindPossibleGenerationForm() {
if (!enabled_ || !render_frame()) if (!enabled_ || !render_frame())
return; return;
...@@ -711,6 +722,8 @@ bool PasswordGenerationAgent::TextDidChangeInTextField( ...@@ -711,6 +722,8 @@ bool PasswordGenerationAgent::TextDidChangeInTextField(
PasswordNoLongerGenerated(); PasswordNoLongerGenerated();
} else if (current_generation_item_->password_is_generated_) { } else if (current_generation_item_->password_is_generated_) {
current_generation_item_->password_edited_ = true; current_generation_item_->password_edited_ = true;
base::AutoReset<bool> auto_reset_update_confirmation_password(
&current_generation_item_->updating_other_password_fileds_, true);
// Mirror edits to any confirmation password fields. // Mirror edits to any confirmation password fields.
CopyElementValueToOtherInputElements( CopyElementValueToOtherInputElements(
&element, &current_generation_item_->password_elements_); &element, &current_generation_item_->password_elements_);
...@@ -786,6 +799,8 @@ void PasswordGenerationAgent::PasswordNoLongerGenerated() { ...@@ -786,6 +799,8 @@ void PasswordGenerationAgent::PasswordNoLongerGenerated() {
// Clear all other password fields. // Clear all other password fields.
for (WebInputElement& element : for (WebInputElement& element :
current_generation_item_->password_elements_) { current_generation_item_->password_elements_) {
base::AutoReset<bool> auto_reset_update_confirmation_password(
&current_generation_item_->updating_other_password_fileds_, true);
if (current_generation_item_->generation_element_ != element) if (current_generation_item_->generation_element_ != element)
element.SetAutofillValue(blink::WebString()); element.SetAutofillValue(blink::WebString());
} }
......
...@@ -79,6 +79,10 @@ class PasswordGenerationAgent : public content::RenderFrameObserver, ...@@ -79,6 +79,10 @@ class PasswordGenerationAgent : public content::RenderFrameObserver,
// Called right before PasswordAutofillAgent filled |password_element|. // Called right before PasswordAutofillAgent filled |password_element|.
void OnFieldAutofilled(const blink::WebInputElement& password_element); void OnFieldAutofilled(const blink::WebInputElement& password_element);
// Returns true iff the currently handled 'blur' event is fake and should be
// ignored.
bool ShouldIgnoreBlur() const;
#if defined(UNIT_TEST) #if defined(UNIT_TEST)
// This method requests the autofill::mojom::PasswordManagerClient which binds // This method requests the autofill::mojom::PasswordManagerClient which binds
// requests the binding if it wasn't bound yet. // requests the binding if it wasn't bound yet.
......
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