Don't ask the browser for Autofill suggestions for non-autofillable form fields.

BUG=69914
TEST=browser_tests --gtest_filter=FormAutofillTest.*


Review URL: http://codereview.chromium.org/7978048

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@102240 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/autofill/autofill_manager.cc

@@ -818,7 +818,13 @@ bool AutofillManager::FindCachedForm(const FormData& form,
        iter != form_structures_.rend(); ++iter) {
     if (**iter == form) {
       *form_structure = *iter;
-      break;
+
+      // The same form might be cached with multiple field counts: in some
+      // cases, non-autofillable fields are filtered out, whereas in other cases
+      // they are not.  To avoid thrashing the cache, keep scanning until we
+      // find a cached version with the same number of fields, if there is one.
+      if ((*iter)->field_count() == form.fields.size())
+        break;
     }
   }
 
@@ -836,14 +842,14 @@ bool AutofillManager::GetCachedFormAndField(const FormData& form,
   // If we do not have this form in our cache but it is parseable, we'll add it
   // in the call to |UpdateCachedForm()|.
   if (!FindCachedForm(form, form_structure) &&
-      (form_structures_.size() >= kMaxFormCacheSize ||
-       !FormStructure(form).ShouldBeParsed(false))) {
+      !FormStructure(form).ShouldBeParsed(false)) {
     return false;
   }
 
   // Update the cached form to reflect any dynamic changes to the form data, if
   // necessary.
-  UpdateCachedForm(form, *form_structure, form_structure);
+  if (!UpdateCachedForm(form, *form_structure, form_structure))
+    return false;
 
   // No data to return if there are no auto-fillable fields.
   if (!(*form_structure)->autofill_count())
@@ -865,7 +871,7 @@ bool AutofillManager::GetCachedFormAndField(const FormData& form,
   return true;
 }
 
-void AutofillManager::UpdateCachedForm(const FormData& live_form,
+bool AutofillManager::UpdateCachedForm(const FormData& live_form,
                                        const FormStructure* cached_form,
                                        FormStructure** updated_form) {
   bool needs_update =
@@ -876,10 +882,12 @@ void AutofillManager::UpdateCachedForm(const FormData& live_form,
   }
 
   if (!needs_update)
-    return;
+    return true;
+
+  if (form_structures_.size() >= kMaxFormCacheSize)
+    return false;
 
   // Add the new or updated form to our cache.
-  DCHECK(form_structures_.size() < kMaxFormCacheSize);
   form_structures_.push_back(new FormStructure(live_form));
   *updated_form = *form_structures_.rbegin();
   (*updated_form)->DetermineHeuristicTypes();
@@ -910,6 +918,8 @@ void AutofillManager::UpdateCachedForm(const FormData& live_form,
   // Annotate the updated form with its predicted types.
   std::vector<FormStructure*> forms(1, *updated_form);
   SendAutofillTypePredictions(forms);
+
+  return true;
 }
 
 void AutofillManager::GetProfileSuggestions(

chrome/browser/autofill/autofill_manager.h

@@ -162,10 +162,10 @@ class AutofillManager : public TabContentsObserver,
   // Re-parses |live_form| and adds the result to |form_structures_|.
   // |cached_form| should be a pointer to the existing version of the form, or
   // NULL if no cached version exists.  The updated form is then written into
-  // |updated_form|.
-  void UpdateCachedForm(const webkit_glue::FormData& live_form,
+  // |updated_form|.  Returns false if the cache could not be updated.
+  bool UpdateCachedForm(const webkit_glue::FormData& live_form,
                         const FormStructure* cached_form,
-                        FormStructure** updated_form);
+                        FormStructure** updated_form) WARN_UNUSED_RESULT;
 
   // Returns a list of values from the stored profiles that match |type| and the
   // value of |field| and returns the labels of the matching profiles. |labels|

chrome/renderer/autofill/autofill_agent.cc

@@ -222,7 +222,7 @@ void AutofillAgent::TextFieldDidChangeImpl(const WebInputElement& element) {
 
   webkit_glue::FormData form;
   webkit_glue::FormField field;
-  if (FindFormAndFieldForNode(element, &form, &field)) {
+  if (FindFormAndFieldForInputElement(element, &form, &field, REQUIRE_NONE)) {
     Send(new AutofillHostMsg_TextFieldDidChange(routing_id(), form, field,
                                                 base::TimeTicks::Now()));
   }
@@ -399,7 +399,8 @@ void AutofillAgent::QueryAutofillSuggestions(const WebInputElement& element,
 
   webkit_glue::FormData form;
   webkit_glue::FormField field;
-  if (!FindFormAndFieldForNode(element, &form, &field)) {
+  if (!FindFormAndFieldForInputElement(element, &form, &field,
+                                       REQUIRE_AUTOCOMPLETE)) {
     // If we didn't find the cached form, at least let autocomplete have a shot
     // at providing suggestions.
     WebFormControlElementToFormField(element, EXTRACT_VALUE, &field);
@@ -417,8 +418,10 @@ void AutofillAgent::FillAutofillFormData(const WebNode& node,
 
   webkit_glue::FormData form;
   webkit_glue::FormField field;
-  if (!FindFormAndFieldForNode(node, &form, &field))
+  if (!FindFormAndFieldForInputElement(node.toConst<WebInputElement>(), &form,
+                                       &field, REQUIRE_AUTOCOMPLETE)) {
     return;
+  }
 
   autofill_action_ = action;
   was_query_node_autofilled_ = field.is_autofilled;
@@ -426,11 +429,4 @@ void AutofillAgent::FillAutofillFormData(const WebNode& node,
       routing_id(), autofill_query_id_, form, field, unique_id));
 }
 
-bool AutofillAgent::FindFormAndFieldForNode(const WebNode& node,
-                                            webkit_glue::FormData* form,
-                                            webkit_glue::FormField* field) {
-  const WebInputElement& element = node.toConst<WebInputElement>();
-  return FindFormAndFieldForFormControlElement(element, form, field);
-}
-
 }  // namespace autofill

chrome/renderer/autofill/autofill_browsertest.cc

@@ -29,8 +29,9 @@ TEST_F(RenderViewTest, SendForms) {
 
   LoadHTML("<form method=\"POST\">"
            "  <input type=\"text\" id=\"firstname\"/>"
-           "  <input type=\"text\" id=\"middlename\" autoComplete=\"off\"/>"
-           "  <input type=\"hidden\" id=\"lastname\"/>"
+           "  <input type=\"text\" id=\"middlename\"/>"
+           "  <input type=\"text\" id=\"lastname\" autoComplete=\"off\"/>"
+           "  <input type=\"hidden\" id=\"email\"/>"
            "  <select id=\"state\"/>"
            "    <option>?</option>"
            "    <option>California</option>"
@@ -47,7 +48,7 @@ TEST_F(RenderViewTest, SendForms) {
   AutofillHostMsg_FormsSeen::Read(message, &params);
   const std::vector<FormData>& forms = params.a;
   ASSERT_EQ(1UL, forms.size());
-  ASSERT_EQ(3UL, forms[0].fields.size());
+  ASSERT_EQ(4UL, forms[0].fields.size());
 
   FormField expected;
 
@@ -63,11 +64,17 @@ TEST_F(RenderViewTest, SendForms) {
   expected.max_length = WebInputElement::defaultMaxLength();
   EXPECT_FORM_FIELD_EQUALS(expected, forms[0].fields[1]);
 
+  expected.name = ASCIIToUTF16("lastname");
+  expected.value = string16();
+  expected.form_control_type = ASCIIToUTF16("text");
+  expected.max_length = WebInputElement::defaultMaxLength();
+  EXPECT_FORM_FIELD_EQUALS(expected, forms[0].fields[2]);
+
   expected.name = ASCIIToUTF16("state");
   expected.value = ASCIIToUTF16("?");
   expected.form_control_type = ASCIIToUTF16("select-one");
   expected.max_length = 0;
-  EXPECT_FORM_FIELD_EQUALS(expected, forms[0].fields[2]);
+  EXPECT_FORM_FIELD_EQUALS(expected, forms[0].fields[3]);
 
   // Verify that |didAcceptAutofillSuggestion()| sends the expected number of
   // fields.

chrome/renderer/autofill/form_autofill_util.cc

@@ -781,9 +781,10 @@ bool WebFormElementToFormData(
   return true;
 }
 
-bool FindFormAndFieldForFormControlElement(const WebFormControlElement& element,
-                                           FormData* form,
-                                           webkit_glue::FormField* field) {
+bool FindFormAndFieldForInputElement(const WebInputElement& element,
+                                     FormData* form,
+                                     webkit_glue::FormField* field,
+                                     RequirementsMask requirements) {
   if (!IsAutofillableElement(element))
     return false;
 
@@ -795,7 +796,7 @@ bool FindFormAndFieldForFormControlElement(const WebFormControlElement& element,
       static_cast<ExtractMask>(EXTRACT_VALUE | EXTRACT_OPTIONS);
   return WebFormElementToFormData(form_element,
                                   element,
-                                  REQUIRE_NONE,
+                                  requirements,
                                   extract_mask,
                                   form,
                                   field);

chrome/renderer/autofill/form_autofill_util.h

@@ -85,10 +85,10 @@ bool WebFormElementToFormData(
 // Finds the form that contains |element| and returns it in |form|.  Fills
 // |field| with the |FormField| representation for element.
 // Returns false if the form is not found.
-bool FindFormAndFieldForFormControlElement(
-    const WebKit::WebFormControlElement& element,
-    webkit_glue::FormData* form,
-    webkit_glue::FormField* field);
+bool FindFormAndFieldForInputElement(const WebKit::WebInputElement& element,
+                                     webkit_glue::FormData* form,
+                                     webkit_glue::FormField* field,
+                                     RequirementsMask requirements);
 
 // Fills the form represented by |form|.  |element| is the input element that
 // initiated the auto-fill process.