[Autofill/Autocomplete Feature] Substring matching instead of prefix matching.

components/password_manager/core/browser/password_autofill_manager.cc

Index: components/password_manager/core/browser/password_autofill_manager.cc
diff --git a/components/password_manager/core/browser/password_autofill_manager.cc b/components/password_manager/core/browser/password_autofill_manager.cc
index 15d5f7e3c8da005d4853f4dc3a8e78098bcaaebb..b9b7e8aae605ea41dec76b0472c047c8f4dcdb65 100644
--- a/components/password_manager/core/browser/password_autofill_manager.cc
+++ b/components/password_manager/core/browser/password_autofill_manager.cc
@@ -15,6 +15,7 @@
 #include "components/autofill/core/browser/suggestion.h"
 #include "components/autofill/core/common/autofill_constants.h"
 #include "components/autofill/core/common/autofill_data_validation.h"
+#include "components/autofill/core/common/autofill_util.h"
 #include "components/password_manager/core/browser/affiliation_utils.h"
 #include "components/password_manager/core/browser/password_manager_driver.h"
 #include "components/strings/grit/components_strings.h"
@@ -57,13 +58,24 @@ base::string16 GetHumanReadableRealm(const std::string& signon_realm) {
   return base::UTF8ToUTF16(signon_realm);
 }
 
+// Returns |true| if |kEnableSuggestionsWithSubstringMatch| command line switch
+// is on and |field_contents| matches substring token within |field_suggestion|

[vabr (Chromium), 2015/03/24 13:32:28]

Now the vague part is "matches" (no indication of prefix matching).
I suggest giving up on repeating the comment for autofill::HasTokenStartsWith.
The comment here does not have to explain what the 2-line function does (it is
easy to read  right below).
Instead, you can comment on how this should be used:
// Use this to check if a given suggestion matches the current contents of a
field.

I also suggest renaming to
DoesSuggestionMatchContents
(The name of the function will never explain fully how the match is done, and
just saying that substrings are involved is not helpful, while making the name
longer.)

[Pritam Nikam, 2015/03/25 14:25:27]

Done.

+// delimited by characters ' ', '.', ',', '-', '_' and '@'; otherwise |false|.
+bool DoesSuggestionsSubstringMatchContents(
+    const base::string16& field_suggestion,
+    const base::string16& field_contents) {
+  return autofill::IsFeatureSubstringMatchEnabled() &&
+         autofill::HasTokenStartsWith(field_suggestion, field_contents);
+}
+
 // This function attempts to fill |suggestions| and |realms| form |fill_data|
 // based on |current_username|. Unless |show_all| is true, it only picks
-// suggestions where the username has |current_username| as a prefix.
+// suggestions where the username has |current_username| as a substring.
 void GetSuggestions(const autofill::PasswordFormFillData& fill_data,
                     const base::string16& current_username,
                     std::vector<autofill::Suggestion>* suggestions,
                     bool show_all) {
+  std::vector<autofill::Suggestion> substring_matched_suggestions;
   if (show_all ||
       StartsWith(fill_data.username_field.value, current_username, false)) {
     autofill::Suggestion suggestion(
@@ -71,6 +83,13 @@ void GetSuggestions(const autofill::PasswordFormFillData& fill_data,
     suggestion.label = GetHumanReadableRealm(fill_data.preferred_realm);
     suggestion.frontend_id = autofill::POPUP_ITEM_ID_PASSWORD_ENTRY;
     suggestions->push_back(suggestion);
+  } else if (DoesSuggestionsSubstringMatchContents(
+                 fill_data.username_field.value, current_username)) {
+    autofill::Suggestion suggestion(

[vabr (Chromium), 2015/03/24 13:32:28]

I am a bit concerned about the code duplication. It was not ideal with 3 blocks
of nearly-identical code, but now there are 6. Could you please separate the
common part of creating the autofill::Suggestion into a separate function, and
call it appropriately?

[Pritam Nikam, 2015/03/25 14:25:26]

Done.

+        ReplaceEmptyUsername(fill_data.username_field.value));
+    suggestion.label = GetHumanReadableRealm(fill_data.preferred_realm);
+    suggestion.frontend_id = autofill::POPUP_ITEM_ID_PASSWORD_ENTRY;
+    substring_matched_suggestions.push_back(suggestion);
   }
 
   for (const auto& login : fill_data.additional_logins) {
@@ -79,6 +98,12 @@ void GetSuggestions(const autofill::PasswordFormFillData& fill_data,
       suggestion.label = GetHumanReadableRealm(login.second.realm);
       suggestion.frontend_id = autofill::POPUP_ITEM_ID_PASSWORD_ENTRY;
       suggestions->push_back(suggestion);
+    } else if (DoesSuggestionsSubstringMatchContents(login.first,
+                                                     current_username)) {
+      autofill::Suggestion suggestion(ReplaceEmptyUsername(login.first));
+      suggestion.label = GetHumanReadableRealm(login.second.realm);
+      suggestion.frontend_id = autofill::POPUP_ITEM_ID_PASSWORD_ENTRY;
+      substring_matched_suggestions.push_back(suggestion);
     }
   }
 
@@ -91,9 +116,21 @@ void GetSuggestions(const autofill::PasswordFormFillData& fill_data,
         suggestion.label = GetHumanReadableRealm(usernames.first.realm);
         suggestion.frontend_id = autofill::POPUP_ITEM_ID_PASSWORD_ENTRY;
         suggestions->push_back(suggestion);
+      } else if (DoesSuggestionsSubstringMatchContents(usernames.second[i],
+                                                       current_username)) {
+        autofill::Suggestion suggestion(
+            ReplaceEmptyUsername(usernames.second[i]));
+        suggestion.label = GetHumanReadableRealm(usernames.first.realm);
+        suggestion.frontend_id = autofill::POPUP_ITEM_ID_PASSWORD_ENTRY;
+        substring_matched_suggestions.push_back(suggestion);
       }
     }
   }
+
+  // Now append usernames having substring matching.
+  for (const auto& suggestion : substring_matched_suggestions) {
+    suggestions->push_back(suggestion);
+  }
 }
 
 }  // namespace