Teach PasswordAutofillAgent sometimes match prefixes of usernames

components/autofill/core/common/autofill_util_unittest.cc

 // 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 <stddef.h>
 
 #include "base/command_line.h"
 #include "base/macros.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().
-struct FieldIsTokenBoundarySubstringCase {
+// Tests for IsSubstringStartingOnTokenBoundary().
+struct TokenBoundarySubstringCase {
   const char* const field_suggestion;
   const char* const field_contents;
   const bool case_sensitive;
   const bool expected_result;
 };
 
-class FieldIsTokenBoundarySubstringCaseTest
-    : public testing::TestWithParam<FieldIsTokenBoundarySubstringCase> {};
+struct TokenBoundaryPrefixCase {

[vabr (Chromium), 2017/06/12 18:56:59]

nit: Please move this just before its first use.

[melandory, 2017/06/19 10:53:48]

Done.

+  const char* const field_suggestion;
+  const char* const field_contents;
+  const bool expected_result;
+};
 
-TEST_P(FieldIsTokenBoundarySubstringCaseTest,
-       FieldIsSuggestionSubstringStartingOnTokenBoundary) {
-  // FieldIsSuggestionSubstringStartingOnTokenBoundary should not work yet
+class SubstringStartingOnTokenBoundaryTest

[vabr (Chromium), 2017/06/12 18:56:59]

nit: Similarly to the name of FieldIsSuggestionSubstringStartingOnTokenBoundary,
also here I suggest not renaming tests not introduced for your change.

[melandory, 2017/06/19 10:53:48]

Done.

+    : public testing::TestWithParam<TokenBoundarySubstringCase> {};
+
+TEST_P(SubstringStartingOnTokenBoundaryTest,
+       IsSubstringStartingOnTokenBoundary) {
+  // IsSubstringStartingOnTokenBoundary should not work yet
   // without a flag.
-  EXPECT_FALSE(FieldIsSuggestionSubstringStartingOnTokenBoundary(
+  EXPECT_FALSE(IsSubstringStartingOnTokenBoundary(
       base::ASCIIToUTF16("ab@cd.b"), base::ASCIIToUTF16("b"), false));
 
   // Token matching is currently behind a flag.
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableSuggestionsWithSubstringMatch);
 
   auto test_case = GetParam();
   SCOPED_TRACE(testing::Message()
                << "suggestion = " << test_case.field_suggestion
                << ", contents = " << test_case.field_contents
                << ", case_sensitive = " << test_case.case_sensitive);
 
   EXPECT_EQ(test_case.expected_result,
-            FieldIsSuggestionSubstringStartingOnTokenBoundary(
+            IsSubstringStartingOnTokenBoundary(
                 base::ASCIIToUTF16(test_case.field_suggestion),
                 base::ASCIIToUTF16(test_case.field_contents),
                 test_case.case_sensitive));
 }
 
 INSTANTIATE_TEST_CASE_P(
     AutofillUtilTest,
-    FieldIsTokenBoundarySubstringCaseTest,
-    testing::Values(
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "a", false, true},
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "b", false, true},
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "Ab", false, true},
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "Ab", true, false},
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "cd", true, true},
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "d", false, false},
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "b@", true, false},
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "ab", false, true},
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "cd.b", true, true},
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "b@cd", false, false},
-        FieldIsTokenBoundarySubstringCase{"ab@cd.b", "ab@c", false, true},
-        FieldIsTokenBoundarySubstringCase{"ba.a.ab", "a.a", false, true},
-        FieldIsTokenBoundarySubstringCase{"", "ab", false, false},
-        FieldIsTokenBoundarySubstringCase{"", "ab", true, false},
-        FieldIsTokenBoundarySubstringCase{"ab", "", false, true},
-        FieldIsTokenBoundarySubstringCase{"ab", "", true, true}));
+    SubstringStartingOnTokenBoundaryTest,
+    testing::Values(TokenBoundarySubstringCase{"ab@cd.b", "a", false, true},
+                    TokenBoundarySubstringCase{"ab@cd.b", "b", false, true},
+                    TokenBoundarySubstringCase{"ab@cd.b", "Ab", false, true},
+                    TokenBoundarySubstringCase{"ab@cd.b", "Ab", true, false},
+                    TokenBoundarySubstringCase{"ab@cd.b", "cd", true, true},
+                    TokenBoundarySubstringCase{"ab@cd.b", "d", false, false},
+                    TokenBoundarySubstringCase{"ab@cd.b", "b@", true, false},
+                    TokenBoundarySubstringCase{"ab@cd.b", "ab", false, true},
+                    TokenBoundarySubstringCase{"ab@cd.b", "cd.b", true, true},
+                    TokenBoundarySubstringCase{"ab@cd.b", "b@cd", false, false},
+                    TokenBoundarySubstringCase{"ab@cd.b", "ab@c", false, true},
+                    TokenBoundarySubstringCase{"ba.a.ab", "a.a", false, true},
+                    TokenBoundarySubstringCase{"", "ab", false, false},
+                    TokenBoundarySubstringCase{"", "ab", true, false},
+                    TokenBoundarySubstringCase{"ab", "", false, true},
+                    TokenBoundarySubstringCase{"ab", "", true, true}));
+
+class PrefixEndingOnTokenBoundaryTest
+    : public testing::TestWithParam<TokenBoundaryPrefixCase> {};
+
+TEST_P(PrefixEndingOnTokenBoundaryTest, IsPrefixEndingOnTokenBoundary) {
+  auto test_case = GetParam();
+  SCOPED_TRACE(testing::Message()
+               << "suggestion = " << test_case.field_suggestion
+               << ", contents = " << test_case.field_contents);
+
+  EXPECT_EQ(test_case.expected_result,
+            IsPrefixEndingOnTokenBoundary(
+                base::ASCIIToUTF16(test_case.field_suggestion),
+                base::ASCIIToUTF16(test_case.field_contents)));
+}
+
+INSTANTIATE_TEST_CASE_P(
+    AutofillUtilTest,
+    PrefixEndingOnTokenBoundaryTest,
+    testing::Values(TokenBoundaryPrefixCase{"ab@cd.b", "a", false},
+                    TokenBoundaryPrefixCase{"ab@cd.b", "b", false},
+                    TokenBoundaryPrefixCase{"ab@cd.b", "Ab", false},
+                    TokenBoundaryPrefixCase{"ab@cd.b", "cd", false},
+                    TokenBoundaryPrefixCase{"ab@cd.b", "d", false},
+                    TokenBoundaryPrefixCase{"ab@cd.b", "b@", false},
+                    TokenBoundaryPrefixCase{"ab@cd.b", "ab", true},
+                    TokenBoundaryPrefixCase{"ab@cd.b", "cd.b", false},
+                    TokenBoundaryPrefixCase{"ab@cd.b", "b@cd", false},
+                    TokenBoundaryPrefixCase{"ab@cd.b", "ab@c", false},
+                    TokenBoundaryPrefixCase{"ba.a.ab", "a.a", false},
+                    TokenBoundaryPrefixCase{"", "ab", false},
+                    TokenBoundaryPrefixCase{"", "ab", false},

[vabr (Chromium), 2017/06/12 18:56:59]

Let's not duplicate the test cases :).
(Also below.)

[melandory, 2017/06/19 10:53:48]

Done.

+                    TokenBoundaryPrefixCase{"ab", "", false},
+                    TokenBoundaryPrefixCase{"ab", "", false}));

[vabr (Chromium), 2017/06/12 18:56:59]

More cases to add:
ab@c, ab@, false
ab@cd@g, ab, false
ab@cd@g, ab@cd, false
abc, abc, false

[melandory, 2017/06/19 10:53:48]

Done.

 
 // Tests for GetTextSelectionStart().
 struct GetTextSelectionStartCase {
   const char* const field_suggestion;
   const char* const field_contents;
   const bool case_sensitive;
   const size_t expected_start;
 };
 
 class GetTextSelectionStartTest
     : public testing::TestWithParam<GetTextSelectionStartCase> {};
 
 TEST_P(GetTextSelectionStartTest, GetTextSelectionStart) {
   auto test_case = GetParam();
   SCOPED_TRACE(testing::Message()
-               << "suggestion = " << test_case.field_suggestion
-               << ", contents = " << test_case.field_contents
+               << "full string = " << test_case.field_suggestion

[vabr (Chromium), 2017/06/12 18:56:59]

nit: Also here, please do not change the unrelated test. Independently of
whether this makes the log more readable, let's not split the focus of this CL.
I'm happy to approve a separate CL with these 2 lines, if you strongly want to
make this change.

[melandory, 2017/06/19 10:53:48]

Done.

+               << ", prefix = " << test_case.field_contents
                << ", case_sensitive = " << test_case.case_sensitive);
   EXPECT_EQ(
       test_case.expected_start,
       GetTextSelectionStart(base::ASCIIToUTF16(test_case.field_suggestion),
                             base::ASCIIToUTF16(test_case.field_contents),
                             test_case.case_sensitive));
 }
 
 INSTANTIATE_TEST_CASE_P(
     AutofillUtilTest,
@@ skipping 51 matching lines @@
         LowercaseAndTokenizeAttributeStringCase{"foO baR bAz",
                                                 {"foo", "bar", "baz"}},
 
         // Test collapsing of multiple whitespace characters in a row
         LowercaseAndTokenizeAttributeStringCase{"  \t\t\n\n   ",
                                                 std::vector<std::string>()},
         LowercaseAndTokenizeAttributeStringCase{"foO    baR bAz",
                                                 {"foo", "bar", "baz"}}));
 
 }  // namespace autofill