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

components/password_manager/core/browser/password_autofill_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/password_manager/core/browser/password_autofill_manager.h"
 
 #include <vector>
 
 #include "base/logging.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autofill_driver.h"
 #include "components/autofill/core/browser/popup_item_ids.h"
 #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"
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace password_manager {
 
 namespace {
 
@@ skipping 22 matching lines @@
 base::string16 GetHumanReadableRealm(const std::string& signon_realm) {
   // For Android application realms, remove the hash component. Otherwise, make
   // no changes.
   FacetURI maybe_facet_uri(FacetURI::FromPotentiallyInvalidSpec(signon_realm));
   if (maybe_facet_uri.IsValidAndroidFacetURI())
     return base::UTF8ToUTF16("android://" +
                              maybe_facet_uri.android_package_name() + "/");
   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(
         ReplaceEmptyUsername(fill_data.username_field.value));
     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) {
     if (show_all || StartsWith(login.first, current_username, false)) {
       autofill::Suggestion suggestion(ReplaceEmptyUsername(login.first));
       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);
     }
   }
 
   for (const auto& usernames : fill_data.other_possible_usernames) {
     for (size_t i = 0; i < usernames.second.size(); ++i) {
       if (show_all ||
           StartsWith(usernames.second[i], current_username, false)) {
         autofill::Suggestion suggestion(
             ReplaceEmptyUsername(usernames.second[i]));
         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
 
 ////////////////////////////////////////////////////////////////////////////////
 // PasswordAutofillManager, public:
 
 PasswordAutofillManager::PasswordAutofillManager(
     PasswordManagerDriver* password_manager_driver,
     autofill::AutofillClient* autofill_client)
@@ skipping 169 matching lines @@
     autofill::PasswordFormFillData* found_password) {
   LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(key);
   if (iter == login_to_password_info_.end())
     return false;
 
   *found_password = iter->second;
   return true;
 }
 
 }  //  namespace password_manager