Commit afc4c45a authored by Vadym Doroshenko's avatar Vadym Doroshenko Committed by Commit Bot

Using unique WebInput ids instead of WebInput elements.

Password Manager keeps the map from WebFormControlElement to last
non-JavaScript value in FieldValueAndPropertiesMaskMap. That's bad from
memory consumption perspective since it keeps input elements in memory
even if they were removed from DOM (and WebFormControlElement keep
s whole WebForm). Now we have reliable mechanism - unique renderer ids,
which might be used instead of WebFormControlElement. This CL replaces
usages of WebFormControlElement with unique renderer ids.

Also this CL contains small clean-up in password_autofill_agent.h:
removing not used typedef and typedef to using conversion.

Bug: 831123, 734427
Change-Id: I12a7b53859fccb9c5398997c61250ccf1a4f5c89
Reviewed-on: https://chromium-review.googlesource.com/1085460
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: default avatarVaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565623}
parent 1710969d
...@@ -1264,10 +1264,7 @@ bool ScriptModifiedUsernameAcceptable( ...@@ -1264,10 +1264,7 @@ bool ScriptModifiedUsernameAcceptable(
const base::string16* typed_from_key = map_key.second.first.get(); const base::string16* typed_from_key = map_key.second.first.get();
if (!typed_from_key) if (!typed_from_key)
continue; continue;
const WebInputElement* input_element = ToWebInputElement(&map_key.first); if (typed_from_key->size() >= kMinMatchSize &&
if (input_element && input_element->IsTextField() &&
!input_element->IsPasswordFieldForAutofill() &&
typed_from_key->size() >= kMinMatchSize &&
lowercase.find(base::i18n::ToLower(*typed_from_key)) != lowercase.find(base::i18n::ToLower(*typed_from_key)) !=
base::string16::npos) { base::string16::npos) {
return true; return true;
...@@ -1511,7 +1508,8 @@ void WebFormControlElementToFormField( ...@@ -1511,7 +1508,8 @@ void WebFormControlElementToFormField(
if (field_value_and_properties_map) { if (field_value_and_properties_map) {
FieldValueAndPropertiesMaskMap::const_iterator it = FieldValueAndPropertiesMaskMap::const_iterator it =
field_value_and_properties_map->find(element); field_value_and_properties_map->find(
element.UniqueRendererFormControlId());
if (it != field_value_and_properties_map->end()) if (it != field_value_and_properties_map->end())
field->properties_mask = it->second.second; field->properties_mask = it->second.second;
} }
...@@ -1596,7 +1594,9 @@ void WebFormControlElementToFormField( ...@@ -1596,7 +1594,9 @@ void WebFormControlElementToFormField(
field->properties_mask & (FieldPropertiesFlags::USER_TYPED | field->properties_mask & (FieldPropertiesFlags::USER_TYPED |
FieldPropertiesFlags::AUTOFILLED)) { FieldPropertiesFlags::AUTOFILLED)) {
const base::string16 typed_value = const base::string16 typed_value =
*field_value_and_properties_map->at(element).first; *field_value_and_properties_map
->at(element.UniqueRendererFormControlId())
.first;
// The typed value is preserved for all passwords. It is also preserved for // The typed value is preserved for all passwords. It is also preserved for
// potential usernames, as long as the |value| is not deemed acceptable. // potential usernames, as long as the |value| is not deemed acceptable.
......
...@@ -408,19 +408,22 @@ void UpdateFieldValueAndPropertiesMaskMap( ...@@ -408,19 +408,22 @@ void UpdateFieldValueAndPropertiesMaskMap(
FieldPropertiesMask added_flags, FieldPropertiesMask added_flags,
FieldValueAndPropertiesMaskMap* field_value_and_properties_map) { FieldValueAndPropertiesMaskMap* field_value_and_properties_map) {
FieldValueAndPropertiesMaskMap::iterator it = FieldValueAndPropertiesMaskMap::iterator it =
field_value_and_properties_map->find(element); field_value_and_properties_map->find(
element.UniqueRendererFormControlId());
if (it != field_value_and_properties_map->end()) { if (it != field_value_and_properties_map->end()) {
if (value) if (value)
it->second.first.reset(new base::string16(*value)); it->second.first.reset(new base::string16(*value));
it->second.second |= added_flags; it->second.second |= added_flags;
} else { } else {
(*field_value_and_properties_map)[element] = std::make_pair( (*field_value_and_properties_map)[element.UniqueRendererFormControlId()] =
value ? std::make_unique<base::string16>(*value) : nullptr, std::make_pair(
added_flags); value ? std::make_unique<base::string16>(*value) : nullptr,
added_flags);
} }
// Reset USER_TYPED and AUTOFILLED flags if the value is empty. // Reset USER_TYPED and AUTOFILLED flags if the value is empty.
if (value && value->empty()) { if (value && value->empty()) {
(*field_value_and_properties_map)[element].second &= (*field_value_and_properties_map)[element.UniqueRendererFormControlId()]
.second &=
~(FieldPropertiesFlags::USER_TYPED | FieldPropertiesFlags::AUTOFILLED); ~(FieldPropertiesFlags::USER_TYPED | FieldPropertiesFlags::AUTOFILLED);
} }
} }
......
...@@ -188,11 +188,10 @@ class PasswordAutofillAgent : public content::RenderFrameObserver, ...@@ -188,11 +188,10 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
// The key under which PasswordAutofillManager can find info for filling. // The key under which PasswordAutofillManager can find info for filling.
int key = -1; int key = -1;
}; };
typedef std::map<blink::WebInputElement, PasswordInfo> using WebInputToPasswordInfoMap =
WebInputToPasswordInfoMap; std::map<blink::WebInputElement, PasswordInfo>;
typedef std::map<blink::WebElement, int> WebElementToPasswordInfoKeyMap; using PasswordToLoginMap =
typedef std::map<blink::WebInputElement, blink::WebInputElement> std::map<blink::WebInputElement, blink::WebInputElement>;
PasswordToLoginMap;
// This class keeps track of autofilled password input elements and makes sure // This class keeps track of autofilled password input elements and makes sure
// the autofilled password value is not accessible to JavaScript code until // the autofilled password value is not accessible to JavaScript code until
......
...@@ -20,7 +20,6 @@ ...@@ -20,7 +20,6 @@
namespace blink { namespace blink {
class WebFormElement; class WebFormElement;
class WebFormControlElement;
class WebInputElement; class WebInputElement;
class WebLocalFrame; class WebLocalFrame;
} }
...@@ -65,10 +64,11 @@ bool IsGaiaReauthenticationForm(const blink::WebFormElement& form); ...@@ -65,10 +64,11 @@ bool IsGaiaReauthenticationForm(const blink::WebFormElement& form);
// Tests whether the given form is a GAIA form with a skip password argument. // Tests whether the given form is a GAIA form with a skip password argument.
bool IsGaiaWithSkipSavePasswordForm(const blink::WebFormElement& form); bool IsGaiaWithSkipSavePasswordForm(const blink::WebFormElement& form);
typedef std::map< // TODO(https://crbug.com/849291): Create separate class for keeping information
const blink::WebFormControlElement, // from FieldValueAndPropertiesMaskMap.
std::pair<std::unique_ptr<base::string16>, FieldPropertiesMask>> using FieldValueAndPropertiesMaskMap =
FieldValueAndPropertiesMaskMap; std::map<uint32_t,
std::pair<std::unique_ptr<base::string16>, FieldPropertiesMask>>;
// Create a PasswordForm from DOM form. Webkit doesn't allow storing // Create a PasswordForm from DOM form. Webkit doesn't allow storing
// custom metadata to DOM nodes, so we have to do this every time an event // custom metadata to DOM nodes, so we have to do this every time an event
......
...@@ -215,7 +215,7 @@ class MAYBE_PasswordFormConversionUtilsTest : public content::RenderViewTest { ...@@ -215,7 +215,7 @@ class MAYBE_PasswordFormConversionUtilsTest : public content::RenderViewTest {
input_element->SetActivatedSubmit(true); input_element->SetActivatedSubmit(true);
if (with_user_input) { if (with_user_input) {
const base::string16 element_value = input_element->Value().Utf16(); const base::string16 element_value = input_element->Value().Utf16();
user_input[control_elements[i]] = user_input[control_elements[i].UniqueRendererFormControlId()] =
std::make_pair(std::make_unique<base::string16>(element_value), std::make_pair(std::make_unique<base::string16>(element_value),
FieldPropertiesFlags::USER_TYPED); FieldPropertiesFlags::USER_TYPED);
} }
...@@ -736,12 +736,14 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, ...@@ -736,12 +736,14 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest,
// "email" field should be ignored despite it is more reliable than prediction // "email" field should be ignored despite it is more reliable than prediction
// for "id" field. // for "id" field.
FieldValueAndPropertiesMaskMap user_input; FieldValueAndPropertiesMaskMap user_input;
user_input[control_elements[2]] = std::make_pair( // id user_input[control_elements[2].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(control_elements[2].Value().Utf16()), std::make_pair( // id
FieldPropertiesFlags::USER_TYPED); std::make_unique<base::string16>(control_elements[2].Value().Utf16()),
user_input[control_elements[3]] = std::make_pair( // password FieldPropertiesFlags::USER_TYPED);
std::make_unique<base::string16>(control_elements[3].Value().Utf16()), user_input[control_elements[3].UniqueRendererFormControlId()] =
FieldPropertiesFlags::USER_TYPED); std::make_pair( // password
std::make_unique<base::string16>(control_elements[3].Value().Utf16()),
FieldPropertiesFlags::USER_TYPED);
std::unique_ptr<PasswordForm> password_form = CreatePasswordFormFromWebForm( std::unique_ptr<PasswordForm> password_form = CreatePasswordFormFromWebForm(
form, &user_input, nullptr, &username_detector_cache); form, &user_input, nullptr, &username_detector_cache);
...@@ -1575,14 +1577,16 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, UserInput) { ...@@ -1575,14 +1577,16 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, UserInput) {
WebVector<WebFormControlElement> control_elements; WebVector<WebFormControlElement> control_elements;
form.GetFormControlElements(control_elements); form.GetFormControlElements(control_elements);
ASSERT_EQ("nonvisible_text", control_elements[0].NameForAutofill().Utf8()); ASSERT_EQ("nonvisible_text", control_elements[0].NameForAutofill().Utf8());
user_input[control_elements[0]] = std::make_pair( user_input[control_elements[0].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(control_elements[0].Value().Utf16()), std::make_pair(
FieldPropertiesFlags::USER_TYPED); std::make_unique<base::string16>(control_elements[0].Value().Utf16()),
FieldPropertiesFlags::USER_TYPED);
ASSERT_EQ("nonvisible_password", ASSERT_EQ("nonvisible_password",
control_elements[2].NameForAutofill().Utf8()); control_elements[2].NameForAutofill().Utf8());
user_input[control_elements[2]] = std::make_pair( user_input[control_elements[2].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(control_elements[2].Value().Utf16()), std::make_pair(
FieldPropertiesFlags::USER_TYPED); std::make_unique<base::string16>(control_elements[2].Value().Utf16()),
FieldPropertiesFlags::USER_TYPED);
std::unique_ptr<PasswordForm> password_form = std::unique_ptr<PasswordForm> password_form =
CreatePasswordFormFromWebForm(form, &user_input, nullptr, nullptr); CreatePasswordFormFromWebForm(form, &user_input, nullptr, nullptr);
...@@ -1633,14 +1637,16 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, ...@@ -1633,14 +1637,16 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest,
form.GetFormControlElements(control_elements); form.GetFormControlElements(control_elements);
ASSERT_EQ("password_with_user_input1", ASSERT_EQ("password_with_user_input1",
control_elements[9].NameForAutofill().Utf8()); control_elements[9].NameForAutofill().Utf8());
user_input[control_elements[9]] = std::make_pair( user_input[control_elements[9].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(control_elements[9].Value().Utf16()), std::make_pair(
FieldPropertiesFlags::USER_TYPED); std::make_unique<base::string16>(control_elements[9].Value().Utf16()),
FieldPropertiesFlags::USER_TYPED);
ASSERT_EQ("password_with_user_input2", ASSERT_EQ("password_with_user_input2",
control_elements[10].NameForAutofill().Utf8()); control_elements[10].NameForAutofill().Utf8());
user_input[control_elements[10]] = std::make_pair( user_input[control_elements[10].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(control_elements[10].Value().Utf16()), std::make_pair(std::make_unique<base::string16>(
FieldPropertiesFlags::USER_TYPED); control_elements[10].Value().Utf16()),
FieldPropertiesFlags::USER_TYPED);
std::unique_ptr<PasswordForm> password_form = std::unique_ptr<PasswordForm> password_form =
CreatePasswordFormFromWebForm(form, &user_input, nullptr, nullptr); CreatePasswordFormFromWebForm(form, &user_input, nullptr, nullptr);
...@@ -1691,14 +1697,16 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, ...@@ -1691,14 +1697,16 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest,
form.GetFormControlElements(control_elements); form.GetFormControlElements(control_elements);
ASSERT_EQ("password_with_user_input1", ASSERT_EQ("password_with_user_input1",
control_elements[7].NameForAutofill().Utf8()); control_elements[7].NameForAutofill().Utf8());
user_input[control_elements[7]] = std::make_pair( user_input[control_elements[7].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(control_elements[7].Value().Utf16()), std::make_pair(
FieldPropertiesFlags::USER_TYPED); std::make_unique<base::string16>(control_elements[7].Value().Utf16()),
FieldPropertiesFlags::USER_TYPED);
ASSERT_EQ("password_with_user_input2", ASSERT_EQ("password_with_user_input2",
control_elements[9].NameForAutofill().Utf8()); control_elements[9].NameForAutofill().Utf8());
user_input[control_elements[9]] = std::make_pair( user_input[control_elements[9].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(control_elements[9].Value().Utf16()), std::make_pair(
FieldPropertiesFlags::USER_TYPED); std::make_unique<base::string16>(control_elements[9].Value().Utf16()),
FieldPropertiesFlags::USER_TYPED);
std::unique_ptr<PasswordForm> password_form = std::unique_ptr<PasswordForm> password_form =
CreatePasswordFormFromWebForm(form, &user_input, nullptr, nullptr); CreatePasswordFormFromWebForm(form, &user_input, nullptr, nullptr);
...@@ -1854,13 +1862,14 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, ...@@ -1854,13 +1862,14 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest,
FieldPropertiesMask mask = FieldPropertiesFlags::AUTOFILLED; FieldPropertiesMask mask = FieldPropertiesFlags::AUTOFILLED;
if (autofilled_value_was_modified_by_user) if (autofilled_value_was_modified_by_user)
mask |= FieldPropertiesFlags::USER_TYPED; mask |= FieldPropertiesFlags::USER_TYPED;
user_input[control_elements[1]] = user_input[control_elements[1].UniqueRendererFormControlId()] =
std::make_pair(std::make_unique<base::string16>( std::make_pair(std::make_unique<base::string16>(
base::ASCIIToUTF16("autofilled_value")), base::ASCIIToUTF16("autofilled_value")),
mask); mask);
user_input[control_elements[2]] = std::make_pair( user_input[control_elements[2].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(base::ASCIIToUTF16("user_value")), std::make_pair(
FieldPropertiesFlags::USER_TYPED); std::make_unique<base::string16>(base::ASCIIToUTF16("user_value")),
FieldPropertiesFlags::USER_TYPED);
std::unique_ptr<PasswordForm> password_form( std::unique_ptr<PasswordForm> password_form(
CreatePasswordFormFromWebForm(form, &user_input, nullptr, nullptr)); CreatePasswordFormFromWebForm(form, &user_input, nullptr, nullptr));
...@@ -2041,7 +2050,7 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, ...@@ -2041,7 +2050,7 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest,
FieldValueAndPropertiesMaskMap user_input; FieldValueAndPropertiesMaskMap user_input;
WebInputElement* input_element = ToWebInputElement(&control_elements[3]); WebInputElement* input_element = ToWebInputElement(&control_elements[3]);
const base::string16 element_value = input_element->Value().Utf16(); const base::string16 element_value = input_element->Value().Utf16();
user_input[control_elements[3]] = user_input[control_elements[3].UniqueRendererFormControlId()] =
std::make_pair(std::make_unique<base::string16>(element_value), std::make_pair(std::make_unique<base::string16>(element_value),
FieldPropertiesFlags::USER_TYPED); FieldPropertiesFlags::USER_TYPED);
...@@ -2462,21 +2471,24 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, TypedValuePreserved) { ...@@ -2462,21 +2471,24 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, TypedValuePreserved) {
ASSERT_EQ(3u, control_elements.size()); ASSERT_EQ(3u, control_elements.size());
ASSERT_EQ("fine", control_elements[0].NameForAutofill().Utf8()); ASSERT_EQ("fine", control_elements[0].NameForAutofill().Utf8());
control_elements[0].SetAutofillValue("same_value"); control_elements[0].SetAutofillValue("same_value");
user_input[control_elements[0]] = std::make_pair( user_input[control_elements[0].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(control_elements[0].Value().Utf16()), std::make_pair(
FieldPropertiesFlags::USER_TYPED); std::make_unique<base::string16>(control_elements[0].Value().Utf16()),
FieldPropertiesFlags::USER_TYPED);
ASSERT_EQ("mangled", control_elements[1].NameForAutofill().Utf8()); ASSERT_EQ("mangled", control_elements[1].NameForAutofill().Utf8());
control_elements[1].SetAutofillValue("mangled_value"); control_elements[1].SetAutofillValue("mangled_value");
user_input[control_elements[1]] = std::make_pair( user_input[control_elements[1].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(base::UTF8ToUTF16("original_value")), std::make_pair(
FieldPropertiesFlags::USER_TYPED); std::make_unique<base::string16>(base::UTF8ToUTF16("original_value")),
FieldPropertiesFlags::USER_TYPED);
ASSERT_EQ("completed_for_user", control_elements[2].NameForAutofill().Utf8()); ASSERT_EQ("completed_for_user", control_elements[2].NameForAutofill().Utf8());
control_elements[2].SetAutofillValue("email@gmail.com"); control_elements[2].SetAutofillValue("email@gmail.com");
user_input[control_elements[2]] = std::make_pair( user_input[control_elements[2].UniqueRendererFormControlId()] =
std::make_unique<base::string16>(base::UTF8ToUTF16("email")), std::make_pair(
FieldPropertiesFlags::USER_TYPED); std::make_unique<base::string16>(base::UTF8ToUTF16("email")),
FieldPropertiesFlags::USER_TYPED);
std::unique_ptr<PasswordForm> password_form = std::unique_ptr<PasswordForm> password_form =
CreatePasswordFormFromWebForm(form, &user_input, nullptr, nullptr); CreatePasswordFormFromWebForm(form, &user_input, nullptr, nullptr);
......
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