Teach PasswordAutofillAgent sometimes match prefixes of usernames

components/autofill/content/renderer/password_autofill_agent.cc

Index: components/autofill/content/renderer/password_autofill_agent.cc
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index 893d84f30b0ab1569f50c664b8d93779c0df27b7..e55f1988a3cea78d960e13538346351a51c9e673 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -268,13 +268,13 @@ bool IsElementEditable(const blink::WebInputElement& element) {
   return element.IsEnabled() && !element.IsReadOnly();
 }
 
-bool DoUsernamesMatch(const base::string16& username1,
-                      const base::string16& username2,
+bool DoUsernamesMatch(const base::string16& potential_suggestion,
+                      const base::string16& current_username,
                       bool exact_match) {
-  if (exact_match)
-    return username1 == username2;
-  return FieldIsSuggestionSubstringStartingOnTokenBoundary(username1, username2,
-                                                           true);
+  if (potential_suggestion == current_username)
+    return true;
+  return !exact_match && IsPrefixOfEmailEndingWithAtSign(current_username,
+                                                         potential_suggestion);
 }
 
 // Returns whether the given |element| is editable.
@@ -373,6 +373,54 @@ void UpdateFieldValueAndPropertiesMaskMap(
   }
 }
 
+// This function attempts to find the matching credentials for the
+// |current_username| by scanning |fill_data|. The result is written in
+// |username| and |password| parameters.
+void FindMatchesByUsername(const PasswordFormFillData& fill_data,
+                           const base::string16& current_username,
+                           bool exact_username_match,
+                           RendererSavePasswordProgressLogger* logger,
+                           base::string16* username,
+                           base::string16* password) {
+  // Look for any suitable matches to current field text.
+  if (DoUsernamesMatch(fill_data.username_field.value, current_username,
+                       exact_username_match)) {
+    *username = fill_data.username_field.value;
+    *password = fill_data.password_field.value;
+    if (logger)
+      logger->LogMessage(Logger::STRING_USERNAMES_MATCH);
+  } else {
+    // Scan additional logins for a match.
+    for (const auto& it : fill_data.additional_logins) {
+      if (DoUsernamesMatch(it.first, current_username, exact_username_match)) {
+        *username = it.first;
+        *password = it.second.password;
+        break;
+      }
+    }
+    if (logger) {
+      logger->LogBoolean(Logger::STRING_MATCH_IN_ADDITIONAL,
+                         !(username->empty() && password->empty()));
+    }
+
+    // Check possible usernames.
+    if (username->empty() && password->empty()) {
+      for (const auto& it : fill_data.other_possible_usernames) {
+        for (size_t i = 0; i < it.second.size(); ++i) {
+          if (DoUsernamesMatch(it.second[i], current_username,
+                               exact_username_match)) {
+            *username = it.second[i];
+            *password = it.first.password;
+            break;
+          }
+        }
+        if (!password->empty())
+          break;
+      }
+    }
+  }
+}
+
 // This function attempts to fill |username_element| and |password_element|
 // with values from |fill_data|. The |password_element| will only have the
 // suggestedValue set, and will be registered for copying that to the real
@@ -404,43 +452,9 @@ bool FillUserNameAndPassword(
   base::string16 username;
   base::string16 password;
 
-  // Look for any suitable matches to current field text.
-  if (DoUsernamesMatch(fill_data.username_field.value, current_username,
-                       exact_username_match)) {
-    username = fill_data.username_field.value;
-    password = fill_data.password_field.value;
-    if (logger)
-      logger->LogMessage(Logger::STRING_USERNAMES_MATCH);
-  } else {
-    // Scan additional logins for a match.
-    for (const auto& it : fill_data.additional_logins) {
-      if (DoUsernamesMatch(it.first, current_username, exact_username_match)) {
-        username = it.first;
-        password = it.second.password;
-        break;
-      }
-    }
-    if (logger) {
-      logger->LogBoolean(Logger::STRING_MATCH_IN_ADDITIONAL,
-                         !(username.empty() && password.empty()));
-    }
+  FindMatchesByUsername(fill_data, current_username, exact_username_match,

[vabr (Chromium), 2017/06/21 12:28:07]

There is still one issue:

In the rare case the user saved two credentials:
username / password1
username@example.com / password2
it may now happen that Chrome autofills password2 for username, creating an
incompatible mix.

We need to ensure that the exact match is always preferred. So instead of just
one pass trying to find everything, prefix- or exactly matching, we should first
check all sources for exact match, and only after that for the prefix match.


The easiest way to do this is to call FindMatchesByUsername here with the third
parameter as true in any case, and then call it once more with false if
exact_username_match is false.

We should also add a test for the scenario I described above.

+                        logger, &username, &password);
 
-    // Check possible usernames.
-    if (username.empty() && password.empty()) {
-      for (const auto& it : fill_data.other_possible_usernames) {
-        for (size_t i = 0; i < it.second.size(); ++i) {
-          if (DoUsernamesMatch(
-                  it.second[i], current_username, exact_username_match)) {
-            username = it.second[i];
-            password = it.first.password;
-            break;
-          }
-        }
-        if (!username.empty() && !password.empty())
-          break;
-      }
-    }
-  }
   if (password.empty())
     return false;
 
@@ -462,10 +476,6 @@ bool FillUserNameAndPassword(
       form_util::PreviewSuggestion(username, current_username,
                                    username_element);
     }
-  } else if (current_username != username) {
-    // If the username can't be filled and it doesn't match a saved password
-    // as is, don't autofill a password.
-    return false;
   }
 
   // Wait to fill in the password until a user gesture occurs. This is to make
@@ -550,12 +560,14 @@ bool FillFormOnPasswordReceived(
         blink::WebString::FromUTF16(fill_data.username_field.value));
   }
 
-  // Fill if we have an exact match for the username. Note that this sets
-  // username to autofilled.
+  bool exact_username_match =
+      username_element.IsNull() || IsElementEditable(username_element);
+  // Use the exact match for the editable username fields and allow prefix
+  // match for read-only username fields.
   return FillUserNameAndPassword(
-      &username_element, &password_element, fill_data,
-      true /* exact_username_match */, false /* set_selection */,
-      field_value_and_properties_map, registration_callback, logger);
+      &username_element, &password_element, fill_data, exact_username_match,
+      false /* set_selection */, field_value_and_properties_map,
+      registration_callback, logger);
 }
 
 // Annotate |forms| with form and field signatures as HTML attributes.