Add some basic success/failure UMA logging for autofill.
BUG=none
TEST=unit_tests --gtest_filter=AutoFillMetricsTest.*
Review URL: http://codereview.chromium.org/5703002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69232 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/autofill/autofill_download.cc b/chrome/browser/autofill/autofill_download.cc
index 9b2e747..94d9129f 100644
--- a/chrome/browser/autofill/autofill_download.cc
+++ b/chrome/browser/autofill/autofill_download.cc
@@ -67,7 +67,8 @@
}
bool AutoFillDownloadManager::StartQueryRequest(
- const ScopedVector<FormStructure>& forms) {
+ const ScopedVector<FormStructure>& forms,
+ const AutoFillMetrics& metric_logger) {
if (next_query_request_ > base::Time::Now()) {
// We are in back-off mode: do not do the request.
return false;
@@ -79,7 +80,7 @@
return false;
request_data.request_type = AutoFillDownloadManager::REQUEST_QUERY;
- autofill_metrics::LogServerQueryMetric(autofill_metrics::QUERY_SENT);
+ metric_logger.Log(AutoFillMetrics::QUERY_SENT);
return StartRequest(form_xml, request_data);
}
diff --git a/chrome/browser/autofill/autofill_download.h b/chrome/browser/autofill/autofill_download.h
index 5987c9d..488c3b3 100644
--- a/chrome/browser/autofill/autofill_download.h
+++ b/chrome/browser/autofill/autofill_download.h
@@ -16,6 +16,7 @@
#include "chrome/browser/autofill/form_structure.h"
#include "chrome/common/net/url_fetcher.h"
+class AutoFillMetrics;
class Profile;
// Handles getting and updating AutoFill heuristics.
@@ -60,7 +61,8 @@
// Starts a query request to AutoFill servers. The observer is called with the
// list of the fields of all requested forms.
// |forms| - array of forms aggregated in this request.
- bool StartQueryRequest(const ScopedVector<FormStructure>& forms);
+ bool StartQueryRequest(const ScopedVector<FormStructure>& forms,
+ const AutoFillMetrics& metric_logger);
// Start upload request if necessary. The probability of request going
// over the wire are GetPositiveUploadRate() if it was matched by
diff --git a/chrome/browser/autofill/autofill_download_unittest.cc b/chrome/browser/autofill/autofill_download_unittest.cc
index 68173c6..a2f9d78 100644
--- a/chrome/browser/autofill/autofill_download_unittest.cc
+++ b/chrome/browser/autofill/autofill_download_unittest.cc
@@ -8,9 +8,11 @@
#include "base/test/test_timeouts.h"
#include "base/utf_string_conversions.h"
#include "chrome/browser/autofill/autofill_download.h"
+#include "chrome/browser/autofill/autofill_metrics.h"
#include "chrome/common/net/test_url_fetcher_factory.h"
#include "chrome/test/testing_profile.h"
#include "net/url_request/url_request_status.h"
+#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/WebKit/WebKit/chromium/public/WebInputElement.h"
#include "webkit/glue/form_data.h"
@@ -18,6 +20,19 @@
using webkit_glue::FormData;
using WebKit::WebInputElement;
+namespace {
+
+class MockAutoFillMetrics : public AutoFillMetrics {
+ public:
+ MockAutoFillMetrics() {}
+ MOCK_CONST_METHOD1(Log, void(ServerQueryMetric metric));
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockAutoFillMetrics);
+};
+
+} // namespace
+
// This tests AutoFillDownloadManager. AutoFillDownloadTestHelper implements
// AutoFillDownloadManager::Observer and creates an instance of
// AutoFillDownloadManager. Then it records responses to different initiated
@@ -86,8 +101,6 @@
AutoFillDownloadManager download_manager;
};
-namespace {
-
TEST(AutoFillDownloadTest, QueryAndUploadTest) {
MessageLoopForUI message_loop;
// Create and register factory.
@@ -173,7 +186,10 @@
form_structures.push_back(form_structure);
// Request with id 0.
- EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures));
+ MockAutoFillMetrics mock_metric_logger;
+ EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(1);
+ EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures,
+ mock_metric_logger));
// Set upload to 100% so requests happen.
helper.download_manager.SetPositiveUploadRate(1.0);
helper.download_manager.SetNegativeUploadRate(1.0);
@@ -258,7 +274,9 @@
EXPECT_EQ(NULL, fetcher);
// Request with id 3.
- EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures));
+ EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(1);
+ EXPECT_TRUE(helper.download_manager.StartQueryRequest(form_structures,
+ mock_metric_logger));
fetcher = factory.GetFetcherByID(3);
ASSERT_TRUE(fetcher);
fetcher->set_backoff_delay(
@@ -274,7 +292,9 @@
helper.responses_.pop_front();
// Query requests should be ignored for the next 10 seconds.
- EXPECT_FALSE(helper.download_manager.StartQueryRequest(form_structures));
+ EXPECT_CALL(mock_metric_logger, Log(AutoFillMetrics::QUERY_SENT)).Times(0);
+ EXPECT_FALSE(helper.download_manager.StartQueryRequest(form_structures,
+ mock_metric_logger));
fetcher = factory.GetFetcherByID(4);
EXPECT_EQ(NULL, fetcher);
@@ -304,5 +324,3 @@
// Make sure consumer of URLFetcher does the right thing.
URLFetcher::set_factory(NULL);
}
-
-} // namespace
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index e74f6b3..bb19e5f0 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -13,6 +13,7 @@
#include "base/utf_string_conversions.h"
#include "chrome/browser/autofill/autofill_cc_infobar_delegate.h"
#include "chrome/browser/autofill/autofill_dialog.h"
+#include "chrome/browser/autofill/autofill_metrics.h"
#include "chrome/browser/autofill/form_structure.h"
#include "chrome/browser/autofill/phone_number.h"
#include "chrome/browser/autofill/select_control_handler.h"
@@ -128,6 +129,7 @@
personal_data_(NULL),
download_manager_(tab_contents_->profile()),
disable_download_manager_requests_(false),
+ metric_logger_(new AutoFillMetrics),
cc_infobar_(NULL) {
DCHECK(tab_contents);
@@ -174,12 +176,17 @@
// Grab a copy of the form data.
upload_form_structure_.reset(new FormStructure(form));
+ // Disregard forms that we wouldn't ever autofill in the first place.
+ if (!upload_form_structure_->ShouldBeParsed(true))
+ return;
+
+ DeterminePossibleFieldTypesForUpload();
+ // TODO(isherman): Consider uploading to server here rather than in
+ // HandleSubmit().
+
if (!upload_form_structure_->IsAutoFillable(true))
return;
- // Determine the possible field types and upload the form structure to the
- // PersonalDataManager.
- DeterminePossibleFieldTypes(upload_form_structure_.get());
HandleSubmit();
}
@@ -398,7 +405,8 @@
UploadRequired upload_required;
FormStructure::ParseQueryResponse(heuristic_xml,
form_structures_.get(),
- &upload_required);
+ &upload_required,
+ *metric_logger_);
}
void AutoFillManager::OnUploadedAutoFillHeuristics(
@@ -425,13 +433,38 @@
return prefs->GetBoolean(prefs::kAutoFillEnabled);
}
-void AutoFillManager::DeterminePossibleFieldTypes(
- FormStructure* form_structure) {
- for (size_t i = 0; i < form_structure->field_count(); i++) {
- const AutoFillField* field = form_structure->field(i);
+void AutoFillManager::DeterminePossibleFieldTypesForUpload() {
+ for (size_t i = 0; i < upload_form_structure_->field_count(); i++) {
+ const AutoFillField* field = upload_form_structure_->field(i);
FieldTypeSet field_types;
personal_data_->GetPossibleFieldTypes(field->value(), &field_types);
- form_structure->set_possible_types(i, field_types);
+ DCHECK(!field_types.empty());
+ upload_form_structure_->set_possible_types(i, field_types);
+
+ if (field->form_control_type() == ASCIIToUTF16("select-one")) {
+ // TODO(isherman): <select> fields don't support |is_autofilled()|. Since
+ // this is heavily relied upon by our metrics, we just don't log anything
+ // for all <select> fields. Better to have less data than misleading data.
+ continue;
+ }
+
+ // Log various quality metrics.
+ 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);
+
+ // 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
+ // * Whether the server type matches the heursitic type
+ // - Perhaps only if at least one of the types is not unknown/no data.
+ }
}
}
@@ -495,7 +528,8 @@
: tab_contents_(NULL),
personal_data_(NULL),
download_manager_(NULL),
- disable_download_manager_requests_(false),
+ disable_download_manager_requests_(true),
+ metric_logger_(new AutoFillMetrics),
cc_infobar_(NULL) {
}
@@ -504,11 +538,17 @@
: tab_contents_(tab_contents),
personal_data_(personal_data),
download_manager_(NULL),
- disable_download_manager_requests_(false),
+ disable_download_manager_requests_(true),
+ metric_logger_(new AutoFillMetrics),
cc_infobar_(NULL) {
DCHECK(tab_contents);
}
+void AutoFillManager::set_metric_logger(
+ const AutoFillMetrics* metric_logger) {
+ metric_logger_.reset(metric_logger);
+}
+
bool AutoFillManager::GetHost(const std::vector<AutoFillProfile*>& profiles,
const std::vector<CreditCard*>& credit_cards,
RenderViewHost** host) {
@@ -709,8 +749,10 @@
}
// 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_);
+ if (!form_structures_.empty() && !disable_download_manager_requests_) {
+ download_manager_.StartQueryRequest(form_structures_,
+ *metric_logger_);
+ }
for (std::vector<FormStructure *>::const_iterator iter =
non_queryable_forms.begin();
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index 29e6b23..3a68336 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -20,6 +20,7 @@
class AutoFillCCInfoBarDelegate;
class AutoFillProfile;
+class AutoFillMetrics;
class CreditCard;
class FormStructure;
class PrefService;
@@ -74,10 +75,6 @@
// Returns the value of the AutoFillEnabled pref.
virtual bool IsAutoFillEnabled() const;
- // Uses heuristics and existing personal data to determine the possible field
- // types.
- void DeterminePossibleFieldTypes(FormStructure* form_structure);
-
// Handles the form data submitted by the user.
void HandleSubmit();
@@ -94,6 +91,11 @@
personal_data_ = personal_data;
}
+ const AutoFillMetrics* metric_logger() const {
+ return metric_logger_.get();
+ }
+ void set_metric_logger(const AutoFillMetrics* metric_logger);
+
// 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);
@@ -160,10 +162,9 @@
// Parses the forms using heuristic matching and querying the AutoFill server.
void ParseForms(const std::vector<webkit_glue::FormData>& forms);
- // The following function is meant to be called from unit-test only.
- void set_disable_download_manager_requests(bool value) {
- disable_download_manager_requests_ = value;
- }
+ // Uses existing personal data to determine possible field types for the
+ // |upload_form_structure_|.
+ void DeterminePossibleFieldTypesForUpload();
// The TabContents hosting this AutoFillManager.
// Weak reference.
@@ -182,9 +183,13 @@
// Should be set to true in AutoFillManagerTest and other tests, false in
// AutoFillDownloadManagerTest and in non-test environment. Is false by
- // default.
+ // default for the public constructor, and true by default for the test-only
+ // constructors.
bool disable_download_manager_requests_;
+ // For logging UMA metrics. Overridden by metrics tests.
+ scoped_ptr<const AutoFillMetrics> metric_logger_;
+
// Our copy of the form data.
ScopedVector<FormStructure> form_structures_;
@@ -200,7 +205,6 @@
std::map<int, std::string> id_guid_map_;
friend class FormStructureBrowserTest;
- friend class TestAutoFillManager;
FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillCreditCardForm);
FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillAddressForm);
FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillAddressAndCreditCardForm);
diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc
index ffaa5363..bf6877f 100644
--- a/chrome/browser/autofill/autofill_manager_unittest.cc
+++ b/chrome/browser/autofill/autofill_manager_unittest.cc
@@ -183,7 +183,7 @@
form->fields.push_back(field);
}
-// Populates |form| with data corresponding to a simple credit card form, with.
+// Populates |form| with data corresponding to a simple credit card form.
// Note that this actually appends fields to the form data, which can be useful
// for building up more complex test forms.
void CreateTestCreditCardFormData(FormData* form, bool is_https) {
@@ -364,19 +364,13 @@
has_address_fields, true);
}
-} // namespace
-
class TestAutoFillManager : public AutoFillManager {
public:
TestAutoFillManager(TabContents* tab_contents,
TestPersonalDataManager* personal_manager)
- : AutoFillManager(tab_contents, NULL),
+ : AutoFillManager(tab_contents, personal_manager),
autofill_enabled_(true) {
test_personal_data_ = personal_manager;
- set_personal_data_manager(personal_manager);
- // Download manager requests are disabled for purposes of this unit test.
- // These requests are tested in autofill_download_unittest.cc.
- set_disable_download_manager_requests(true);
}
virtual bool IsAutoFillEnabled() const { return autofill_enabled_; }
@@ -425,6 +419,8 @@
DISALLOW_COPY_AND_ASSIGN(TestAutoFillManager);
};
+} // namespace
+
class AutoFillManagerTest : public RenderViewHostTestHarness {
public:
AutoFillManagerTest() {}
diff --git a/chrome/browser/autofill/autofill_metrics.cc b/chrome/browser/autofill/autofill_metrics.cc
index 85b5517..bae11d7 100644
--- a/chrome/browser/autofill/autofill_metrics.cc
+++ b/chrome/browser/autofill/autofill_metrics.cc
@@ -6,13 +6,22 @@
#include "base/metrics/histogram.h"
-namespace autofill_metrics {
+AutoFillMetrics::AutoFillMetrics() {
+}
-void LogServerQueryMetric(ServerQueryMetricType type) {
- DCHECK(type < NUM_SERVER_QUERY_METRICS);
+AutoFillMetrics::~AutoFillMetrics() {
+}
- UMA_HISTOGRAM_ENUMERATION("AutoFill.ServerQueryResponse", type,
+void AutoFillMetrics::Log(ServerQueryMetric metric) const {
+ DCHECK(metric < NUM_SERVER_QUERY_METRICS);
+
+ UMA_HISTOGRAM_ENUMERATION("AutoFill.ServerQueryResponse", metric,
NUM_SERVER_QUERY_METRICS);
}
-} // namespace autofill_metrics
+void AutoFillMetrics::Log(QualityMetric metric) const {
+ DCHECK(metric < NUM_QUALITY_METRICS);
+
+ UMA_HISTOGRAM_ENUMERATION("AutoFill.Quality", metric,
+ NUM_QUALITY_METRICS);
+}
diff --git a/chrome/browser/autofill/autofill_metrics.h b/chrome/browser/autofill/autofill_metrics.h
index d461668..6318274f 100644
--- a/chrome/browser/autofill/autofill_metrics.h
+++ b/chrome/browser/autofill/autofill_metrics.h
@@ -6,32 +6,56 @@
#define CHROME_BROWSER_AUTOFILL_AUTOFILL_METRICS_H_
#pragma once
-namespace autofill_metrics {
+#include "base/basictypes.h"
-// Each of these should be logged at most once per query to the server, which in
-// turn should occur at most once per page load.
-enum ServerQueryMetricType {
- // Logged for each query sent to the server
- QUERY_SENT = 0,
- // Logged for each query response received from the server
- QUERY_RESPONSE_RECEIVED,
- // Logged for each parsable response received from the server
- QUERY_RESPONSE_PARSED,
- // Logged for each parsable response that provided no improvements relative to
- // our heuristics.
- QUERY_RESPONSE_MATCHED_LOCAL_HEURISTICS,
- // Logged for each page for which our heuristics detected at least one
- // auto-fillable field, but the server response overrode the type of at least
- // one field
- QUERY_RESPONSE_OVERRODE_LOCAL_HEURISTICS,
- // Logged for each page for which our heuristics did not detect any
- // auto-fillable fields, but the server response did detect some.
- QUERY_RESPONSE_WITH_NO_LOCAL_HEURISTICS,
- NUM_SERVER_QUERY_METRICS
+class AutoFillMetrics {
+ public:
+ // Each of these is logged at most once per query to the server, which in turn
+ // occurs at most once per page load.
+ enum ServerQueryMetric {
+ // Logged for each query sent to the server.
+ QUERY_SENT = 0,
+ // Logged for each query response received from the server.
+ QUERY_RESPONSE_RECEIVED,
+ // Logged for each parsable response received from the server.
+ QUERY_RESPONSE_PARSED,
+ // Logged for each parsable response that provided no improvements relative
+ // to our heuristics.
+ QUERY_RESPONSE_MATCHED_LOCAL_HEURISTICS,
+ // Logged for each page for which our heuristics detected at least one
+ // auto-fillable field, but the server response overrode the type of at
+ // least one field.
+ QUERY_RESPONSE_OVERRODE_LOCAL_HEURISTICS,
+ // Logged for each page for which our heuristics did not detect any
+ // auto-fillable fields, but the server response did detect some.
+ QUERY_RESPONSE_WITH_NO_LOCAL_HEURISTICS,
+ NUM_SERVER_QUERY_METRICS
+ };
+
+ // Each of these is logged at most once per form submission.
+ enum QualityMetric {
+ // Logged for each field in a submitted form.
+ FIELD_SUBMITTED = 0,
+ // A simple successs metric, logged for each field that returns true for
+ // |is_autofilled()| and has a value that is present in the personal data
+ // manager. There is a small chance of false positives from filling via
+ // autocomplete rather than autofill.
+ FIELD_AUTOFILLED,
+ // A simple failure metric, logged for each field that returns false for
+ // |is_autofilled()| but as a value that is present in the personal data
+ // manager.
+ FIELD_AUTOFILL_FAILED,
+ NUM_QUALITY_METRICS
+ };
+
+ AutoFillMetrics();
+ virtual ~AutoFillMetrics();
+
+ virtual void Log(ServerQueryMetric metric) const;
+ virtual void Log(QualityMetric metric) const;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(AutoFillMetrics);
};
-void LogServerQueryMetric(ServerQueryMetricType type);
-
-} // namespace autofill_metrics
-
#endif // CHROME_BROWSER_AUTOFILL_AUTOFILL_METRICS_H_
diff --git a/chrome/browser/autofill/autofill_metrics_unittest.cc b/chrome/browser/autofill/autofill_metrics_unittest.cc
new file mode 100644
index 0000000..4521296
--- /dev/null
+++ b/chrome/browser/autofill/autofill_metrics_unittest.cc
@@ -0,0 +1,229 @@
+// Copyright (c) 2010 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/ref_counted.h"
+#include "base/scoped_ptr.h"
+#include "base/string16.h"
+#include "base/utf_string_conversions.h"
+#include "chrome/browser/autofill/autofill_common_test.h"
+#include "chrome/browser/autofill/autofill_manager.h"
+#include "chrome/browser/autofill/autofill_metrics.h"
+#include "chrome/browser/autofill/personal_data_manager.h"
+#include "chrome/browser/renderer_host/test/test_render_view_host.h"
+#include "chrome/browser/tab_contents/test_tab_contents.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/glue/form_data.h"
+#include "webkit/glue/form_field.h"
+
+using webkit_glue::FormData;
+using webkit_glue::FormField;
+
+namespace {
+
+// TODO(isherman): Move this into autofill_common_test.h?
+class TestPersonalDataManager : public PersonalDataManager {
+ public:
+ TestPersonalDataManager() {
+ CreateTestAutoFillProfiles(&web_profiles_);
+ CreateTestCreditCards(&credit_cards_);
+ }
+
+ virtual void InitializeIfNeeded() {}
+ virtual void SaveImportedFormData() {}
+ virtual bool IsDataLoaded() const { return true; }
+
+ private:
+ void CreateTestAutoFillProfiles(ScopedVector<AutoFillProfile>* profiles) {
+ AutoFillProfile* profile = new AutoFillProfile;
+ autofill_test::SetProfileInfo(profile, "Home", "Elvis", "Aaron",
+ "Presley", "[email protected]", "RCA",
+ "3734 Elvis Presley Blvd.", "Apt. 10",
+ "Memphis", "Tennessee", "38116", "USA",
+ "12345678901", "");
+ profile->set_guid("00000000-0000-0000-0000-000000000001");
+ profiles->push_back(profile);
+ profile = new AutoFillProfile;
+ autofill_test::SetProfileInfo(profile, "Work", "Charles", "Hardin",
+ "Holley", "[email protected]", "Decca",
+ "123 Apple St.", "unit 6", "Lubbock",
+ "Texas", "79401", "USA", "23456789012",
+ "");
+ profile->set_guid("00000000-0000-0000-0000-000000000002");
+ profiles->push_back(profile);
+ profile = new AutoFillProfile;
+ autofill_test::SetProfileInfo(profile, "Empty", "", "", "", "", "", "", "",
+ "", "", "", "", "", "");
+ profile->set_guid("00000000-0000-0000-0000-000000000003");
+ profiles->push_back(profile);
+ }
+
+ void CreateTestCreditCards(ScopedVector<CreditCard>* credit_cards) {
+ CreditCard* credit_card = new CreditCard;
+ autofill_test::SetCreditCardInfo(credit_card, "First", "Elvis Presley",
+ "4234567890123456", // Visa
+ "04", "2012");
+ credit_card->set_guid("00000000-0000-0000-0000-000000000004");
+ credit_cards->push_back(credit_card);
+ credit_card = new CreditCard;
+ autofill_test::SetCreditCardInfo(credit_card, "Second", "Buddy Holly",
+ "5187654321098765", // Mastercard
+ "10", "2014");
+ credit_card->set_guid("00000000-0000-0000-0000-000000000005");
+ credit_cards->push_back(credit_card);
+ credit_card = new CreditCard;
+ autofill_test::SetCreditCardInfo(credit_card, "Empty", "", "", "", "");
+ credit_card->set_guid("00000000-0000-0000-0000-000000000006");
+ credit_cards->push_back(credit_card);
+ }
+
+ DISALLOW_COPY_AND_ASSIGN(TestPersonalDataManager);
+};
+
+class MockAutoFillMetrics : public AutoFillMetrics {
+ public:
+ MockAutoFillMetrics() {}
+ MOCK_CONST_METHOD1(Log, void(ServerQueryMetric metric));
+ MOCK_CONST_METHOD1(Log, void(QualityMetric metric));
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockAutoFillMetrics);
+};
+
+class TestAutoFillManager : public AutoFillManager {
+ public:
+ TestAutoFillManager(TabContents* tab_contents,
+ TestPersonalDataManager* personal_manager)
+ : AutoFillManager(tab_contents, personal_manager),
+ autofill_enabled_(true) {
+ set_metric_logger(new MockAutoFillMetrics);
+ }
+ virtual ~TestAutoFillManager() {}
+
+ virtual bool IsAutoFillEnabled() const { return autofill_enabled_; }
+
+ void set_autofill_enabled(bool autofill_enabled) {
+ autofill_enabled_ = autofill_enabled;
+ }
+
+ const MockAutoFillMetrics* metric_logger() const {
+ return static_cast<const MockAutoFillMetrics*>(
+ AutoFillManager::metric_logger());
+ }
+
+ private:
+ bool autofill_enabled_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestAutoFillManager);
+};
+
+} // namespace
+
+class AutoFillMetricsTest : public RenderViewHostTestHarness {
+ public:
+ AutoFillMetricsTest() {}
+ virtual ~AutoFillMetricsTest() {
+ // Order of destruction is important as AutoFillManager relies on
+ // PersonalDataManager to be around when it gets destroyed.
+ autofill_manager_.reset(NULL);
+ test_personal_data_ = NULL;
+ }
+
+ virtual void SetUp() {
+ RenderViewHostTestHarness::SetUp();
+ test_personal_data_ = new TestPersonalDataManager();
+ autofill_manager_.reset(new TestAutoFillManager(contents(),
+ test_personal_data_.get()));
+ }
+
+ protected:
+ scoped_ptr<TestAutoFillManager> autofill_manager_;
+ scoped_refptr<TestPersonalDataManager> test_personal_data_;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(AutoFillMetricsTest);
+};
+
+// Test that we log quality metrics appropriately.
+TEST_F(AutoFillMetricsTest, QualityMetrics) {
+ // 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;
+
+ FormField field;
+ autofill_test::CreateTestFormField(
+ "Autofilled", "autofilled", "Elvis Presley", "text", &field);
+ field.set_autofilled(true);
+ form.fields.push_back(field);
+ autofill_test::CreateTestFormField(
+ "Autofill Failed", "autofillfailed", "[email protected]", "text", &field);
+ form.fields.push_back(field);
+ autofill_test::CreateTestFormField(
+ "Empty", "empty", "", "text", &field);
+ form.fields.push_back(field);
+ autofill_test::CreateTestFormField(
+ "Unknown", "unknown", "garbage", "text", &field);
+ form.fields.push_back(field);
+ autofill_test::CreateTestFormField(
+ "Select", "select", "USA", "select-one", &field);
+ form.fields.push_back(field);
+
+ // Establish our expectations.
+ ::testing::InSequence dummy;
+ EXPECT_CALL(*autofill_manager_->metric_logger(),
+ Log(AutoFillMetrics::FIELD_SUBMITTED));
+ EXPECT_CALL(*autofill_manager_->metric_logger(),
+ Log(AutoFillMetrics::FIELD_AUTOFILLED));
+ 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(AutoFillMetrics::FIELD_SUBMITTED));
+ EXPECT_CALL(*autofill_manager_->metric_logger(),
+ Log(AutoFillMetrics::FIELD_SUBMITTED));
+
+ // Simulate form submission.
+ EXPECT_NO_FATAL_FAILURE(autofill_manager_->FormSubmitted(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.
+ 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;
+
+ FormField field;
+ autofill_test::CreateTestFormField(
+ "Autofilled", "autofilled", "Elvis Presley", "text", &field);
+ field.set_autofilled(true);
+ form.fields.push_back(field);
+ autofill_test::CreateTestFormField(
+ "Autofill Failed", "autofillfailed", "[email protected]", "text", &field);
+ form.fields.push_back(field);
+
+ // Simulate form submission.
+ EXPECT_CALL(*autofill_manager_->metric_logger(),
+ Log(AutoFillMetrics::FIELD_SUBMITTED)).Times(0);
+ EXPECT_NO_FATAL_FAILURE(autofill_manager_->FormSubmitted(form));
+
+ // Search forms are not auto-fillable.
+ form.action = GURL("http://example.com/search?q=Elvis%20Presley");
+ autofill_test::CreateTestFormField(
+ "Empty", "empty", "", "text", &field);
+ form.fields.push_back(field);
+
+ // Simulate form submission.
+ EXPECT_CALL(*autofill_manager_->metric_logger(),
+ Log(AutoFillMetrics::FIELD_SUBMITTED)).Times(0);
+ EXPECT_NO_FATAL_FAILURE(autofill_manager_->FormSubmitted(form));
+}
diff --git a/chrome/browser/autofill/form_structure.cc b/chrome/browser/autofill/form_structure.cc
index 2958df67..8ae8eb23 100644
--- a/chrome/browser/autofill/form_structure.cc
+++ b/chrome/browser/autofill/form_structure.cc
@@ -194,9 +194,9 @@
// static
void FormStructure::ParseQueryResponse(const std::string& response_xml,
const std::vector<FormStructure*>& forms,
- UploadRequired* upload_required) {
- autofill_metrics::LogServerQueryMetric(
- autofill_metrics::QUERY_RESPONSE_RECEIVED);
+ UploadRequired* upload_required,
+ const AutoFillMetrics& metric_logger) {
+ metric_logger.Log(AutoFillMetrics::QUERY_RESPONSE_RECEIVED);
// Parse the field types from the server response to the query.
std::vector<AutoFillFieldType> field_types;
@@ -206,8 +206,7 @@
if (!parse_handler.succeeded())
return;
- autofill_metrics::LogServerQueryMetric(
- autofill_metrics::QUERY_RESPONSE_PARSED);
+ metric_logger.Log(AutoFillMetrics::QUERY_RESPONSE_PARSED);
bool heuristics_detected_fillable_field = false;
bool query_response_overrode_heuristics = false;
@@ -250,17 +249,17 @@
form->UpdateAutoFillCount();
}
- autofill_metrics::ServerQueryMetricType metric_type;
+ AutoFillMetrics::ServerQueryMetric metric;
if (query_response_overrode_heuristics) {
if (heuristics_detected_fillable_field) {
- metric_type = autofill_metrics::QUERY_RESPONSE_OVERRODE_LOCAL_HEURISTICS;
+ metric = AutoFillMetrics::QUERY_RESPONSE_OVERRODE_LOCAL_HEURISTICS;
} else {
- metric_type = autofill_metrics::QUERY_RESPONSE_WITH_NO_LOCAL_HEURISTICS;
+ metric = AutoFillMetrics::QUERY_RESPONSE_WITH_NO_LOCAL_HEURISTICS;
}
} else {
- metric_type = autofill_metrics::QUERY_RESPONSE_MATCHED_LOCAL_HEURISTICS;
+ metric = AutoFillMetrics::QUERY_RESPONSE_MATCHED_LOCAL_HEURISTICS;
}
- autofill_metrics::LogServerQueryMetric(metric_type);
+ metric_logger.Log(metric);
}
std::string FormStructure::FormSignature() const {
diff --git a/chrome/browser/autofill/form_structure.h b/chrome/browser/autofill/form_structure.h
index fe4c5da..ad0c411f 100644
--- a/chrome/browser/autofill/form_structure.h
+++ b/chrome/browser/autofill/form_structure.h
@@ -31,6 +31,8 @@
USE_UPLOAD_RATES
};
+class AutoFillMetrics;
+
// FormStructure stores a single HTML form together with the values entered
// in the fields along with additional information needed by AutoFill.
class FormStructure {
@@ -55,7 +57,8 @@
// same as the one passed to EncodeQueryRequest when constructing the query.
static void ParseQueryResponse(const std::string& response_xml,
const std::vector<FormStructure*>& forms,
- UploadRequired* upload_required);
+ UploadRequired* upload_required,
+ const AutoFillMetrics& metric_logger);
// The unique signature for this form, composed of the target url domain,
// the form name, and the form field names in a 64-bit hash.
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 4b9116b..37f27a2 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1025,6 +1025,7 @@
'browser/autofill/autofill_field_unittest.cc',
'browser/autofill/autofill_ie_toolbar_import_win_unittest.cc',
'browser/autofill/autofill_manager_unittest.cc',
+ 'browser/autofill/autofill_metrics_unittest.cc',
'browser/autofill/autofill_profile_unittest.cc',
'browser/autofill/autofill_type_unittest.cc',
'browser/autofill/autofill_xml_parser_unittest.cc',
