[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..9a4f3ff77348596370c1bc7c515fd8573817c442 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,95 @@ TEST_F(PasswordAutofillManagerTest, FillSuggestionPasswordField) {
       autofill::IS_PASSWORD_FIELD, element_bounds);
 }
 
+// Test that when |kEnableSuggestionsWithSubstringMatch| command line switch is
+// on we return only username suggestions having substring token begins with the

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

This sentence has broken grammar ("having token begins with"). Also, mentioning
the switch in the comment is not necessary, if you comment about that below (see
my comment there).
What about:

// Test that suggestion tokens (substrings separated by characters from
// " .,-_@") are matched against field contents.

No need to say "only", because you test the negative case in the new test below.

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

Done.

+// selected form field's partially filled contents.
+TEST_F(PasswordAutofillManagerTest, ExtractSuggestionsWithSubstringMatchToken) {

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

I propose removing "Substring" from the test name.
It does not describe the matching criterion (it's prefix matching, not substring
on the tokens, right?).
Also, Match->Matching, and I don't fully understand Extract -- did you mean
display?
In total:
DisplaySuggestionsWithMatchingTokens

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

Done.

+  base::CommandLine::ForCurrentProcess()->AppendSwitch(

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

Instead of mentioning the flag in the test comment, you can just say here:
// Token matching is currently behind a flag.

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

Done.

+      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_username1(base::ASCIIToUTF16("example@foo.com"));
+  other_names.push_back(other_username1);
+  base::string16 other_username2(base::ASCIIToUTF16("example@bar.com"));

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

Do not include this username -- you should test the negative cases in different
tests. Try to keep the test as focussed and small as possible. To test all the
newly added codepaths, you need more tests anyway (here you only check negative
filtering of other usernames).

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

Done.

+  other_names.push_back(other_username2);
+  data.other_possible_usernames[usernames_key] = other_names;
+
+  int dummy_key = 0;
+  password_autofill_manager_->OnAddPasswordFormMapping(dummy_key, data);
+
+  // Simulate displaying suggestions with substring token begins with "foo".

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

Sentence has broken grammar. What about:

// Simulate displaying suggestions for field contents "foo", check that matching
ones are displayed.

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

Done.

+  EXPECT_CALL(
+      *autofill_client,
+      ShowAutofillPopup(element_bounds, _,
+                        SuggestionVectorValuesAre(testing::UnorderedElementsAre(
+                            username, additional_username, other_username1)),
+                        _));
+  password_autofill_manager_->OnShowPasswordSuggestions(
+      dummy_key, base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("foo"), false,
+      element_bounds);
+}
+
+// Test that when |kEnableSuggestionsWithSubstringMatch| command line switch is

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

Similarly to above, consider shortening the comment to:

// Test that suggestions are not matched when the field contents
// spans multiple tokens (substrings separated by characters
// from " .,-_@").

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

Done.

+// on we fail to return username suggestions having substring match but tokens
+// do not begin with the selected form field's partially filled contents.
+TEST_F(PasswordAutofillManagerTest,
+       NoSuggestionWithSubstringMatchButTokenDoesNot) {

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

The name has broken grammar (Match is a noun, so "does not" cannot be applied).
What about
NoSuggestionForNonPrefixTokenMatch
?

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

Done.

+  base::CommandLine::ForCurrentProcess()->AppendSwitch(

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

Instead of mentioning the flag in the test comment, you can just say here:
// Token matching is currently behind a flag.

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

Done.

+      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_username1(base::ASCIIToUTF16("example@foo.com"));
+  other_names.push_back(other_username1);
+  base::string16 other_username2(base::ASCIIToUTF16("example@bar.com"));
+  other_names.push_back(other_username2);
+  data.other_possible_usernames[usernames_key] = other_names;
+
+  int dummy_key = 0;
+  password_autofill_manager_->OnAddPasswordFormMapping(dummy_key, data);
+
+  // Simulate no suggestion popup appears with substring matching "oo".

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

This is not really simulating that nothing appears. Please rephrase, e.g., as
// Simulate displaying suggestions for field contents "oo". Check that none
appear, because none has a token with a prefix "oo".

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

Done.

+  EXPECT_CALL(*autofill_client, ShowAutofillPopup(_, _, _, _)).Times(0);
+
+  password_autofill_manager_->OnShowPasswordSuggestions(
+      dummy_key, base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("oo"), false,
+      element_bounds);
+}
+
 }  // namespace password_manager

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

Further suggestions for tests:
(1) Test (and thereby document) what happens if the field contents contains the
token separators, e.g., "user.typed".
(2) If UI confirms that the prefix-matched suggestions should be displayed
before the token-matched ones, add a test for that as well.

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

Done.
Will add this test after confirmation from UX.