[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 46 matching lines @@
         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
 // is imposed by WebFormElementToFormData().
 bool ShouldIgnoreForm(size_t num_editable_elements,
                       size_t num_control_elements) {
-  return (num_editable_elements < kRequiredAutofillFields &&
-          num_control_elements > 0);
+  return (!num_editable_elements && num_control_elements);
 }
 
 }  // 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 &&

[Evan Stade, 2015/10/21 19:12:31]

Why are we changing the point at which forms might get filtered out based on
size? Seems like you should be using ShouldBeParsed to filter things out here.

And ShouldBeParsed is a weird name given that you have to parse the form to
answer the question...

[sebsg, 2015/10/26 15:57:55]

This is the point where we filtered out forms with less than 3 fields before
they reached the browser. These fields didn't get to the autofill_manager
before, but now we need to allow them to reach it so they can be parsed for
autocomplete attribute and so we can make some suggestions. So I'm not sure I
understand what you mean. Do you mean converting to a form_structure, calling
ParseFieldTypesFromAutocompleteAttributes then calling ShouldBeParsed here?

[Evan Stade, 2015/10/27 17:08:37]

only if they actually have that attribute
something like that

[Mathieu, 2015/10/28 12:50:48]

I don't think it makes sense to move FormStructure and its associated parsing to
the renderer, at least for this trivial change. If anything I think the password
team is working towards moving their business logic to the browser, with the
renderer only handling <form>-to-FormData/PasswordForm. It makes sense for the
renderer code to be the link between WebKit and the browser, but not much more.

Here, you can have a single function that looks at the FormData and determines
if it's interesting enough to send to the browser. I'm bad with naming, but it
does sounds like it should be called something like ShouldBeParsed, with the
conditions that it has at least 3 fields or fields with autocomplete attributes.

[Evan Stade, 2015/10/28 18:51:53]

IsFormInteresting?

[sebsg, 2015/11/02 19:55:12]

Acknowledged.

[sebsg, 2015/11/02 19:55:12]

Acknowledged.

[sebsg, 2015/11/02 19:55:12]

Done.

-        !ContainsKey(parsed_forms_, form)) {
+    if (!form.fields.empty() && !ContainsKey(parsed_forms_, 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 (!synthetic_form.fields.empty() && !parsed_forms_.count(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 173 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