Password Manager should skip fields with credit card autocomplete attribute.

components/autofill/content/renderer/password_form_conversion_utils.cc

 // Copyright 2013 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/content/renderer/password_form_conversion_utils.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <string>
@@ skipping 75 matching lines @@
 // (N*PPP?.*) and forms which use password fields to store private but
 // non-password data (could look like, e.g., PN+P.*).
 const char kLoginAndSignupRegex[] =
     "NP"   // Login section.
     "N+P"  // Sign-up section.
     ".*";  // Anything beyond that.
 
 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;
   options.set_case_sensitive(false);
   // Use placement new to initialize the instance in the preallocated space.
   // The "(instance)" is very important to force POD type initialization.
   re2::RE2* matcher = new (instance) re2::RE2(pattern, options);
   DCHECK(matcher->ok());
   return matcher;
 }
@@ skipping 312 matching lines @@
   std::string layout_sequence;
   layout_sequence.reserve(form.control_elements.size());
   size_t number_of_non_empty_text_non_password_fields = 0;
   for (size_t i = 0; i < form.control_elements.size(); ++i) {
     WebFormControlElement control_element = form.control_elements[i];
 
     WebInputElement* input_element = toWebInputElement(&control_element);
     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()) {
         if (element_is_invisible && ignore_invisible_passwords)
           continue;
         layout_sequence.push_back('P');
       } else {
         if (FieldHasNonscriptModifiedValue(field_value_and_properties_map,
                                            *input_element))
           ++number_of_non_empty_text_non_password_fields;
@@ skipping 298 matching lines @@
 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));
 
   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