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

components/autofill/core/browser/personal_data_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/autofill/core/browser/personal_data_manager.h"
 
 #include <algorithm>
 #include <functional>
 #include <iterator>
 
@@ skipping 12 matching lines @@
 #include "components/autofill/core/browser/autofill_experiments.h"
 #include "components/autofill/core/browser/autofill_field.h"
 #include "components/autofill/core/browser/autofill_metrics.h"
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/personal_data_manager_observer.h"
 #include "components/autofill/core/browser/phone_number.h"
 #include "components/autofill/core/browser/phone_number_i18n.h"
 #include "components/autofill/core/browser/validation.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
 #include "components/autofill/core/common/autofill_switches.h"
+#include "components/autofill/core/common/autofill_util.h"
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/common/signin_pref_names.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_data.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_formatter.h"
 
 namespace autofill {
 namespace {
 
 using ::i18n::addressinput::AddressField;
 using ::i18n::addressinput::GetStreetAddressLinesAsSingleLine;
@@ skipping 745 matching lines @@
 
   std::vector<Suggestion> suggestions;
   // Match based on a prefix search.
   std::vector<AutofillProfile*> matched_profiles;
   for (AutofillProfile* profile : profiles) {
     base::string16 value = GetInfoInOneLine(profile, type, app_locale_);
     if (value.empty())
       continue;
     base::string16 value_canon =
         AutofillProfile::CanonicalizeProfileString(value);
-    if (base::StartsWith(value_canon, field_contents_canon, true)) {
-      // Prefix match, add suggestion.
+    // CanonicalizeProfileString() returns lower-case string with all

[please use gerrit instead, 2015/06/29 22:06:29]

CanonicalizeProfileString() does not replace "all separators". Please correct
your comment.

[Pritam Nikam, 2015/06/30 15:05:50]

Done.

I reffered it from here [1]. 

[1].
https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...

Rephrased it to:

// CanonicalizeProfileString() returns lower-case string with separators;
// includes space, line and paragraph along with punctuations like dash,
// start, end and others {U_DASH_PUNCTUATION, U_START_PUNCTUATION,
// U_END_PUNCTUATION, U_CONNECTOR_PUNCTUATION, U_OTHER_PUNCTUATION,
// U_SPACE_SEPARATOR, U_LINE_SEPARATOR, U_PARAGRAPH_SEPARATOR}; substituted
// by whitespaces and trims off trailing whitespace if any. Passing
// |value_canon| and |field_contents_canon| to ContainsTokenThatStartsWith()
// may lead to unexpected result, hence preferred passing |value| and
// |field_contents| instead.

+    // separator substituted by whitespaces and trims off trailing whitespace
+    // if any. Passing |value_canon| and |field_contents_canon| to
+    // ContainsTokenThatStartsWith() may lead to unexpected result, hence
+    // preferred passing |values[i]| and |field_contents| instead.

[please use gerrit instead, 2015/06/29 22:06:29]

This part of the comment is outdated. There's no values[i] anywhere in this
file.

[Pritam Nikam, 2015/06/30 15:05:50]

Done.

+    bool substring_matched_suggestion = false;
+    if (base::StartsWith(value_canon, field_contents_canon, true) ||
+        (substring_matched_suggestion =
+             ContainsTokenThatStartsWith(value, field_contents, false))) {

[please use gerrit instead, 2015/06/29 22:06:29]

It's rare in Chromium code to assign a variable inside of an if clause. Is there
a simpler way to write this without such assignment?

[Pritam Nikam, 2015/06/30 15:05:50]

Done.

Changes to:
   
   bool prefix_matched_suggestion = base::StartsWith(...);
   if (prefix_matched_suggestion || ContainsTokenThatStartsWith(...)) {
     ...
     suggestions.back().match = prefix_matched_suggestion
                                    ? Suggestion::PREFIX_MATCH
                                    : Suggestion::SUBSTRING_MATCH;
   }

[please use gerrit instead, 2015/06/30 19:06:22]

Nice.

       matched_profiles.push_back(profile);
       suggestions.push_back(Suggestion(value));
       suggestions.back().backend_id = profile->guid();
+      suggestions.back().match = substring_matched_suggestion
+                                     ? Suggestion::SUBSTRING_MATCH
+                                     : Suggestion::PREFIX_MATCH;
     }
   }
 
+  // Prefix matches should precede other token matches.
+  if (IsFeatureSubstringMatchEnabled()) {
+    std::stable_sort(suggestions.begin(), suggestions.end(),
+                     [](const Suggestion& a, const Suggestion& b) {
+                       return a.match < b.match;
+                     });

[please use gerrit instead, 2015/06/29 22:06:29]

Is this clang formatted?

[Pritam Nikam, 2015/06/30 15:05:50]

Yes.

+  }
+
   // Don't show two suggestions if one is a subset of the other.
   std::vector<AutofillProfile*> unique_matched_profiles;
   std::vector<Suggestion> unique_suggestions;
   ServerFieldTypeSet types(other_field_types.begin(), other_field_types.end());
   for (size_t i = 0; i < matched_profiles.size(); ++i) {
     bool include = true;
     AutofillProfile* profile_a = matched_profiles[i];
     for (size_t j = 0; j < matched_profiles.size(); ++j) {
       AutofillProfile* profile_b = matched_profiles[j];
       // Check if profile A is a subset of profile B. If not, continue.
@@ skipping 31 matching lines @@
   return unique_suggestions;
 }
 
 std::vector<Suggestion> PersonalDataManager::GetCreditCardSuggestions(
     const AutofillType& type,
     const base::string16& field_contents) {
   if (IsInAutofillSuggestionsDisabledExperiment())
     return std::vector<Suggestion>();
 
   std::list<const CreditCard*> cards_to_suggest;
+  std::list<const CreditCard*> substring_matched_suggestions;

[please use gerrit instead, 2015/06/29 22:06:29]

s/substring_matched_suggestions/substring_matched_cards

[Pritam Nikam, 2015/06/30 15:05:50]

Done.

   for (const CreditCard* credit_card : GetCreditCards()) {
     // The value of the stored data for this field type in the |credit_card|.
     base::string16 creditcard_field_value =
         credit_card->GetInfo(type, app_locale_);
     if (creditcard_field_value.empty())
       continue;
 
     // For card number fields, suggest the card if:
     // - the number matches any part of the card, or
     // - it's a masked card and there are 6 or fewers typed so far.
     // For other fields, require that the field contents match the beginning of
     // the stored data.
     if (type.GetStorableType() == CREDIT_CARD_NUMBER) {
       if (creditcard_field_value.find(field_contents) == base::string16::npos &&
           (credit_card->record_type() != CreditCard::MASKED_SERVER_CARD ||
            field_contents.size() >= 6)) {
         continue;
       }
-    } else if (!base::StartsWith(creditcard_field_value, field_contents,
-                                 false)) {
-      continue;
+      cards_to_suggest.push_back(credit_card);
+    } else if (base::StartsWith(creditcard_field_value, field_contents,
+                                false)) {
+      cards_to_suggest.push_back(credit_card);
+    } else if (ContainsTokenThatStartsWith(creditcard_field_value,
+                                           field_contents, false)) {
+      substring_matched_suggestions.push_back(credit_card);
     }
+  }
 
-    cards_to_suggest.push_back(credit_card);
+  cards_to_suggest.sort(RankByMfu);
+
+  // Prefix matches should precede other token matches.
+  if (IsFeatureSubstringMatchEnabled()) {
+    substring_matched_suggestions.sort(RankByMfu);
+    cards_to_suggest.insert(cards_to_suggest.end(),
+                            substring_matched_suggestions.begin(),
+                            substring_matched_suggestions.end());
   }
 
   // De-dupe card suggestions. Full server cards shadow local cards, and
   // local cards shadow masked server cards.
   for (auto outer_it = cards_to_suggest.begin();
        outer_it != cards_to_suggest.end();
        ++outer_it) {
 
     if ((*outer_it)->record_type() == CreditCard::FULL_SERVER_CARD) {
       for (auto inner_it = cards_to_suggest.begin();
            inner_it != cards_to_suggest.end();) {
         auto inner_it_copy = inner_it++;
         if ((*inner_it_copy)->IsLocalDuplicateOfServerCard(**outer_it))
           cards_to_suggest.erase(inner_it_copy);
       }
     } else if ((*outer_it)->record_type() == CreditCard::LOCAL_CARD) {
       for (auto inner_it = cards_to_suggest.begin();
            inner_it != cards_to_suggest.end();) {
         auto inner_it_copy = inner_it++;
         if ((*inner_it_copy)->record_type() == CreditCard::MASKED_SERVER_CARD &&
             (*outer_it)->IsLocalDuplicateOfServerCard(**inner_it_copy)) {
           cards_to_suggest.erase(inner_it_copy);
         }
       }
     }
   }
 
-  cards_to_suggest.sort(RankByMfu);
-
   std::vector<Suggestion> suggestions;
   for (const CreditCard* credit_card : cards_to_suggest) {
     // Make a new suggestion.
     suggestions.push_back(Suggestion());
     Suggestion* suggestion = &suggestions.back();
 
     suggestion->value = credit_card->GetInfo(type, app_locale_);
     suggestion->icon = base::UTF8ToUTF16(credit_card->type());
     suggestion->backend_id = credit_card->guid();
 
@@ skipping 413 matching lines @@
   }
   if (IsExperimentalWalletIntegrationEnabled() &&
       pref_service_->GetBoolean(prefs::kAutofillWalletImportEnabled)) {
     profiles_.insert(
         profiles_.end(), server_profiles_.begin(), server_profiles_.end());
   }
   return profiles_;
 }
 
 }  // namespace autofill