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..9e16edff6c90289095c3807e6633d3f76d4811f9 100644
--- a/components/autofill/content/renderer/password_form_conversion_utils.cc
+++ b/components/autofill/content/renderer/password_form_conversion_utils.cc
@@ -93,6 +93,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 +426,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()) {
@@ -743,4 +747,23 @@ bool HasAutocompleteAttributeValue(const blink::WebInputElement& element,
   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()));
+  // All credit cards autocomplete attributes start with "cc-". So if there is a

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

I wonder whether this would be simpler to code with SplitStringPiece:

for (auto attr : base::SplitStringPiece(autocomplete_value_lowercase, " \t\n\r",
base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY)) {
  if (base::StartsWith(attr, kAutocompleteCreditCardPrefix,
base::CompareCase::SENSITIVE)
    return true;
}
return false;

[dvadym, 2017/03/24 09:34:33]

Sure it's simpler. I had a dilemma because of this, to use SplitStringPiece or
not. Not it's written more effective (no creating of extra objects and not doing
split operation), but of course with splitting it's easier to understand. Anyway
HasAutocompleteAttributeValue (it's next function up) uses SplitStringPiece, but
we can make it more effective too.
So for me, any option is ok. What option would you prefer?

[Roger McFarlane (Chromium), 2017/03/24 15:47:16]

You could use a regular expression with case insensitive matching:
 (^|\s)cc\-

[vabr (Chromium), 2017/03/24 19:19:36]

The extra object created is a vector of StringPieces, which should not be too
expensive to create (it will also usually have very few items). I would be
worried if strings were copied, but they are not. If in doubt, feel free to do a
benchmark.

The code readability is significantly better with SplitStringPiece, without
needing 3-line explanation comments and with much shorter code.

Altogether, SplitStringPiece seems like a win to me here.

[dvadym, 2017/03/27 10:51:40]

Changed to call of SplitStringPiece, in this function and in
HasAutocompleteAttributeValue.

+  // credit card autocomplete attribute then |autocomplete_value_lowercase|
+  // should start from "cc-" or have a substring "<whitespace>cc-".
+  for (size_t index = -1;;) {
+    index = autocomplete_value_lowercase.find(kAutocompleteCreditCardPrefix,
+                                              index + 1);
+    if (index == std::string::npos)
+      break;
+    if (index == 0 ||
+        base::IsAsciiWhitespace(autocomplete_value_lowercase[index - 1]))
+      return true;
+  }
+  return false;
+}
+
 }  // namespace autofill