summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormathp <mathp@chromium.org>2015-04-22 09:03:34 -0700
committerCommit bot <commit-bot@chromium.org>2015-04-22 16:03:51 +0000
commitc4db2b38971c03fc5f3b8f49d7cd7dc52e7986c6 (patch)
tree372523f6d426c5a2637bb3efec7a1ec167a6c232
parent4555ac061b6bd3eaf934ee9ae0c6ceff91afcae0 (diff)
downloadchromium_src-c4db2b38971c03fc5f3b8f49d7cd7dc52e7986c6.zip
chromium_src-c4db2b38971c03fc5f3b8f49d7cd7dc52e7986c6.tar.gz
chromium_src-c4db2b38971c03fc5f3b8f49d7cd7dc52e7986c6.tar.bz2
[Autofill] Log Rappor metric in the case of no server data received
For some sites, the autofill server returns no data at all. Use Rappor to log those cases. We log here and not after form submission because we'd like to capture this metric for all query events, not only those that result in a submission. BUG=478869 TEST=FormStructureTest Review URL: https://codereview.chromium.org/1087403006 Cr-Commit-Position: refs/heads/master@{#326313}
-rw-r--r--components/autofill/core/browser/autofill_manager.cc3
-rw-r--r--components/autofill/core/browser/form_structure.cc16
-rw-r--r--components/autofill/core/browser/form_structure.h4
-rw-r--r--components/autofill/core/browser/form_structure_unittest.cc162
-rw-r--r--components/password_manager/core/browser/password_generation_manager_unittest.cc2
-rw-r--r--tools/metrics/rappor/rappor.xml9
6 files changed, 188 insertions, 8 deletions
diff --git a/components/autofill/core/browser/autofill_manager.cc b/components/autofill/core/browser/autofill_manager.cc
index b7b3d5d..beb4281 100644
--- a/components/autofill/core/browser/autofill_manager.cc
+++ b/components/autofill/core/browser/autofill_manager.cc
@@ -767,7 +767,8 @@ void AutofillManager::OnSetDataList(const std::vector<base::string16>& values,
void AutofillManager::OnLoadedServerPredictions(
const std::string& response_xml) {
// Parse and store the server predictions.
- FormStructure::ParseQueryResponse(response_xml, form_structures_.get());
+ FormStructure::ParseQueryResponse(response_xml, form_structures_.get(),
+ client_->GetRapporService());
// Forward form structures to the password generation manager to detect
// account creation forms.
diff --git a/components/autofill/core/browser/form_structure.cc b/components/autofill/core/browser/form_structure.cc
index a582b0a..5c76c62 100644
--- a/components/autofill/core/browser/form_structure.cc
+++ b/components/autofill/core/browser/form_structure.cc
@@ -550,9 +550,9 @@ bool FormStructure::EncodeQueryRequest(
}
// static
-void FormStructure::ParseQueryResponse(
- const std::string& response_xml,
- const std::vector<FormStructure*>& forms) {
+void FormStructure::ParseQueryResponse(const std::string& response_xml,
+ const std::vector<FormStructure*>& forms,
+ rappor::RapporService* rappor_service) {
AutofillMetrics::LogServerQueryMetric(
AutofillMetrics::QUERY_RESPONSE_RECEIVED);
@@ -579,6 +579,7 @@ void FormStructure::ParseQueryResponse(
FormStructure* form = *iter;
form->upload_required_ = upload_required;
+ bool query_response_has_no_server_data = true;
for (std::vector<AutofillField*>::iterator field = form->fields_.begin();
field != form->fields_.end(); ++field) {
if (form->ShouldSkipField(**field))
@@ -589,6 +590,9 @@ void FormStructure::ParseQueryResponse(
if (current_info == field_infos.end())
break;
+ query_response_has_no_server_data &=
+ current_info->field_type == NO_SERVER_DATA;
+
// If |form->has_author_specified_types| only password fields should be
// updated.
if (!form->has_author_specified_types_ ||
@@ -612,6 +616,12 @@ void FormStructure::ParseQueryResponse(
++current_info;
}
+ if (query_response_has_no_server_data && form->source_url().is_valid()) {
+ rappor::SampleDomainAndRegistryFromGURL(
+ rappor_service, "Autofill.QueryResponseHasNoServerDataForForm",
+ form->source_url());
+ }
+
form->UpdateAutofillCount();
form->IdentifySections(false);
}
diff --git a/components/autofill/core/browser/form_structure.h b/components/autofill/core/browser/form_structure.h
index 8e2e7a1..5a3f5065 100644
--- a/components/autofill/core/browser/form_structure.h
+++ b/components/autofill/core/browser/form_structure.h
@@ -77,8 +77,10 @@ class FormStructure {
// Parses the field types from the server query response. |forms| must be the
// same as the one passed to EncodeQueryRequest when constructing the query.
+ // |rappor_service| may be null.
static void ParseQueryResponse(const std::string& response_xml,
- const std::vector<FormStructure*>& forms);
+ const std::vector<FormStructure*>& forms,
+ rappor::RapporService* rappor_service);
// Returns predictions using the details from the given |form_structures| and
// their fields' predicted types.
diff --git a/components/autofill/core/browser/form_structure_unittest.cc b/components/autofill/core/browser/form_structure_unittest.cc
index a437aa4..7d05b72 100644
--- a/components/autofill/core/browser/form_structure_unittest.cc
+++ b/components/autofill/core/browser/form_structure_unittest.cc
@@ -11,10 +11,12 @@
#include "components/autofill/core/common/autofill_switches.h"
#include "components/autofill/core/common/form_data.h"
#include "components/autofill/core/common/form_field_data.h"
+#include "components/rappor/test_rappor_service.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
using base::ASCIIToUTF16;
+using rappor::TestRapporService;
namespace autofill {
@@ -2379,7 +2381,9 @@ TEST(FormStructureTest, PossibleValues) {
}
TEST(FormStructureTest, ParseQueryResponse) {
+ TestRapporService rappor_service;
FormData form;
+ form.origin = GURL("http://foo.com");
FormFieldData field;
field.form_control_type = "text";
@@ -2420,7 +2424,7 @@ TEST(FormStructureTest, ParseQueryResponse) {
"<field autofilltype=\"0\" />"
"</autofillqueryresponse>";
- FormStructure::ParseQueryResponse(response, forms.get());
+ FormStructure::ParseQueryResponse(response, forms.get(), &rappor_service);
ASSERT_GE(forms[0]->field_count(), 2U);
ASSERT_GE(forms[1]->field_count(), 2U);
@@ -2428,11 +2432,17 @@ TEST(FormStructureTest, ParseQueryResponse) {
EXPECT_EQ(30, forms[0]->field(1)->server_type());
EXPECT_EQ(9, forms[1]->field(0)->server_type());
EXPECT_EQ(0, forms[1]->field(1)->server_type());
+
+ // No RAPPOR metrics are logged in the case there is server data available for
+ // all forms.
+ EXPECT_EQ(0, rappor_service.GetReportsCount());
}
// If user defined types are present, only parse password fields.
TEST(FormStructureTest, ParseQueryResponseAuthorDefinedTypes) {
+ TestRapporService rappor_service;
FormData form;
+ form.origin = GURL("http://foo.com");
FormFieldData field;
field.label = ASCIIToUTF16("email");
@@ -2457,11 +2467,159 @@ TEST(FormStructureTest, ParseQueryResponseAuthorDefinedTypes) {
"<field autofilltype=\"76\" />"
"</autofillqueryresponse>";
- FormStructure::ParseQueryResponse(response, forms.get());
+ FormStructure::ParseQueryResponse(response, forms.get(), &rappor_service);
ASSERT_GE(forms[0]->field_count(), 2U);
EXPECT_EQ(NO_SERVER_DATA, forms[0]->field(0)->server_type());
EXPECT_EQ(76, forms[0]->field(1)->server_type());
}
+// If the server returns NO_SERVER_DATA for one of the forms, expect RAPPOR
+// logging.
+TEST(FormStructureTest, ParseQueryResponse_RapporLogging_OneFormNoServerData) {
+ TestRapporService rappor_service;
+ FormData form;
+ form.origin = GURL("http://foo.com");
+ FormFieldData field;
+ field.form_control_type = "text";
+
+ field.label = ASCIIToUTF16("fullname");
+ field.name = ASCIIToUTF16("fullname");
+ form.fields.push_back(field);
+
+ field.label = ASCIIToUTF16("address");
+ field.name = ASCIIToUTF16("address");
+ form.fields.push_back(field);
+
+ ScopedVector<FormStructure> forms;
+ forms.push_back(new FormStructure(form));
+
+ field.label = ASCIIToUTF16("email");
+ field.name = ASCIIToUTF16("email");
+ form.fields.push_back(field);
+
+ field.label = ASCIIToUTF16("password");
+ field.name = ASCIIToUTF16("password");
+ field.form_control_type = "password";
+ form.fields.push_back(field);
+
+ forms.push_back(new FormStructure(form));
+
+ std::string response =
+ "<autofillqueryresponse>"
+ "<field autofilltype=\"0\" />"
+ "<field autofilltype=\"0\" />"
+ "<field autofilltype=\"9\" />"
+ "<field autofilltype=\"0\" />"
+ "</autofillqueryresponse>";
+
+ FormStructure::ParseQueryResponse(response, forms.get(), &rappor_service);
+
+ EXPECT_EQ(1, rappor_service.GetReportsCount());
+ std::string sample;
+ rappor::RapporType type;
+ EXPECT_TRUE(rappor_service.GetRecordedSampleForMetric(
+ "Autofill.QueryResponseHasNoServerDataForForm", &sample, &type));
+ EXPECT_EQ("foo.com", sample);
+ EXPECT_EQ(rappor::ETLD_PLUS_ONE_RAPPOR_TYPE, type);
+}
+
+// If the server returns NO_SERVER_DATA for both of the forms, expect RAPPOR
+// logging.
+TEST(FormStructureTest, ParseQueryResponse_RapporLogging_AllFormsNoServerData) {
+ TestRapporService rappor_service;
+ FormData form;
+ form.origin = GURL("http://foo.com");
+ FormFieldData field;
+ field.form_control_type = "text";
+
+ field.label = ASCIIToUTF16("fullname");
+ field.name = ASCIIToUTF16("fullname");
+ form.fields.push_back(field);
+
+ field.label = ASCIIToUTF16("address");
+ field.name = ASCIIToUTF16("address");
+ form.fields.push_back(field);
+
+ ScopedVector<FormStructure> forms;
+ forms.push_back(new FormStructure(form));
+
+ field.label = ASCIIToUTF16("email");
+ field.name = ASCIIToUTF16("email");
+ form.fields.push_back(field);
+
+ field.label = ASCIIToUTF16("password");
+ field.name = ASCIIToUTF16("password");
+ field.form_control_type = "password";
+ form.fields.push_back(field);
+
+ forms.push_back(new FormStructure(form));
+
+ std::string response =
+ "<autofillqueryresponse>"
+ "<field autofilltype=\"0\" />"
+ "<field autofilltype=\"0\" />"
+ "<field autofilltype=\"0\" />"
+ "<field autofilltype=\"0\" />"
+ "</autofillqueryresponse>";
+
+ FormStructure::ParseQueryResponse(response, forms.get(), &rappor_service);
+
+ // Even though both forms are logging to RAPPOR, there is only one sample for
+ // a given eTLD+1.
+ EXPECT_EQ(1, rappor_service.GetReportsCount());
+ std::string sample;
+ rappor::RapporType type;
+ EXPECT_TRUE(rappor_service.GetRecordedSampleForMetric(
+ "Autofill.QueryResponseHasNoServerDataForForm", &sample, &type));
+ EXPECT_EQ("foo.com", sample);
+ EXPECT_EQ(rappor::ETLD_PLUS_ONE_RAPPOR_TYPE, type);
+}
+
+// If the server returns NO_SERVER_DATA for only some of the fields, expect no
+// RAPPOR logging.
+TEST(FormStructureTest, ParseQueryResponse_RapporLogging_PartialNoServerData) {
+ TestRapporService rappor_service;
+ FormData form;
+ form.origin = GURL("http://foo.com");
+ FormFieldData field;
+ field.form_control_type = "text";
+
+ field.label = ASCIIToUTF16("fullname");
+ field.name = ASCIIToUTF16("fullname");
+ form.fields.push_back(field);
+
+ field.label = ASCIIToUTF16("address");
+ field.name = ASCIIToUTF16("address");
+ form.fields.push_back(field);
+
+ ScopedVector<FormStructure> forms;
+ forms.push_back(new FormStructure(form));
+
+ field.label = ASCIIToUTF16("email");
+ field.name = ASCIIToUTF16("email");
+ form.fields.push_back(field);
+
+ field.label = ASCIIToUTF16("password");
+ field.name = ASCIIToUTF16("password");
+ field.form_control_type = "password";
+ form.fields.push_back(field);
+
+ forms.push_back(new FormStructure(form));
+
+ std::string response =
+ "<autofillqueryresponse>"
+ "<field autofilltype=\"0\" />"
+ "<field autofilltype=\"10\" />"
+ "<field autofilltype=\"0\" />"
+ "<field autofilltype=\"11\" />"
+ "</autofillqueryresponse>";
+
+ FormStructure::ParseQueryResponse(response, forms.get(), &rappor_service);
+
+ // No RAPPOR metrics are logged in the case there is at least some server data
+ // available for all forms.
+ EXPECT_EQ(0, rappor_service.GetReportsCount());
+}
+
} // namespace autofill
diff --git a/components/password_manager/core/browser/password_generation_manager_unittest.cc b/components/password_manager/core/browser/password_generation_manager_unittest.cc
index d01d5f5..eda8962 100644
--- a/components/password_manager/core/browser/password_generation_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_generation_manager_unittest.cc
@@ -186,7 +186,7 @@ TEST_F(PasswordGenerationManagerTest, DetectAccountCreationForms) {
"<field autofilltype=\"76\" />"
"<field autofilltype=\"75\" />"
"</autofillqueryresponse>";
- autofill::FormStructure::ParseQueryResponse(kServerResponse, forms);
+ autofill::FormStructure::ParseQueryResponse(kServerResponse, forms, NULL);
DetectAccountCreationForms(forms);
EXPECT_EQ(1u, GetTestDriver()->GetFoundAccountCreationForms().size());
diff --git a/tools/metrics/rappor/rappor.xml b/tools/metrics/rappor/rappor.xml
index c32d6bd..33502e0 100644
--- a/tools/metrics/rappor/rappor.xml
+++ b/tools/metrics/rappor/rappor.xml
@@ -107,6 +107,15 @@ components/rappor/rappor_service.cc.
</summary>
</rappor-metric>
+<rappor-metric name="Autofill.QueryResponseHasNoServerDataForForm"
+ type="ETLD_PLUS_ONE">
+ <owner>mathp@chromium.org</owner>
+ <summary>
+ The eTLD+1 of a URL for which there was a server query response for which
+ the server had no data at all for at least one form.
+ </summary>
+</rappor-metric>
+
<rappor-metric name="ContentSettings.MixedScript.DisplayedShield"
type="ETLD_PLUS_ONE">
<owner>lgarron@chromium.org</owner>