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>
 
 #include "base/i18n/case_conversion.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #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"
 #include "components/autofill/core/common/autofill_util.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/autofill/core/common/password_form_field_prediction_map.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebVector.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
@@ skipping 58 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 290 matching lines @@
     return std::unique_ptr<PasswordForm>();
 
   // No actual action on the form, so use the the origin as the action.
   password_form->action = password_form->origin;
 
   return password_form;
 }
 
 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