Password filling by unique ids.

This CL implements filling based on WebForm and WebFormControlElement
unique renderer ids instead of complicated heuristics that are used
currently.

This CL contains:
1.Adding renderer ids to PasswordForm and PasswordFormFillData classes
2.Adding MOJO for PasswordFormFillData
3.Setting renderer ids in FormParser
4.Setting renderer ids in password_form_fill_data.cc
5.Filling using renderer ids in PasswordAutofillAgent
  - extracting to separate methods common code between new and old
filling
  - implementing finding username/password elements by renderer ids.

Bug: 831123
Change-Id: Ie29964db51014b140e806a6e3f78c9c854a8df34
Reviewed-on: https://chromium-review.googlesource.com/1090729Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567238}

chrome/renderer/autofill/password_autofill_agent_browsertest.cc

@@ -389,6 +389,24 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
     password_element_ = element.To<WebInputElement>();
   }
 
+  // TODO(https://crbug.com/831123): Make the code of this function to be part
+  // of UpdateUsernameAndPasswordElements() when the old parsing is removed and
+  // filling by renderer ids is by default.
+  void UpdateRendererIDs() {
+    fill_data_.has_renderer_ids = true;
+    if (!username_element_.IsNull()) {
+      fill_data_.username_field.unique_renderer_id =
+          username_element_.UniqueRendererFormControlId();
+    }
+    ASSERT_FALSE(password_element_.IsNull());
+    fill_data_.password_field.unique_renderer_id =
+        password_element_.UniqueRendererFormControlId();
+    WebFormElement form = password_element_.Form();
+    fill_data_.form_renderer_id = form.IsNull()
+                                      ? FormData::kNotSetFormRendererId
+                                      : form.UniqueRendererFormId();
+  }
+
   void UpdateOnlyPasswordElement() {
     WebDocument document = GetMainFrame()->GetDocument();
     WebElement element =
@@ -3477,4 +3495,43 @@ TEST_F(PasswordAutofillAgentTest, PSLMatchedPasswordIsNotAutofill) {
                                               false);
 }
 
+// Tests that the password form is filled as expected on load with using
+// renderer ids.
+TEST_F(PasswordAutofillAgentTest, FillOnLoadWithRendererIDs) {
+  UpdateRendererIDs();
+  SimulateOnFillPasswordForm(fill_data_);
+  CheckTextFieldsSuggestedState(kAliceUsername, true, kAlicePassword, true);
+}
+
+// Tests that the password form is filled as expected on load even if form/field
+// attributes were changed between from load and filling.
+TEST_F(PasswordAutofillAgentTest, FillOnLoadWithRendererIDsFormChanged) {
+  UpdateRendererIDs();
+  // Simulate JavaScript changed field names and form name.
+  fill_data_.name += ASCIIToUTF16("1");
+  fill_data_.username_field.name += ASCIIToUTF16("1");
+  fill_data_.password_field.name += ASCIIToUTF16("1");
+
+  SimulateOnFillPasswordForm(fill_data_);
+  CheckTextFieldsSuggestedState(kAliceUsername, true, kAlicePassword, true);
+}
+
+TEST_F(PasswordAutofillAgentTest, FillOnLoadWithRendererIDsNoForm) {
+  LoadHTML(kNoFormHTML);
+  UpdateUsernameAndPasswordElements();
+  UpdateRendererIDs();
+  SimulateOnFillPasswordForm(fill_data_);
+  CheckTextFieldsSuggestedState(kAliceUsername, true, kAlicePassword, true);
+}
+
+TEST_F(PasswordAutofillAgentTest, FillOnLoadWithRendererIDsNoUsername) {
+  LoadHTML(kTwoNoUsernameFormsHTML);
+  username_element_.Reset();
+  fill_data_.username_field.value.clear();
+  password_element_ = GetInputElementByID("password2");
+  UpdateRendererIDs();
+  SimulateOnFillPasswordForm(fill_data_);
+  EXPECT_EQ(kAlicePassword, password_element_.SuggestedValue().Utf8());
+}
+
 }  // namespace autofill

components/autofill/content/common/autofill_types.mojom

@@ -171,6 +171,7 @@ struct PasswordAndRealm {
 
 // autofill::PasswordFormFillData
 struct PasswordFormFillData {
+  uint32 form_renderer_id;
   mojo_base.mojom.String16 name;
   url.mojom.Url origin;
   url.mojom.Url action;
@@ -180,6 +181,7 @@ struct PasswordFormFillData {
   map<mojo_base.mojom.String16, PasswordAndRealm> additional_logins;
   bool wait_for_username;
   bool is_possible_change_password_form;
+  bool has_renderer_ids;
 };
 
 // autofill::PasswordFormGenerationData

components/autofill/content/common/autofill_types_struct_traits.cc

@@ -682,9 +682,11 @@ bool StructTraits<autofill::mojom::PasswordFormFillDataDataView,
       !data.ReadAdditionalLogins(&out->additional_logins))
     return false;
 
+  out->form_renderer_id = data.form_renderer_id();
   out->wait_for_username = data.wait_for_username();
   out->is_possible_change_password_form =
       data.is_possible_change_password_form();
+  out->has_renderer_ids = data.has_renderer_ids();
 
   return true;
 }

components/autofill/content/common/autofill_types_struct_traits.h

@@ -349,6 +349,9 @@ struct StructTraits<autofill::mojom::PasswordAndRealmDataView,
 template <>
 struct StructTraits<autofill::mojom::PasswordFormFillDataDataView,
                     autofill::PasswordFormFillData> {
+  static uint32_t form_renderer_id(const autofill::PasswordFormFillData& r) {
+    return r.form_renderer_id;
+  }
 
   static const base::string16& name(const autofill::PasswordFormFillData& r) {
     return r.name;
@@ -391,6 +394,10 @@ struct StructTraits<autofill::mojom::PasswordFormFillDataDataView,
     return r.is_possible_change_password_form;
   }
 
+  static bool has_renderer_ids(const autofill::PasswordFormFillData& r) {
+    return r.has_renderer_ids;
+  }
+
   static bool Read(autofill::mojom::PasswordFormFillDataDataView data,
                    autofill::PasswordFormFillData* out);
 };

components/autofill/content/common/autofill_types_struct_traits_unittest.cc

@@ -36,6 +36,7 @@ void CreateTestFieldDataPredictions(const std::string& signature,
 }
 
 void CreateTestPasswordFormFillData(PasswordFormFillData* fill_data) {
+  fill_data->form_renderer_id = 1234;
   fill_data->name = base::ASCIIToUTF16("TestName");
   fill_data->origin = GURL("https://foo.com/");
   fill_data->action = GURL("https://foo.com/login");
@@ -146,6 +147,7 @@ void CreateTestFormsPredictionsMap(FormsPredictionsMap* predictions) {
 
 void CheckEqualPasswordFormFillData(const PasswordFormFillData& expected,
                                     const PasswordFormFillData& actual) {
+  EXPECT_EQ(expected.form_renderer_id, actual.form_renderer_id);
   EXPECT_EQ(expected.name, actual.name);
   EXPECT_EQ(expected.origin, actual.origin);
   EXPECT_EQ(expected.action, actual.action);

components/autofill/content/renderer/password_autofill_agent.cc

@@ -56,6 +56,9 @@
 #include "url/gurl.h"
 
 using blink::WebAutofillState;
+using blink::WebDocument;
+using blink::WebInputElement;
+using blink::WebFormControlElement;
 
 namespace autofill {
 namespace {
@@ -622,6 +625,13 @@ PasswordForm::SubmissionIndicatorEvent ToSubmissionIndicatorEvent(
   }
 }
 
+WebInputElement ConvertToWebInput(const WebFormControlElement& element) {
+  if (element.IsNull())
+    return WebInputElement();
+  const WebInputElement* input = blink::ToWebInputElement(&element);
+  return input ? *input : WebInputElement();
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1366,10 +1376,38 @@ void PasswordAutofillAgent::OnProbablyFormSubmitted() {
   }
 }
 
+void PasswordAutofillAgent::FillUsingRendererIDs(
+    int key,
+    const PasswordFormFillData& form_data) {
+  std::unique_ptr<RendererSavePasswordProgressLogger> logger;
+  if (logging_state_active_) {
+    logger.reset(new RendererSavePasswordProgressLogger(
+        GetPasswordManagerDriver().get()));
+    logger->LogMessage(Logger::STRING_ON_FILL_PASSWORD_FORM_METHOD);
+  }
+  WebInputElement username_element, password_element;
+  std::tie(username_element, password_element) =
+      FindUsernamePasswordElements(form_data);
+  if (password_element.IsNull()) {
+    MaybeStoreFallbackData(key, form_data);
+    return;
+  }
+
+  StoreDataForFillOnAccountSelect(key, form_data, username_element,
+                                  password_element);
+  FillFormOnPasswordReceived(form_data, username_element, password_element,
+                             &field_value_and_properties_map_, logger.get());
+}
+
 // mojom::PasswordAutofillAgent:
 void PasswordAutofillAgent::FillPasswordForm(
     int key,
     const PasswordFormFillData& form_data) {
+  if (form_data.has_renderer_ids) {
+    FillUsingRendererIDs(key, form_data);
+    return;
+  }
+
   std::vector<blink::WebInputElement> elements;
   std::unique_ptr<RendererSavePasswordProgressLogger> logger;
   if (logging_state_active_) {
@@ -1461,33 +1499,13 @@ void PasswordAutofillAgent::GetFillableElementFromFormData(
 
     blink::WebInputElement main_element =
         username_element.IsNull() ? password_element : username_element;
-
-    PasswordInfo password_info;
-    password_info.fill_data = form_data;
-    password_info.key = key;
-    password_info.password_field = password_element;
-    web_input_to_password_info_[main_element] = password_info;
-    last_supplied_password_info_iter_ =
-        web_input_to_password_info_.find(main_element);
-    if (!main_element.IsPasswordFieldForAutofill())
-      password_to_username_[password_element] = username_element;
     if (elements)
       elements->push_back(main_element);
+    StoreDataForFillOnAccountSelect(key, form_data, username_element,
+                                    password_element);
   }
 
-  // This is a fallback, if for some reasons elements for filling were not found
-  // (for example because they were renamed by JavaScript) then add fill data
-  // for |web_input_to_password_info_|. When the user clicks on a password
-  // field which is not a key in |web_input_to_password_info_|, the first
-  // element from |web_input_to_password_info_| will be used in
-  // PasswordAutofillAgent::FindPasswordInfoForElement to propose to fill.
-  if (web_input_to_password_info_.empty()) {
-    PasswordInfo password_info;
-    password_info.fill_data = form_data;
-    password_info.key = key;
-    web_input_to_password_info_[blink::WebInputElement()] = password_info;
-    last_supplied_password_info_iter_ = web_input_to_password_info_.begin();
-  }
+  MaybeStoreFallbackData(key, form_data);
 }
 
 void PasswordAutofillAgent::FocusedNodeHasChanged(const blink::WebNode& node) {
@@ -1891,8 +1909,82 @@ PasswordAutofillAgent::GetPasswordManagerDriver() {
     render_frame()->GetRemoteInterfaces()->GetInterface(
         mojo::MakeRequest(&password_manager_driver_));
   }
-
   return password_manager_driver_;
 }
 
+std::pair<WebInputElement, WebInputElement>
+PasswordAutofillAgent::FindUsernamePasswordElements(
+    const PasswordFormFillData& form_data) {
+  const uint32_t username_renderer_id =
+      form_data.username_field.unique_renderer_id;
+  const uint32_t password_renderer_id =
+      form_data.password_field.unique_renderer_id;
+  const bool is_username_present =
+      username_renderer_id != FormFieldData::kNotSetFormControlRendererId;
+  const bool is_password_present =
+      password_renderer_id != FormFieldData::kNotSetFormControlRendererId;
+
+  DCHECK(is_password_present);
+
+  std::vector<uint32_t> element_ids = {password_renderer_id};
+  if (is_username_present)
+    element_ids.push_back(username_renderer_id);
+
+  WebDocument doc = render_frame()->GetWebFrame()->GetDocument();
+  bool is_form_tag =
+      form_data.form_renderer_id != FormData::kNotSetFormRendererId;
+  std::vector<WebFormControlElement> elements =
+      is_form_tag ? form_util::FindFormControlElementsByUniqueRendererId(
+                        doc, form_data.form_renderer_id, element_ids)
+                  : form_util::FindFormControlElementsByUniqueRendererId(
+                        doc, element_ids);
+
+  // Set password element.
+  WebInputElement password_field = ConvertToWebInput(elements[0]);
+
+  // Set username element.
+  WebInputElement username_field;
+  if (is_username_present)
+    username_field = ConvertToWebInput(elements[1]);
+
+  return std::make_pair(username_field, password_field);
+}
+
+void PasswordAutofillAgent::StoreDataForFillOnAccountSelect(
+    int key,
+    const PasswordFormFillData& form_data,
+    WebInputElement username_element,
+    WebInputElement password_element) {
+  WebInputElement main_element =
+      username_element.IsNull() ? password_element : username_element;
+
+  PasswordInfo password_info;
+  password_info.fill_data = form_data;
+  password_info.key = key;
+  password_info.password_field = password_element;
+  web_input_to_password_info_[main_element] = password_info;
+  last_supplied_password_info_iter_ =
+      web_input_to_password_info_.find(main_element);
+  if (!main_element.IsPasswordFieldForAutofill())
+    password_to_username_[password_element] = username_element;
+}
+
+void PasswordAutofillAgent::MaybeStoreFallbackData(
+    int key,
+    const PasswordFormFillData& form_data) {
+  if (!web_input_to_password_info_.empty())
+    return;
+  // If for some reasons elements for filling were not found (for example
+  // because they were renamed by JavaScript) then add fill data for
+  // |web_input_to_password_info_|. When the user clicks on a password field
+  // which is not a key in |web_input_to_password_info_|, the first element from
+  // |web_input_to_password_info_| will be used in
+  // PasswordAutofillAgent::FindPasswordInfoForElement to propose to fill.
+  PasswordInfo password_info;
+  password_info.fill_data = form_data;
+  password_info.key = key;
+  web_input_to_password_info_[WebInputElement()] = password_info;
+  last_supplied_password_info_iter_ = web_input_to_password_info_.begin();
+}
+
 }  // namespace autofill

components/autofill/content/renderer/password_autofill_agent.h

@@ -312,6 +312,30 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
 
   void HidePopup();
 
+  // TODO(https://crbug.com/831123): Rename to FillPasswordForm when browser
+  // form parsing is launched.
+  void FillUsingRendererIDs(int key, const PasswordFormFillData& form_data);
+
+  // Returns pair(username_element, password_element) based on renderer ids from
+  // |username_field| and |password_field| from |form_data|.
+  std::pair<blink::WebInputElement, blink::WebInputElement>
+  FindUsernamePasswordElements(const PasswordFormFillData& form_data);
+
+  // Populates |web_input_to_password_info_| and |password_to_username_| in
+  // order to provide fill on account select on |username_element| and
+  // |password_element| with credentials from |form_data|.
+  void StoreDataForFillOnAccountSelect(int key,
+                                       const PasswordFormFillData& form_data,
+                                       blink::WebInputElement username_element,
+                                       blink::WebInputElement password_element);
+
+  // In case when |web_input_to_password_info_| is empty (i.e. no fill on
+  // account select data yet) this function populates
+  // |web_input_to_password_info_| in order to provide fill on account select on
+  // any password field (aka filling fallback) with credentials from
+  // |form_data|.
+  void MaybeStoreFallbackData(int key, const PasswordFormFillData& form_data);
+
   // The logins we have filled so far with their associated info.
   WebInputToPasswordInfoMap web_input_to_password_info_;
   // A (sort-of) reverse map to |web_input_to_password_info_|.

components/autofill/core/common/password_form.cc

@@ -28,7 +28,10 @@ void PasswordFormToJSON(const PasswordForm& form,
   target->SetString("origin", form.origin.possibly_invalid_spec());
   target->SetString("action", form.action.possibly_invalid_spec());
   target->SetString("submit_element", form.submit_element);
-  target->SetString("username_elem", form.username_element);
+  target->SetBoolean("has_renderer_ids", form.has_renderer_ids);
+  target->SetString("username_element", form.username_element);
+  target->SetInteger("username_element_renderer_id",
+                     form.username_element_renderer_id);
   target->SetBoolean("username_marked_by_site", form.username_marked_by_site);
   target->SetString("username_value", form.username_value);
   target->SetString("password_elem", form.password_element);
@@ -36,6 +39,8 @@ void PasswordFormToJSON(const PasswordForm& form,
   target->SetBoolean("password_value_is_default",
                      form.password_value_is_default);
   target->SetString("new_password_element", form.new_password_element);
+  target->SetInteger("password_element_renderer_id",
+                     form.password_element_renderer_id);
   target->SetString("new_password_value", form.new_password_value);
   target->SetBoolean("new_password_value_is_default",
                      form.new_password_value_is_default);
@@ -118,13 +123,16 @@ bool PasswordForm::operator==(const PasswordForm& form) const {
   return scheme == form.scheme && signon_realm == form.signon_realm &&
          origin == form.origin && action == form.action &&
          submit_element == form.submit_element &&
+         has_renderer_ids == form.has_renderer_ids &&
          username_element == form.username_element &&
+         username_element_renderer_id == form.username_element_renderer_id &&
          username_marked_by_site == form.username_marked_by_site &&
          username_value == form.username_value &&
          other_possible_usernames == form.other_possible_usernames &&
          all_possible_passwords == form.all_possible_passwords &&
          form_has_autofilled_value == form.form_has_autofilled_value &&
          password_element == form.password_element &&
+         password_element_renderer_id == form.password_element_renderer_id &&
          password_value == form.password_value &&
          new_password_element == form.new_password_element &&
          new_password_marked_by_site == form.new_password_marked_by_site &&

components/autofill/core/common/password_form.h

@@ -161,11 +161,22 @@ struct PasswordForm {
   // When parsing an HTML form, this must always be set.
   base::string16 submit_element;
 
+  // True if renderer ids for username and password fields are present. Only set
+  // on form parsing, and not persisted.
+  // TODO(https://crbug.com/831123): Remove this field when old parsing is
+  // removed and filling by renderer ids is by default.
+  bool has_renderer_ids = false;
+
   // The name of the username input element. Optional (improves scoring).
   //
   // When parsing an HTML form, this must always be set.
   base::string16 username_element;
 
+  // The renderer id of the username input element. It is set during the new
+  // form parsing and not persisted.
+  uint32_t username_element_renderer_id =
+      FormFieldData::kNotSetFormControlRendererId;
+
   // Whether the |username_element| has an autocomplete=username attribute. This
   // is only used in parsed HTML forms.
   bool username_marked_by_site;
@@ -199,6 +210,11 @@ struct PasswordForm {
   // In these two cases the |new_password_element| will always be set.
   base::string16 password_element;
 
+  // The renderer id of the password input element. It is set during the new
+  // form parsing and not persisted.
+  uint32_t password_element_renderer_id =
+      FormFieldData::kNotSetFormControlRendererId;
+
   // The current password. Must be non-empty for PasswordForm instances that are
   // meant to be persisted to the password store.
   //

components/autofill/core/common/password_form_fill_data.cc

@@ -27,8 +27,7 @@ PasswordFormFillData::PasswordFormFillData()
 PasswordFormFillData::PasswordFormFillData(const PasswordFormFillData& other) =
     default;
 
-PasswordFormFillData::~PasswordFormFillData() {
-}
+PasswordFormFillData::~PasswordFormFillData() = default;
 
 void InitPasswordFormFillData(
     const PasswordForm& form_on_page,
@@ -42,12 +41,15 @@ void InitPasswordFormFillData(
   FormFieldData username_field;
   username_field.name = form_on_page.username_element;
   username_field.value = preferred_match->username_value;
+  username_field.unique_renderer_id = form_on_page.username_element_renderer_id;
   FormFieldData password_field;
   password_field.name = form_on_page.password_element;
   password_field.value = preferred_match->password_value;
+  password_field.unique_renderer_id = form_on_page.password_element_renderer_id;
   password_field.form_control_type = "password";
 
   // Fill basic form data.
+  result->form_renderer_id = form_on_page.form_data.unique_renderer_id;
   result->name = form_on_page.form_data.name;
   result->origin = form_on_page.origin;
   result->action = form_on_page.action;
@@ -56,6 +58,7 @@ void InitPasswordFormFillData(
   result->wait_for_username = wait_for_username_before_autofill;
   result->is_possible_change_password_form =
       form_on_page.IsPossibleChangePasswordForm();
+  result->has_renderer_ids = form_on_page.has_renderer_ids;
 
   if (preferred_match->is_public_suffix_match ||
       preferred_match->is_affiliation_based_match)

components/autofill/core/common/password_form_fill_data.h

@@ -38,6 +38,17 @@ struct PasswordFormFillData {
   typedef std::map<UsernamesCollectionKey,
                    std::vector<base::string16> > UsernamesCollection;
 
+  // If |has_renderer_ids| == true then |form_renderer_id| contains the unique
+  // renderer form id. No special values for |has_renderer_ids| == false case
+  // was introduced because the absent of ids is just temprorary situation while
+  // the old form parsing still exists.
+  // If there is no form tag then |form_renderer_id| ==
+  // FormData::kNotSetFormRendererId.
+  // Username and Password elements renderer ids are in
+  // |username_field.unique_renderer_id| and |password_field.unique_renderer_id|
+  // correspondingly.
+  uint32_t form_renderer_id;
+
   // The name of the form.
   base::string16 name;
 
@@ -69,6 +80,11 @@ struct PasswordFormFillData {
   // True if this form is a change password form.
   bool is_possible_change_password_form;
 
+  // True if renderer ids for form, username and password fields are present.
+  // TODO(https://crbug.com/831123): Remove this field when old parsing is
+  // removed and filling by renderer ids is by default.
+  bool has_renderer_ids = false;
+
   PasswordFormFillData();
   PasswordFormFillData(const PasswordFormFillData& other);
   ~PasswordFormFillData();

components/autofill/core/common/password_form_fill_data_unittest.cc

@@ -241,4 +241,42 @@ TEST(PasswordFormFillDataTest, TestAffiliationMatch) {
   EXPECT_EQ(iter->second.realm, affiliated_match.signon_realm);
 }
 
+// Tests that renderer ids are passed correctly.
+TEST(PasswordFormFillDataTest, RendererIDs) {
+  // Create the current form on the page.
+  PasswordForm form_on_page;
+  form_on_page.origin = GURL("https://foo.com/");
+  form_on_page.action = GURL("https://foo.com/login");
+  form_on_page.username_element = ASCIIToUTF16("username");
+  form_on_page.password_element = ASCIIToUTF16("password");
+
+  // Create an exact match in the database.
+  PasswordForm preferred_match = form_on_page;
+  preferred_match.username_value = ASCIIToUTF16("test@gmail.com");
+  preferred_match.password_value = ASCIIToUTF16("test");
+  preferred_match.preferred = true;
+
+  // Set renderer id related fields.
+  FormData form_data;
+  form_data.unique_renderer_id = 42;
+  form_data.is_form_tag = true;
+  form_on_page.form_data = form_data;
+  form_on_page.has_renderer_ids = true;
+  form_on_page.username_element_renderer_id = 123;
+  form_on_page.password_element_renderer_id = 456;
+
+  std::map<base::string16, const PasswordForm*> matches;
+
+  PasswordFormFillData result;
+  InitPasswordFormFillData(form_on_page, matches, &preferred_match, true,
+                           &result);
+
+  EXPECT_EQ(form_data.unique_renderer_id, result.form_renderer_id);
+  EXPECT_EQ(form_on_page.has_renderer_ids, result.has_renderer_ids);
+  EXPECT_EQ(form_on_page.username_element_renderer_id,
+            result.username_field.unique_renderer_id);
+  EXPECT_EQ(form_on_page.password_element_renderer_id,
+            result.password_field.unique_renderer_id);
+}
+
 }  // namespace autofill

components/password_manager/core/browser/form_parsing/form_parser.cc

@@ -416,14 +416,19 @@ std::unique_ptr<ParseResult> ParseUsingBaseHeuristics(
 
 // Set username and password fields from |parse_result| in |password_form|.
 void SetFields(const ParseResult& parse_result, PasswordForm* password_form) {
+  password_form->has_renderer_ids = true;
   if (parse_result.username_field) {
     password_form->username_element = parse_result.username_field->name;
     password_form->username_value = parse_result.username_field->value;
+    password_form->username_element_renderer_id =
+        parse_result.username_field->unique_renderer_id;
   }
 
   if (parse_result.password_field) {
     password_form->password_element = parse_result.password_field->name;
     password_form->password_value = parse_result.password_field->value;
+    password_form->password_element_renderer_id =
+        parse_result.password_field->unique_renderer_id;
   }
 
   if (parse_result.new_password_field) {

components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

@@ -227,10 +227,14 @@ void CheckPasswordFormFields(const PasswordForm& password_form,
   CheckField(form_data.fields, expectations.username_id,
              password_form.username_element, &password_form.username_value,
              "username");
+  EXPECT_EQ(expectations.username_id,
+            password_form.username_element_renderer_id);
 
   CheckField(form_data.fields, expectations.password_id,
              password_form.password_element, &password_form.password_value,
              "password");
+  EXPECT_EQ(expectations.password_id,
+            password_form.password_element_renderer_id);
 
   CheckField(form_data.fields, expectations.new_password_id,
              password_form.new_password_element,
@@ -266,6 +270,7 @@ void CheckTestData(const std::vector<FormParsingTestCase>& test_cases) {
         EXPECT_FALSE(parsed_form) << "Expected no parsed results";
       } else {
         ASSERT_TRUE(parsed_form) << "Expected successful parsing";
+        EXPECT_TRUE(parsed_form->has_renderer_ids);
         CheckPasswordFormFields(*parsed_form, form_data, expected_ids);
       }
     }