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

components/password_manager/core/browser/password_autofill_manager_unittest.cc

Index: components/password_manager/core/browser/password_autofill_manager_unittest.cc
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 d811161d32361fe8fa19de0febdc71e5532c0f16..48a59d0379125b973d4559d61610c834df86be56 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,140 @@ TEST_F(PasswordAutofillManagerTest, FillSuggestionPasswordField) {
       autofill::IS_PASSWORD_FIELD, element_bounds);
 }
 
+// Test that suggestion tokens (substrings separated by characters from "

[vabr (Chromium), 2015/03/27 10:13:35]

Note that clangformat discarded the space character in the token separators
list. Please break manually, as I suggested in the previous comment.

[Pritam Nikam, 2015/03/27 14:57:43]

Done.

+// .,-_@") are matched against field contents.
+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);
+
+  // Simulate displaying suggestions for field contents "foo", check that
+  // matching ones are displayed.
+  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);
+}
+
+// Test that suggestions are not matched when the field contents spans multiple

[vabr (Chromium), 2015/03/27 10:13:35]

I'm sorry, when I was suggesting this comment, I got into an inconsistent state
-- wanted to suggest a test for field contents spanning multiple tokens (which
you added below), and commented in that spirit on this test.
Please adjust to match the actual test. Something like
// Test that suggestions which do not have a prefix match or prefix-token match
with the field contents are not matched.

[Pritam Nikam, 2015/03/27 14:57:43]

Done.

+// tokens (substrings separated by characters from " .,-_@").
+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);
+
+  // Simulate displaying suggestions for field contents "oo". Check that none
+  // appear, because none has a token with a prefix "oo".
+  EXPECT_CALL(*autofill_client, ShowAutofillPopup(_, _, _, _)).Times(0);
+
+  password_autofill_manager_->OnShowPasswordSuggestions(
+      dummy_key, base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("oo"), false,
+      element_bounds);
+}
+
+// Test that suggestion tokens (substrings separated by characters from "

[vabr (Chromium), 2015/03/27 10:13:35]

Again: the space in the token separator list got discarded.

[Pritam Nikam, 2015/03/27 14:57:43]

Done.

+// .,-_@") are matched against field contents containing tokens along the token

[vabr (Chromium), 2015/03/27 10:13:35]

I don't understand this sentence. What does "containing tokens along (...)
separators" mean?
Also, how is matching tested? Did you mean something like:
// Test matching when field contents contains suggestion token separators.
?

[Pritam Nikam, 2015/03/27 14:57:43]

Done.

+// separators.
+TEST_F(PasswordAutofillManagerTest,
+       DisplaySuggestionsWithMatchingTokensAndContentsHaveSeperator) {

[vabr (Chromium), 2015/03/27 10:13:35]

typo: Seperator
Also, proposed simplification of the test name:
MatchingContentsWithSuggestionTokenSeparator

[Pritam Nikam, 2015/03/27 14:57:43]

Done.

+  // 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);
+
+  // Simulate displaying suggestions for field contents "foo@exam", check that
+  // matching ones are displayed.

[vabr (Chromium), 2015/03/27 10:13:35]

nit: It is slightly confusing, that you use plural (matching ones) while there
is only one matching suggestion. I would also appreciate a mention of the
importance of token separators present. Ideally, could you explain why each of
the usernames is or is not matched? I'm actually surprised that any of them
match -- the presence of the token separator should prevent token matching, and
foo@exam is also not a prefix of any of them.

[Pritam Nikam, 2015/03/27 14:57:43]

Done.

I've rephrased it as below:

// Simulate displaying suggestion for field contents "foo@exam", check that
matching one is displayed. Please also note here that field contents contains
token separators '@', |IsContentsPrefixOfSuggestionToken()| only picks
suggestions that passes 2 constrains; firstly perform substring match, and
secondly |Tokenize| field contents and suggestion and check whether the field
contents's first token matches any of the suggestion's token. In our testcase,
only |additional_username| passes both the constrains and apparently we could
see only that in matching suggestion list.

+  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);
+}
+
 }  // namespace password_manager