[Autofill] Include fields with autocomplete="off" in server upload requests.

BUG=177419
TEST=browser_tests
R=estade@chromium.org, vadimb@chromium.org

Review URL: https://codereview.chromium.org/407083002

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

chrome/browser/autofill/autofill_server_browsertest.cc

+// Copyright 2014 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 "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/message_loop/message_loop.h"
+#include "base/prefs/pref_service.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/autofill/personal_data_manager_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "components/autofill/core/browser/autofill_profile.h"
+#include "components/autofill/core/browser/autofill_test_utils.h"
+#include "components/autofill/core/browser/personal_data_manager.h"
+#include "components/autofill/core/browser/personal_data_manager_observer.h"
+#include "components/autofill/core/common/autofill_pref_names.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_utils.h"
+#include "net/url_request/test_url_fetcher_factory.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace autofill {
+namespace {
+
+// TODO(isherman): Similar classes are defined in a few other Autofill browser
+// tests. It would be good to factor out the shared code into a helper file.
+class WindowedPersonalDataManagerObserver : public PersonalDataManagerObserver {
+ public:
+  explicit WindowedPersonalDataManagerObserver(Profile* profile)
+      : profile_(profile),
+        message_loop_runner_(new content::MessageLoopRunner){
+    PersonalDataManagerFactory::GetForProfile(profile_)->AddObserver(this);
+  }
+  virtual ~WindowedPersonalDataManagerObserver() {}
+
+  // Waits for the PersonalDataManager's list of profiles to be updated.
+  void Wait() {
+    message_loop_runner_->Run();
+    PersonalDataManagerFactory::GetForProfile(profile_)->RemoveObserver(this);
+  }
+
+  // PersonalDataManagerObserver:
+  virtual void OnPersonalDataChanged() OVERRIDE {
+    message_loop_runner_->Quit();
+  }
+
+ private:
+  Profile* profile_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+};
+
+class WindowedNetworkObserver : public net::TestURLFetcher::DelegateForTests {
+ public:
+  explicit WindowedNetworkObserver(const std::string& expected_upload_data)
+      : factory_(new net::TestURLFetcherFactory),
+        expected_upload_data_(expected_upload_data),
+        message_loop_runner_(new content::MessageLoopRunner) {
+    factory_->SetDelegateForTests(this);
+  }
+  ~WindowedNetworkObserver() {}
+
+  // Waits for a network request with the |expected_upload_data_|.
+  void Wait() {
+    message_loop_runner_->Run();
+    factory_.reset();
+  }
+
+  // net::TestURLFetcher::DelegateForTests:
+  virtual void OnRequestStart(int fetcher_id) OVERRIDE {
+    net::TestURLFetcher* fetcher = factory_->GetFetcherByID(fetcher_id);
+    if (fetcher->upload_data() == expected_upload_data_)
+      message_loop_runner_->Quit();
+
+    // Not interested in any further status updates from this fetcher.
+    fetcher->SetDelegateForTests(NULL);
+  }
+  virtual void OnChunkUpload(int fetcher_id) OVERRIDE {}
+  virtual void OnRequestEnd(int fetcher_id) OVERRIDE {}
+
+ private:
+  // Mocks out network requests.
+  scoped_ptr<net::TestURLFetcherFactory> factory_;
+
+  const std::string expected_upload_data_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(WindowedNetworkObserver);
+};
+
+}  // namespace
+
+class AutofillServerTest : public InProcessBrowserTest  {
+ public:
+  virtual void SetUpOnMainThread() OVERRIDE {
+    // Disable interactions with the Mac Keychain.
+    PrefService* pref_service = browser()->profile()->GetPrefs();
+    test::DisableSystemServices(pref_service);
+
+    // Enable uploads, and load a new tab to force the AutofillDownloadManager
+    // to update its cached view of the prefs.
+    pref_service->SetDouble(prefs::kAutofillPositiveUploadRate, 1.0);
+    pref_service->SetDouble(prefs::kAutofillNegativeUploadRate, 1.0);
+    AddBlankTabAndShow(browser());
+  }
+};
+
+// Regression test for http://crbug.com/177419
+IN_PROC_BROWSER_TEST_F(AutofillServerTest,
+                       QueryAndUploadBothIncludeFieldsWithAutocompleteOff) {
+  // Seed some test Autofill profile data, as upload requests are only made when
+  // there is local data available to use as a baseline.
+  WindowedPersonalDataManagerObserver personal_data_observer(
+      browser()->profile());
+  PersonalDataManagerFactory::GetForProfile(browser()->profile())
+      ->AddProfile(test::GetFullProfile());
+  personal_data_observer.Wait();
+
+  // Load the test page. Expect a query request upon loading the page.
+  const char kDataURIPrefix[] = "data:text/html;charset=utf-8,";
+  const char kFormHtml[] =
+      "<form id='test_form'>"
+      "  <input id='one'>"
+      "  <input id='two' autocomplete='off'>"
+      "  <input id='three'>"
+      "  <input id='four' autocomplete='off'>"
+      "  <input type='submit'>"
+      "</form>"
+      "<script>"
+      "  document.onclick = function() {"
+      "    document.getElementById('test_form').submit();"
+      "  };"
+      "</script>";
+  const char kQueryRequest[] =
+      "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+      "<autofillquery clientversion=\"6.1.1715.1442/en (GGLL)\">"
+      "<form signature=\"15916856893790176210\">"
+      "<field signature=\"2594484045\"/>"
+      "<field signature=\"2750915947\"/>"
+      "<field signature=\"3494787134\"/>"
+      "<field signature=\"1236501728\"/>"
+      "</form>"
+      "</autofillquery>";
+  WindowedNetworkObserver query_network_observer(kQueryRequest);
+  ui_test_utils::NavigateToURL(
+      browser(), GURL(std::string(kDataURIPrefix) + kFormHtml));
+  query_network_observer.Wait();
+
+  // Submit the form, using a simulated mouse click because form submissions not
+  // triggered by user gestures are ignored. Expect an upload request upon form
+  // submission, with form fields matching those from the query request.
+  const char kUploadRequest[] =
+      "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+      "<autofillupload clientversion=\"6.1.1715.1442/en (GGLL)\""
+      " formsignature=\"15916856893790176210\""
+      " autofillused=\"false\""
+      " datapresent=\"1f7e0003780000080014\">"
+      "<field signature=\"2594484045\" autofilltype=\"2\"/>"
+      "<field signature=\"2750915947\" autofilltype=\"2\"/>"
+      "<field signature=\"3494787134\" autofilltype=\"2\"/>"
+      "<field signature=\"1236501728\" autofilltype=\"2\"/>"
+      "</autofillupload>";
+  WindowedNetworkObserver upload_network_observer(kUploadRequest);
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::SimulateMouseClick(
+      web_contents, 0, blink::WebMouseEvent::ButtonLeft);
+  upload_network_observer.Wait();
+}
+
+}  // namespace autofill

chrome/chrome_tests.gypi

@@ -882,6 +882,7 @@
         'browser/apps/window_controls_browsertest.cc',
         'browser/autocomplete/autocomplete_browsertest.cc',
         'browser/autofill/autofill_browsertest.cc',
+        'browser/autofill/autofill_server_browsertest.cc',
         'browser/autofill/content_autofill_driver_browsertest.cc',
         'browser/autofill/form_structure_browsertest.cc',
         'browser/bitmap_fetcher/bitmap_fetcher_browsertest.cc',

chrome/renderer/autofill/form_autocomplete_browsertest.cc

@@ -49,7 +49,7 @@ TEST_F(FormAutocompleteTest, NormalFormSubmit) {
   EXPECT_EQ(WebString("Deckard"), form_field.value);
 }
 
-// Tests that submitting a form that has autocomplete="off" does not generate a
+// Tests that submitting a form that has autocomplete="off" generates a
 // FormSubmitted message.
 TEST_F(FormAutocompleteTest, AutoCompleteOffFormSubmit) {
   // Load a form.
@@ -62,12 +62,25 @@ TEST_F(FormAutocompleteTest, AutoCompleteOffFormSubmit) {
   ExecuteJavaScript("document.getElementById('myForm').submit();");
   ProcessPendingMessages();
 
-  // No FormSubmitted message should have been sent.
-  EXPECT_FALSE(render_thread_->sink().GetFirstMessageMatching(
-      AutofillHostMsg_FormSubmitted::ID));
+  const IPC::Message* message = render_thread_->sink().GetFirstMessageMatching(
+      AutofillHostMsg_FormSubmitted::ID);
+  ASSERT_TRUE(message != NULL);
+
+  // The tuple also includes a timestamp, which is ignored.
+  Tuple2<FormData, base::TimeTicks> forms;
+  AutofillHostMsg_FormSubmitted::Read(message, &forms);
+  ASSERT_EQ(2U, forms.a.fields.size());
+
+  FormFieldData& form_field = forms.a.fields[0];
+  EXPECT_EQ(WebString("fname"), form_field.name);
+  EXPECT_EQ(WebString("Rick"), form_field.value);
+
+  form_field = forms.a.fields[1];
+  EXPECT_EQ(WebString("lname"), form_field.name);
+  EXPECT_EQ(WebString("Deckard"), form_field.value);
 }
 
-// Tests that fields with autocomplete off are not submitted.
+// Tests that fields with autocomplete off are submitted.
 TEST_F(FormAutocompleteTest, AutoCompleteOffInputSubmit) {
   // Load a form.
   LoadHTML("<html><form id='myForm'>"
@@ -79,7 +92,6 @@ TEST_F(FormAutocompleteTest, AutoCompleteOffInputSubmit) {
   ExecuteJavaScript("document.getElementById('myForm').submit();");
   ProcessPendingMessages();
 
-  // No FormSubmitted message should have been sent.
   const IPC::Message* message = render_thread_->sink().GetFirstMessageMatching(
       AutofillHostMsg_FormSubmitted::ID);
   ASSERT_TRUE(message != NULL);
@@ -87,16 +99,20 @@ TEST_F(FormAutocompleteTest, AutoCompleteOffInputSubmit) {
   // The tuple also includes a timestamp, which is ignored.
   Tuple2<FormData, base::TimeTicks> forms;
   AutofillHostMsg_FormSubmitted::Read(message, &forms);
-  ASSERT_EQ(1U, forms.a.fields.size());
+  ASSERT_EQ(2U, forms.a.fields.size());
 
   FormFieldData& form_field = forms.a.fields[0];
   EXPECT_EQ(WebString("fname"), form_field.name);
   EXPECT_EQ(WebString("Rick"), form_field.value);
+
+  form_field = forms.a.fields[1];
+  EXPECT_EQ(WebString("lname"), form_field.name);
+  EXPECT_EQ(WebString("Deckard"), form_field.value);
 }
 
 // Tests that submitting a form that has been dynamically set as autocomplete
-// off does not generate a FormSubmitted message.
-// http://crbug.com/36520
+// off generates a FormSubmitted message.
+// Note: We previously did the opposite, for bug http://crbug.com/36520
 TEST_F(FormAutocompleteTest, DynamicAutoCompleteOffFormSubmit) {
   LoadHTML("<html><form id='myForm'><input name='fname' value='Rick'/>"
            "<input name='lname' value='Deckard'/></form></html>");
@@ -117,9 +133,22 @@ TEST_F(FormAutocompleteTest, DynamicAutoCompleteOffFormSubmit) {
   ExecuteJavaScript("document.getElementById('myForm').submit();");
   ProcessPendingMessages();
 
-  // No FormSubmitted message should have been sent.
-  EXPECT_FALSE(render_thread_->sink().GetFirstMessageMatching(
-      AutofillHostMsg_FormSubmitted::ID));
+  const IPC::Message* message = render_thread_->sink().GetFirstMessageMatching(
+      AutofillHostMsg_FormSubmitted::ID);
+  ASSERT_TRUE(message != NULL);
+
+  // The tuple also includes a timestamp, which is ignored.
+  Tuple2<FormData, base::TimeTicks> forms;
+  AutofillHostMsg_FormSubmitted::Read(message, &forms);
+  ASSERT_EQ(2U, forms.a.fields.size());
+
+  FormFieldData& form_field = forms.a.fields[0];
+  EXPECT_EQ(WebString("fname"), form_field.name);
+  EXPECT_EQ(WebString("Rick"), form_field.value);
+
+  form_field = forms.a.fields[1];
+  EXPECT_EQ(WebString("lname"), form_field.name);
+  EXPECT_EQ(WebString("Deckard"), form_field.value);
 }
 
 }  // namespace autofill

components/autofill/content/renderer/autofill_agent.cc

@@ -206,7 +206,7 @@ void AutofillAgent::WillSubmitForm(WebLocalFrame* frame,
   FormData form_data;
   if (WebFormElementToFormData(form,
                                WebFormControlElement(),
-                               REQUIRE_AUTOCOMPLETE,
+                               REQUIRE_NONE,
                                static_cast<ExtractMask>(
                                    EXTRACT_VALUE | EXTRACT_OPTION_TEXT),
                                &form_data,