Commit 71f087ae authored by Christoph Schwering's avatar Christoph Schwering Committed by Commit Bot

[Autofill] Use renderer ID for comparing FormStructure to FormData.

This CL removes FormStructure::operator==() and replaces it with direct
comparison of the unique renderer ID.

It appears that operator==() attempts to implement equality in terms of
underlying <form> tags, i.e., FormStructure == FormData iff they
originate from the same <form> tag. However, the implementation preceded
renderer IDs and thus could only approximate the intended relation.

Since there is no canonical way of defining equivalence between
FormStructure and FormData, it seems best to compare the renderer IDs
directly.

Renderer IDs suffice to uniquely identify forms and fields within
AutofillHandler since there is a one-to-one mapping from
RenderFrameHosts to AutofillDrivers and from AutofillDrivers to
AutofillHandlers.

Bug: 1007974,1064709
Change-Id: I9a3f1b02d06bab345d6e9df86273020ee888903a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2113574
Commit-Queue: Christoph Schwering <schwering@google.com>
Reviewed-by: default avatarVadym Doroshenko  <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776928}
parent 1cafa697
...@@ -256,7 +256,7 @@ bool AutofillHandler::FindCachedForm(const FormData& form, ...@@ -256,7 +256,7 @@ bool AutofillHandler::FindCachedForm(const FormData& form,
// of form signature. Compare it to all the forms in the cache to look for a // of form signature. Compare it to all the forms in the cache to look for a
// match. // match.
for (const auto& it : form_structures_) { for (const auto& it : form_structures_) {
if (*it.second == form) { if (it.second->unique_renderer_id() == form.unique_renderer_id) {
*form_structure = it.second.get(); *form_structure = it.second.get();
return true; return true;
} }
......
...@@ -1401,23 +1401,6 @@ FormData FormStructure::ToFormData() const { ...@@ -1401,23 +1401,6 @@ FormData FormStructure::ToFormData() const {
return data; return data;
} }
bool FormStructure::operator==(const FormData& form) const {
// TODO(jhawkins): Is this enough to differentiate a form?
if (form_name_ == form.name && source_url_ == form.url &&
target_url_ == form.action) {
return true;
}
// TODO(jhawkins): Compare field names, IDs and labels once we have labels
// set up.
return false;
}
bool FormStructure::operator!=(const FormData& form) const {
return !operator==(form);
}
FormStructure::SectionedFieldsIndexes::SectionedFieldsIndexes() {} FormStructure::SectionedFieldsIndexes::SectionedFieldsIndexes() {}
FormStructure::SectionedFieldsIndexes::~SectionedFieldsIndexes() {} FormStructure::SectionedFieldsIndexes::~SectionedFieldsIndexes() {}
......
...@@ -351,8 +351,6 @@ class FormStructure { ...@@ -351,8 +351,6 @@ class FormStructure {
void set_submission_source(mojom::SubmissionSource submission_source) { void set_submission_source(mojom::SubmissionSource submission_source) {
submission_source_ = submission_source; submission_source_ = submission_source;
} }
bool operator==(const FormData& form) const;
bool operator!=(const FormData& form) const;
// Returns an identifier that is used by the refill logic. Takes the first non // Returns an identifier that is used by the refill logic. Takes the first non
// empty of these or returns an empty string: // empty of these or returns an empty string:
......
...@@ -26,7 +26,8 @@ void TestAutofillDownloadManager::VerifyLastQueriedForms( ...@@ -26,7 +26,8 @@ void TestAutofillDownloadManager::VerifyLastQueriedForms(
const std::vector<FormData>& expected_forms) { const std::vector<FormData>& expected_forms) {
ASSERT_EQ(expected_forms.size(), last_queried_forms_.size()); ASSERT_EQ(expected_forms.size(), last_queried_forms_.size());
for (size_t i = 0; i < expected_forms.size(); ++i) { for (size_t i = 0; i < expected_forms.size(); ++i) {
EXPECT_EQ(*last_queried_forms_[i], expected_forms[i]); EXPECT_EQ(last_queried_forms_[i]->unique_renderer_id(),
expected_forms[i].unique_renderer_id);
} }
} }
......
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