AutoFill: Fix a bug that caused AutoFill to not fill the field the user is

typing in. This change also conveniently removes filling the default profile,
since this feature has been removed from AutoFill.

BUG=46219
TEST=FormManagerTest.FillFormNonEmptyField

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

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

5 files changed, 123 insertions, 37 deletions

diff --git a/chrome/renderer/form_manager.cc b/chrome/renderer/form_manager.cc
index 6cc4e17..3868582 100644
--- a/chrome/renderer/form_manager.cc
+++ b/chrome/renderer/form_manager.cc
@@ -514,7 +514,7 @@ bool FormManager::FindFormWithFormControlElement(
   return false;
 }
 
-bool FormManager::FillForm(const FormData& form) {
+bool FormManager::FillForm(const FormData& form, const WebKit::WebNode& node) {
   FormElement* form_element = NULL;
   if (!FindCachedFormElement(form, &form_element))
     return false;
@@ -522,6 +522,7 @@ bool FormManager::FillForm(const FormData& form) {
   RequirementsMask requirements = static_cast<RequirementsMask>(
       REQUIRE_AUTOCOMPLETE | REQUIRE_ENABLED | REQUIRE_EMPTY);
   ForEachMatchingFormField(form_element,
+                           node,
                            requirements,
                            form,
                            NewCallback(this, &FormManager::FillFormField));
@@ -537,6 +538,7 @@ bool FormManager::PreviewForm(const FormData& form) {
   RequirementsMask requirements = static_cast<RequirementsMask>(
       REQUIRE_AUTOCOMPLETE | REQUIRE_ENABLED | REQUIRE_EMPTY);
   ForEachMatchingFormField(form_element,
+                           WebNode(),
                            requirements,
                            form,
                            NewCallback(this, &FormManager::PreviewFormField));
@@ -567,13 +569,6 @@ void FormManager::ClearPreviewedForm(const FormData& form) {
   }
 }
 
-void FormManager::FillForms(const std::vector<FormData>& forms) {
-  for (std::vector<FormData>::const_iterator iter = forms.begin();
-       iter != forms.end(); ++iter) {
-    FillForm(*iter);
-  }
-}
-
 void FormManager::Reset() {
   for (WebFrameFormElementMap::iterator iter = form_elements_map_.begin();
        iter != form_elements_map_.end(); ++iter) {
@@ -682,6 +677,7 @@ bool FormManager::FindCachedFormElement(const FormData& form,
 }
 
 void FormManager::ForEachMatchingFormField(FormElement* form,
+                                           const WebKit::WebNode& node,
                                            RequirementsMask requirements,
                                            const FormData& data,
                                            Callback* callback) {
@@ -723,7 +719,12 @@ void FormManager::ForEachMatchingFormField(FormElement* form,
       if (requirements & REQUIRE_AUTOCOMPLETE && !input_element.autoComplete())
         continue;
 
-      if (requirements & REQUIRE_EMPTY && !input_element.value().isEmpty())
+      // Don't require the node that initiated the auto-fill process to be
+      // empty.  The user is typing in this field and we should complete the
+      // value when the user selects a value to fill out.
+      if (requirements & REQUIRE_EMPTY &&
+          input_element != node &&
+          !input_element.value().isEmpty())
         continue;
     }
 
diff --git a/chrome/renderer/form_manager.h b/chrome/renderer/form_manager.h
index ad4dee7..4d6aa5c 100644
--- a/chrome/renderer/form_manager.h
+++ b/chrome/renderer/form_manager.h
@@ -87,10 +87,11 @@ class FormManager {
 
   // Fills the form represented by |form|.  |form| should have the name set to
   // the name of the form to fill out, and the number of elements and values
-  // must match the number of stored elements in the form.
+  // must match the number of stored elements in the form. |node| is the form
+  // control element that initiated the auto-fill process.
   // TODO(jhawkins): Is matching on name alone good enough?  It's possible to
   // store multiple forms with the same names from different frames.
-  bool FillForm(const webkit_glue::FormData& form);
+  bool FillForm(const webkit_glue::FormData& form, const WebKit::WebNode& node);
 
   // Previews the form represented by |form|.  Same conditions as FillForm.
   bool PreviewForm(const webkit_glue::FormData& form);
@@ -99,9 +100,6 @@ class FormManager {
   // in |form| that have been previewed.
   void ClearPreviewedForm(const webkit_glue::FormData& form);
 
-  // Fills all of the forms in the cache with form data from |forms|.
-  void FillForms(const std::vector<webkit_glue::FormData>& forms);
-
   // Resets the stored set of forms.
   void Reset();
 
@@ -144,9 +142,11 @@ class FormManager {
 
   // For each field in |data| that matches the corresponding field in |form|
   // and meets the |requirements|, |callback| is called with the actual
-  // WebFormControlElement and the FormField data from |form|. This method owns
-  // |callback|.
+  // WebFormControlElement and the FormField data from |form|. The field that
+  // matches |node| is not required to be empty if |requirements| includes
+  // REQUIRE_EMPTY.  This method owns |callback|.
   void ForEachMatchingFormField(FormElement* form,
+                                const WebKit::WebNode& node,
                                 RequirementsMask requirements,
                                 const webkit_glue::FormData& data,
                                 Callback* callback);
diff --git a/chrome/renderer/form_manager_unittest.cc b/chrome/renderer/form_manager_unittest.cc
index 4344499..1731f47 100644
--- a/chrome/renderer/form_manager_unittest.cc
+++ b/chrome/renderer/form_manager_unittest.cc
@@ -9,6 +9,7 @@
 #include "third_party/WebKit/WebKit/chromium/public/WebElement.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebFormElement.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebInputElement.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebNode.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebString.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebVector.h"
 #include "webkit/glue/form_data.h"
@@ -18,6 +19,7 @@ using WebKit::WebElement;
 using WebKit::WebFormElement;
 using WebKit::WebFrame;
 using WebKit::WebInputElement;
+using WebKit::WebNode;
 using WebKit::WebString;
 using WebKit::WebVector;
 
@@ -420,7 +422,7 @@ TEST_F(FormManagerTest, FillForm) {
   form.fields[3].set_value(ASCIIToUTF16("Beta"));
   form.fields[4].set_value(ASCIIToUTF16("Gamma"));
   form.fields[5].set_value(ASCIIToUTF16("Delta"));
-  EXPECT_TRUE(form_manager.FillForm(form));
+  EXPECT_TRUE(form_manager.FillForm(form, WebNode()));
 
   // Verify the filled elements.
   WebDocument document = web_frame->document();
@@ -1271,7 +1273,7 @@ TEST_F(FormManagerTest, FillFormMaxLength) {
   // Fill the form.
   form.fields[0].set_value(ASCIIToUTF16("Brother"));
   form.fields[1].set_value(ASCIIToUTF16("Jonathan"));
-  EXPECT_TRUE(form_manager.FillForm(form));
+  EXPECT_TRUE(form_manager.FillForm(form, WebNode()));
 
   // Find the newly-filled form that contains the input element.
   FormData form2;
@@ -1359,7 +1361,7 @@ TEST_F(FormManagerTest, FillFormNegativeMaxLength) {
   // Fill the form.
   form.fields[0].set_value(ASCIIToUTF16("Brother"));
   form.fields[1].set_value(ASCIIToUTF16("Jonathan"));
-  EXPECT_TRUE(form_manager.FillForm(form));
+  EXPECT_TRUE(form_manager.FillForm(form, WebNode()));
 
   // Find the newly-filled form that contains the input element.
   FormData form2;
@@ -1459,7 +1461,7 @@ TEST_F(FormManagerTest, FillFormMoreFormDataFields) {
   form->fields[4].set_value(ASCIIToUTF16("Beta"));
   form->fields[5].set_value(ASCIIToUTF16("Jonathan"));
   form->fields[6].set_value(ASCIIToUTF16("Omega"));
-  EXPECT_TRUE(form_manager.FillForm(*form));
+  EXPECT_TRUE(form_manager.FillForm(*form, WebNode()));
 
   // Get the input element we want to find.
   WebElement element = web_frame->document().getElementById("firstname");
@@ -1537,7 +1539,7 @@ TEST_F(FormManagerTest, FillFormFewerFormDataFields) {
   form->fields[0].set_value(ASCIIToUTF16("Brother"));
   form->fields[1].set_value(ASCIIToUTF16("Joseph"));
   form->fields[2].set_value(ASCIIToUTF16("Jonathan"));
-  EXPECT_TRUE(form_manager.FillForm(*form));
+  EXPECT_TRUE(form_manager.FillForm(*form, WebNode()));
 
   // Get the input element we want to find.
   WebElement element = web_frame->document().getElementById("firstname");
@@ -1632,7 +1634,7 @@ TEST_F(FormManagerTest, FillFormChangedFormDataFields) {
   form->fields[1].set_label(ASCIIToUTF16("bogus"));
   form->fields[1].set_name(ASCIIToUTF16("bogus"));
 
-  EXPECT_TRUE(form_manager.FillForm(*form));
+  EXPECT_TRUE(form_manager.FillForm(*form, WebNode()));
 
   // Get the input element we want to find.
   WebElement element = web_frame->document().getElementById("firstname");
@@ -1704,7 +1706,7 @@ TEST_F(FormManagerTest, FillFormExtraFieldInCache) {
   form->fields[0].set_value(ASCIIToUTF16("Brother"));
   form->fields[1].set_value(ASCIIToUTF16("Joseph"));
   form->fields[2].set_value(ASCIIToUTF16("Jonathan"));
-  EXPECT_TRUE(form_manager.FillForm(*form));
+  EXPECT_TRUE(form_manager.FillForm(*form, WebNode()));
 
   // Get the input element we want to find.
   WebElement element = web_frame->document().getElementById("firstname");
@@ -1801,7 +1803,7 @@ TEST_F(FormManagerTest, FillFormEmptyName) {
   // Fill the form.
   form.fields[0].set_value(ASCIIToUTF16("Wyatt"));
   form.fields[1].set_value(ASCIIToUTF16("Earp"));
-  EXPECT_TRUE(form_manager.FillForm(form));
+  EXPECT_TRUE(form_manager.FillForm(form, WebNode()));
 
   // Find the newly-filled form that contains the input element.
   FormData form2;
@@ -1893,7 +1895,7 @@ TEST_F(FormManagerTest, FillFormEmptyFormNames) {
   // Fill the form.
   form.fields[0].set_value(ASCIIToUTF16("Red"));
   form.fields[1].set_value(ASCIIToUTF16("Yellow"));
-  EXPECT_TRUE(form_manager.FillForm(form));
+  EXPECT_TRUE(form_manager.FillForm(form, WebNode()));
 
   // Find the newly-filled form that contains the input element.
   FormData form2;
@@ -2070,4 +2072,96 @@ TEST_F(FormManagerTest, SizeFields) {
             fields[6]);
 }
 
+// This test re-creates the experience of typing in a field then selecting a
+// profile from the AutoFill suggestions popup.  The field that is being typed
+// into should be filled even though it's not technically empty.
+TEST_F(FormManagerTest, FillFormNonEmptyField) {
+  LoadHTML("<FORM name=\"TestForm\" action=\"http://buh.com\" method=\"post\">"
+           "  <INPUT type=\"text\" id=\"firstname\"/>"
+           "  <INPUT type=\"text\" id=\"lastname\"/>"
+           "  <INPUT type=\"submit\" value=\"Send\"/>"
+           "</FORM>");
+
+  WebFrame* web_frame = GetMainFrame();
+  ASSERT_NE(static_cast<WebFrame*>(NULL), web_frame);
+
+  FormManager form_manager;
+  form_manager.ExtractForms(web_frame);
+
+  // Verify that we have the form.
+  std::vector<FormData> forms;
+  form_manager.GetForms(FormManager::REQUIRE_NONE, &forms);
+  ASSERT_EQ(1U, forms.size());
+
+  // Get the input element we want to find.
+  WebElement element = web_frame->document().getElementById("firstname");
+  WebInputElement input_element = element.to<WebInputElement>();
+
+  // Simulate typing by modifying the field value.
+  input_element.setValue(ASCIIToUTF16("Wy"));
+
+  // Find the form that contains the input element.
+  FormData form;
+  EXPECT_TRUE(form_manager.FindFormWithFormControlElement(
+      input_element, FormManager::REQUIRE_NONE, &form));
+  EXPECT_EQ(ASCIIToUTF16("TestForm"), form.name);
+  EXPECT_EQ(GURL(web_frame->url()), form.origin);
+  EXPECT_EQ(GURL("http://buh.com"), form.action);
+
+  const std::vector<FormField>& fields = form.fields;
+  ASSERT_EQ(3U, fields.size());
+  EXPECT_EQ(FormField(string16(),
+                      ASCIIToUTF16("firstname"),
+                      string16(),
+                      ASCIIToUTF16("text"),
+                      20),
+            fields[0]);
+  EXPECT_EQ(FormField(string16(),
+                      ASCIIToUTF16("lastname"),
+                      string16(),
+                      ASCIIToUTF16("text"),
+                      20),
+            fields[1]);
+  EXPECT_EQ(FormField(string16(),
+                      string16(),
+                      ASCIIToUTF16("Send"),
+                      ASCIIToUTF16("submit"),
+                      0),
+            fields[2]);
+
+  // Fill the form.
+  form.fields[0].set_value(ASCIIToUTF16("Wyatt"));
+  form.fields[1].set_value(ASCIIToUTF16("Earp"));
+  EXPECT_TRUE(form_manager.FillForm(form, input_element));
+
+  // Find the newly-filled form that contains the input element.
+  FormData form2;
+  EXPECT_TRUE(form_manager.FindFormWithFormControlElement(
+      input_element, FormManager::REQUIRE_NONE, &form2));
+  EXPECT_EQ(ASCIIToUTF16("TestForm"), form2.name);
+  EXPECT_EQ(GURL(web_frame->url()), form2.origin);
+  EXPECT_EQ(GURL("http://buh.com"), form2.action);
+
+  const std::vector<FormField>& fields2 = form2.fields;
+  ASSERT_EQ(3U, fields2.size());
+  EXPECT_EQ(FormField(string16(),
+                      ASCIIToUTF16("firstname"),
+                      ASCIIToUTF16("Wyatt"),
+                      ASCIIToUTF16("text"),
+                      20),
+                      fields2[0]);
+  EXPECT_EQ(FormField(string16(),
+                      ASCIIToUTF16("lastname"),
+                      ASCIIToUTF16("Earp"),
+                      ASCIIToUTF16("text"),
+                      20),
+                      fields2[1]);
+  EXPECT_EQ(FormField(string16(),
+                      string16(),
+                      ASCIIToUTF16("Send"),
+                      ASCIIToUTF16("submit"),
+                      0),
+                      fields2[2]);
+}
+
 }  // namespace
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index c9e89c52..74dd7ac 100755
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -682,7 +682,6 @@ void RenderView::OnMessageReceived(const IPC::Message& message) {
                         OnDisassociateFromPopupCount)
     IPC_MESSAGE_HANDLER(ViewMsg_AutoFillSuggestionsReturned,
                         OnAutoFillSuggestionsReturned)
-    IPC_MESSAGE_HANDLER(ViewMsg_AutoFillForms, OnAutoFillForms)
     IPC_MESSAGE_HANDLER(ViewMsg_AutocompleteSuggestionsReturned,
                         OnAutocompleteSuggestionsReturned)
     IPC_MESSAGE_HANDLER(ViewMsg_AutoFillFormDataFilled,
@@ -1515,12 +1514,6 @@ void RenderView::OnAutoFillSuggestionsReturned(
     webview()->applyAutoFillSuggestions(
         autofill_query_node_, values, labels, default_suggestion_index);
   }
-  autofill_query_node_.reset();
-}
-
-void RenderView::OnAutoFillForms(
-    const std::vector<webkit_glue::FormData>& forms) {
-  form_manager_.FillForms(forms);
 }
 
 void RenderView::OnAutocompleteSuggestionsReturned(
@@ -1531,7 +1524,6 @@ void RenderView::OnAutocompleteSuggestionsReturned(
     webview()->applyAutocompleteSuggestions(
         autofill_query_node_, suggestions, default_suggestion_index);
   }
-  autofill_query_node_.reset();
 }
 
 void RenderView::OnAutoFillFormDataFilled(int query_id,
@@ -1542,7 +1534,7 @@ void RenderView::OnAutoFillFormDataFilled(int query_id,
   DCHECK_NE(AUTOFILL_NONE, autofill_action_);
 
   if (autofill_action_ == AUTOFILL_FILL)
-    form_manager_.FillForm(form);
+    form_manager_.FillForm(form, autofill_query_node_);
   else if (autofill_action_ == AUTOFILL_PREVIEW)
     form_manager_.PreviewForm(form);
 
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index 4a06534..6b6016c 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -683,7 +683,6 @@ class RenderView : public RenderWidget,
       int default_suggestions_index);
   void OnAutoFillFormDataFilled(int query_id,
                                 const webkit_glue::FormData& form);
-  void OnAutoFillForms(const std::vector<webkit_glue::FormData>& forms);
   void OnAutoFillSuggestionsReturned(
       int query_id,
       const std::vector<string16>& values,
@@ -1136,12 +1135,12 @@ class RenderView : public RenderWidget,
 
   // Autofill ------------------------------------------------------------------
 
-  // The id of the last request sent for form field autofill.  Used to ignore
+  // The id of the last request sent for form field AutoFill.  Used to ignore
   // out of date responses.
   int autofill_query_id_;
 
   // The id of the node corresponding to the last request sent for form field
-  // autofill.
+  // AutoFill.
   WebKit::WebNode autofill_query_node_;
 
   // The action to take when receiving AutoFill data from the AutoFillManager.