Recognise second hinted new-password field as confirmation

Chrome can see multiple hints from the server that a field is a
new-password field. This happens, in particular, if two fields
have the same name and hence the same signature -- because server
hints are keyed by the signature, there is no way to specify two
different types for two fields with the same name.

If there is no other explicitly hinted confirmation field, then it
is likely that in reality one of the new-password fields was supposed
to be a confirmation field.


Knowing which field is the confirmation field will be useful for
Chrome to understand where to copy the generated password to.

Therefore, this CL makes sure that the second hinted same-signature
new-password field is marked as a confirmation field, unless the server
also explicitly hints that some field is a confirmation field.

This is analogous to what the parser already does in case of two
fields with the autocomplete="new-password" attribute.

Bug: 902700
Change-Id: I06e75a6d1059976c68d04903351d9bd207de2ee9
Reviewed-on: https://chromium-review.googlesource.com/c/1326511
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608818}

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

@@ -1623,10 +1623,10 @@ TEST(FormParserTest, MultipleUsernames) {
 // new-password. That way the generation can be offered before the user has
 // thought of and typed their new password elsewhere. See
 // https://crbug.com/902700 for more details.
-TEST(FormParserTest, NewPasswordFirst) {
+TEST(FormParserTest, MultipleNewPasswords) {
   CheckTestData({
       {
-          "More than two usernames are ignored.",
+          "Only one new-password recognised.",
           {
               {.role = ElementRole::USERNAME,
                .form_control_type = "text",
@@ -1638,6 +1638,22 @@ TEST(FormParserTest, NewPasswordFirst) {
                .prediction = {.type = autofill::ACCOUNT_CREATION_PASSWORD}},
           },
       },
+      {
+          "Only one new-password recognised, confirmation unaffected.",
+          {
+              {.role = ElementRole::USERNAME,
+               .form_control_type = "text",
+               .prediction = {.type = autofill::USERNAME}},
+              {.role = ElementRole::NEW_PASSWORD,
+               .form_control_type = "password",
+               .prediction = {.type = autofill::ACCOUNT_CREATION_PASSWORD}},
+              {.form_control_type = "password",
+               .prediction = {.type = autofill::ACCOUNT_CREATION_PASSWORD}},
+              {.role = ElementRole::CONFIRMATION_PASSWORD,
+               .form_control_type = "password",
+               .prediction = {.type = autofill::CONFIRMATION_PASSWORD}},
+          },
+      },
   });
 }
 

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

@@ -7,7 +7,9 @@
 #include "base/logging.h"
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/common/form_data.h"
+#include "components/autofill/core/common/signatures_util.h"
 
+using autofill::FieldSignature;
 using autofill::FormData;
 using autofill::FormStructure;
 using autofill::ServerFieldType;
@@ -41,9 +43,42 @@ CredentialFieldType DeriveFromServerFieldType(ServerFieldType type) {
 }
 
 FormPredictions ConvertToFormPredictions(const FormStructure& form_structure) {
+  // This is a mostly mechanical transformation, except for the following case:
+  // If there is no explicit CONFIRMATION_PASSWORD field, and there are two
+  // fields with the same signature and one of the "new password" types, then
+  // the latter of those two should be marked as CONFIRMATION_PASSWORD. For
+  // fields which have the same signature, the server has no means to hint
+  // different types, and it is likely that one of them is the confirmation
+  // field.
+
+  // Stores the signature of the last field with the server type
+  // ACCOUNT_CREATION_PASSWORD or NEW_PASSWORD. The value 0 represents "no
+  // field with the 'new password' type seen yet".
+  FieldSignature last_new_password = 0;
+
+  bool explicit_confirmation_hint_present = false;
+  for (const auto& field : form_structure) {
+    if (field->server_type() == autofill::CONFIRMATION_PASSWORD) {
+      explicit_confirmation_hint_present = true;
+      break;
+    }
+  }
+
   FormPredictions result;
   for (const auto& field : form_structure) {
     ServerFieldType server_type = field->server_type();
+
+    if (!explicit_confirmation_hint_present &&
+        (server_type == autofill::ACCOUNT_CREATION_PASSWORD ||
+         server_type == autofill::NEW_PASSWORD)) {
+      FieldSignature current_signature = field->GetFieldSignature();
+      if (last_new_password && last_new_password == current_signature) {
+        server_type = autofill::CONFIRMATION_PASSWORD;
+      } else {
+        last_new_password = current_signature;
+      }
+    }
+
     if (IsCredentialRelatedPrediction(server_type)) {
       bool may_use_prefilled_placeholder = false;
       for (const auto& predictions : field->server_predictions()) {

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

@@ -4,6 +4,8 @@
 
 #include "components/password_manager/core/browser/form_parsing/password_field_prediction.h"
 
+#include <vector>
+
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/form_structure.h"
@@ -95,6 +97,71 @@ TEST(FormPredictionsTest, ConvertToFormPredictions) {
   }
 }
 
+TEST(FormPredictionsTest, ConvertToFormPredictions_SynthesiseConfirmation) {
+  struct TestField {
+    std::string name;
+    std::string form_control_type;
+    ServerFieldType input_type;
+    ServerFieldType expected_type;
+  };
+  const std::vector<TestField> kTestForms[] = {
+      {
+          {"username", "text", USERNAME, USERNAME},
+          {"new password", "password", ACCOUNT_CREATION_PASSWORD,
+           ACCOUNT_CREATION_PASSWORD},
+          // Same name and type means same signature. As a second new-password
+          // field with this signature, the next field should be re-classified
+          // to confirmation password.
+          {"new password", "password", ACCOUNT_CREATION_PASSWORD,
+           CONFIRMATION_PASSWORD},
+      },
+      {
+          {"username", "text", USERNAME, USERNAME},
+          {"new password duplicate", "password", ACCOUNT_CREATION_PASSWORD,
+           ACCOUNT_CREATION_PASSWORD},
+          // An explicit confirmation password above should override the
+          // 2-new-passwords heuristic.
+          {"new password duplicate", "password", ACCOUNT_CREATION_PASSWORD,
+           ACCOUNT_CREATION_PASSWORD},
+          {"confirm_password", "password", CONFIRMATION_PASSWORD,
+           CONFIRMATION_PASSWORD},
+      },
+  };
+
+  for (const std::vector<TestField>& test_form : kTestForms) {
+    FormData form_data;
+    for (size_t i = 0; i < test_form.size(); ++i) {
+      FormFieldData field;
+      field.unique_renderer_id = i + 1000;
+      field.name = ASCIIToUTF16(test_form[i].name);
+      field.form_control_type = test_form[i].form_control_type;
+      form_data.fields.push_back(field);
+    }
+
+    FormStructure form_structure(form_data);
+    // Set server predictions and create expected votes.
+    for (size_t i = 0; i < test_form.size(); ++i) {
+      AutofillField* field = form_structure.field(i);
+      field->set_server_type(test_form[i].input_type);
+    }
+
+    FormPredictions actual_predictions =
+        ConvertToFormPredictions(form_structure);
+
+    for (size_t i = 0; i < form_data.fields.size(); ++i) {
+      SCOPED_TRACE(testing::Message()
+                   << "field description: name=" << test_form[i].name
+                   << ", form control type=" << test_form[i].form_control_type
+                   << ", input type=" << test_form[i].input_type
+                   << ", expected type=" << test_form[i].expected_type
+                   << ", synthesised FormFieldData=" << form_data.fields[i]);
+      auto it = actual_predictions.find(form_data.fields[i].unique_renderer_id);
+      ASSERT_NE(actual_predictions.end(), it);
+      EXPECT_EQ(test_form[i].expected_type, it->second.type);
+    }
+  }
+}
+
 TEST(FormPredictionsTest, DeriveFromServerFieldType) {
   struct TestCase {
     const char* name;