Commit 4f27fd28 authored by Vaclav Brozek's avatar Vaclav Brozek Committed by Commit Bot

Add filling fallback for new-password forms

Forms with only "new password" fields, i.e., fields which should
contain previously unknown passwords, are generally not to be filled.
A previously unknown password is unlikely to be among those stored
with Chrome.

However, due to errors in form field classification, it can happen
that despite Chrome's conclusion, the form actually contains fields
worth filling.

This CL makes it possible to fill such fields on-demand.

Bug: 854123
Change-Id: I800290c4aabb91fb0fd661d083dc76af02c6895c
Reviewed-on: https://chromium-review.googlesource.com/1108209
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: default avatarDominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569575}
parent 09ab62ef
...@@ -3601,4 +3601,37 @@ TEST_F(PasswordAutofillAgentTest, FillOnLoadWithRendererIDsNoUsername) { ...@@ -3601,4 +3601,37 @@ TEST_F(PasswordAutofillAgentTest, FillOnLoadWithRendererIDsNoUsername) {
EXPECT_EQ(kAlicePassword, password_element_.SuggestedValue().Utf8()); EXPECT_EQ(kAlicePassword, password_element_.SuggestedValue().Utf8());
} }
TEST_F(PasswordAutofillAgentTest, FillDataWithNoPasswordId) {
ClearUsernameAndPasswordFields();
// Prepare fill data which contain the form ID, to trigger filling by IDs.
PasswordFormFillData data = fill_data_;
WebFormElement form_element = GetMainFrame()
->GetDocument()
.GetElementById("LoginTestForm")
.To<WebFormElement>();
data.form_renderer_id = form_element.UniqueRendererFormId();
data.has_renderer_ids = true;
// Simulate that no field was found into which a password should be filled
// (i.e. no "current-password" field).
data.password_field.unique_renderer_id =
FormFieldData::kNotSetFormControlRendererId;
SimulateOnFillPasswordForm(data);
// The username and password should not have been autocompleted.
CheckTextFieldsSuggestedState("", false, "", false);
SimulateElementClick(kPasswordName);
SimulateSuggestionChoice(password_element_);
// Check that the correct suggestions were requested.
base::RunLoop().RunUntilIdle();
ASSERT_TRUE(fake_driver_.called_show_pw_suggestions());
EXPECT_EQ(kPasswordFillFormDataId, fake_driver_.show_pw_suggestions_key());
fake_driver_.reset_show_pw_suggestions();
CheckTextFieldsDOMState(kAliceUsername, true, kAlicePassword, true);
}
} // namespace autofill } // namespace autofill
...@@ -1955,28 +1955,32 @@ PasswordAutofillAgent::FindUsernamePasswordElements( ...@@ -1955,28 +1955,32 @@ PasswordAutofillAgent::FindUsernamePasswordElements(
const bool is_password_present = const bool is_password_present =
password_renderer_id != FormFieldData::kNotSetFormControlRendererId; password_renderer_id != FormFieldData::kNotSetFormControlRendererId;
DCHECK(is_password_present); std::vector<uint32_t> element_ids;
if (is_password_present)
std::vector<uint32_t> element_ids = {password_renderer_id}; element_ids.push_back(password_renderer_id);
if (is_username_present) if (is_username_present)
element_ids.push_back(username_renderer_id); element_ids.push_back(username_renderer_id);
WebDocument doc = render_frame()->GetWebFrame()->GetDocument(); WebDocument doc = render_frame()->GetWebFrame()->GetDocument();
bool is_form_tag = bool wrapped_in_form_tag =
form_data.form_renderer_id != FormData::kNotSetFormRendererId; form_data.form_renderer_id != FormData::kNotSetFormRendererId;
std::vector<WebFormControlElement> elements = std::vector<WebFormControlElement> elements =
is_form_tag ? form_util::FindFormControlElementsByUniqueRendererId( wrapped_in_form_tag
doc, form_data.form_renderer_id, element_ids) ? form_util::FindFormControlElementsByUniqueRendererId(
: form_util::FindFormControlElementsByUniqueRendererId( doc, form_data.form_renderer_id, element_ids)
doc, element_ids); : form_util::FindFormControlElementsByUniqueRendererId(doc,
element_ids);
// Set password element. // Set password element.
WebInputElement password_field = ConvertToWebInput(elements[0]); WebInputElement password_field;
size_t current_index = 0;
if (is_password_present)
password_field = ConvertToWebInput(elements[current_index++]);
// Set username element. // Set username element.
WebInputElement username_field; WebInputElement username_field;
if (is_username_present) if (is_username_present)
username_field = ConvertToWebInput(elements[1]); username_field = ConvertToWebInput(elements[current_index++]);
return std::make_pair(username_field, password_field); return std::make_pair(username_field, password_field);
} }
......
...@@ -226,11 +226,6 @@ void NewPasswordFormManager::Fill() { ...@@ -226,11 +226,6 @@ void NewPasswordFormManager::Fill() {
if (!driver_ || best_matches_.empty() || filled_) if (!driver_ || best_matches_.empty() || filled_)
return; return;
// Do not fill forms without password field.
if (observed_password_form->password_element_renderer_id ==
FormFieldData::kNotSetFormControlRendererId)
return;
// TODO(https://crbug.com/831123). Implement correct treating of federated // TODO(https://crbug.com/831123). Implement correct treating of federated
// matches. // matches.
std::vector<const autofill::PasswordForm*> federated_matches; std::vector<const autofill::PasswordForm*> federated_matches;
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/test/test_mock_time_task_runner.h" #include "base/test/test_mock_time_task_runner.h"
#include "components/autofill/core/browser/form_structure.h" #include "components/autofill/core/browser/form_structure.h"
#include "components/autofill/core/common/form_data.h" #include "components/autofill/core/common/form_data.h"
#include "components/autofill/core/common/form_field_data.h"
#include "components/autofill/core/common/password_form.h" #include "components/autofill/core/common/password_form.h"
#include "components/autofill/core/common/password_form_fill_data.h" #include "components/autofill/core/common/password_form_fill_data.h"
#include "components/password_manager/core/browser/fake_form_fetcher.h" #include "components/password_manager/core/browser/fake_form_fetcher.h"
...@@ -137,19 +138,29 @@ TEST_F(NewPasswordFormManagerTest, Autofill) { ...@@ -137,19 +138,29 @@ TEST_F(NewPasswordFormManagerTest, Autofill) {
EXPECT_EQ(saved_match_.password_value, fill_data.password_field.value); EXPECT_EQ(saved_match_.password_value, fill_data.password_field.value);
} }
TEST_F(NewPasswordFormManagerTest, NoAutofillSignUpForm) { // NewPasswordFormManager should always send fill data to renderer, even for
// sign-up forms (no "current-password" field, i.e., no password field to fill
// into). However, for sign-up forms, no particular password field should be
// identified for filling. That way, Chrome won't disturb the user by filling
// the sign-up form, but will be able to offer a manual fallback for filling if
// the form was misclassified.
TEST_F(NewPasswordFormManagerTest, AutofillSignUpForm) {
TestMockTimeTaskRunner::ScopedContext scoped_context(task_runner_.get()); TestMockTimeTaskRunner::ScopedContext scoped_context(task_runner_.get());
FakeFormFetcher fetcher; FakeFormFetcher fetcher;
fetcher.Fetch(); fetcher.Fetch();
// Make |observed_form_| to be sign-up form. // Make |observed_form_| to be sign-up form.
observed_form_.fields.back().autocomplete_attribute = "new-password"; observed_form_.fields.back().autocomplete_attribute = "new-password";
EXPECT_CALL(driver_, FillPasswordForm(_)).Times(0); PasswordFormFillData fill_data;
EXPECT_CALL(driver_, FillPasswordForm(_)).WillOnce(SaveArg<0>(&fill_data));
NewPasswordFormManager form_manager(&client_, driver_.AsWeakPtr(), NewPasswordFormManager form_manager(&client_, driver_.AsWeakPtr(),
observed_form_, &fetcher); observed_form_, &fetcher);
fetcher.SetNonFederated({&saved_match_}, 0u); fetcher.SetNonFederated({&saved_match_}, 0u);
task_runner_->FastForwardUntilNoTasksRemain(); task_runner_->FastForwardUntilNoTasksRemain();
constexpr uint32_t kNoID = FormFieldData::kNotSetFormControlRendererId;
EXPECT_EQ(kNoID, fill_data.password_field.unique_renderer_id);
EXPECT_EQ(saved_match_.password_value, fill_data.password_field.value);
} }
TEST_F(NewPasswordFormManagerTest, SetSubmitted) { TEST_F(NewPasswordFormManagerTest, SetSubmitted) {
......
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