diff options
author | ramankk@chromium.org <ramankk@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-12-19 10:08:45 +0000 |
---|---|---|
committer | ramankk@chromium.org <ramankk@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-12-19 10:08:45 +0000 |
commit | b32c7e7452dd53a9907083ae6a09523400197419 (patch) | |
tree | 41d272ec478ea77374ce7d06ae088c30f236383f /chrome/browser/autofill | |
parent | 8f3119100192dff88fb3e4e563a4852349c00e7b (diff) | |
download | chromium_src-b32c7e7452dd53a9907083ae6a09523400197419.zip chromium_src-b32c7e7452dd53a9907083ae6a09523400197419.tar.gz chromium_src-b32c7e7452dd53a9907083ae6a09523400197419.tar.bz2 |
Add support for autofilling radio buttons and checkboxes.
Autofill server is equiped with provision to say a field type as
FIELD_WITH_DEFAULT_VALUE and specify what value to use by default it
client wants to fill it. For radio buttons, if the default value
specified by the autofill server is same as its value, Chrome checks that
input element.
all changes are behind switch.
BUG=157636
Review URL: https://chromiumcodereview.appspot.com/11415221
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@173889 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/autofill')
-rw-r--r-- | chrome/browser/autofill/autofill_field.cc | 3 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_field.h | 6 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_manager.cc | 20 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_manager_unittest.cc | 54 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_server_field_info.h | 20 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_xml_parser.cc | 51 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_xml_parser.h | 7 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_xml_parser_unittest.cc | 110 | ||||
-rw-r--r-- | chrome/browser/autofill/field_types.h | 6 | ||||
-rw-r--r-- | chrome/browser/autofill/form_field.cc | 12 | ||||
-rw-r--r-- | chrome/browser/autofill/form_field_unittest.cc | 30 | ||||
-rw-r--r-- | chrome/browser/autofill/form_structure.cc | 57 | ||||
-rw-r--r-- | chrome/browser/autofill/form_structure.h | 4 | ||||
-rw-r--r-- | chrome/browser/autofill/form_structure_unittest.cc | 61 |
14 files changed, 356 insertions, 85 deletions
diff --git a/chrome/browser/autofill/autofill_field.cc b/chrome/browser/autofill/autofill_field.cc index 7058c53..974af94 100644 --- a/chrome/browser/autofill/autofill_field.cc +++ b/chrome/browser/autofill/autofill_field.cc @@ -43,7 +43,8 @@ AutofillField::AutofillField(const FormFieldData& field, AutofillField::~AutofillField() {} void AutofillField::set_heuristic_type(AutofillFieldType type) { - if (type >= 0 && type < MAX_VALID_FIELD_TYPE) { + if (type >= 0 && type < MAX_VALID_FIELD_TYPE && + type != FIELD_WITH_DEFAULT_VALUE) { heuristic_type_ = type; } else { NOTREACHED(); diff --git a/chrome/browser/autofill/autofill_field.h b/chrome/browser/autofill/autofill_field.h index 0b21098..e688fad 100644 --- a/chrome/browser/autofill/autofill_field.h +++ b/chrome/browser/autofill/autofill_field.h @@ -56,6 +56,9 @@ class AutofillField : public FormFieldData { // field). bool IsFieldFillable() const; + void set_default_value(const std::string& value) { default_value_ = value; } + const std::string& default_value() const { return default_value_; } + private: // The unique name of this field, generated by Autofill. string16 unique_name_; @@ -76,6 +79,9 @@ class AutofillField : public FormFieldData { // Used to track whether this field is a phone prefix or suffix. PhonePart phone_part_; + // The default value returned by the Autofill server. + std::string default_value_; + DISALLOW_COPY_AND_ASSIGN(AutofillField); }; diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc index efb9b33..82f0cd9 100644 --- a/chrome/browser/autofill/autofill_manager.cc +++ b/chrome/browser/autofill/autofill_manager.cc @@ -654,6 +654,26 @@ void AutofillManager::OnFillAutofillFormData(int query_id, // Mark the cached field as autofilled, so that we can detect when a user // edits an autofilled field (for metrics). form_structure->field(i)->is_autofilled = true; + } else if (cached_field->type() == FIELD_WITH_DEFAULT_VALUE && + cached_field->is_checkable) { + // For a form with radio buttons, like: + // <form> + // <input type="radio" name="sex" value="male">Male<br> + // <input type="radio" name="sex" value="female">Female + // </form> + // If the default value specified at the server is "female", then + // Autofill server responds back with following field mappings + // (fieldtype: FIELD_WITH_DEFAULT_VALUE, value: "female") + // (fieldtype: FIELD_WITH_DEFAULT_VALUE, value: "female") + // Note that, the field mapping is repeated twice to respond to both the + // input elements with the same name/signature in the form. + string16 default_value = UTF8ToUTF16(cached_field->default_value()); + // Mark the field checked if server says the default value of the field + // to be this field's value. + result.fields[i].is_checked = (default_value == result.fields[i].value); + // Mark the cached field as autofilled, so that we can detect when a user + // edits an autofilled field (for metrics). + form_structure->field(i)->is_autofilled = true; } } diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc index 111d2cb..ff0126f 100644 --- a/chrome/browser/autofill/autofill_manager_unittest.cc +++ b/chrome/browser/autofill/autofill_manager_unittest.cc @@ -672,6 +672,10 @@ class AutofillManagerTest : public ChromeRenderViewHostTestHarness { autofill_manager_->OnFormsSeen(forms, base::TimeTicks()); } + void LoadServerPredictions(const std::string& response_xml) { + autofill_manager_->OnLoadedServerPredictions(response_xml); + } + void FormSubmitted(const FormData& form) { if (autofill_manager_->OnFormSubmitted(form, base::TimeTicks::Now())) autofill_manager_->WaitForAsyncFormSubmit(); @@ -1821,6 +1825,56 @@ TEST_F(AutofillManagerTest, FillCreditCardForm) { ExpectFilledCreditCardFormElvis(page_id, results, kDefaultPageID, false); } +TEST_F(AutofillManagerTest, FillCheckableElements) { + FormData form; + CreateTestAddressFormData(&form); + + // Add a checkbox field. + FormFieldData field; + autofill_test::CreateTestFormField( + "Checkbox", "checkbx", "fill-me", "checkbox", &field); + field.is_checkable = true; + form.fields.push_back(field); + autofill_test::CreateTestFormField( + "Checkbox", "checkbx", "fill-me", "checkbox", &field); + field.is_checkable = true; + form.fields.push_back(field); + + std::vector<FormData> forms(1, form); + FormsSeen(forms); + + // Replicate server response with XML. + const std::string response_xml = + "<autofillqueryresponse>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"0\"/>" + "<field autofilltype=\"61\" defaultvalue=\"fill-me\"/>" + "<field autofilltype=\"61\" defaultvalue=\"dont-fill-me\"/>" + "</autofillqueryresponse>"; + LoadServerPredictions(response_xml); + + GUIDPair guid("00000000-0000-0000-0000-000000000001", 0); + GUIDPair empty(std::string(), 0); + FillAutofillFormData(kDefaultPageID, form, form.fields[0], + PackGUIDs(empty, guid)); + int page_id = 0; + FormData results; + EXPECT_TRUE(GetAutofillFormDataFilledMessage(&page_id, &results)); + // Second to last field should be checked. + EXPECT_TRUE(results.fields[results.fields.size()-2].is_checked); + // Last field shouldn't be checked as values are not same. + EXPECT_FALSE(results.fields[results.fields.size()-1].is_checked); +} + // Test that we correctly fill a credit card form with month input type. // 1. year empty, month empty TEST_F(AutofillManagerTest, FillCreditCardFormNoYearNoMonth) { diff --git a/chrome/browser/autofill/autofill_server_field_info.h b/chrome/browser/autofill/autofill_server_field_info.h new file mode 100644 index 0000000..e07ec26 --- /dev/null +++ b/chrome/browser/autofill/autofill_server_field_info.h @@ -0,0 +1,20 @@ +// Copyright (c) 2012 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. + +#ifndef CHROME_BROWSER_AUTOFILL_AUTOFILL_SERVER_FIELD_INFO_H_ +#define CHROME_BROWSER_AUTOFILL_AUTOFILL_SERVER_FIELD_INFO_H_ + +#include <string> + +#include "chrome/browser/autofill/field_types.h" + +struct AutofillServerFieldInfo { + // The predicted type returned by the Autofill server for this field. + AutofillFieldType field_type; + // Default value to be used for the field (only applies to + // FIELD_WITH_DEFAULT_TYPE field type) + std::string default_value; +}; + +#endif // CHROME_BROWSER_AUTOFILL_AUTOFILL_SERVER_FIELD_INFO_H_ diff --git a/chrome/browser/autofill/autofill_xml_parser.cc b/chrome/browser/autofill/autofill_xml_parser.cc index 92a1ede..1fdda9e 100644 --- a/chrome/browser/autofill/autofill_xml_parser.cc +++ b/chrome/browser/autofill/autofill_xml_parser.cc @@ -8,6 +8,7 @@ #include <string.h> #include "base/logging.h" +#include "chrome/browser/autofill/autofill_server_field_info.h" #include "third_party/libjingle/source/talk/xmllite/qname.h" AutofillXmlParser::AutofillXmlParser() @@ -28,10 +29,10 @@ void AutofillXmlParser::Error(buzz::XmlParseContext* context, } AutofillQueryXmlParser::AutofillQueryXmlParser( - std::vector<AutofillFieldType>* field_types, + std::vector<AutofillServerFieldInfo>* field_infos, UploadRequired* upload_required, std::string* experiment_id) - : field_types_(field_types), + : field_infos_(field_infos), upload_required_(upload_required), experiment_id_(experiment_id) { DCHECK(upload_required_); @@ -52,22 +53,21 @@ void AutofillQueryXmlParser::StartElement(buzz::XmlParseContext* context, // |attrs| is a NULL-terminated list of (attribute, value) pairs. while (*attrs) { - buzz::QName attribute_qname = context->ResolveQName(attrs[0], true); + buzz::QName attribute_qname = context->ResolveQName(*attrs, true); + ++attrs; const std::string& attribute_name = attribute_qname.LocalPart(); if (attribute_name.compare("uploadrequired") == 0) { - if (strcmp(attrs[1], "true") == 0) + if (strcmp(*attrs, "true") == 0) *upload_required_ = UPLOAD_REQUIRED; - else if (strcmp(attrs[1], "false") == 0) + else if (strcmp(*attrs, "false") == 0) *upload_required_ = UPLOAD_NOT_REQUIRED; } else if (attribute_name.compare("experimentid") == 0) { - *experiment_id_ = attrs[1]; + *experiment_id_ = *attrs; } - - // Advance to the next (attribute, value) pair. - attrs += 2; + ++attrs; } } else if (element.compare("field") == 0) { - if (!attrs[0]) { + if (!*attrs) { // Missing the "autofilltype" attribute, abort. context->RaiseError(XML_ERROR_ABORTED); return; @@ -75,20 +75,29 @@ void AutofillQueryXmlParser::StartElement(buzz::XmlParseContext* context, // Determine the field type from the attribute value. There should be one // attribute (autofilltype) with an integer value. - AutofillFieldType field_type = UNKNOWN_TYPE; - buzz::QName attribute_qname = context->ResolveQName(attrs[0], true); - const std::string& attribute_name = attribute_qname.LocalPart(); - - if (attribute_name.compare("autofilltype") == 0) { - int value = GetIntValue(context, attrs[1]); - field_type = static_cast<AutofillFieldType>(value); - if (field_type < 0 || field_type > MAX_VALID_FIELD_TYPE) { - field_type = NO_SERVER_DATA; + AutofillServerFieldInfo field_info; + field_info.field_type = UNKNOWN_TYPE; + + // |attrs| is a NULL-terminated list of (attribute, value) pairs. + while (*attrs) { + buzz::QName attribute_qname = context->ResolveQName(*attrs, true); + ++attrs; + const std::string& attribute_name = attribute_qname.LocalPart(); + if (attribute_name.compare("autofilltype") == 0) { + int value = GetIntValue(context, *attrs); + if (value >= 0 && value < MAX_VALID_FIELD_TYPE) + field_info.field_type = static_cast<AutofillFieldType>(value); + else + field_info.field_type = NO_SERVER_DATA; + } else if (field_info.field_type == FIELD_WITH_DEFAULT_VALUE && + attribute_name.compare("defaultvalue") == 0) { + field_info.default_value = *attrs; } + ++attrs; } - // Record this field type. - field_types_->push_back(field_type); + // Record this field type, default value pair. + field_infos_->push_back(field_info); } } diff --git a/chrome/browser/autofill/autofill_xml_parser.h b/chrome/browser/autofill/autofill_xml_parser.h index da3d5bf..5ae9774 100644 --- a/chrome/browser/autofill/autofill_xml_parser.h +++ b/chrome/browser/autofill/autofill_xml_parser.h @@ -10,6 +10,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" +#include "chrome/browser/autofill/autofill_server_field_info.h" #include "chrome/browser/autofill/field_types.h" #include "chrome/browser/autofill/form_structure.h" #include "third_party/libjingle/source/talk/xmllite/xmlparser.h" @@ -66,7 +67,7 @@ class AutofillXmlParser : public buzz::XmlParseHandler { // unknown, other types are documented in chrome/browser/autofill/field_types.h. class AutofillQueryXmlParser : public AutofillXmlParser { public: - AutofillQueryXmlParser(std::vector<AutofillFieldType>* field_types, + AutofillQueryXmlParser(std::vector<AutofillServerFieldInfo>* field_infos, UploadRequired* upload_required, std::string* experiment_id); @@ -85,8 +86,8 @@ class AutofillQueryXmlParser : public AutofillXmlParser { // |value| is the string to convert. int GetIntValue(buzz::XmlParseContext* context, const char* attribute); - // The parsed field types. - std::vector<AutofillFieldType>* field_types_; + // The parsed <field type, default value> pairs. + std::vector<AutofillServerFieldInfo>* field_infos_; // A flag indicating whether the client should upload Autofill data when this // form is submitted. diff --git a/chrome/browser/autofill/autofill_xml_parser_unittest.cc b/chrome/browser/autofill/autofill_xml_parser_unittest.cc index a0a1024..2363862 100644 --- a/chrome/browser/autofill/autofill_xml_parser_unittest.cc +++ b/chrome/browser/autofill/autofill_xml_parser_unittest.cc @@ -6,6 +6,7 @@ #include <vector> #include "base/memory/scoped_ptr.h" +#include "base/string_number_conversions.h" #include "chrome/browser/autofill/autofill_xml_parser.h" #include "chrome/browser/autofill/field_types.h" #include "testing/gtest/include/gtest/gtest.h" @@ -20,31 +21,36 @@ TEST(AutofillQueryXmlParserTest, BasicQuery) { "<field autofilltype=\"1\" />" "<field autofilltype=\"3\" />" "<field autofilltype=\"2\" />" + "<field autofilltype=\"61\" defaultvalue=\"default\"/>" "</autofillqueryresponse>"; - // Create a vector of AutofillFieldTypes, to assign the parsed field types to. - std::vector<AutofillFieldType> field_types; + // Create a vector of AutofillServerFieldInfos, to assign the parsed field + // types to. + std::vector<AutofillServerFieldInfo> field_infos; UploadRequired upload_required = USE_UPLOAD_RATES; std::string experiment_id; // Create a parser. - AutofillQueryXmlParser parse_handler(&field_types, &upload_required, + AutofillQueryXmlParser parse_handler(&field_infos, &upload_required, &experiment_id); buzz::XmlParser parser(&parse_handler); parser.Parse(xml.c_str(), xml.length(), true); EXPECT_TRUE(parse_handler.succeeded()); EXPECT_EQ(USE_UPLOAD_RATES, upload_required); - ASSERT_EQ(4U, field_types.size()); - EXPECT_EQ(NO_SERVER_DATA, field_types[0]); - EXPECT_EQ(UNKNOWN_TYPE, field_types[1]); - EXPECT_EQ(NAME_FIRST, field_types[2]); - EXPECT_EQ(EMPTY_TYPE, field_types[3]); + ASSERT_EQ(5U, field_infos.size()); + EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type); + EXPECT_EQ(UNKNOWN_TYPE, field_infos[1].field_type); + EXPECT_EQ(NAME_FIRST, field_infos[2].field_type); + EXPECT_EQ(EMPTY_TYPE, field_infos[3].field_type); + EXPECT_EQ("", field_infos[3].default_value); + EXPECT_EQ(FIELD_WITH_DEFAULT_VALUE, field_infos[4].field_type); + EXPECT_EQ("default", field_infos[4].default_value); EXPECT_EQ(std::string(), experiment_id); } // Test parsing the upload required attribute. TEST(AutofillQueryXmlParserTest, TestUploadRequired) { - std::vector<AutofillFieldType> field_types; + std::vector<AutofillServerFieldInfo> field_infos; UploadRequired upload_required = USE_UPLOAD_RATES; std::string experiment_id; @@ -53,50 +59,50 @@ TEST(AutofillQueryXmlParserTest, TestUploadRequired) { "</autofillqueryresponse>"; scoped_ptr<AutofillQueryXmlParser> parse_handler( - new AutofillQueryXmlParser(&field_types, &upload_required, + new AutofillQueryXmlParser(&field_infos, &upload_required, &experiment_id)); scoped_ptr<buzz::XmlParser> parser(new buzz::XmlParser(parse_handler.get())); parser->Parse(xml.c_str(), xml.length(), true); EXPECT_TRUE(parse_handler->succeeded()); EXPECT_EQ(UPLOAD_REQUIRED, upload_required); - ASSERT_EQ(1U, field_types.size()); - EXPECT_EQ(NO_SERVER_DATA, field_types[0]); + ASSERT_EQ(1U, field_infos.size()); + EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type); EXPECT_EQ(std::string(), experiment_id); - field_types.clear(); + field_infos.clear(); xml = "<autofillqueryresponse uploadrequired=\"false\">" "<field autofilltype=\"0\" />" "</autofillqueryresponse>"; - parse_handler.reset(new AutofillQueryXmlParser(&field_types, &upload_required, + parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required, &experiment_id)); parser.reset(new buzz::XmlParser(parse_handler.get())); parser->Parse(xml.c_str(), xml.length(), true); EXPECT_TRUE(parse_handler->succeeded()); EXPECT_EQ(UPLOAD_NOT_REQUIRED, upload_required); - ASSERT_EQ(1U, field_types.size()); - EXPECT_EQ(NO_SERVER_DATA, field_types[0]); + ASSERT_EQ(1U, field_infos.size()); + EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type); EXPECT_EQ(std::string(), experiment_id); - field_types.clear(); + field_infos.clear(); xml = "<autofillqueryresponse uploadrequired=\"bad_value\">" "<field autofilltype=\"0\" />" "</autofillqueryresponse>"; - parse_handler.reset(new AutofillQueryXmlParser(&field_types, &upload_required, + parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required, &experiment_id)); parser.reset(new buzz::XmlParser(parse_handler.get())); parser->Parse(xml.c_str(), xml.length(), true); EXPECT_TRUE(parse_handler->succeeded()); EXPECT_EQ(USE_UPLOAD_RATES, upload_required); - ASSERT_EQ(1U, field_types.size()); - EXPECT_EQ(NO_SERVER_DATA, field_types[0]); + ASSERT_EQ(1U, field_infos.size()); + EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type); EXPECT_EQ(std::string(), experiment_id); } // Test parsing the experiment id attribute TEST(AutofillQueryXmlParserTest, ParseExperimentId) { - std::vector<AutofillFieldType> field_types; + std::vector<AutofillServerFieldInfo> field_infos; UploadRequired upload_required = USE_UPLOAD_RATES; std::string experiment_id; @@ -107,34 +113,34 @@ TEST(AutofillQueryXmlParserTest, ParseExperimentId) { "</autofillqueryresponse>"; scoped_ptr<AutofillQueryXmlParser> parse_handler( - new AutofillQueryXmlParser(&field_types, &upload_required, + new AutofillQueryXmlParser(&field_infos, &upload_required, &experiment_id)); scoped_ptr<buzz::XmlParser> parser(new buzz::XmlParser(parse_handler.get())); parser->Parse(xml.c_str(), xml.length(), true); EXPECT_TRUE(parse_handler->succeeded()); EXPECT_EQ(USE_UPLOAD_RATES, upload_required); - ASSERT_EQ(1U, field_types.size()); - EXPECT_EQ(NO_SERVER_DATA, field_types[0]); + ASSERT_EQ(1U, field_infos.size()); + EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type); EXPECT_EQ(std::string(), experiment_id); - field_types.clear(); + field_infos.clear(); // When the attribute is present, make sure we parse it. xml = "<autofillqueryresponse experimentid=\"FancyNewAlgorithm\">" "<field autofilltype=\"0\" />" "</autofillqueryresponse>"; - parse_handler.reset(new AutofillQueryXmlParser(&field_types, &upload_required, + parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required, &experiment_id)); parser.reset(new buzz::XmlParser(parse_handler.get())); parser->Parse(xml.c_str(), xml.length(), true); EXPECT_TRUE(parse_handler->succeeded()); EXPECT_EQ(USE_UPLOAD_RATES, upload_required); - ASSERT_EQ(1U, field_types.size()); - EXPECT_EQ(NO_SERVER_DATA, field_types[0]); + ASSERT_EQ(1U, field_infos.size()); + EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type); EXPECT_EQ(std::string("FancyNewAlgorithm"), experiment_id); - field_types.clear(); + field_infos.clear(); // Make sure that we can handle parsing both the upload required and the // experiment id attribute together. @@ -143,20 +149,20 @@ TEST(AutofillQueryXmlParserTest, ParseExperimentId) { "<field autofilltype=\"0\" />" "</autofillqueryresponse>"; - parse_handler.reset(new AutofillQueryXmlParser(&field_types, &upload_required, + parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required, &experiment_id)); parser.reset(new buzz::XmlParser(parse_handler.get())); parser->Parse(xml.c_str(), xml.length(), true); EXPECT_TRUE(parse_handler->succeeded()); EXPECT_EQ(UPLOAD_NOT_REQUIRED, upload_required); - ASSERT_EQ(1U, field_types.size()); - EXPECT_EQ(NO_SERVER_DATA, field_types[0]); + ASSERT_EQ(1U, field_infos.size()); + EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type); EXPECT_EQ(std::string("ServerSmartyPants"), experiment_id); } // Test badly formed XML queries. TEST(AutofillQueryXmlParserTest, ParseErrors) { - std::vector<AutofillFieldType> field_types; + std::vector<AutofillServerFieldInfo> field_infos; UploadRequired upload_required = USE_UPLOAD_RATES; std::string experiment_id; @@ -166,46 +172,62 @@ TEST(AutofillQueryXmlParserTest, ParseErrors) { "</autofillqueryresponse>"; scoped_ptr<AutofillQueryXmlParser> parse_handler( - new AutofillQueryXmlParser(&field_types, &upload_required, + new AutofillQueryXmlParser(&field_infos, &upload_required, &experiment_id)); scoped_ptr<buzz::XmlParser> parser(new buzz::XmlParser(parse_handler.get())); parser->Parse(xml.c_str(), xml.length(), true); EXPECT_FALSE(parse_handler->succeeded()); EXPECT_EQ(USE_UPLOAD_RATES, upload_required); - EXPECT_EQ(0U, field_types.size()); + EXPECT_EQ(0U, field_infos.size()); EXPECT_EQ(std::string(), experiment_id); // Test an incorrect Autofill type. xml = "<autofillqueryresponse>" - "<field autofilltype=\"307\"/>" + "<field autofilltype=\"-1\"/>" "</autofillqueryresponse>"; - parse_handler.reset(new AutofillQueryXmlParser(&field_types, &upload_required, + parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required, &experiment_id)); parser.reset(new buzz::XmlParser(parse_handler.get())); parser->Parse(xml.c_str(), xml.length(), true); EXPECT_TRUE(parse_handler->succeeded()); EXPECT_EQ(USE_UPLOAD_RATES, upload_required); - ASSERT_EQ(1U, field_types.size()); + ASSERT_EQ(1U, field_infos.size()); // AutofillType was out of range and should be set to NO_SERVER_DATA. - EXPECT_EQ(NO_SERVER_DATA, field_types[0]); + EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type); + EXPECT_EQ(std::string(), experiment_id); + + // Test upper bound for the field type, MAX_VALID_FIELD_TYPE. + field_infos.clear(); + xml = "<autofillqueryresponse><field autofilltype=\"" + + base::IntToString(MAX_VALID_FIELD_TYPE) + "\"/></autofillqueryresponse>"; + + parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required, + &experiment_id)); + parser.reset(new buzz::XmlParser(parse_handler.get())); + parser->Parse(xml.c_str(), xml.length(), true); + EXPECT_TRUE(parse_handler->succeeded()); + EXPECT_EQ(USE_UPLOAD_RATES, upload_required); + ASSERT_EQ(1U, field_infos.size()); + // AutofillType was out of range and should be set to NO_SERVER_DATA. + EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type); EXPECT_EQ(std::string(), experiment_id); // Test an incorrect Autofill type. - field_types.clear(); + field_infos.clear(); xml = "<autofillqueryresponse>" "<field autofilltype=\"No Type\"/>" "</autofillqueryresponse>"; - // Parse fails but an entry is still added to field_types. - parse_handler.reset(new AutofillQueryXmlParser(&field_types, &upload_required, + // Parse fails but an entry is still added to field_infos. + parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required, &experiment_id)); parser.reset(new buzz::XmlParser(parse_handler.get())); parser->Parse(xml.c_str(), xml.length(), true); EXPECT_FALSE(parse_handler->succeeded()); EXPECT_EQ(USE_UPLOAD_RATES, upload_required); - ASSERT_EQ(1U, field_types.size()); - EXPECT_EQ(NO_SERVER_DATA, field_types[0]); + ASSERT_EQ(1U, field_infos.size()); + EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type); EXPECT_EQ(std::string(), experiment_id); } diff --git a/chrome/browser/autofill/field_types.h b/chrome/browser/autofill/field_types.h index 832b94a..d45f1aa 100644 --- a/chrome/browser/autofill/field_types.h +++ b/chrome/browser/autofill/field_types.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_AUTOFILL_FIELD_TYPES_H_ #include <set> +#include <string> // NOTE: This list MUST not be modified. The server aggregates and stores these // types over several versions, so we must remain fully compatible with the @@ -77,9 +78,12 @@ enum AutofillFieldType { COMPANY_NAME = 60, + // Generic type whose default value is known. + FIELD_WITH_DEFAULT_VALUE = 61, + // No new types can be added. - MAX_VALID_FIELD_TYPE = 61, + MAX_VALID_FIELD_TYPE = 62, }; typedef std::set<AutofillFieldType> FieldTypeSet; diff --git a/chrome/browser/autofill/form_field.cc b/chrome/browser/autofill/form_field.cc index b8f941b..e722c5f 100644 --- a/chrome/browser/autofill/form_field.cc +++ b/chrome/browser/autofill/form_field.cc @@ -45,6 +45,10 @@ bool IsSelectField(const std::string& type) { return type == "select-one"; } +bool IsCheckable(const AutofillField* field) { + return field->is_checkable; +} + } // namespace // static @@ -54,6 +58,14 @@ void FormField::ParseFormFields(const std::vector<AutofillField*>& fields, std::vector<const AutofillField*> remaining_fields(fields.size()); std::copy(fields.begin(), fields.end(), remaining_fields.begin()); + // Ignore checkable fields as they interfere with parsers assuming context. + // Eg., while parsing address, "Is PO box" checkbox after ADDRESS_LINE1 + // interferes with correctly understanding ADDRESS_LINE2. + remaining_fields.erase( + std::remove_if(remaining_fields.begin(), remaining_fields.end(), + IsCheckable), + remaining_fields.end()); + const CommandLine& command_line = *CommandLine::ForCurrentProcess(); bool parse_new_field_types = command_line.HasSwitch(switches::kEnableNewAutofillHeuristics); diff --git a/chrome/browser/autofill/form_field_unittest.cc b/chrome/browser/autofill/form_field_unittest.cc index 16b51a0..8ab1127 100644 --- a/chrome/browser/autofill/form_field_unittest.cc +++ b/chrome/browser/autofill/form_field_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/memory/scoped_vector.h" #include "base/string16.h" #include "base/utf_string_conversions.h" #include "chrome/browser/autofill/autofill_field.h" @@ -114,3 +115,32 @@ TEST(FormFieldTest, Match) { EXPECT_FALSE(FormField::Match(&field, ASCIIToUTF16("\\bcr\\b"), FormField::MATCH_LABEL)); } + +// Test that we ignore checkable elements. +TEST(FormFieldTest, ParseFormFields) { + ScopedVector<AutofillField> fields; + FormFieldData field_data; + field_data.form_control_type = "text"; + + field_data.label = ASCIIToUTF16("Address line1"); + fields.push_back(new AutofillField(field_data, field_data.label)); + + field_data.is_checkable = true; + field_data.label = ASCIIToUTF16("Is PO Box"); + fields.push_back(new AutofillField(field_data, field_data.label)); + + // reset is_checkable to false. + field_data.is_checkable = false; + field_data.label = ASCIIToUTF16("Address line2"); + fields.push_back(new AutofillField(field_data, field_data.label)); + + FieldTypeMap field_type_map; + FormField::ParseFormFields(fields.get(), &field_type_map); + // Checkable element shouldn't interfere with inference of Address line2. + EXPECT_EQ(2U, field_type_map.size()); + + EXPECT_EQ(ADDRESS_HOME_LINE1, + field_type_map.find(ASCIIToUTF16("Address line1"))->second); + EXPECT_EQ(ADDRESS_HOME_LINE2, + field_type_map.find(ASCIIToUTF16("Address line2"))->second); +} diff --git a/chrome/browser/autofill/form_structure.cc b/chrome/browser/autofill/form_structure.cc index d1ca6d1..9214ff1 100644 --- a/chrome/browser/autofill/form_structure.cc +++ b/chrome/browser/autofill/form_structure.cc @@ -227,6 +227,7 @@ FormStructure::FormStructure(const FormData& form) source_url_(form.origin), target_url_(form.action), autofill_count_(0), + checkable_field_count_(0), upload_required_(USE_UPLOAD_RATES), server_experiment_id_("no server response"), has_author_specified_types_(false) { @@ -235,10 +236,17 @@ FormStructure::FormStructure(const FormData& form) for (std::vector<FormFieldData>::const_iterator field = form.fields.begin(); field != form.fields.end(); field++) { - // Add all supported form fields (including with empty names) to the - // signature. This is a requirement for Autofill servers. - form_signature_field_names_.append("&"); - form_signature_field_names_.append(UTF16ToUTF8(field->name)); + // Skipping checkable elements when flag is not set, else these fields will + // interfere with existing field signatures with Autofill servers. + // TODO(ramankk): Add checkable elements only on whitelisted pages + if (!field->is_checkable || + CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableExperimentalFormFilling)) { + // Add all supported form fields (including with empty names) to the + // signature. This is a requirement for Autofill servers. + form_signature_field_names_.append("&"); + form_signature_field_names_.append(UTF16ToUTF8(field->name)); + } // Generate a unique name for this field by appending a counter to the name. // Make sure to prepend the counter with a non-numeric digit so that we are @@ -250,6 +258,9 @@ FormStructure::FormStructure(const FormData& form) string16 unique_name = field->name + ASCIIToUTF16("_") + base::IntToString16(unique_names[field->name]); fields_.push_back(new AutofillField(*field, unique_name)); + + if (field->is_checkable) + ++checkable_field_count_; } std::string method = UTF16ToUTF8(form.method); @@ -406,10 +417,10 @@ void FormStructure::ParseQueryResponse(const std::string& response_xml, metric_logger.LogServerQueryMetric(AutofillMetrics::QUERY_RESPONSE_RECEIVED); // Parse the field types from the server response to the query. - std::vector<AutofillFieldType> field_types; + std::vector<AutofillServerFieldInfo> field_infos; UploadRequired upload_required; std::string experiment_id; - AutofillQueryXmlParser parse_handler(&field_types, &upload_required, + AutofillQueryXmlParser parse_handler(&field_infos, &upload_required, &experiment_id); buzz::XmlParser parser(&parse_handler); parser.Parse(response_xml.c_str(), response_xml.length(), true); @@ -423,7 +434,8 @@ void FormStructure::ParseQueryResponse(const std::string& response_xml, bool query_response_overrode_heuristics = false; // Copy the field types into the actual form. - std::vector<AutofillFieldType>::iterator current_type = field_types.begin(); + std::vector<AutofillServerFieldInfo>::iterator current_info = + field_infos.begin(); for (std::vector<FormStructure*>::const_iterator iter = forms.begin(); iter != forms.end(); ++iter) { FormStructure* form = *iter; @@ -431,22 +443,26 @@ void FormStructure::ParseQueryResponse(const std::string& response_xml, form->server_experiment_id_ = experiment_id; for (std::vector<AutofillField*>::iterator field = form->fields_.begin(); - field != form->fields_.end(); ++field, ++current_type) { + field != form->fields_.end(); ++field, ++current_info) { // In some cases *successful* response does not return all the fields. // Quit the update of the types then. - if (current_type == field_types.end()) + if (current_info == field_infos.end()) break; // UNKNOWN_TYPE is reserved for use by the client. - DCHECK_NE(*current_type, UNKNOWN_TYPE); + DCHECK_NE(current_info->field_type, UNKNOWN_TYPE); AutofillFieldType heuristic_type = (*field)->type(); if (heuristic_type != UNKNOWN_TYPE) heuristics_detected_fillable_field = true; - (*field)->set_server_type(*current_type); + (*field)->set_server_type(current_info->field_type); if (heuristic_type != (*field)->type()) query_response_overrode_heuristics = true; + + // Copy default value into the field if available. + if (!current_info->default_value.empty()) + (*field)->set_default_value(current_info->default_value); } form->UpdateAutofillCount(); @@ -551,7 +567,10 @@ bool FormStructure::ShouldBeParsed(bool require_method_post) const { switches::kEnableExperimentalFormFilling)) return true; - if (field_count() < kRequiredFillableFields) + // Ignore counting checkable elements towards minimum number of elements + // required to parse. This avoids trying to crowdsource forms with few text + // or select elements. + if ((field_count() - checkable_field_count()) < kRequiredFillableFields) return false; // Rule out http(s)://*/search?... @@ -813,6 +832,10 @@ size_t FormStructure::field_count() const { return fields_.size(); } +size_t FormStructure::checkable_field_count() const { + return checkable_field_count_; +} + std::string FormStructure::server_experiment_id() const { return server_experiment_id_; } @@ -884,6 +907,10 @@ bool FormStructure::EncodeFormRequest( for (size_t index = 0; index < field_count(); ++index) { const AutofillField* field = fields_[index]; if (request_type == FormStructure::UPLOAD) { + // Don't upload checkable fields. + if (field->is_checkable) + continue; + FieldTypeSet types = field->possible_types(); // |types| could be empty in unit-tests only. for (FieldTypeSet::iterator field_type = types.begin(); @@ -898,6 +925,12 @@ bool FormStructure::EncodeFormRequest( encompassing_xml_element->AddElement(field_element); } } else { + // Skip putting checkable fields in the request if the flag is not set. + if (field->is_checkable && + !CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableExperimentalFormFilling)) + continue; + buzz::XmlElement *field_element = new buzz::XmlElement( buzz::QName(kXMLElementField)); field_element->SetAttr(buzz::QName(kAttributeSignature), diff --git a/chrome/browser/autofill/form_structure.h b/chrome/browser/autofill/form_structure.h index 45cf2bb..6ac65c1 100644 --- a/chrome/browser/autofill/form_structure.h +++ b/chrome/browser/autofill/form_structure.h @@ -127,6 +127,7 @@ class FormStructure { const AutofillField* field(size_t index) const; AutofillField* field(size_t index); size_t field_count() const; + size_t checkable_field_count() const; // Returns the number of fields that are able to be autofilled. size_t autofill_count() const { return autofill_count_; } @@ -192,6 +193,9 @@ class FormStructure { // A vector of all the input fields in the form. ScopedVector<AutofillField> fields_; + // The number of fields able to be checked. + size_t checkable_field_count_; + // The names of the form input elements, that are part of the form signature. // The string starts with "&" and the names are also separated by the "&" // character. E.g.: "&form_input1_name&form_input2_name&...&form_inputN_name" diff --git a/chrome/browser/autofill/form_structure_unittest.cc b/chrome/browser/autofill/form_structure_unittest.cc index bd2bded..4dbc3ce 100644 --- a/chrome/browser/autofill/form_structure_unittest.cc +++ b/chrome/browser/autofill/form_structure_unittest.cc @@ -4,10 +4,12 @@ #include "chrome/browser/autofill/form_structure.h" +#include "base/command_line.h" #include "base/memory/scoped_ptr.h" #include "base/string_util.h" #include "base/utf_string_conversions.h" #include "chrome/browser/autofill/autofill_metrics.h" +#include "chrome/common/chrome_switches.h" #include "chrome/common/form_data.h" #include "chrome/common/form_field_data.h" #include "googleurl/src/gurl.h" @@ -215,6 +217,16 @@ TEST(FormStructureTest, ShouldBeParsed) { field.form_control_type = "text"; form.fields.push_back(field); + FormFieldData checkable_field; + checkable_field.is_checkable = true; + checkable_field.name = ASCIIToUTF16("radiobtn"); + checkable_field.form_control_type = "radio"; + form.fields.push_back(checkable_field); + + checkable_field.name = ASCIIToUTF16("checkbox"); + checkable_field.form_control_type = "checkbox"; + form.fields.push_back(checkable_field); + form_structure.reset(new FormStructure(form)); EXPECT_FALSE(form_structure->ShouldBeParsed(true)); @@ -1430,6 +1442,13 @@ TEST(FormStructureTest, EncodeQueryRequest) { field.name = ASCIIToUTF16("expiration_year"); form.fields.push_back(field); + // Add checkable field. + FormFieldData checkable_field; + checkable_field.is_checkable = true; + checkable_field.label = ASCIIToUTF16("Checkable1"); + checkable_field.name = ASCIIToUTF16("Checkable1"); + form.fields.push_back(checkable_field); + ScopedVector<FormStructure> forms; forms.push_back(new FormStructure(form)); std::vector<std::string> encoded_signatures; @@ -1487,15 +1506,16 @@ TEST(FormStructureTest, EncodeQueryRequest) { "signature=\"509334676\"/></form></autofillquery>"; EXPECT_EQ(kResponse2, encoded_xml); + FormData malformed_form(form); // Add 50 address fields - the form is not valid anymore, but previous ones // are. The result should be the same as in previous test. for (size_t i = 0; i < 50; ++i) { field.label = ASCIIToUTF16("Address"); field.name = ASCIIToUTF16("address"); - form.fields.push_back(field); + malformed_form.fields.push_back(field); } - forms.push_back(new FormStructure(form)); + forms.push_back(new FormStructure(malformed_form)); ASSERT_TRUE(FormStructure::EncodeQueryRequest(forms.get(), &encoded_signatures, &encoded_xml)); @@ -1506,12 +1526,37 @@ TEST(FormStructureTest, EncodeQueryRequest) { // Check that we fail if there are only bad form(s). ScopedVector<FormStructure> bad_forms; - bad_forms.push_back(new FormStructure(form)); + bad_forms.push_back(new FormStructure(malformed_form)); EXPECT_FALSE(FormStructure::EncodeQueryRequest(bad_forms.get(), &encoded_signatures, &encoded_xml)); EXPECT_EQ(0U, encoded_signatures.size()); EXPECT_EQ("", encoded_xml); + + // Check the behaviour with kEnableExperimentalFormFilling switch on. + CommandLine::ForCurrentProcess()->AppendSwitch( + switches::kEnableExperimentalFormFilling); + // Add the previous form but with flag set. + ScopedVector<FormStructure> checkable_forms; + checkable_forms.push_back(new FormStructure(form)); + + ASSERT_TRUE(FormStructure::EncodeQueryRequest(checkable_forms.get(), + &encoded_signatures, + &encoded_xml)); + const char * const kSignature3 = "7747357776717901584"; + const char * const kResponse3 = + "<?xml version=\"1.0\" encoding=\"UTF-8\"?><autofillquery " + "clientversion=\"6.1.1715.1442/en (GGLL)\" accepts=\"e\">" + "<form signature=\"7747357776717901584\"><field signature=\"412125936\"/>" + "<field signature=\"1917667676\"/><field signature=\"2226358947\"/><field" + " signature=\"747221617\"/><field signature=\"4108155786\"/><field " + "signature=\"3410250678\"/><field signature=\"509334676\"/><field " + "signature=\"509334676\"/><field signature=\"509334676\"/><field " + "signature=\"509334676\"/><field signature=\"509334676\"/></form>" + "</autofillquery>"; + ASSERT_EQ(1U, encoded_signatures.size()); + EXPECT_EQ(kSignature3, encoded_signatures[0]); + EXPECT_EQ(kResponse3, encoded_xml); } TEST(FormStructureTest, EncodeUploadRequest) { @@ -1557,6 +1602,16 @@ TEST(FormStructureTest, EncodeUploadRequest) { form.fields.push_back(field); possible_field_types.push_back(FieldTypeSet()); possible_field_types.back().insert(ADDRESS_HOME_COUNTRY); + + // Add checkable field. + FormFieldData checkable_field; + checkable_field.is_checkable = true; + checkable_field.label = ASCIIToUTF16("Checkable1"); + checkable_field.name = ASCIIToUTF16("Checkable1"); + form.fields.push_back(checkable_field); + possible_field_types.push_back(FieldTypeSet()); + possible_field_types.back().insert(ADDRESS_HOME_COUNTRY); + form_structure.reset(new FormStructure(form)); ASSERT_EQ(form_structure->field_count(), possible_field_types.size()); |