Implement autocomplete='current-password' and 'new-password'.

BUG=375333
# Depends on recently landed CL, ran try bots with manually specified base revison.
NOTRY=True

Review URL: https://codereview.chromium.org/369323003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282356 0039d316-1c4b-4281-b951-d872f2087c98

components/autofill/content/renderer/password_form_conversion_utils.cc

@@ -28,21 +28,46 @@ static const size_t kMaxPasswords = 3;
 
 // Checks in a case-insensitive way if the autocomplete attribute for the given
 // |element| is present and has the specified |value_in_lowercase|.
-bool HasAutocompleteAttributeValue(const WebInputElement* element,
+bool HasAutocompleteAttributeValue(const WebInputElement& element,
                                    const char* value_in_lowercase) {
-  return LowerCaseEqualsASCII(element->getAttribute("autocomplete"),
+  return LowerCaseEqualsASCII(element.getAttribute("autocomplete"),
                               value_in_lowercase);
 }
 
 // Helper to determine which password is the main (current) one, and which is
 // the new password (e.g., on a sign-up or change password form), if any.
 bool LocateSpecificPasswords(std::vector<WebInputElement> passwords,
-                             WebInputElement* password,
+                             WebInputElement* current_password,
                              WebInputElement* new_password) {
+  DCHECK(current_password && current_password->isNull());
+  DCHECK(new_password && new_password->isNull());
+
+  // First, look for elements marked with either autocomplete='current-password'
+  // or 'new-password' -- if we find any, take the hint, and treat the first of
+  // each kind as the element we are looking for.
+  for (std::vector<WebInputElement>::const_iterator it = passwords.begin();
+       it != passwords.end(); it++) {
+    if (HasAutocompleteAttributeValue(*it, "current-password") &&
+        current_password->isNull()) {
+      *current_password = *it;
+    } else if (HasAutocompleteAttributeValue(*it, "new-password") &&
+        new_password->isNull()) {
+      *new_password = *it;
+    }
+  }
+
+  // If we have seen an element with either of autocomplete attributes above,
+  // take that as a signal that the page author must have intentionally left the
+  // rest of the password fields unmarked. Perhaps they are used for other
+  // purposes, e.g., PINs, OTPs, and the like. So we skip all the heuristics we
+  // normally do, and ignore the rest of the password fields.
+  if (!current_password->isNull() || !new_password->isNull())
+    return true;
+
   switch (passwords.size()) {
     case 1:
       // Single password, easy.
-      *password = passwords[0];
+      *current_password = passwords[0];
       break;
     case 2:
       if (passwords[0].value() == passwords[1].value()) {
@@ -52,7 +77,7 @@ bool LocateSpecificPasswords(std::vector<WebInputElement> passwords,
         *new_password = passwords[0];
       } else {
         // Assume first is old password, second is new (no choice but to guess).
-        *password = passwords[0];
+        *current_password = passwords[0];
         *new_password = passwords[1];
       }
       break;
@@ -66,12 +91,12 @@ bool LocateSpecificPasswords(std::vector<WebInputElement> passwords,
       } else if (passwords[1].value() == passwords[2].value()) {
         // New password is the duplicated one, and comes second; or empty form
         // with 3 password fields, in which case we will assume this layout.
-        *password = passwords[0];
+        *current_password = passwords[0];
         *new_password = passwords[1];
       } else if (passwords[0].value() == passwords[1].value()) {
         // It is strange that the new password comes first, but trust more which
         // fields are duplicated than the ordering of fields.
-        *password = passwords[2];
+        *current_password = passwords[2];
         *new_password = passwords[0];
       } else {
         // Three different passwords, or first and last match with middle
@@ -128,7 +153,7 @@ void GetPasswordForm(const WebFormElement& form, PasswordForm* password_form) {
 
     // Various input types such as text, url, email can be a username field.
     if (input_element->isTextField() && !input_element->isPasswordField()) {
-      if (HasAutocompleteAttributeValue(input_element, "username")) {
+      if (HasAutocompleteAttributeValue(*input_element, "username")) {
         if (has_seen_element_with_autocomplete_username_before) {
           // A second or subsequent element marked with autocomplete='username'.
           // This makes us less confident that we have understood the form. We

components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc

@@ -39,11 +39,11 @@ class PasswordFormBuilder {
   // |name_and_id|, |value|, and |autocomplete| attributes. The |autocomplete|
   // argument can take two special values, namely:
   //  1.) NULL, causing no autocomplete attribute to be added,
-  //  2.) "", causing an empty attribute (i.e. autocomplete='') to be added.
+  //  2.) "", causing an empty attribute (i.e. autocomplete="") to be added.
   void AddUsernameField(const char* name_and_id,
                         const char* value,
                         const char* autocomplete) {
-    std::string autocomplete_attribute(autocomplete != NULL ?
+    std::string autocomplete_attribute(autocomplete ?
         base::StringPrintf("autocomplete=\"%s\"", autocomplete) : "");
     base::StringAppendF(
         &html_,
@@ -52,12 +52,17 @@ class PasswordFormBuilder {
   }
 
   // Appends a new password-type field at the end of the form, having the
-  // specified |name_and_id| and |value| attributes.
-  void AddPasswordField(const char* name_and_id, const char* value) {
+  // specified |name_and_id|, |value|, and |autocomplete| attributes. Special
+  // values for |autocomplete| are the same as in AddUsernameField.
+  void AddPasswordField(const char* name_and_id,
+                        const char* value,
+                        const char* autocomplete) {
+    std::string autocomplete_attribute(autocomplete ?
+        base::StringPrintf("autocomplete=\"%s\"", autocomplete): "");
     base::StringAppendF(
         &html_,
-        "<INPUT type=\"password\" name=\"%s\" id=\"%s\" value=\"%s\"/>",
-        name_and_id, name_and_id, value);
+        "<INPUT type=\"password\" name=\"%s\" id=\"%s\" value=\"%s\" %s/>",
+        name_and_id, name_and_id, value, autocomplete_attribute.c_str());
   }
 
   // Appends a new submit-type field at the end of the form.
@@ -111,7 +116,7 @@ TEST_F(PasswordFormConversionUtilsTest, ValidWebFormElementToPasswordForm) {
   PasswordFormBuilder builder(kTestFormActionURL);
   builder.AddUsernameField("username", "johnsmith", NULL);
   builder.AddSubmitButton();
-  builder.AddPasswordField("password", "secret");
+  builder.AddPasswordField("password", "secret", NULL);
   std::string html = builder.ProduceHTML();
 
   scoped_ptr<PasswordForm> password_form;
@@ -135,7 +140,7 @@ TEST_F(PasswordFormConversionUtilsTest, InvalidWebFormElementToPasswordForm) {
   PasswordFormBuilder builder("invalid_target");
   builder.AddUsernameField("username", "johnsmith", NULL);
   builder.AddSubmitButton();
-  builder.AddPasswordField("password", "secret");
+  builder.AddPasswordField("password", "secret", NULL);
   std::string html = builder.ProduceHTML();
 
   scoped_ptr<PasswordForm> password_form;
@@ -147,11 +152,11 @@ TEST_F(PasswordFormConversionUtilsTest,
        WebFormWithMultipleUseNameAndPassWordFieldsToPasswordForm) {
   PasswordFormBuilder builder(kTestFormActionURL);
   builder.AddUsernameField("username1", "John", NULL);
-  builder.AddPasswordField("password1", "oldsecret");
+  builder.AddPasswordField("password1", "oldsecret", NULL);
   builder.AddUsernameField("username2", "William", NULL);
-  builder.AddPasswordField("password2", "secret");
+  builder.AddPasswordField("password2", "secret", NULL);
   builder.AddUsernameField("username3", "Smith", NULL);
-  builder.AddPasswordField("password3", "secret");
+  builder.AddPasswordField("password3", "secret", NULL);
   builder.AddSubmitButton();
   std::string html = builder.ProduceHTML();
 
@@ -185,9 +190,9 @@ TEST_F(PasswordFormConversionUtilsTest,
        WebFormwithThreeDifferentPasswordsToPasswordForm) {
   PasswordFormBuilder builder(kTestFormActionURL);
   builder.AddUsernameField("username1", "John", NULL);
-  builder.AddPasswordField("password1", "alpha");
-  builder.AddPasswordField("password2", "beta");
-  builder.AddPasswordField("password3", "gamma");
+  builder.AddPasswordField("password1", "alpha", NULL);
+  builder.AddPasswordField("password2", "beta", NULL);
+  builder.AddPasswordField("password3", "gamma", NULL);
   builder.AddSubmitButton();
   std::string html = builder.ProduceHTML();
 
@@ -218,12 +223,12 @@ TEST_F(PasswordFormConversionUtilsTest,
       {{"username", NULL, "username"}, "username1", "John", "Smith"},
       {{"username", "username", "username"}, "username1", "John",
        "William+Smith"},
-      // When there is an empty autocomplete attribute (i.e. autocomplete=''),
+      // When there is an empty autocomplete attribute (i.e. autocomplete=""),
       // it should have the same effect as having no attribute whatsoever.
       {{"", "", ""}, "username2", "William", "John+Smith"},
       {{"", "", "username"}, "username3", "Smith", ""},
       {{"username", "", "username"}, "username1", "John", "Smith"},
-      // Whether attribute values are upper or mixed case, it should not matter.
+      // It should not matter if attribute values are upper or mixed case.
       {{"USERNAME", NULL, "uSeRNaMe"}, "username1", "John", "Smith"},
       {{"uSeRNaMe", NULL, "USERNAME"}, "username1", "John", "Smith"}};
 
@@ -233,7 +238,7 @@ TEST_F(PasswordFormConversionUtilsTest,
     PasswordFormBuilder builder(kTestFormActionURL);
     builder.AddUsernameField("username1", "John", cases[i].autocomplete[0]);
     builder.AddUsernameField("username2", "William", cases[i].autocomplete[1]);
-    builder.AddPasswordField("password", "secret");
+    builder.AddPasswordField("password", "secret", NULL);
     builder.AddUsernameField("username3", "Smith", cases[i].autocomplete[2]);
     builder.AddSubmitButton();
     std::string html = builder.ProduceHTML();
@@ -253,4 +258,138 @@ TEST_F(PasswordFormConversionUtilsTest,
   }
 }
 
+TEST_F(PasswordFormConversionUtilsTest,
+       PasswordFieldsWithAutocompleteAttributes) {
+  // Each test case consists of a set of parameters to be plugged into the
+  // PasswordFormBuilder below, plus the corresponding expectations.
+  struct TestCase {
+    const char* autocomplete[3];
+    const char* expected_password_element;
+    const char* expected_password_value;
+    const char* expected_new_password_element;
+    const char* expected_new_password_value;
+  } cases[] = {
+      // When there are elements marked with autocomplete='current-password',
+      // but no elements with 'new-password', we should treat the first of the
+      // former kind as the current password, and ignore all other password
+      // fields, assuming they are not intentionally not marked. They might be
+      // for other purposes, such as PINs, OTPs, and the like. Actual values in
+      // the password fields should be ignored in all cases below.
+      {{"current-password", NULL, NULL},
+       "password1", "alpha", "", ""},
+      {{NULL, "current-password", NULL},
+       "password2", "beta", "", ""},
+      {{NULL, NULL, "current-password"},
+       "password3", "gamma", "", ""},
+      {{NULL, "current-password", "current-password"},
+       "password2", "beta", "", ""},
+      {{"current-password", NULL, "current-password"},
+       "password1", "alpha", "", ""},
+      {{"current-password", "current-password", NULL},
+       "password1", "alpha", "", ""},
+      {{"current-password", "current-password", "current-password"},
+       "password1", "alpha", "", ""},
+      // The same goes vice versa for autocomplete='new-password'.
+      {{"new-password", NULL, NULL},
+       "", "", "password1", "alpha"},
+      {{NULL, "new-password", NULL},
+       "", "", "password2", "beta"},
+      {{NULL, NULL, "new-password"},
+       "", "", "password3", "gamma"},
+      {{NULL, "new-password", "new-password"},
+       "", "", "password2", "beta"},
+      {{"new-password", NULL, "new-password"},
+       "", "", "password1", "alpha"},
+      {{"new-password", "new-password", NULL},
+       "", "", "password1", "alpha"},
+      {{"new-password", "new-password", "new-password"},
+       "", "", "password1", "alpha"},
+      // When there is one element marked with autocomplete='current-password',
+      // and one with 'new-password', just comply, regardless of their order.
+      // Ignore the unmarked password field(s) for the same reason as above.
+      {{"current-password", "new-password", NULL},
+       "password1", "alpha", "password2", "beta"},
+      {{"current-password", NULL, "new-password"},
+       "password1", "alpha", "password3", "gamma"},
+      {{NULL, "current-password", "new-password"},
+       "password2", "beta", "password3", "gamma"},
+      {{"new-password", "current-password", NULL},
+       "password2", "beta", "password1", "alpha"},
+      {{"new-password", NULL, "current-password"},
+       "password3", "gamma", "password1", "alpha"},
+      {{NULL, "new-password", "current-password"},
+       "password3", "gamma", "password2", "beta"},
+      // In case of duplicated elements of either kind, go with the first one of
+      // its kind.
+      {{"current-password", "current-password", "new-password"},
+       "password1", "alpha", "password3", "gamma"},
+      {{"current-password", "new-password", "current-password"},
+       "password1", "alpha", "password2", "beta"},
+      {{"new-password", "current-password", "current-password"},
+       "password2", "beta", "password1", "alpha"},
+      {{"current-password", "new-password", "new-password"},
+       "password1", "alpha", "password2", "beta"},
+      {{"new-password", "current-password", "new-password"},
+       "password2", "beta", "password1", "alpha"},
+      {{"new-password", "new-password", "current-password"},
+       "password3", "gamma", "password1", "alpha"},
+      // When there is an empty autocomplete attribute (i.e. autocomplete=""),
+      // it should have the same effect as having no attribute whatsoever.
+      {{"current-password", "", ""},
+       "password1", "alpha", "", ""},
+      {{"", "", "new-password"},
+       "", "", "password3", "gamma"},
+      {{"", "new-password", ""},
+       "", "", "password2", "beta"},
+      {{"", "current-password", "current-password"},
+       "password2", "beta", "", ""},
+      {{"new-password", "", "new-password"},
+       "", "", "password1", "alpha"},
+      {{"new-password", "", "current-password"},
+       "password3", "gamma", "password1", "alpha"},
+      // It should not matter if attribute values are upper or mixed case.
+      {{NULL, "current-password", NULL},
+       "password2", "beta", "", ""},
+      {{NULL, "CURRENT-PASSWORD", NULL},
+       "password2", "beta", "", ""},
+      {{NULL, "new-password", NULL},
+       "", "", "password2", "beta"},
+      {{NULL, "nEw-PaSsWoRd", NULL},
+       "", "", "password2", "beta"}};
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) {
+    SCOPED_TRACE(testing::Message() << "Iteration " << i);
+
+    PasswordFormBuilder builder(kTestFormActionURL);
+    builder.AddPasswordField("password1", "alpha", cases[i].autocomplete[0]);
+    builder.AddUsernameField("username1", "William", NULL);
+    builder.AddPasswordField("password2", "beta", cases[i].autocomplete[1]);
+    builder.AddUsernameField("username2", "Smith", NULL);
+    builder.AddPasswordField("password3", "gamma", cases[i].autocomplete[2]);
+    builder.AddSubmitButton();
+    std::string html = builder.ProduceHTML();
+
+    scoped_ptr<PasswordForm> password_form;
+    ASSERT_NO_FATAL_FAILURE(LoadHTMLAndConvertForm(html, &password_form));
+    ASSERT_NE(static_cast<PasswordForm*>(NULL), password_form.get());
+
+    // Any constellation of password autocomplete attributes should have no
+    // effect on that the first text-type input field before a password field
+    // should be selected as the username.
+    EXPECT_EQ(base::UTF8ToUTF16("username1"), password_form->username_element);
+    EXPECT_EQ(base::UTF8ToUTF16("William"), password_form->username_value);
+    EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_password_element),
+              password_form->password_element);
+    EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_password_value),
+              password_form->password_value);
+    EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_new_password_element),
+              password_form->new_password_element);
+    EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_new_password_value),
+              password_form->new_password_value);
+    ASSERT_EQ(1u, password_form->other_possible_usernames.size());
+    EXPECT_EQ(base::UTF8ToUTF16("Smith"),
+              password_form->other_possible_usernames[0]);
+  }
+}
+
 }  // namespace autofill