diff options
| -rw-r--r-- | chrome/browser/autofill/autofill_manager.cc | 55 | ||||
| -rw-r--r-- | chrome/browser/autofill/autofill_manager.h | 6 | ||||
| -rw-r--r-- | chrome/browser/autofill/autofill_metrics.h | 7 | ||||
| -rw-r--r-- | chrome/browser/autofill/autofill_metrics_unittest.cc | 147 | ||||
| -rw-r--r-- | chrome/browser/autofill/form_structure.h | 6 |
5 files changed, 204 insertions, 17 deletions
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc index 4fa5241..28dba23 100644 --- a/chrome/browser/autofill/autofill_manager.cc +++ b/chrome/browser/autofill/autofill_manager.cc @@ -212,7 +212,14 @@ void AutoFillManager::OnFormSubmitted(const FormData& form) { if (!upload_form_structure_->ShouldBeParsed(true)) return; - DeterminePossibleFieldTypesForUpload(); + FormStructure* cached_upload_form_structure = NULL; + AutoFillField* ignored; + if (!FindCachedFormAndField(form, form.fields.front(), + &cached_upload_form_structure, &ignored)) { + cached_upload_form_structure = NULL; + } + + DeterminePossibleFieldTypesForUpload(cached_upload_form_structure); // TODO(isherman): Consider uploading to server here rather than in // HandleSubmit(). @@ -477,7 +484,8 @@ bool AutoFillManager::IsAutoFillEnabled() const { return prefs->GetBoolean(prefs::kAutoFillEnabled); } -void AutoFillManager::DeterminePossibleFieldTypesForUpload() { +void AutoFillManager::DeterminePossibleFieldTypesForUpload( + const FormStructure* cached_upload_form_structure) { for (size_t i = 0; i < upload_form_structure_->field_count(); i++) { const AutoFillField* field = upload_form_structure_->field(i); FieldTypeSet field_types; @@ -496,13 +504,34 @@ void AutoFillManager::DeterminePossibleFieldTypesForUpload() { metric_logger_->Log(AutoFillMetrics::FIELD_SUBMITTED); if (field_types.find(EMPTY_TYPE) == field_types.end() && field_types.find(UNKNOWN_TYPE) == field_types.end()) { - if (field->is_autofilled()) - metric_logger_->Log(AutoFillMetrics::FIELD_AUTOFILLED); - else - metric_logger_->Log(AutoFillMetrics::FIELD_AUTOFILL_FAILED); + if (field->is_autofilled()) { + metric_logger_->Log(AutoFillMetrics::FIELD_AUTOFILLED); + } else { + metric_logger_->Log(AutoFillMetrics::FIELD_AUTOFILL_FAILED); + + AutoFillFieldType heuristic_type = cached_upload_form_structure? + cached_upload_form_structure->field(i)->heuristic_type() : + UNKNOWN_TYPE; + if (heuristic_type == UNKNOWN_TYPE) + metric_logger_->Log(AutoFillMetrics::FIELD_HEURISTIC_TYPE_UNKNOWN); + else if (field_types.count(heuristic_type)) + metric_logger_->Log(AutoFillMetrics::FIELD_HEURISTIC_TYPE_MATCH); + else + metric_logger_->Log(AutoFillMetrics::FIELD_HEURISTIC_TYPE_MISMATCH); + + AutoFillFieldType server_type = cached_upload_form_structure? + cached_upload_form_structure->field(i)->server_type() : + NO_SERVER_DATA; + if (server_type == NO_SERVER_DATA) + metric_logger_->Log(AutoFillMetrics::FIELD_SERVER_TYPE_UNKNOWN); + else if (field_types.count(server_type)) + metric_logger_->Log(AutoFillMetrics::FIELD_SERVER_TYPE_MATCH); + else + metric_logger_->Log(AutoFillMetrics::FIELD_SERVER_TYPE_MISMATCH); + } + // TODO(isherman): Other things we might want to log here: - // * Did the field's heuristic type match the PDM type? // * Per Vadim's email, a combination of (1) whether heuristics fired, // (2) whether the server returned something interesting, (3) whether // the user filled the field @@ -622,7 +651,7 @@ bool AutoFillManager::FindCachedFormAndField(const FormData& form, // Find the FormStructure that corresponds to |form|. *form_structure = NULL; for (std::vector<FormStructure*>::const_iterator iter = - form_structures_.begin(); + form_structures_.begin(); iter != form_structures_.end(); ++iter) { if (**iter == form) { *form_structure = *iter; @@ -782,7 +811,7 @@ void AutoFillManager::FillPhoneNumberField(const AutoFillProfile* profile, } void AutoFillManager::ParseForms(const std::vector<FormData>& forms) { - std::vector<FormStructure *> non_queryable_forms; + std::vector<FormStructure*> non_queryable_forms; for (std::vector<FormData>::const_iterator iter = forms.begin(); iter != forms.end(); ++iter) { scoped_ptr<FormStructure> form_structure(new FormStructure(*iter)); @@ -798,12 +827,10 @@ void AutoFillManager::ParseForms(const std::vector<FormData>& forms) { } // If none of the forms were parsed, no use querying the server. - if (!form_structures_.empty() && !disable_download_manager_requests_) { - download_manager_.StartQueryRequest(form_structures_, - *metric_logger_); - } + if (!form_structures_.empty() && !disable_download_manager_requests_) + download_manager_.StartQueryRequest(form_structures_, *metric_logger_); - for (std::vector<FormStructure *>::const_iterator iter = + for (std::vector<FormStructure*>::const_iterator iter = non_queryable_forms.begin(); iter != non_queryable_forms.end(); ++iter) { form_structures_.push_back(*iter); diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h index 08be41c..76ed0d7 100644 --- a/chrome/browser/autofill/autofill_manager.h +++ b/chrome/browser/autofill/autofill_manager.h @@ -91,6 +91,8 @@ class AutoFillManager : public IPC::Channel::Listener, } void set_metric_logger(const AutoFillMetrics* metric_logger); + ScopedVector<FormStructure>* form_structures() { return &form_structures_; } + // Maps GUIDs to and from IDs that are used to identify profiles and credit // cards sent to and from the renderer process. virtual int GUIDToID(const std::string& guid); @@ -172,7 +174,8 @@ class AutoFillManager : public IPC::Channel::Listener, // Uses existing personal data to determine possible field types for the // |upload_form_structure_|. - void DeterminePossibleFieldTypesForUpload(); + void DeterminePossibleFieldTypesForUpload( + const FormStructure* cached_upload_form_structure); // The TabContents hosting this AutoFillManager. // Weak reference. @@ -224,6 +227,7 @@ class AutoFillManager : public IPC::Channel::Listener, FRIEND_TEST_ALL_PREFIXES(AutoFillMetricsTest, QualityMetrics); FRIEND_TEST_ALL_PREFIXES(AutoFillMetricsTest, NoQualityMetricsForNonAutoFillableForms); + FRIEND_TEST_ALL_PREFIXES(AutoFillMetricsTest, QualityMetricsForFailure); DISALLOW_COPY_AND_ASSIGN(AutoFillManager); }; diff --git a/chrome/browser/autofill/autofill_metrics.h b/chrome/browser/autofill/autofill_metrics.h index 6318274f..6f7e5a4 100644 --- a/chrome/browser/autofill/autofill_metrics.h +++ b/chrome/browser/autofill/autofill_metrics.h @@ -45,6 +45,13 @@ class AutoFillMetrics { // |is_autofilled()| but as a value that is present in the personal data // manager. FIELD_AUTOFILL_FAILED, + // The below are only logged when |FIELD_AUTOFILL_FAILED| is also logged. + FIELD_HEURISTIC_TYPE_UNKNOWN, + FIELD_HEURISTIC_TYPE_MATCH, + FIELD_HEURISTIC_TYPE_MISMATCH, + FIELD_SERVER_TYPE_UNKNOWN, + FIELD_SERVER_TYPE_MATCH, + FIELD_SERVER_TYPE_MISMATCH, NUM_QUALITY_METRICS }; diff --git a/chrome/browser/autofill/autofill_metrics_unittest.cc b/chrome/browser/autofill/autofill_metrics_unittest.cc index ca4dce3..41df1e9 100644 --- a/chrome/browser/autofill/autofill_metrics_unittest.cc +++ b/chrome/browser/autofill/autofill_metrics_unittest.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include <vector> + #include "base/ref_counted.h" #include "base/scoped_ptr.h" #include "base/string16.h" @@ -112,12 +114,40 @@ class TestAutoFillManager : public AutoFillManager { AutoFillManager::metric_logger()); } + void AddSeenForm(FormStructure* form) { + form_structures()->push_back(form); + } + private: bool autofill_enabled_; DISALLOW_COPY_AND_ASSIGN(TestAutoFillManager); }; +class TestFormStructure : public FormStructure { + public: + explicit TestFormStructure(const FormData& form) : FormStructure(form) {} + virtual ~TestFormStructure() {} + + void SetFieldTypes(const std::vector<AutoFillFieldType>& heuristic_types, + const std::vector<AutoFillFieldType>& server_types) { + ASSERT_EQ(field_count(), heuristic_types.size()); + ASSERT_EQ(field_count(), server_types.size()); + + for (size_t i = 0; i < field_count(); ++i) { + AutoFillField* field = (*fields())[i]; + ASSERT_TRUE(field); + field->set_heuristic_type(heuristic_types[i]); + field->set_server_type(server_types[i]); + } + + UpdateAutoFillCount(); + } + + private: + DISALLOW_COPY_AND_ASSIGN(TestFormStructure); +}; + } // namespace class AutoFillMetricsTest : public RenderViewHostTestHarness { @@ -182,7 +212,11 @@ TEST_F(AutoFillMetricsTest, QualityMetrics) { EXPECT_CALL(*autofill_manager_->metric_logger(), Log(AutoFillMetrics::FIELD_SUBMITTED)); EXPECT_CALL(*autofill_manager_->metric_logger(), - Log(AutoFillMetrics::FIELD_AUTOFILL_FAILED )); + Log(AutoFillMetrics::FIELD_AUTOFILL_FAILED)); + EXPECT_CALL(*autofill_manager_->metric_logger(), + Log(AutoFillMetrics::FIELD_HEURISTIC_TYPE_UNKNOWN)); + EXPECT_CALL(*autofill_manager_->metric_logger(), + Log(AutoFillMetrics::FIELD_SERVER_TYPE_UNKNOWN)); EXPECT_CALL(*autofill_manager_->metric_logger(), Log(AutoFillMetrics::FIELD_SUBMITTED)); EXPECT_CALL(*autofill_manager_->metric_logger(), @@ -192,6 +226,117 @@ TEST_F(AutoFillMetricsTest, QualityMetrics) { EXPECT_NO_FATAL_FAILURE(autofill_manager_->OnFormSubmitted(form)); } +// Test that we log the appropriate additional metrics when AutoFill failed. +TEST_F(AutoFillMetricsTest, QualityMetricsForFailure) { + // Set up our form data. + FormData form; + form.name = ASCIIToUTF16("TestForm"); + form.method = ASCIIToUTF16("POST"); + form.origin = GURL("http://example.com/form.html"); + form.action = GURL("http://example.com/submit.html"); + form.user_submitted = true; + + struct { + const char* label; + const char* name; + const char* value; + AutoFillFieldType heuristic_type; + AutoFillFieldType server_type; + AutoFillMetrics::QualityMetric heuristic_metric; + AutoFillMetrics::QualityMetric server_metric; + } failure_cases[] = { + { + "Heuristics unknown, server unknown", "0,0", "Elvis", + UNKNOWN_TYPE, NO_SERVER_DATA, + AutoFillMetrics::FIELD_HEURISTIC_TYPE_UNKNOWN, + AutoFillMetrics::FIELD_SERVER_TYPE_UNKNOWN + }, + { + "Heuristics match, server unknown", "1,0", "Aaron", + NAME_MIDDLE, NO_SERVER_DATA, + AutoFillMetrics::FIELD_HEURISTIC_TYPE_MATCH, + AutoFillMetrics::FIELD_SERVER_TYPE_UNKNOWN + }, + { + "Heuristics mismatch, server unknown", "2,0", "Presley", + PHONE_HOME_NUMBER, NO_SERVER_DATA, + AutoFillMetrics::FIELD_HEURISTIC_TYPE_MISMATCH, + AutoFillMetrics::FIELD_SERVER_TYPE_UNKNOWN + }, + { + "Heuristics unknown, server match", "0,1", "theking@gmail.com", + UNKNOWN_TYPE, EMAIL_ADDRESS, + AutoFillMetrics::FIELD_HEURISTIC_TYPE_UNKNOWN, + AutoFillMetrics::FIELD_SERVER_TYPE_MATCH + }, + { + "Heuristics match, server match", "1,1", "3734 Elvis Presley Blvd.", + ADDRESS_HOME_LINE1, ADDRESS_HOME_LINE1, + AutoFillMetrics::FIELD_HEURISTIC_TYPE_MATCH, + AutoFillMetrics::FIELD_SERVER_TYPE_MATCH + }, + { + "Heuristics mismatch, server match", "2,1", "Apt. 10", + PHONE_HOME_NUMBER, ADDRESS_HOME_LINE2, + AutoFillMetrics::FIELD_HEURISTIC_TYPE_MISMATCH, + AutoFillMetrics::FIELD_SERVER_TYPE_MATCH + }, + { + "Heuristics unknown, server mismatch", "0,2", "Memphis", + UNKNOWN_TYPE, PHONE_HOME_NUMBER, + AutoFillMetrics::FIELD_HEURISTIC_TYPE_UNKNOWN, + AutoFillMetrics::FIELD_SERVER_TYPE_MISMATCH + }, + { + "Heuristics match, server mismatch", "1,2", "Tennessee", + ADDRESS_HOME_STATE, PHONE_HOME_NUMBER, + AutoFillMetrics::FIELD_HEURISTIC_TYPE_MATCH, + AutoFillMetrics::FIELD_SERVER_TYPE_MISMATCH + }, + { + "Heuristics mismatch, server mismatch", "2,2", "38116", + PHONE_HOME_NUMBER, PHONE_HOME_NUMBER, + AutoFillMetrics::FIELD_HEURISTIC_TYPE_MISMATCH, + AutoFillMetrics::FIELD_SERVER_TYPE_MISMATCH + } + }; + + std::vector<AutoFillFieldType> heuristic_types, server_types; + for (size_t i = 0; i < ARRAYSIZE_UNSAFE(failure_cases); ++i) { + FormField field; + autofill_test::CreateTestFormField(failure_cases[i].label, + failure_cases[i].name, + failure_cases[i].value, "text", &field); + form.fields.push_back(field); + heuristic_types.push_back(failure_cases[i].heuristic_type); + server_types.push_back(failure_cases[i].server_type); + + } + + // Simulate having seen this form with the desired heuristic and server types. + // |form_structure| will be owned by |autofill_manager_|. + TestFormStructure* form_structure = new TestFormStructure(form); + form_structure->SetFieldTypes(heuristic_types, server_types); + autofill_manager_->AddSeenForm(form_structure); + + // Establish our expectations. Only print gmock errors, as the warnings are + // too verbose. + ::testing::InSequence dummy; + for (size_t i = 0; i < ARRAYSIZE_UNSAFE(failure_cases); ++i) { + EXPECT_CALL(*autofill_manager_->metric_logger(), + Log(AutoFillMetrics::FIELD_SUBMITTED)); + EXPECT_CALL(*autofill_manager_->metric_logger(), + Log(AutoFillMetrics::FIELD_AUTOFILL_FAILED)); + EXPECT_CALL(*autofill_manager_->metric_logger(), + Log(failure_cases[i].heuristic_metric)); + EXPECT_CALL(*autofill_manager_->metric_logger(), + Log(failure_cases[i].server_metric)); + } + + // Simulate form submission. + EXPECT_NO_FATAL_FAILURE(autofill_manager_->OnFormSubmitted(form)); +} + // Test that we don't log quality metrics for non-autofillable forms. TEST_F(AutoFillMetricsTest, NoQualityMetricsForNonAutoFillableForms) { // Forms must include at least three fields to be auto-fillable. diff --git a/chrome/browser/autofill/form_structure.h b/chrome/browser/autofill/form_structure.h index ad0c411..1c5e7af 100644 --- a/chrome/browser/autofill/form_structure.h +++ b/chrome/browser/autofill/form_structure.h @@ -38,7 +38,7 @@ class AutoFillMetrics; class FormStructure { public: explicit FormStructure(const webkit_glue::FormData& form); - ~FormStructure(); + virtual ~FormStructure(); // Encodes the XML upload request from this FormStructure. bool EncodeUploadRequest(bool auto_fill_used, @@ -106,6 +106,10 @@ class FormStructure { bool operator==(const webkit_glue::FormData& form) const; bool operator!=(const webkit_glue::FormData& form) const; + protected: + // For tests. + ScopedVector<AutoFillField>* fields() { return &fields_; } + private: enum EncodeRequestType { QUERY, |