[Autofill] Always show available data when encountering autocomplete attributes

components/autofill/content/renderer/form_cache.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/form_cache.h"
 
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/content/renderer/form_autofill_util.h"
@@ skipping 42 matching lines @@
     std::string msg = std::string("autocomplete='") + deprecated[i] +
         "' is deprecated and will soon be ignored. See http://goo.gl/YjeSsW";
     WebConsoleMessage console_message = WebConsoleMessage(
         WebConsoleMessage::LevelWarning,
         WebString(base::ASCIIToUTF16(msg)));
     element.document().frame()->addMessageToConsole(console_message);
   }
 }
 
 // To avoid overly expensive computation, we impose a minimum number of
 // allowable fields.  The corresponding maximum number of allowable fields

[Evan Stade, 2015/11/04 17:15:18]

that minimum number is now one, so this comment needs updating

[sebsg, 2015/11/10 19:04:50]

Done.

 // is imposed by WebFormElementToFormData().

[Evan Stade, 2015/11/04 17:15:18]

I think this comment needs further updating because I can't grasp the logic of
why you require there to be control elements in order to ignore the form

[sebsg, 2015/11/10 19:04:50]

As I read the code more I realized that num_editable_elements is a subset of
num_control_elements (the editable ones) which is really the number of
autofillable fields. Talked to isherman@ and it was used with has_skipped_forms
https://chromiumcodereview.appspot.com/13831013 which is no longer there. So I
removed it. DO you know of a reason why it would still be useful?

 bool ShouldIgnoreForm(size_t num_editable_elements,
                       size_t num_control_elements) {
-  return (num_editable_elements < kRequiredAutofillFields &&
-          num_control_elements > 0);

[Evan Stade, 2015/11/04 17:15:18]

prefer this way of checking against 0 rather than implicit cast to bool

[sebsg, 2015/11/10 19:04:50]

Done.

+  return (!num_editable_elements && num_control_elements);

[Evan Stade, 2015/11/04 17:15:18]

nit: remove excess parens

[sebsg, 2015/11/10 19:04:50]

Done.

 }
 
 }  // namespace
 
 FormCache::FormCache(const WebFrame& frame) : frame_(frame) {
 }
 
 FormCache::~FormCache() {
 }
 
@@ skipping 31 matching lines @@
     FormData form;
     if (!WebFormElementToFormData(form_element, WebFormControlElement(),
                                   extract_mask, &form, nullptr)) {
       continue;
     }
 
     num_fields_seen += form.fields.size();
     if (num_fields_seen > form_util::kMaxParseableFields)
       return forms;
 
-    if (form.fields.size() >= kRequiredAutofillFields &&
-        !ContainsKey(parsed_forms_, form)) {
+    if (!ContainsKey(parsed_forms_, form) && IsFormInteresting(form)) {
       for (auto it = parsed_forms_.begin(); it != parsed_forms_.end(); ++it) {
         if (it->SameFormAs(form)) {
           parsed_forms_.erase(it);
           break;
         }
       }
 
       SaveInitialValues(control_elements);
       forms.push_back(form);
       parsed_forms_.insert(form);
@@ skipping 16 matching lines @@
   if (!UnownedCheckoutFormElementsAndFieldSetsToFormData(
           fieldsets, control_elements, nullptr, document, extract_mask,
           &synthetic_form, nullptr)) {
     return forms;
   }
 
   num_fields_seen += synthetic_form.fields.size();
   if (num_fields_seen > form_util::kMaxParseableFields)
     return forms;
 
-  if (synthetic_form.fields.size() >= kRequiredAutofillFields &&
-      !parsed_forms_.count(synthetic_form)) {
+  if (!parsed_forms_.count(synthetic_form) &&
+      IsFormInteresting(synthetic_form)) {
     SaveInitialValues(control_elements);
     forms.push_back(synthetic_form);
     parsed_forms_.insert(synthetic_form);
     parsed_forms_.erase(synthetic_form_);
     synthetic_form_ = synthetic_form;
   }
   return forms;
 }
 
 void FormCache::Reset() {
@@ skipping 128 matching lines @@
         base::UTF8ToUTF16(form.fields[i].server_type),
         base::UTF8ToUTF16(form.fields[i].heuristic_type),
         base::UTF8ToUTF16(form.fields[i].signature),
         base::UTF8ToUTF16(form.signature));
     element.setAttribute("title", WebString(title));
   }
 
   return true;
 }
 
+// static
+bool FormCache::IsFormInteresting(FormData form) {
+  if (form.fields.empty())
+    return false;
+
+  // If the form has at least one field with an autocomplete attribute,
+  // autocomplete suggestions will be shown.

[Evan Stade, 2015/11/04 17:15:18]

this comment is misleading, we don't know if suggestions will be shown or not

[sebsg, 2015/11/10 19:04:50]

Done.

+  for (auto field : form.fields) {
+    if (!field.autocomplete_attribute.empty())
+      return true;
+  }
+
+  // If there are no autocomplete attributes, the form needs to have at least
+  // the required number of fields to be a candidate for autofill.
+  return form.fields.size() >= kRequiredAutofillFields;
+}
+
 size_t FormCache::ScanFormControlElements(
     const std::vector<WebFormControlElement>& control_elements,
     bool log_deprecation_messages) {
   size_t num_editable_elements = 0;
   for (size_t i = 0; i < control_elements.size(); ++i) {
     const WebFormControlElement& element = control_elements[i];
 
     if (log_deprecation_messages)
       LogDeprecationMessages(element);
 
@@ skipping 23 matching lines @@
       const WebInputElement* input_element = toWebInputElement(&element);
       if (form_util::IsCheckableElement(input_element)) {
         initial_checked_state_.insert(
             std::make_pair(*input_element, input_element->isChecked()));
       }
     }
   }
 }
 
 }  // namespace autofill