diff options
25 files changed, 856 insertions, 72 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index fc02045..bcbca66 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -6415,6 +6415,12 @@ Keep your key file in a safe place. You will need it to create new versions of y <message name="IDS_PASSWORD_MANAGER_SAVE_PASSWORD_SMART_LOCK_NO_THANKS_BUTTON" desc="Text for the button the user clicks to dismiss save password bubble."> No thanks </message> + <message name="IDS_FLAGS_ENABLE_SUGGESTIONS_WITH_SUB_STRING_MATCH_NAME" desc="Name of the flag to enable substring matching for Autofill suggestions."> + Substring matching for Autofill suggestions. + </message> + <message name="IDS_FLAGS_ENABLE_SUGGESTIONS_WITH_SUB_STRING_MATCH_DESCRIPTION" desc="Description of the flag to enable substring matching for Autofill suggestions."> + Match Autofill suggestions based on substrings (token prefixes) rather than just prefixes. + </message> <message name="IDS_FLAGS_ENABLE_AFFILIATION_BASED_MATCHING_NAME" desc="Name of the flag to enable affiliation based matching, so that credentials stored for an Android application will also be considered matches for, and be filled into corresponding websites (so-called 'affiliated' websites)."> Enable affiliation based matching in password manager. </message> diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc index 94b1ee1..7a56094 100644 --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc @@ -1138,6 +1138,12 @@ const Experiment kExperiments[] = { kOsAndroid | kOsCrOS, ENABLE_DISABLE_VALUE_TYPE(switches::kEnableSuggestionsService, switches::kDisableSuggestionsService)}, + {"enable-suggestions-with-substring-match", + IDS_FLAGS_ENABLE_SUGGESTIONS_WITH_SUB_STRING_MATCH_NAME, + IDS_FLAGS_ENABLE_SUGGESTIONS_WITH_SUB_STRING_MATCH_DESCRIPTION, + kOsAll, + SINGLE_VALUE_TYPE( + autofill::switches::kEnableSuggestionsWithSubstringMatch)}, {"enable-supervised-user-managed-bookmarks-folder", IDS_FLAGS_ENABLE_SUPERVISED_USER_MANAGED_BOOKMARKS_FOLDER_NAME, IDS_FLAGS_ENABLE_SUPERVISED_USER_MANAGED_BOOKMARKS_FOLDER_DESCRIPTION, diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index 1ea6058..e00b5e1 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -1409,6 +1409,7 @@ void ChromeContentBrowserClient::AppendExtraCommandLineSwitches( autofill::switches::kEnableFillOnAccountSelectNoHighlighting, autofill::switches::kEnablePasswordGeneration, autofill::switches::kEnableSingleClickAutofill, + autofill::switches::kEnableSuggestionsWithSubstringMatch, autofill::switches::kIgnoreAutocompleteOffForAutofill, autofill::switches::kLocalHeuristicsOnlyForPasswordGeneration, #if defined(ENABLE_EXTENSIONS) diff --git a/components/autofill.gypi b/components/autofill.gypi index 99cc85d..4b1abe4 100644 --- a/components/autofill.gypi +++ b/components/autofill.gypi @@ -31,6 +31,8 @@ 'autofill/core/common/autofill_regexes.h', 'autofill/core/common/autofill_switches.cc', 'autofill/core/common/autofill_switches.h', + 'autofill/core/common/autofill_util.cc', + 'autofill/core/common/autofill_util.h', 'autofill/core/common/form_data.cc', 'autofill/core/common/form_data.h', 'autofill/core/common/form_data_predictions.cc', diff --git a/components/autofill/content/renderer/autofill_agent.cc b/components/autofill/content/renderer/autofill_agent.cc index 3c81afe..3412281 100644 --- a/components/autofill/content/renderer/autofill_agent.cc +++ b/components/autofill/content/renderer/autofill_agent.cc @@ -739,8 +739,7 @@ void AutofillAgent::PreviewFieldWithValue(const base::string16& value, was_query_node_autofilled_ = element_.isAutofilled(); node->setSuggestedValue(value.substr(0, node->maxLength())); node->setAutofilled(true); - node->setSelectionRange(node->value().length(), - node->suggestedValue().length()); + PreviewSuggestion(node->suggestedValue(), node->value(), node); } void AutofillAgent::ProcessForms() { diff --git a/components/autofill/content/renderer/form_autofill_util.cc b/components/autofill/content/renderer/form_autofill_util.cc index 51a2251..4cf695f 100644 --- a/components/autofill/content/renderer/form_autofill_util.cc +++ b/components/autofill/content/renderer/form_autofill_util.cc @@ -16,6 +16,7 @@ #include "components/autofill/core/common/autofill_data_validation.h" #include "components/autofill/core/common/autofill_regexes.h" #include "components/autofill/core/common/autofill_switches.h" +#include "components/autofill/core/common/autofill_util.h" #include "components/autofill/core/common/form_data.h" #include "components/autofill/core/common/form_field_data.h" #include "third_party/WebKit/public/platform/WebString.h" @@ -871,9 +872,7 @@ void PreviewFormField(const FormFieldData& data, if (is_initiating_node && (IsTextInput(input_element) || IsTextAreaElement(*field))) { // Select the part of the text that the user didn't type. - int start = field->value().length(); - int end = field->suggestedValue().length(); - field->setSelectionRange(start, end); + PreviewSuggestion(field->suggestedValue(), field->value(), field); } } @@ -1534,4 +1533,19 @@ gfx::RectF GetScaledBoundingBox(float scale, WebElement* element) { bounding_box.height() * scale); } +void PreviewSuggestion(const base::string16& suggestion, + const base::string16& user_input, + blink::WebFormControlElement* input_element) { + size_t selection_start = user_input.length(); + if (IsFeatureSubstringMatchEnabled()) { + size_t offset = + autofill::GetTextSelectionStart(suggestion, user_input, false); + // Zero selection start is for password manager, which can show usernames + // that do not begin with the user input value. + selection_start = (offset == base::string16::npos) ? 0 : offset; + } + + input_element->setSelectionRange(selection_start, suggestion.length()); +} + } // namespace autofill diff --git a/components/autofill/content/renderer/form_autofill_util.h b/components/autofill/content/renderer/form_autofill_util.h index 1be05cd..985ea16 100644 --- a/components/autofill/content/renderer/form_autofill_util.h +++ b/components/autofill/content/renderer/form_autofill_util.h @@ -176,6 +176,15 @@ bool IsWebElementEmpty(const blink::WebElement& element); // Return a gfx::RectF that is the bounding box for |element| scaled by |scale|. gfx::RectF GetScaledBoundingBox(float scale, blink::WebElement* element); +// Previews |suggestion| in |input_element| and highlights the suffix of +// |suggestion| not included in the |input_element| text. |input_element| must +// not be null. |user_input| should be the text typed by the user into +// |input_element|. Note that |user_input| cannot be easily derived from +// |input_element| by calling value(), because of http://crbug.com/507714. +void PreviewSuggestion(const base::string16& suggestion, + const base::string16& user_input, + blink::WebFormControlElement* input_element); + } // namespace autofill #endif // COMPONENTS_AUTOFILL_CONTENT_RENDERER_FORM_AUTOFILL_UTIL_H_ diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc index 6b87d60..1c836a1 100644 --- a/components/autofill/content/renderer/password_autofill_agent.cc +++ b/components/autofill/content/renderer/password_autofill_agent.cc @@ -18,6 +18,7 @@ #include "components/autofill/content/renderer/renderer_save_password_progress_logger.h" #include "components/autofill/core/common/autofill_constants.h" #include "components/autofill/core/common/autofill_switches.h" +#include "components/autofill/core/common/autofill_util.h" #include "components/autofill/core/common/form_field_data.h" #include "components/autofill/core/common/password_form.h" #include "components/autofill/core/common/password_form_fill_data.h" @@ -226,7 +227,8 @@ bool DoUsernamesMatch(const base::string16& username1, bool exact_match) { if (exact_match) return username1 == username2; - return base::StartsWith(username1, username2, base::CompareCase::SENSITIVE); + return FieldIsSuggestionSubstringStartingOnTokenBoundary(username1, username2, + true); } // Returns |true| if the given element is editable. Otherwise, returns |false|. @@ -408,14 +410,12 @@ bool FillUserNameAndPassword( // Input matches the username, fill in required values. if (!username_element->isNull() && IsElementAutocompletable(*username_element)) { + // TODO(vabr): Why not setSuggestedValue? http://crbug.com/507714 username_element->setValue(username, true); (*nonscript_modified_values)[*username_element] = username; username_element->setAutofilled(true); - - if (set_selection) { - username_element->setSelectionRange(current_username.length(), - username.length()); - } + if (set_selection) + 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. @@ -561,7 +561,6 @@ PasswordAutofillAgent::PasswordAutofillAgent(content::RenderFrame* render_frame) logging_state_active_(false), was_username_autofilled_(false), was_password_autofilled_(false), - username_selection_start_(0), did_stop_loading_(false), weak_ptr_factory_(this) { Send(new AutofillHostMsg_PasswordAutofillAgentConstructed(routing_id())); @@ -752,14 +751,14 @@ bool PasswordAutofillAgent::PreviewSuggestion( return false; } + if (username_query_prefix_.empty()) + username_query_prefix_ = username_element.value(); + was_username_autofilled_ = username_element.isAutofilled(); - username_selection_start_ = username_element.selectionStart(); username_element.setSuggestedValue(username); username_element.setAutofilled(true); - username_element.setSelectionRange( - username_selection_start_, - username_element.suggestedValue().length()); - + ::autofill::PreviewSuggestion(username_element.suggestedValue(), + username_query_prefix_, &username_element); was_password_autofilled_ = password_info->password_field.isAutofilled(); password_info->password_field.setSuggestedValue(password); password_info->password_field.setAutofilled(true); @@ -1304,6 +1303,7 @@ bool PasswordAutofillAgent::ShowSuggestionPopup( Send(new AutofillHostMsg_ShowPasswordSuggestions( routing_id(), key_it->second, field.text_direction, username_string, options, bounding_box_scaled)); + username_query_prefix_ = username_string; return CanShowSuggestion(fill_data, username_string, show_all); } @@ -1339,7 +1339,7 @@ void PasswordAutofillAgent::ClearPreview( if (!username->suggestedValue().isEmpty()) { username->setSuggestedValue(blink::WebString()); username->setAutofilled(was_username_autofilled_); - username->setSelectionRange(username_selection_start_, + username->setSelectionRange(username_query_prefix_.length(), username->value().length()); } if (!password->suggestedValue().isEmpty()) { diff --git a/components/autofill/content/renderer/password_autofill_agent.h b/components/autofill/content/renderer/password_autofill_agent.h index 459f7b6..3461d4c 100644 --- a/components/autofill/content/renderer/password_autofill_agent.h +++ b/components/autofill/content/renderer/password_autofill_agent.h @@ -272,9 +272,8 @@ class PasswordAutofillAgent : public content::RenderFrameObserver { // True indicates that the password field was autofilled, false otherwise. bool was_password_autofilled_; - // Records original starting point of username element's selection range - // before preview. - int username_selection_start_; + // Records the username typed before suggestions preview. + base::string16 username_query_prefix_; // True indicates that all frames in a page have been rendered. bool did_stop_loading_; diff --git a/components/autofill/core/browser/autofill_manager_unittest.cc b/components/autofill/core/browser/autofill_manager_unittest.cc index 8c960b5..6477ace 100644 --- a/components/autofill/core/browser/autofill_manager_unittest.cc +++ b/components/autofill/core/browser/autofill_manager_unittest.cc @@ -3120,4 +3120,176 @@ TEST_F(AutofillManagerTest, FillInUpdatedExpirationDate) { CREDIT_CARD_EXP_4_DIGIT_YEAR)); } +// Verify that typing "gmail" will match "theking@gmail.com" and +// "buddy@gmail.com" when substring matching is enabled. +TEST_F(AutofillManagerTest, DisplaySuggestionsWithMatchingTokens) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + // Set up our form data. + FormData form; + test::CreateTestAddressFormData(&form); + std::vector<FormData> forms(1, form); + FormsSeen(forms); + + FormFieldData field; + test::CreateTestFormField("Email", "email", "gmail", "email", &field); + GetAutofillSuggestions(form, field); + AutocompleteSuggestionsReturned(std::vector<base::string16>()); + + external_delegate_->CheckSuggestions( + kDefaultPageID, + Suggestion("theking@gmail.com", "3734 Elvis Presley Blvd.", "", 1), + Suggestion("buddy@gmail.com", "123 Apple St.", "", 2)); +} + +// Verify that typing "apple" will match "123 Apple St." when substring matching +// is enabled. +TEST_F(AutofillManagerTest, DisplaySuggestionsWithMatchingTokens_CaseIgnored) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + // Set up our form data. + FormData form; + test::CreateTestAddressFormData(&form); + std::vector<FormData> forms(1, form); + FormsSeen(forms); + + FormFieldData field; + test::CreateTestFormField("Address Line 2", "addr2", "apple", "text", &field); + GetAutofillSuggestions(form, field); + AutocompleteSuggestionsReturned(std::vector<base::string16>()); + + external_delegate_->CheckSuggestions( + kDefaultPageID, + Suggestion("123 Apple St., unit 6", "123 Apple St.", "", 1)); +} + +// Verify that typing "mail" will not match any of the "@gmail.com" email +// addresses when substring matching is enabled. +TEST_F(AutofillManagerTest, NoSuggestionForNonPrefixTokenMatch) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + // Set up our form data. + FormData form; + test::CreateTestAddressFormData(&form); + std::vector<FormData> forms(1, form); + FormsSeen(forms); + + FormFieldData field; + test::CreateTestFormField("Email", "email", "mail", "email", &field); + GetAutofillSuggestions(form, field); + EXPECT_FALSE(external_delegate_->on_suggestions_returned_seen()); +} + +// Verify that typing "pres" will match "Elvis Presley" when substring matching +// is enabled. +TEST_F(AutofillManagerTest, DisplayCreditCardSuggestionsWithMatchingTokens) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + // Set up our form data. + FormData form; + CreateTestCreditCardFormData(&form, true, false); + std::vector<FormData> forms(1, form); + FormsSeen(forms); + + FormFieldData field; + test::CreateTestFormField("Name on Card", "nameoncard", "pres", "text", + &field); + GetAutofillSuggestions(form, field); + + // No suggestions provided, so send an empty vector as the results. + // This triggers the combined message send. + AutocompleteSuggestionsReturned(std::vector<base::string16>()); + +#if defined(OS_ANDROID) + static const char* kVisaSuggestion = + "Visa\xC2\xA0\xE2\x8B\xAF" + "3456"; +#else + static const char* kVisaSuggestion = "*3456"; +#endif + + external_delegate_->CheckSuggestions( + kDefaultPageID, Suggestion("Elvis Presley", kVisaSuggestion, kVisaCard, + autofill_manager_->GetPackedCreditCardID(4))); +} + +// Verify that typing "lvis" will not match any of the credit card name when +// substring matching is enabled. +TEST_F(AutofillManagerTest, NoCreditCardSuggestionsForNonPrefixTokenMatch) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + // Set up our form data. + FormData form; + CreateTestCreditCardFormData(&form, true, false); + std::vector<FormData> forms(1, form); + FormsSeen(forms); + + FormFieldData field; + test::CreateTestFormField("Name on Card", "nameoncard", "lvis", "text", + &field); + GetAutofillSuggestions(form, field); + EXPECT_FALSE(external_delegate_->on_suggestions_returned_seen()); +} + +// Verify that typing "S" into the middle name field will match and order middle +// names "Shawn Smith" followed by "Adam Smith" i.e. prefix matched followed by +// substring matched. +TEST_F(AutofillManagerTest, + DisplaySuggestionsWithPrefixesPrecedeSubstringMatched) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + // Set up our form data. + FormData form; + test::CreateTestAddressFormData(&form); + std::vector<FormData> forms(1, form); + FormsSeen(forms); + + AutofillProfile* profile1 = new AutofillProfile; + profile1->set_guid("00000000-0000-0000-0000-000000000103"); + profile1->SetInfo(AutofillType(NAME_FIRST), ASCIIToUTF16("Robin"), "en-US"); + profile1->SetInfo(AutofillType(NAME_MIDDLE), ASCIIToUTF16("Adam Smith"), + "en-US"); + profile1->SetInfo(AutofillType(NAME_LAST), ASCIIToUTF16("Grimes"), "en-US"); + profile1->SetInfo(AutofillType(ADDRESS_HOME_LINE1), + ASCIIToUTF16("1234 Smith Blvd."), "en-US"); + autofill_manager_->AddProfile(profile1); + + AutofillProfile* profile2 = new AutofillProfile; + profile2->set_guid("00000000-0000-0000-0000-000000000124"); + profile2->SetInfo(AutofillType(NAME_FIRST), ASCIIToUTF16("Carl"), "en-US"); + profile2->SetInfo(AutofillType(NAME_MIDDLE), ASCIIToUTF16("Shawn Smith"), + "en-US"); + profile2->SetInfo(AutofillType(NAME_LAST), ASCIIToUTF16("Grimes"), "en-US"); + profile2->SetInfo(AutofillType(ADDRESS_HOME_LINE1), + ASCIIToUTF16("1234 Smith Blvd."), "en-US"); + autofill_manager_->AddProfile(profile2); + + FormFieldData field; + test::CreateTestFormField("Middle Name", "middlename", "S", "text", &field); + GetAutofillSuggestions(form, field); + + // No suggestions provided, so send an empty vector as the results. + // This triggers the combined message send. + AutocompleteSuggestionsReturned(std::vector<base::string16>()); + + external_delegate_->CheckSuggestions( + kDefaultPageID, + Suggestion("Shawn Smith", "1234 Smith Blvd., Robin Adam Smith Grimes", "", + 1), + Suggestion("Adam Smith", "1234 Smith Blvd., Carl Shawn Smith Grimes", "", + 2)); +} + } // namespace autofill diff --git a/components/autofill/core/browser/personal_data_manager.cc b/components/autofill/core/browser/personal_data_manager.cc index 980448b..da9a59f 100644 --- a/components/autofill/core/browser/personal_data_manager.cc +++ b/components/autofill/core/browser/personal_data_manager.cc @@ -31,6 +31,7 @@ #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" @@ -796,15 +797,28 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions( continue; base::string16 value_canon = AutofillProfile::CanonicalizeProfileString(value); - if (base::StartsWith(value_canon, field_contents_canon, - base::CompareCase::SENSITIVE)) { - // Prefix match, add suggestion. + bool prefix_matched_suggestion = base::StartsWith( + value_canon, field_contents_canon, base::CompareCase::SENSITIVE); + if (prefix_matched_suggestion || + FieldIsSuggestionSubstringStartingOnTokenBoundary(value, field_contents, + false)) { matched_profiles.push_back(profile); suggestions.push_back(Suggestion(value)); suggestions.back().backend_id = profile->guid(); + suggestions.back().match = prefix_matched_suggestion + ? Suggestion::PREFIX_MATCH + : Suggestion::SUBSTRING_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; + }); + } + // Don't show two suggestions if one is a subset of the other. std::vector<AutofillProfile*> unique_matched_profiles; std::vector<Suggestion> unique_suggestions; @@ -856,6 +870,7 @@ std::vector<Suggestion> PersonalDataManager::GetCreditCardSuggestions( return std::vector<Suggestion>(); std::list<const CreditCard*> cards_to_suggest; + std::list<const CreditCard*> substring_matched_cards; base::string16 field_contents_lower = base::i18n::ToLower(field_contents); for (const CreditCard* credit_card : GetCreditCards()) { // The value of the stored data for this field type in the |credit_card|. @@ -878,12 +893,24 @@ std::vector<Suggestion> PersonalDataManager::GetCreditCardSuggestions( field_contents.size() >= 6)) { continue; } - } else if (!base::StartsWith(creditcard_field_lower, field_contents_lower, - base::CompareCase::SENSITIVE)) { - continue; + cards_to_suggest.push_back(credit_card); + } else if (base::StartsWith(creditcard_field_lower, field_contents_lower, + base::CompareCase::SENSITIVE)) { + cards_to_suggest.push_back(credit_card); + } else if (FieldIsSuggestionSubstringStartingOnTokenBoundary( + creditcard_field_lower, field_contents_lower, true)) { + substring_matched_cards.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_cards.sort(RankByMfu); + cards_to_suggest.insert(cards_to_suggest.end(), + substring_matched_cards.begin(), + substring_matched_cards.end()); } // De-dupe card suggestions. Full server cards shadow local cards, and @@ -911,8 +938,6 @@ std::vector<Suggestion> PersonalDataManager::GetCreditCardSuggestions( } } - cards_to_suggest.sort(RankByMfu); - std::vector<Suggestion> suggestions; for (const CreditCard* credit_card : cards_to_suggest) { // Make a new suggestion. diff --git a/components/autofill/core/browser/suggestion.cc b/components/autofill/core/browser/suggestion.cc index c46d302..c5b150e 100644 --- a/components/autofill/core/browser/suggestion.cc +++ b/components/autofill/core/browser/suggestion.cc @@ -11,7 +11,8 @@ namespace autofill { Suggestion::Suggestion() - : frontend_id(0) { + : frontend_id(0), + match(PREFIX_MATCH) { } Suggestion::Suggestion(const Suggestion& other) @@ -19,12 +20,14 @@ Suggestion::Suggestion(const Suggestion& other) frontend_id(other.frontend_id), value(other.value), label(other.label), - icon(other.icon) { + icon(other.icon), + match(other.match) { } Suggestion::Suggestion(const base::string16& v) : frontend_id(0), - value(v) { + value(v), + match(PREFIX_MATCH) { } Suggestion::Suggestion(const std::string& v, @@ -34,7 +37,8 @@ Suggestion::Suggestion(const std::string& v, : frontend_id(fid), value(base::UTF8ToUTF16(v)), label(base::UTF8ToUTF16(l)), - icon(base::UTF8ToUTF16(i)) { + icon(base::UTF8ToUTF16(i)), + match(PREFIX_MATCH) { } Suggestion::~Suggestion() { diff --git a/components/autofill/core/browser/suggestion.h b/components/autofill/core/browser/suggestion.h index 6077185..ebf1607 100644 --- a/components/autofill/core/browser/suggestion.h +++ b/components/autofill/core/browser/suggestion.h @@ -17,6 +17,11 @@ class CreditCard; struct Suggestion { public: + enum MatchMode { + PREFIX_MATCH, // for prefix matched suggestions; + SUBSTRING_MATCH // for substring matched suggestions; + }; + Suggestion(); // Copy constructor for STL containers. @@ -46,6 +51,7 @@ struct Suggestion { base::string16 value; base::string16 label; base::string16 icon; + MatchMode match; }; } // namespace autofill diff --git a/components/autofill/core/browser/webdata/autofill_table.cc b/components/autofill/core/browser/webdata/autofill_table.cc index c337f60..2ee8455 100644 --- a/components/autofill/core/browser/webdata/autofill_table.cc +++ b/components/autofill/core/browser/webdata/autofill_table.cc @@ -18,6 +18,7 @@ #include "base/logging.h" #include "base/numerics/safe_conversions.h" #include "base/strings/string_number_conversions.h" +#include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "base/time/time.h" #include "components/autofill/core/browser/autofill_country.h" @@ -28,6 +29,7 @@ #include "components/autofill/core/browser/webdata/autofill_change.h" #include "components/autofill/core/browser/webdata/autofill_entry.h" #include "components/autofill/core/common/autofill_switches.h" +#include "components/autofill/core/common/autofill_util.h" #include "components/autofill/core/common/form_field_data.h" #include "components/os_crypt/os_crypt.h" #include "components/webdata/common/web_database.h" @@ -368,6 +370,28 @@ CreditCard::ServerStatus ServerStatusStringToEnum(const std::string& status) { return CreditCard::OK; } +// Returns |s| with |escaper| in front of each of occurrence of a character from +// |special_chars|. Any occurrence of |escaper| in |s| is doubled. For example, +// Substitute("hello_world!", "_%", '!'') returns "hello!_world!!". +base::string16 Substitute(const base::string16& s, + const base::string16& special_chars, + const base::char16& escaper) { + // Prepend |escaper| to the list of |special_chars|. + base::string16 escape_wildcards(special_chars); + escape_wildcards.insert(escape_wildcards.begin(), escaper); + + // Prepend the |escaper| just before |special_chars| in |s|. + base::string16 result(s); + for (base::char16 c : escape_wildcards) { + for (size_t pos = 0; (pos = result.find(c, pos)) != base::string16::npos; + pos += 2) { + result.insert(result.begin() + pos, escaper); + } + } + + return result; +} + } // namespace // The maximum length allowed for form data. @@ -456,9 +480,10 @@ bool AutofillTable::GetFormValuesForElementName( std::vector<base::string16>* values, int limit) { DCHECK(values); - sql::Statement s; + bool succeeded = false; if (prefix.empty()) { + sql::Statement s; s.Assign(db_->GetUniqueStatement( "SELECT value FROM autofill " "WHERE name = ? " @@ -466,28 +491,62 @@ bool AutofillTable::GetFormValuesForElementName( "LIMIT ?")); s.BindString16(0, name); s.BindInt(1, limit); + + values->clear(); + while (s.Step()) + values->push_back(s.ColumnString16(0)); + + succeeded = s.Succeeded(); } else { base::string16 prefix_lower = base::i18n::ToLower(prefix); base::string16 next_prefix = prefix_lower; next_prefix[next_prefix.length() - 1]++; - s.Assign(db_->GetUniqueStatement( + sql::Statement s1; + s1.Assign(db_->GetUniqueStatement( "SELECT value FROM autofill " "WHERE name = ? AND " "value_lower >= ? AND " "value_lower < ? " "ORDER BY count DESC " "LIMIT ?")); - s.BindString16(0, name); - s.BindString16(1, prefix_lower); - s.BindString16(2, next_prefix); - s.BindInt(3, limit); + s1.BindString16(0, name); + s1.BindString16(1, prefix_lower); + s1.BindString16(2, next_prefix); + s1.BindInt(3, limit); + + values->clear(); + while (s1.Step()) + values->push_back(s1.ColumnString16(0)); + + succeeded = s1.Succeeded(); + + if (IsFeatureSubstringMatchEnabled()) { + sql::Statement s2; + s2.Assign(db_->GetUniqueStatement( + "SELECT value FROM autofill " + "WHERE name = ? AND (" + " value LIKE '% ' || :prefix || '%' ESCAPE '!' OR " + " value LIKE '%.' || :prefix || '%' ESCAPE '!' OR " + " value LIKE '%,' || :prefix || '%' ESCAPE '!' OR " + " value LIKE '%-' || :prefix || '%' ESCAPE '!' OR " + " value LIKE '%@' || :prefix || '%' ESCAPE '!' OR " + " value LIKE '%!_' || :prefix || '%' ESCAPE '!' ) " + "ORDER BY count DESC " + "LIMIT ?")); + + s2.BindString16(0, name); + // escaper as L'!' -> 0x21. + s2.BindString16(1, Substitute(prefix_lower, ASCIIToUTF16("_%"), 0x21)); + s2.BindInt(2, limit); + while (s2.Step()) + values->push_back(s2.ColumnString16(0)); + + succeeded &= s2.Succeeded(); + } } - values->clear(); - while (s.Step()) - values->push_back(s.ColumnString16(0)); - return s.Succeeded(); + return succeeded; } bool AutofillTable::HasFormElements() { diff --git a/components/autofill/core/browser/webdata/autofill_table_unittest.cc b/components/autofill/core/browser/webdata/autofill_table_unittest.cc index f3cf200..af71b6c 100644 --- a/components/autofill/core/browser/webdata/autofill_table_unittest.cc +++ b/components/autofill/core/browser/webdata/autofill_table_unittest.cc @@ -5,6 +5,7 @@ #include <utility> #include <vector> +#include "base/command_line.h" #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" #include "base/guid.h" @@ -21,6 +22,8 @@ #include "components/autofill/core/browser/webdata/autofill_change.h" #include "components/autofill/core/browser/webdata/autofill_entry.h" #include "components/autofill/core/browser/webdata/autofill_table.h" +#include "components/autofill/core/common/autofill_switches.h" +#include "components/autofill/core/common/autofill_util.h" #include "components/autofill/core/common/form_field_data.h" #include "components/os_crypt/os_crypt.h" #include "components/webdata/common/web_database.h" @@ -1856,4 +1859,60 @@ TEST_F(AutofillTableTest, DeleteUnmaskedCard) { outputs.clear(); } +TEST_F(AutofillTableTest, GetFormValuesForElementName_SubstringMatchEnabled) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + const size_t kMaxCount = 2; + const struct { + const char* const field_suggestion[kMaxCount]; + const char* const field_contents; + size_t expected_suggestion_count; + const char* const expected_suggestion[kMaxCount]; + } kTestCases[] = { + {{"user.test", "test_user"}, "TEST", 2, {"test_user", "user.test"}}, + {{"user test", "test-user"}, "user", 2, {"user test", "test-user"}}, + {{"user test", "test-rest"}, "user", 1, {"user test", nullptr}}, + {{"user@test", "test_user"}, "user@t", 1, {"user@test", nullptr}}, + {{"user.test", "test_user"}, "er.tes", 0, {nullptr, nullptr}}, + {{"user test", "test_user"}, "_ser", 0, {nullptr, nullptr}}, + {{"user.test", "test_user"}, "%ser", 0, {nullptr, nullptr}}, + {{"user.test", "test_user"}, + "; DROP TABLE autofill;", + 0, + {nullptr, nullptr}}, + }; + + for (size_t i = 0; i < arraysize(kTestCases); ++i) { + SCOPED_TRACE(testing::Message() + << "suggestion = " << kTestCases[i].field_suggestion[0] + << ", contents = " << kTestCases[i].field_contents); + + Time t1 = Time::Now(); + + // Simulate the submission of a handful of entries in a field called "Name". + AutofillChangeList changes; + FormFieldData field; + for (size_t k = 0; k < kMaxCount; ++k) { + field.name = ASCIIToUTF16("Name"); + field.value = ASCIIToUTF16(kTestCases[i].field_suggestion[k]); + table_->AddFormFieldValue(field, &changes); + } + + std::vector<base::string16> v; + table_->GetFormValuesForElementName( + ASCIIToUTF16("Name"), ASCIIToUTF16(kTestCases[i].field_contents), &v, + 6); + + EXPECT_EQ(kTestCases[i].expected_suggestion_count, v.size()); + for (size_t j = 0; j < kTestCases[i].expected_suggestion_count; ++j) { + EXPECT_EQ(ASCIIToUTF16(kTestCases[i].expected_suggestion[j]), v[j]); + } + + changes.clear(); + table_->RemoveFormElementsAddedBetween(t1, Time(), &changes); + } +} + } // namespace autofill diff --git a/components/autofill/core/common/BUILD.gn b/components/autofill/core/common/BUILD.gn index ed9d589..9a53fe2 100644 --- a/components/autofill/core/common/BUILD.gn +++ b/components/autofill/core/common/BUILD.gn @@ -16,6 +16,8 @@ static_library("common") { "autofill_regexes.h", "autofill_switches.cc", "autofill_switches.h", + "autofill_util.cc", + "autofill_util.h", "form_data.cc", "form_data.h", "form_data_predictions.cc", @@ -55,6 +57,7 @@ source_set("unit_tests") { testonly = true sources = [ "autofill_regexes_unittest.cc", + "autofill_util_unittest.cc", "form_data_unittest.cc", "form_field_data_unittest.cc", "password_form_fill_data_unittest.cc", diff --git a/components/autofill/core/common/autofill_switches.cc b/components/autofill/core/common/autofill_switches.cc index 675475a..0178335 100644 --- a/components/autofill/core/common/autofill_switches.cc +++ b/components/autofill/core/common/autofill_switches.cc @@ -63,6 +63,10 @@ const char kEnablePasswordGeneration[] = "enable-password-generation"; // Enables/disables suggestions without typing anything (on first click). const char kEnableSingleClickAutofill[] = "enable-single-click-autofill"; +// Enables suggestions with substring matching instead of prefix matching. +const char kEnableSuggestionsWithSubstringMatch[] = + "enable-suggestions-with-substring-match"; + // Enables syncing usage counts and last use dates of Wallet addresses and // cards. const char kEnableWalletMetadataSync[] = "enable-wallet-metadata-sync"; diff --git a/components/autofill/core/common/autofill_switches.h b/components/autofill/core/common/autofill_switches.h index 2c6ee91..0ce9bf7 100644 --- a/components/autofill/core/common/autofill_switches.h +++ b/components/autofill/core/common/autofill_switches.h @@ -24,6 +24,7 @@ extern const char kEnableFullFormAutofillIOS[]; extern const char kEnableOfferStoreUnmaskedWalletCards[]; extern const char kEnablePasswordGeneration[]; extern const char kEnableSingleClickAutofill[]; +extern const char kEnableSuggestionsWithSubstringMatch[]; extern const char kEnableWalletMetadataSync[]; extern const char kIgnoreAutocompleteOffForAutofill[]; extern const char kLocalHeuristicsOnlyForPasswordGeneration[]; diff --git a/components/autofill/core/common/autofill_util.cc b/components/autofill/core/common/autofill_util.cc new file mode 100644 index 0000000..a313fc9 --- /dev/null +++ b/components/autofill/core/common/autofill_util.cc @@ -0,0 +1,85 @@ +// Copyright 2015 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/common/autofill_util.h" + +#include <algorithm> +#include <vector> + +#include "base/command_line.h" +#include "base/i18n/case_conversion.h" +#include "base/strings/string_piece.h" +#include "base/strings/string_split.h" +#include "base/strings/string_util.h" +#include "base/strings/utf_string_conversions.h" +#include "components/autofill/core/common/autofill_switches.h" + +namespace autofill { + +namespace { + +const char kSplitCharacters[] = " .,-_@"; + +template <typename Char> +struct Compare : base::CaseInsensitiveCompareASCII<Char> { + explicit Compare(bool case_sensitive) : case_sensitive_(case_sensitive) {} + bool operator()(Char x, Char y) const { + return case_sensitive_ ? (x == y) + : base::CaseInsensitiveCompareASCII<Char>:: + operator()(x, y); + } + + private: + bool case_sensitive_; +}; + +} // namespace + +bool IsFeatureSubstringMatchEnabled() { + return base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableSuggestionsWithSubstringMatch); +} + +bool FieldIsSuggestionSubstringStartingOnTokenBoundary( + const base::string16& suggestion, + const base::string16& field_contents, + bool case_sensitive) { + if (!IsFeatureSubstringMatchEnabled()) { + return base::StartsWith(suggestion, field_contents, + case_sensitive + ? base::CompareCase::SENSITIVE + : base::CompareCase::INSENSITIVE_ASCII); + } + + return suggestion.length() >= field_contents.length() && + GetTextSelectionStart(suggestion, field_contents, case_sensitive) != + base::string16::npos; +} + +size_t GetTextSelectionStart(const base::string16& suggestion, + const base::string16& field_contents, + bool case_sensitive) { + const base::string16 kSplitChars = base::ASCIIToUTF16(kSplitCharacters); + + // Loop until we find either the |field_contents| is a prefix of |suggestion| + // or character right before the match is one of the splitting characters. + for (base::string16::const_iterator it = suggestion.begin(); + (it = std::search( + it, suggestion.end(), field_contents.begin(), field_contents.end(), + Compare<base::string16::value_type>(case_sensitive))) != + suggestion.end(); + ++it) { + if (it == suggestion.begin() || + kSplitChars.find(*(it - 1)) != std::string::npos) { + // Returns the character position right after the |field_contents| within + // |suggestion| text as a caret position for text selection. + return it - suggestion.begin() + field_contents.size(); + } + } + + // Unable to find the |field_contents| in |suggestion| text. + return base::string16::npos; +} + +} // namespace autofill diff --git a/components/autofill/core/common/autofill_util.h b/components/autofill/core/common/autofill_util.h new file mode 100644 index 0000000..18c487e --- /dev/null +++ b/components/autofill/core/common/autofill_util.h @@ -0,0 +1,38 @@ +// Copyright 2015 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. + +#ifndef COMPONENTS_AUTOFILL_CORE_COMMON_AUTOFILL_UTIL_H_ +#define COMPONENTS_AUTOFILL_CORE_COMMON_AUTOFILL_UTIL_H_ + +#include "base/strings/string16.h" + +namespace autofill { + +// Returns true when command line switch |kEnableSuggestionsWithSubstringMatch| +// is on. +bool IsFeatureSubstringMatchEnabled(); + +// A token is a sequences of contiguous characters separated by any of the +// characters that are part of delimiter set {' ', '.', ',', '-', '_', '@'}. + +// Returns true if the |field_contents| is a substring of the |suggestion| +// starting at token boundaries. |field_contents| can span multiple |suggestion| +// tokens. +bool FieldIsSuggestionSubstringStartingOnTokenBoundary( + const base::string16& suggestion, + const base::string16& field_contents, + bool case_sensitive); + +// Finds the first occurrence of a searched substring |field_contents| within +// the string |suggestion| starting at token boundaries and returns the index to +// the end of the located substring, or base::string16::npos if the substring is +// not found. "preview-on-hover" feature is one such use case where the +// substring |field_contents| may not be found within the string |suggestion|. +size_t GetTextSelectionStart(const base::string16& suggestion, + const base::string16& field_contents, + bool case_sensitive); + +} // namespace autofill + +#endif // COMPONENTS_AUTOFILL_CORE_COMMON_AUTOFILL_UTIL_H_ diff --git a/components/autofill/core/common/autofill_util_unittest.cc b/components/autofill/core/common/autofill_util_unittest.cc new file mode 100644 index 0000000..fc7bf61 --- /dev/null +++ b/components/autofill/core/common/autofill_util_unittest.cc @@ -0,0 +1,99 @@ +// Copyright 2015 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/common/autofill_util.h" + +#include "base/command_line.h" +#include "base/strings/utf_string_conversions.h" +#include "components/autofill/core/common/autofill_switches.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace autofill { + +// Tests for FieldIsSuggestionSubstringStartingOnTokenBoundary(). +TEST(AutofillUtilTest, FieldIsSuggestionSubstringStartingOnTokenBoundary) { + // FieldIsSuggestionSubstringStartingOnTokenBoundary should not work yet + // without a flag. + EXPECT_FALSE(FieldIsSuggestionSubstringStartingOnTokenBoundary( + base::ASCIIToUTF16("ab@cd.b"), base::ASCIIToUTF16("b"), false)); + + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + const struct { + const char* const field_suggestion; + const char* const field_contents; + bool case_sensitive; + bool expected_result; + } kTestCases[] = { + {"ab@cd.b", "a", false, true}, + {"ab@cd.b", "b", false, true}, + {"ab@cd.b", "Ab", false, true}, + {"ab@cd.b", "Ab", true, false}, + {"ab@cd.b", "cd", true, true}, + {"ab@cd.b", "d", false, false}, + {"ab@cd.b", "b@", true, false}, + {"ab@cd.b", "ab", false, true}, + {"ab@cd.b", "cd.b", true, true}, + {"ab@cd.b", "b@cd", false, false}, + {"ab@cd.b", "ab@c", false, true}, + {"ba.a.ab", "a.a", false, true}, + {"", "ab", false, false}, + {"", "ab", true, false}, + {"ab", "", false, true}, + {"ab", "", true, true}, + }; + + for (size_t i = 0; i < arraysize(kTestCases); ++i) { + SCOPED_TRACE(testing::Message() + << "suggestion = " << kTestCases[i].field_suggestion + << ", contents = " << kTestCases[i].field_contents + << ", case_sensitive = " << kTestCases[i].case_sensitive); + + EXPECT_EQ(kTestCases[i].expected_result, + FieldIsSuggestionSubstringStartingOnTokenBoundary( + base::ASCIIToUTF16(kTestCases[i].field_suggestion), + base::ASCIIToUTF16(kTestCases[i].field_contents), + kTestCases[i].case_sensitive)); + } +} + +// Tests for GetTextSelectionStart(). +TEST(AutofillUtilTest, GetTextSelectionStart) { + const size_t kInvalid = base::string16::npos; + const struct { + const char* const field_suggestion; + const char* const field_contents; + bool case_sensitive; + size_t expected_start; + } kTestCases[] = { + {"ab@cd.b", "a", false, 1}, + {"ab@cd.b", "A", true, kInvalid}, + {"ab@cd.b", "Ab", false, 2}, + {"ab@cd.b", "Ab", true, kInvalid}, + {"ab@cd.b", "cd", false, 5}, + {"ab@cd.b", "ab@c", false, 4}, + {"ab@cd.b", "cd.b", false, 7}, + {"ab@cd.b", "b@cd", false, kInvalid}, + {"ab@cd.b", "b", false, 7}, + {"ba.a.ab", "a.a", false, 6}, + {"texample@example.com", "example", false, 16}, + }; + + for (size_t i = 0; i < arraysize(kTestCases); ++i) { + SCOPED_TRACE(testing::Message() + << "suggestion = " << kTestCases[i].field_suggestion + << ", contents = " << kTestCases[i].field_contents + << ", case_sensitive = " << kTestCases[i].case_sensitive); + + EXPECT_EQ(kTestCases[i].expected_start, + GetTextSelectionStart( + base::ASCIIToUTF16(kTestCases[i].field_suggestion), + base::ASCIIToUTF16(kTestCases[i].field_contents), + kTestCases[i].case_sensitive)); + } +} + +} // autofill diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 4bf8a14..02c39b3 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -62,6 +62,7 @@ 'autofill/core/browser/webdata/autofill_wallet_metadata_syncable_service_unittest.cc', 'autofill/core/browser/webdata/web_data_service_unittest.cc', 'autofill/core/common/autofill_regexes_unittest.cc', + 'autofill/core/common/autofill_util_unittest.cc', 'autofill/core/common/form_data_unittest.cc', 'autofill/core/common/form_field_data_unittest.cc', 'autofill/core/common/password_form_fill_data_unittest.cc', diff --git a/components/password_manager/core/browser/password_autofill_manager.cc b/components/password_manager/core/browser/password_autofill_manager.cc index d0af275..3100bdd 100644 --- a/components/password_manager/core/browser/password_autofill_manager.cc +++ b/components/password_manager/core/browser/password_autofill_manager.cc @@ -17,6 +17,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/password_manager/core/browser/password_manager_metrics_util.h" @@ -60,6 +61,32 @@ base::string16 GetHumanReadableRealm(const std::string& signon_realm) { return base::UTF8ToUTF16(signon_realm); } +// If |field_suggestion| matches |field_content|, creates a Suggestion out of it +// and appends to |suggestions|. +void AppendSuggestionIfMatching( + const base::string16& field_suggestion, + const base::string16& field_contents, + const std::string& signon_realm, + bool show_all, + std::vector<autofill::Suggestion>* suggestions) { + base::string16 lower_suggestion = base::i18n::ToLower(field_suggestion); + base::string16 lower_contents = base::i18n::ToLower(field_contents); + bool prefix_matched_suggestion = + show_all || base::StartsWith(lower_suggestion, lower_contents, + base::CompareCase::SENSITIVE); + if (prefix_matched_suggestion || + autofill::FieldIsSuggestionSubstringStartingOnTokenBoundary( + lower_suggestion, lower_contents, true)) { + autofill::Suggestion suggestion(ReplaceEmptyUsername(field_suggestion)); + suggestion.label = GetHumanReadableRealm(signon_realm); + suggestion.frontend_id = autofill::POPUP_ITEM_ID_PASSWORD_ENTRY; + suggestion.match = prefix_matched_suggestion + ? autofill::Suggestion::PREFIX_MATCH + : autofill::Suggestion::SUBSTRING_MATCH; + suggestions->push_back(suggestion); + } +} + // 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. @@ -67,42 +94,28 @@ void GetSuggestions(const autofill::PasswordFormFillData& fill_data, const base::string16& current_username, std::vector<autofill::Suggestion>* suggestions, bool show_all) { - base::string16 lower_username = base::i18n::ToLower(current_username); - - if (show_all || - base::StartsWith(base::i18n::ToLower(fill_data.username_field.value), - lower_username, base::CompareCase::SENSITIVE)) { - 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); - } + AppendSuggestionIfMatching(fill_data.username_field.value, current_username, + fill_data.preferred_realm, show_all, suggestions); for (const auto& login : fill_data.additional_logins) { - if (show_all || - base::StartsWith(base::i18n::ToLower(login.first), lower_username, - base::CompareCase::SENSITIVE)) { - 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); - } + AppendSuggestionIfMatching(login.first, current_username, + login.second.realm, show_all, suggestions); } for (const auto& usernames : fill_data.other_possible_usernames) { for (size_t i = 0; i < usernames.second.size(); ++i) { - if (show_all || - base::StartsWith(base::i18n::ToLower(usernames.second[i]), - lower_username, base::CompareCase::SENSITIVE)) { - 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); - } + AppendSuggestionIfMatching(usernames.second[i], current_username, + usernames.first.realm, show_all, suggestions); } } + + // Prefix matches should precede other token matches. + if (autofill::IsFeatureSubstringMatchEnabled()) { + std::sort(suggestions->begin(), suggestions->end(), + [](const autofill::Suggestion& a, const autofill::Suggestion& b) { + return a.match < b.match; + }); + } } } // namespace diff --git a/components/password_manager/core/browser/password_autofill_manager_unittest.cc b/components/password_manager/core/browser/password_autofill_manager_unittest.cc index 7dd87dc..d214998 100644 --- a/components/password_manager/core/browser/password_autofill_manager_unittest.cc +++ b/components/password_manager/core/browser/password_autofill_manager_unittest.cc @@ -4,6 +4,7 @@ #include "components/password_manager/core/browser/password_autofill_manager.h" +#include "base/command_line.h" #include "base/compiler_specific.h" #include "base/message_loop/message_loop.h" #include "base/strings/utf_string_conversions.h" @@ -12,6 +13,7 @@ #include "components/autofill/core/browser/test_autofill_client.h" #include "components/autofill/core/browser/test_autofill_driver.h" #include "components/autofill/core/common/autofill_constants.h" +#include "components/autofill/core/common/autofill_switches.h" #include "components/autofill/core/common/form_field_data.h" #include "components/autofill/core/common/password_form_fill_data.h" #include "components/password_manager/core/browser/stub_password_manager_client.h" @@ -354,4 +356,180 @@ TEST_F(PasswordAutofillManagerTest, FillSuggestionPasswordField) { autofill::IS_PASSWORD_FIELD, element_bounds); } +// Verify that typing "foo" into the username field will match usernames +// "foo.bar@example.com", "bar.foo@example.com" and "example@foo.com". +TEST_F(PasswordAutofillManagerTest, DisplaySuggestionsWithMatchingTokens) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + scoped_ptr<TestPasswordManagerClient> client(new TestPasswordManagerClient); + scoped_ptr<MockAutofillClient> autofill_client(new MockAutofillClient); + InitializePasswordAutofillManager(client.get(), autofill_client.get()); + + gfx::RectF element_bounds; + autofill::PasswordFormFillData data; + base::string16 username = base::ASCIIToUTF16("foo.bar@example.com"); + data.username_field.value = username; + data.password_field.value = base::ASCIIToUTF16("foobar"); + data.preferred_realm = "http://foo.com/"; + + autofill::PasswordAndRealm additional; + additional.realm = "https://foobarrealm.org"; + base::string16 additional_username(base::ASCIIToUTF16("bar.foo@example.com")); + data.additional_logins[additional_username] = additional; + + autofill::UsernamesCollectionKey usernames_key; + usernames_key.realm = "http://yetanother.net"; + std::vector<base::string16> other_names; + base::string16 other_username(base::ASCIIToUTF16("example@foo.com")); + other_names.push_back(other_username); + data.other_possible_usernames[usernames_key] = other_names; + + int dummy_key = 0; + password_autofill_manager_->OnAddPasswordFormMapping(dummy_key, data); + + EXPECT_CALL( + *autofill_client, + ShowAutofillPopup(element_bounds, _, + SuggestionVectorValuesAre(testing::UnorderedElementsAre( + username, additional_username, other_username)), + _)); + password_autofill_manager_->OnShowPasswordSuggestions( + dummy_key, base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("foo"), false, + element_bounds); +} + +// Verify that typing "oo" into the username field will not match any usernames +// "foo.bar@example.com", "bar.foo@example.com" or "example@foo.com". +TEST_F(PasswordAutofillManagerTest, NoSuggestionForNonPrefixTokenMatch) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + scoped_ptr<TestPasswordManagerClient> client(new TestPasswordManagerClient); + scoped_ptr<MockAutofillClient> autofill_client(new MockAutofillClient); + InitializePasswordAutofillManager(client.get(), autofill_client.get()); + + gfx::RectF element_bounds; + autofill::PasswordFormFillData data; + base::string16 username = base::ASCIIToUTF16("foo.bar@example.com"); + data.username_field.value = username; + data.password_field.value = base::ASCIIToUTF16("foobar"); + data.preferred_realm = "http://foo.com/"; + + autofill::PasswordAndRealm additional; + additional.realm = "https://foobarrealm.org"; + base::string16 additional_username(base::ASCIIToUTF16("bar.foo@example.com")); + data.additional_logins[additional_username] = additional; + + autofill::UsernamesCollectionKey usernames_key; + usernames_key.realm = "http://yetanother.net"; + std::vector<base::string16> other_names; + base::string16 other_username(base::ASCIIToUTF16("example@foo.com")); + other_names.push_back(other_username); + data.other_possible_usernames[usernames_key] = other_names; + + int dummy_key = 0; + password_autofill_manager_->OnAddPasswordFormMapping(dummy_key, data); + + EXPECT_CALL(*autofill_client, ShowAutofillPopup(_, _, _, _)).Times(0); + + password_autofill_manager_->OnShowPasswordSuggestions( + dummy_key, base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("oo"), false, + element_bounds); +} + +// Verify that typing "foo@exam" into the username field will match username +// "bar.foo@example.com" even if the field contents span accross multiple +// tokens. +TEST_F(PasswordAutofillManagerTest, + MatchingContentsWithSuggestionTokenSeparator) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + scoped_ptr<TestPasswordManagerClient> client(new TestPasswordManagerClient); + scoped_ptr<MockAutofillClient> autofill_client(new MockAutofillClient); + InitializePasswordAutofillManager(client.get(), autofill_client.get()); + + gfx::RectF element_bounds; + autofill::PasswordFormFillData data; + base::string16 username = base::ASCIIToUTF16("foo.bar@example.com"); + data.username_field.value = username; + data.password_field.value = base::ASCIIToUTF16("foobar"); + data.preferred_realm = "http://foo.com/"; + + autofill::PasswordAndRealm additional; + additional.realm = "https://foobarrealm.org"; + base::string16 additional_username(base::ASCIIToUTF16("bar.foo@example.com")); + data.additional_logins[additional_username] = additional; + + autofill::UsernamesCollectionKey usernames_key; + usernames_key.realm = "http://yetanother.net"; + std::vector<base::string16> other_names; + base::string16 other_username(base::ASCIIToUTF16("example@foo.com")); + other_names.push_back(other_username); + data.other_possible_usernames[usernames_key] = other_names; + + int dummy_key = 0; + password_autofill_manager_->OnAddPasswordFormMapping(dummy_key, data); + + EXPECT_CALL( + *autofill_client, + ShowAutofillPopup(element_bounds, _, + SuggestionVectorValuesAre( + testing::UnorderedElementsAre(additional_username)), + _)); + password_autofill_manager_->OnShowPasswordSuggestions( + dummy_key, base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("foo@exam"), + false, element_bounds); +} + +// Verify that typing "example" into the username field will match and order +// usernames "example@foo.com", "foo.bar@example.com" and "bar.foo@example.com" +// i.e. prefix matched followed by substring matched. +TEST_F(PasswordAutofillManagerTest, + DisplaySuggestionsWithPrefixesPrecedeSubstringMatched) { + // Token matching is currently behind a flag. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + autofill::switches::kEnableSuggestionsWithSubstringMatch); + + scoped_ptr<TestPasswordManagerClient> client(new TestPasswordManagerClient); + scoped_ptr<MockAutofillClient> autofill_client(new MockAutofillClient); + InitializePasswordAutofillManager(client.get(), autofill_client.get()); + + gfx::RectF element_bounds; + autofill::PasswordFormFillData data; + base::string16 username = base::ASCIIToUTF16("foo.bar@example.com"); + data.username_field.value = username; + data.password_field.value = base::ASCIIToUTF16("foobar"); + data.preferred_realm = "http://foo.com/"; + + autofill::PasswordAndRealm additional; + additional.realm = "https://foobarrealm.org"; + base::string16 additional_username(base::ASCIIToUTF16("bar.foo@example.com")); + data.additional_logins[additional_username] = additional; + + autofill::UsernamesCollectionKey usernames_key; + usernames_key.realm = "http://yetanother.net"; + std::vector<base::string16> other_names; + base::string16 other_username(base::ASCIIToUTF16("example@foo.com")); + other_names.push_back(other_username); + data.other_possible_usernames[usernames_key] = other_names; + + int dummy_key = 0; + password_autofill_manager_->OnAddPasswordFormMapping(dummy_key, data); + + EXPECT_CALL( + *autofill_client, + ShowAutofillPopup(element_bounds, _, + SuggestionVectorValuesAre(testing::UnorderedElementsAre( + other_username, username, additional_username)), + _)); + password_autofill_manager_->OnShowPasswordSuggestions( + dummy_key, base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("foo"), false, + element_bounds); +} + } // namespace password_manager diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 9daef96..7a95e99 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -60513,6 +60513,7 @@ To add a new entry, add it with any value and run test to compute valid value. <int value="-964676765" label="enable-accelerated-mjpeg-decode"/> <int value="-951394314" label="top-chrome-md"/> <int value="-949178861" label="enable-new-avatar-menu"/> + <int value="-938178614" label="enable-suggestions-with-substring-match"/> <int value="-926422468" label="disable-embedded-shared-worker"/> <int value="-918618075" label="enable-service-worker"/> <int value="-914210146" label="enable-web-based-signin"/> |