Revert 176189
Performance regression re-occurred.
> Add support for radiobuttons and checkboxes.
>
> This is attempt no 2 to submit this CL. earlier attempt at
> https://chromiumcodereview.appspot.com/11415221/ (which was rolled back due to perf regression)
>
> Changes from last attempt:
> * In chrome/browser/autofill/form_structure.cc: Read switch value into local variable before being used multiple times.
> * In chrome/renderer/autofill/form_autofill_util.cc: Extracted form_control_type strings in WebString constants for reuse.
>
> Due to inconsistency in the timing reported by the page cycler tests on my local setup and unavailablity of good environment to perform these tests, the only data I have now is runtimes were approximatly same during *one* set of runs with and without my change. I have observed ~4ms difference in timing without any code changes, while I am chasing ~2 ms.
>
> If perf regresssion is observed again on the perf machines, I will revert it back.
>
>
> BUG=157636
>
> Review URL: https://chromiumcodereview.appspot.com/11788025
[email protected]
Review URL: https://codereview.chromium.org/11884020
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@176685 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/autofill/autofill_xml_parser_unittest.cc b/chrome/browser/autofill/autofill_xml_parser_unittest.cc
index 2363862..a0a1024 100644
--- a/chrome/browser/autofill/autofill_xml_parser_unittest.cc
+++ b/chrome/browser/autofill/autofill_xml_parser_unittest.cc
@@ -6,7 +6,6 @@
#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"
@@ -21,36 +20,31 @@
"<field autofilltype=\"1\" />"
"<field autofilltype=\"3\" />"
"<field autofilltype=\"2\" />"
- "<field autofilltype=\"61\" defaultvalue=\"default\"/>"
"</autofillqueryresponse>";
- // Create a vector of AutofillServerFieldInfos, to assign the parsed field
- // types to.
- std::vector<AutofillServerFieldInfo> field_infos;
+ // Create a vector of AutofillFieldTypes, to assign the parsed field types to.
+ std::vector<AutofillFieldType> field_types;
UploadRequired upload_required = USE_UPLOAD_RATES;
std::string experiment_id;
// Create a parser.
- AutofillQueryXmlParser parse_handler(&field_infos, &upload_required,
+ AutofillQueryXmlParser parse_handler(&field_types, &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(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);
+ 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]);
EXPECT_EQ(std::string(), experiment_id);
}
// Test parsing the upload required attribute.
TEST(AutofillQueryXmlParserTest, TestUploadRequired) {
- std::vector<AutofillServerFieldInfo> field_infos;
+ std::vector<AutofillFieldType> field_types;
UploadRequired upload_required = USE_UPLOAD_RATES;
std::string experiment_id;
@@ -59,50 +53,50 @@
"</autofillqueryresponse>";
scoped_ptr<AutofillQueryXmlParser> parse_handler(
- new AutofillQueryXmlParser(&field_infos, &upload_required,
+ new AutofillQueryXmlParser(&field_types, &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_infos.size());
- EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type);
+ ASSERT_EQ(1U, field_types.size());
+ EXPECT_EQ(NO_SERVER_DATA, field_types[0]);
EXPECT_EQ(std::string(), experiment_id);
- field_infos.clear();
+ field_types.clear();
xml = "<autofillqueryresponse uploadrequired=\"false\">"
"<field autofilltype=\"0\" />"
"</autofillqueryresponse>";
- parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required,
+ parse_handler.reset(new AutofillQueryXmlParser(&field_types, &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_infos.size());
- EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type);
+ ASSERT_EQ(1U, field_types.size());
+ EXPECT_EQ(NO_SERVER_DATA, field_types[0]);
EXPECT_EQ(std::string(), experiment_id);
- field_infos.clear();
+ field_types.clear();
xml = "<autofillqueryresponse uploadrequired=\"bad_value\">"
"<field autofilltype=\"0\" />"
"</autofillqueryresponse>";
- parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required,
+ parse_handler.reset(new AutofillQueryXmlParser(&field_types, &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());
- EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type);
+ ASSERT_EQ(1U, field_types.size());
+ EXPECT_EQ(NO_SERVER_DATA, field_types[0]);
EXPECT_EQ(std::string(), experiment_id);
}
// Test parsing the experiment id attribute
TEST(AutofillQueryXmlParserTest, ParseExperimentId) {
- std::vector<AutofillServerFieldInfo> field_infos;
+ std::vector<AutofillFieldType> field_types;
UploadRequired upload_required = USE_UPLOAD_RATES;
std::string experiment_id;
@@ -113,34 +107,34 @@
"</autofillqueryresponse>";
scoped_ptr<AutofillQueryXmlParser> parse_handler(
- new AutofillQueryXmlParser(&field_infos, &upload_required,
+ new AutofillQueryXmlParser(&field_types, &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_infos.size());
- EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type);
+ ASSERT_EQ(1U, field_types.size());
+ EXPECT_EQ(NO_SERVER_DATA, field_types[0]);
EXPECT_EQ(std::string(), experiment_id);
- field_infos.clear();
+ field_types.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_infos, &upload_required,
+ parse_handler.reset(new AutofillQueryXmlParser(&field_types, &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());
- EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type);
+ ASSERT_EQ(1U, field_types.size());
+ EXPECT_EQ(NO_SERVER_DATA, field_types[0]);
EXPECT_EQ(std::string("FancyNewAlgorithm"), experiment_id);
- field_infos.clear();
+ field_types.clear();
// Make sure that we can handle parsing both the upload required and the
// experiment id attribute together.
@@ -149,20 +143,20 @@
"<field autofilltype=\"0\" />"
"</autofillqueryresponse>";
- parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required,
+ parse_handler.reset(new AutofillQueryXmlParser(&field_types, &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_infos.size());
- EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type);
+ ASSERT_EQ(1U, field_types.size());
+ EXPECT_EQ(NO_SERVER_DATA, field_types[0]);
EXPECT_EQ(std::string("ServerSmartyPants"), experiment_id);
}
// Test badly formed XML queries.
TEST(AutofillQueryXmlParserTest, ParseErrors) {
- std::vector<AutofillServerFieldInfo> field_infos;
+ std::vector<AutofillFieldType> field_types;
UploadRequired upload_required = USE_UPLOAD_RATES;
std::string experiment_id;
@@ -172,62 +166,46 @@
"</autofillqueryresponse>";
scoped_ptr<AutofillQueryXmlParser> parse_handler(
- new AutofillQueryXmlParser(&field_infos, &upload_required,
+ new AutofillQueryXmlParser(&field_types, &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_infos.size());
+ EXPECT_EQ(0U, field_types.size());
EXPECT_EQ(std::string(), experiment_id);
// Test an incorrect Autofill type.
xml = "<autofillqueryresponse>"
- "<field autofilltype=\"-1\"/>"
+ "<field autofilltype=\"307\"/>"
"</autofillqueryresponse>";
- parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required,
+ parse_handler.reset(new AutofillQueryXmlParser(&field_types, &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());
+ ASSERT_EQ(1U, field_types.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 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(NO_SERVER_DATA, field_types[0]);
EXPECT_EQ(std::string(), experiment_id);
// Test an incorrect Autofill type.
- field_infos.clear();
+ field_types.clear();
xml = "<autofillqueryresponse>"
"<field autofilltype=\"No Type\"/>"
"</autofillqueryresponse>";
- // Parse fails but an entry is still added to field_infos.
- parse_handler.reset(new AutofillQueryXmlParser(&field_infos, &upload_required,
+ // Parse fails but an entry is still added to field_types.
+ parse_handler.reset(new AutofillQueryXmlParser(&field_types, &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_infos.size());
- EXPECT_EQ(NO_SERVER_DATA, field_infos[0].field_type);
+ ASSERT_EQ(1U, field_types.size());
+ EXPECT_EQ(NO_SERVER_DATA, field_types[0]);
EXPECT_EQ(std::string(), experiment_id);
}