[Password Generation] Use signatures for form matching

components/autofill/content/renderer/password_generation_agent.cc

Index: components/autofill/content/renderer/password_generation_agent.cc
diff --git a/components/autofill/content/renderer/password_generation_agent.cc b/components/autofill/content/renderer/password_generation_agent.cc
index 0776b0ebf355bc83112ad59d2f78f5f804ab8522..ec28f365ce63d487028161e1f785dc23d22dfbd4 100644
--- a/components/autofill/content/renderer/password_generation_agent.cc
+++ b/components/autofill/content/renderer/password_generation_agent.cc
@@ -18,6 +18,7 @@
 #include "components/autofill/core/common/password_form.h"
 #include "components/autofill/core/common/password_form_generation_data.h"
 #include "components/autofill/core/common/password_generation_util.h"
+#include "components/autofill/core/common/signatures_util.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_view.h"
 #include "google_apis/gaia/gaia_urls.h"
@@ -57,46 +58,39 @@ bool ContainsURL(const std::vector<GURL>& urls, const GURL& url) {
   return std::find(urls.begin(), urls.end(), url) != urls.end();
 }
 
-// Finds a form in |forms| that has the same action and name as |form|.
-// If the action of a form in |forms| is empty, it uses |base_url| as action. It
-// also strips parameters of the action.
+// Calculates the signature of |form| and searches it in |forms|.
 const PasswordFormGenerationData* FindFormGenerationData(
     const std::vector<PasswordFormGenerationData>& forms,
-    const PasswordForm& form,
-    const GURL& base_url) {
+    const PasswordForm& form) {
+  FormSignature form_signature = CalculateFormSignature(form.form_data);

[dvadym, 2016/09/07 17:29:35]

I was wondering, whether we want to cache this signature. But it seems it
doesn't have sense to do this, since call of this method is not more than once
for a page, and anyway calculating of form.form_data is much more expensive

   for (const auto& form_it : forms) {
-    GURL action = form_it.action;
-    if (action.is_empty())
-      action = base_url;
-    action = form_util::StripAuthAndParams(action);
-    if (form_it.name == form.form_data.name && action == form.action)
+    if (form_it.form_signature == form_signature)
       return &form_it;
   }
   return nullptr;
 }
 
 // This function returns a vector of password fields into which Chrome should
-// fill the generated password. It assumes that |field_data| describes the field
-// where Chrome shows the password generation prompt. It returns no more
+// fill the generated password. It assumes that |field_signature| describes the
+// field where Chrome shows the password generation prompt. It returns no more
 // than 2 elements.
 std::vector<blink::WebInputElement> FindPasswordElementsForGeneration(
     const std::vector<blink::WebInputElement>& all_password_elements,
-    const base::string16& field_name) {
-  auto iter =
-      std::find_if(all_password_elements.begin(), all_password_elements.end(),
-                   [&field_name](const blink::WebInputElement& input) {
-                     // Make explicit conversion before comparing with string16.
-                     base::string16 input_name = input.nameForAutofill();
-                     return input_name == field_name;
-                   });
+    const FieldSignature field_signature) {
+  auto iter = std::find_if(
+      all_password_elements.begin(), all_password_elements.end(),
+      [&field_signature](const blink::WebInputElement& input) {
+        FieldSignature signature = CalculateFieldSignatureByNameAndType(
+            input.nameForAutofill(), input.formControlType().utf8());
+        return signature == field_signature;
+      });
   std::vector<blink::WebInputElement> passwords;
 
   // We copy not more than 2 fields because occasionally there are forms where
   // the security question answers are put in password fields and we don't want
   // to fill those.
-  for (; iter != all_password_elements.end() && passwords.size() < 2; ++iter) {
+  for (; iter != all_password_elements.end() && passwords.size() < 2; ++iter)
     passwords.push_back(*iter);
-  }
   return passwords;
 }
 
@@ -390,9 +384,8 @@ void PasswordGenerationAgent::DetermineGenerationElement() {
                << "blacklisted";
       continue;
     } else {
-      generation_data = FindFormGenerationData(
-          generation_enabled_forms_, *possible_password_form,
-          render_frame()->GetWebFrame()->document().baseURL());
+      generation_data = FindFormGenerationData(generation_enabled_forms_,
+                                               *possible_password_form);
       if (!generation_data) {
         if (AutocompleteAttributesSetForGeneration(*possible_password_form)) {
           VLOG(2) << "Ignoring lack of Autofill signal due to Autocomplete "
@@ -412,7 +405,7 @@ void PasswordGenerationAgent::DetermineGenerationElement() {
     std::vector<blink::WebInputElement> password_elements =
         generation_data ? FindPasswordElementsForGeneration(
                               possible_form_data.password_elements,
-                              generation_data->generation_field.name)
+                              generation_data->field_signature)
                         : possible_form_data.password_elements;
     if (password_elements.empty()) {
       // It might be if JavaScript changes field names.
@@ -574,7 +567,10 @@ void PasswordGenerationAgent::UserTriggeredGeneratePassword() {
   std::vector<blink::WebInputElement> password_elements;
   GetAccountCreationPasswordFields(control_elements, &password_elements);
   password_elements = FindPasswordElementsForGeneration(
-      password_elements, last_focused_password_element_.nameForAutofill());
+      password_elements,
+      CalculateFieldSignatureByNameAndType(
+          last_focused_password_element_.nameForAutofill(),
+          last_focused_password_element_.formControlType().utf8()));
   generation_form_data_.reset(new AccountCreationFormData(
       make_linked_ptr(password_form.release()), password_elements));
   is_manually_triggered_ = true;