Password Manager should skip fields with credit card autocomplete attribute.

components/autofill/content/renderer/password_form_conversion_utils.cc

Index: components/autofill/content/renderer/password_form_conversion_utils.cc
diff --git a/components/autofill/content/renderer/password_form_conversion_utils.cc b/components/autofill/content/renderer/password_form_conversion_utils.cc
index e02d83dbb53d099739cbb77a85eb39c9b49addfa..f128e0b4647cc6673a62357f25d573eeef1842ce 100644
--- a/components/autofill/content/renderer/password_form_conversion_utils.cc
+++ b/components/autofill/content/renderer/password_form_conversion_utils.cc
@@ -15,6 +15,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/content/renderer/form_autofill_util.h"
@@ -93,6 +94,7 @@ const char kLoginAndSignupRegex[] =
 const char kAutocompleteUsername[] = "username";
 const char kAutocompleteCurrentPassword[] = "current-password";
 const char kAutocompleteNewPassword[] = "new-password";
+const char kAutocompleteCreditCardPrefix[] = "cc-";
 
 re2::RE2* CreateMatcher(void* instance, const char* pattern) {
   re2::RE2::Options options;
@@ -425,6 +427,9 @@ bool GetPasswordForm(
     if (!input_element || !input_element->isEnabled())
       continue;
 
+    if (HasCreditCardAutocompleteAttributes(*input_element))
+      continue;
+
     bool element_is_invisible = !form_util::IsWebNodeVisible(*input_element);
     if (input_element->isTextField()) {
       if (input_element->isPasswordField()) {
@@ -735,12 +740,30 @@ std::unique_ptr<PasswordForm> CreatePasswordFormFromUnownedInputElements(
 
 bool HasAutocompleteAttributeValue(const blink::WebInputElement& element,
                                    const char* value_in_lowercase) {
-  base::string16 autocomplete_attribute(
-      element.getAttribute("autocomplete").utf16());
-  std::vector<std::string> tokens = LowercaseAndTokenizeAttributeString(
-      base::UTF16ToUTF8(autocomplete_attribute));
+  std::string autocomplete_value_lowercase = base::ToLowerASCII(
+      base::UTF16ToUTF8(element.getAttribute("autocomplete").utf16()));
+
+  std::vector<base::StringPiece> tokens = base::SplitStringPiece(
+      autocomplete_value_lowercase, base::kWhitespaceASCII,
+      base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
 
   return base::ContainsValue(tokens, value_in_lowercase);
 }
 
+bool HasCreditCardAutocompleteAttributes(
+    const blink::WebInputElement& element) {
+  std::string autocomplete_value_lowercase = base::ToLowerASCII(
+      base::UTF16ToUTF8(element.getAttribute("autocomplete").utf16()));
+
+  for (const auto& token : base::SplitStringPiece(

[Roger McFarlane (Chromium), 2017/03/27 19:06:28]

Another, IMO better, alternative would be to just do startswith "cc-" or
contains " cc-" which would be clearer to read, have zero memory overhead and be
O(n) time, where n is the length of the value.

[vabr (Chromium), 2017/03/27 20:20:03]

It is indeed O(n) because the length of " cc-" is a constant. I'm not sure about
the zero memory overhead, because I don't know how exactly string::find is
implemented.

What you describe is roughly what Vadym had originally, and I would argue that
that was not clearer to read.

I'm fine with anything which at least reaches the bar of the current code: no
explanatory comments needed, and the code is concise.

[dvadym, 2017/03/28 08:33:08]

When using regexp, there are 2 times: creating a regexp object (building state
machine) and matching. Since the creating of the object could take significant
time in general, usually it's better to create the object only once. Then it's
needed to use some machinery to do this for example like here
https://cs.chromium.org/chromium/src/components/autofill/content/renderer/pas...
with lazy loading. My feeling that it would be not easier than just straight
forward splitting and checking with StartsWith. Moreover now both this function
and HasAutocompleteAttributeValue function have the same logic with splitting,
and in HasAutocompleteAttributeValue it doesn't make sense to create a regexp
object since |value_in_lowercase| is different on each call.

+           autocomplete_value_lowercase, base::kWhitespaceASCII,
+           base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY)) {
+    if (base::StartsWith(token, kAutocompleteCreditCardPrefix,
+                         base::CompareCase::SENSITIVE)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace autofill