Add metric for whether Autofill is enabled or disabled.

BUG=none
TEST=unit_tests --gtest_filter=AutofillMetrics*

Review URL: http://codereview.chromium.org/6826047

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@81474 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 109 insertions, 47 deletions

diff --git a/base/metrics/histogram.h b/base/metrics/histogram.h
index b7ae10a..bc58ee5 100644
--- a/base/metrics/histogram.h
+++ b/base/metrics/histogram.h
@@ -229,6 +229,15 @@ class Lock;
 #define UMA_HISTOGRAM_PERCENTAGE(name, under_one_hundred) \
     UMA_HISTOGRAM_ENUMERATION(name, under_one_hundred, 101)
 
+#define UMA_HISTOGRAM_BOOLEAN(name, sample) do { \
+    static base::Histogram* counter(NULL); \
+    if (!counter) \
+      counter = base::BooleanHistogram::FactoryGet(name, \
+          base::Histogram::kUmaTargetedHistogramFlag); \
+    DCHECK_EQ(name, counter->histogram_name()); \
+    counter->AddBoolean(sample); \
+  } while (0)
+
 #define UMA_HISTOGRAM_ENUMERATION(name, sample, boundary_value) do { \
     static base::Histogram* counter(NULL); \
     if (!counter) \
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index ab4e936..ea32437 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -227,6 +227,7 @@ AutofillManager::AutofillManager(TabContents* tab_contents)
       download_manager_(tab_contents->profile()),
       disable_download_manager_requests_(false),
       metric_logger_(new AutofillMetrics),
+      has_logged_autofill_enabled_(false),
       has_logged_address_suggestions_count_(false) {
   DCHECK(tab_contents);
 
@@ -328,7 +329,13 @@ void AutofillManager::OnFormSubmitted(const FormData& form) {
 }
 
 void AutofillManager::OnFormsSeen(const std::vector<FormData>& forms) {
-  if (!IsAutofillEnabled())
+  bool enabled = IsAutofillEnabled();
+  if (!has_logged_autofill_enabled_) {
+    metric_logger_->LogIsAutofillEnabledAtPageLoad(enabled);
+    has_logged_autofill_enabled_ = true;
+  }
+
+  if (!enabled)
     return;
 
   ParseForms(forms);
@@ -601,18 +608,8 @@ void AutofillManager::OnHeuristicsRequestError(
 }
 
 bool AutofillManager::IsAutofillEnabled() const {
-  PrefService* prefs =
-      const_cast<AutofillManager*>(this)->tab_contents()->profile()->GetPrefs();
-
-  // Migrate obsolete Autofill pref.
-  if (prefs->FindPreference(prefs::kFormAutofillEnabled)) {
-    bool enabled = prefs->GetBoolean(prefs::kFormAutofillEnabled);
-    prefs->ClearPref(prefs::kFormAutofillEnabled);
-    prefs->SetBoolean(prefs::kAutofillEnabled, enabled);
-    return enabled;
-  }
-
-  return prefs->GetBoolean(prefs::kAutofillEnabled);
+  return const_cast<AutofillManager*>(this)->tab_contents()->profile()->
+      GetPrefs()->GetBoolean(prefs::kAutofillEnabled);
 }
 
 void AutofillManager::DeterminePossibleFieldTypesForUpload(
@@ -671,6 +668,7 @@ void AutofillManager::UploadFormData(const FormStructure& submitted_form) {
 
 void AutofillManager::Reset() {
   form_structures_.reset();
+  has_logged_autofill_enabled_ = false;
   has_logged_address_suggestions_count_ = false;
 }
 
@@ -681,6 +679,7 @@ AutofillManager::AutofillManager(TabContents* tab_contents,
       download_manager_(NULL),
       disable_download_manager_requests_(true),
       metric_logger_(new AutofillMetrics),
+      has_logged_autofill_enabled_(false),
       has_logged_address_suggestions_count_(false) {
   DCHECK(tab_contents);
 }
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index a19f55b..de509eb9 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -215,12 +215,15 @@ class AutofillManager : public TabContentsObserver,
   // For logging UMA metrics. Overridden by metrics tests.
   scoped_ptr<const AutofillMetrics> metric_logger_;
 
-  // Our copy of the form data.
-  ScopedVector<FormStructure> form_structures_;
+  // Have we logged whether Autofill is enabled for this page load?
+  bool has_logged_autofill_enabled_;
 
   // Have we logged an address suggestions count metric for this page?
   bool has_logged_address_suggestions_count_;
 
+  // Our copy of the form data.
+  ScopedVector<FormStructure> form_structures_;
+
   // GUID to ID mapping.  We keep two maps to convert back and forth.
   std::map<GUIDPair, int> guid_id_map_;
   std::map<int, GUIDPair> id_guid_map_;
@@ -244,6 +247,7 @@ class AutofillManager : public TabContentsObserver,
   FRIEND_TEST_ALL_PREFIXES(AutofillManagerTest, FormSubmitted);
   FRIEND_TEST_ALL_PREFIXES(AutofillManagerTest, FormSubmittedServerTypes);
   FRIEND_TEST_ALL_PREFIXES(AutofillMetricsTest, AddressSuggestionsCount);
+  FRIEND_TEST_ALL_PREFIXES(AutofillMetricsTest, AutofillIsEnabledAtPageLoad);
   FRIEND_TEST_ALL_PREFIXES(AutofillMetricsTest,
                            NoQualityMetricsForNonAutofillableForms);
   FRIEND_TEST_ALL_PREFIXES(AutofillMetricsTest, QualityMetrics);
diff --git a/chrome/browser/autofill/autofill_metrics.cc b/chrome/browser/autofill/autofill_metrics.cc
index 922fdc9..484fd09 100644
--- a/chrome/browser/autofill/autofill_metrics.cc
+++ b/chrome/browser/autofill/autofill_metrics.cc
@@ -94,6 +94,14 @@ void AutofillMetrics::Log(ServerTypeQualityMetric metric,
                              NUM_SERVER_TYPE_QUALITY_METRICS);
 }
 
+void AutofillMetrics::LogIsAutofillEnabledAtStartup(bool enabled) const {
+  UMA_HISTOGRAM_BOOLEAN("Autofill.IsEnabled.Startup", enabled);
+}
+
+void AutofillMetrics::LogIsAutofillEnabledAtPageLoad(bool enabled) const {
+  UMA_HISTOGRAM_BOOLEAN("Autofill.IsEnabled.PageLoad", enabled);
+}
+
 void AutofillMetrics::LogStoredProfileCount(size_t num_profiles) const {
   UMA_HISTOGRAM_COUNTS("Autofill.StoredProfileCount", num_profiles);
 }
diff --git a/chrome/browser/autofill/autofill_metrics.h b/chrome/browser/autofill/autofill_metrics.h
index 1ecab3c..cc2aeb5 100644
--- a/chrome/browser/autofill/autofill_metrics.h
+++ b/chrome/browser/autofill/autofill_metrics.h
@@ -103,7 +103,13 @@ class AutofillMetrics {
   virtual void Log(ServerTypeQualityMetric metric,
                    const std::string& experiment_id) const;
 
-  // This should be called at most once per run.
+  // This should be called each time a page containing forms is loaded.
+  virtual void LogIsAutofillEnabledAtPageLoad(bool enabled) const;
+
+  // This should be called each time a new profile is launched.
+  virtual void LogIsAutofillEnabledAtStartup(bool enabled) const;
+
+  // This should be called each time a new profile is launched.
   virtual void LogStoredProfileCount(size_t num_profiles) const;
 
   // Log the number of Autofill suggestions presented to the user when filling a
diff --git a/chrome/browser/autofill/autofill_metrics_unittest.cc b/chrome/browser/autofill/autofill_metrics_unittest.cc
index 4d3f54e..eb2217d 100644
--- a/chrome/browser/autofill/autofill_metrics_unittest.cc
+++ b/chrome/browser/autofill/autofill_metrics_unittest.cc
@@ -39,6 +39,8 @@ class MockAutofillMetrics : public AutofillMetrics {
                                const std::string& experiment_id));
   MOCK_CONST_METHOD2(Log, void(PredictedTypeQualityMetric metric,
                                const std::string& experiment_id));
+  MOCK_CONST_METHOD1(LogIsAutofillEnabledAtPageLoad, void(bool enabled));
+  MOCK_CONST_METHOD1(LogIsAutofillEnabledAtStartup, void(bool enabled));
   MOCK_CONST_METHOD1(LogStoredProfileCount, void(size_t num_profiles));
   MOCK_CONST_METHOD1(LogAddressSuggestionsCount, void(size_t num_suggestions));
 
@@ -48,14 +50,14 @@ class MockAutofillMetrics : public AutofillMetrics {
 
 class TestPersonalDataManager : public PersonalDataManager {
  public:
-  TestPersonalDataManager() {
+  TestPersonalDataManager() : autofill_enabled_(true) {
     set_metric_logger(new MockAutofillMetrics);
     CreateTestAutofillProfiles(&web_profiles_);
   }
 
   // Overridden to avoid a trip to the database. This should be a no-op except
   // for the side-effect of logging the profile count.
-  virtual void LoadProfiles() {
+  virtual void LoadProfiles() OVERRIDE {
     std::vector<AutofillProfile*> profiles;
     web_profiles_.release(&profiles);
     WDResult<std::vector<AutofillProfile*> > result(AUTOFILL_PROFILES_RESULT,
@@ -63,6 +65,9 @@ class TestPersonalDataManager : public PersonalDataManager {
     ReceiveLoadedProfiles(0, &result);
   }
 
+  // Overridden to avoid a trip to the database.
+  virtual void LoadCreditCards() OVERRIDE {}
+
   // Adds |profile| to |web_profiles_| and takes ownership of the profile's
   // memory.
   virtual void AddProfile(AutofillProfile* profile) {
@@ -74,8 +79,12 @@ class TestPersonalDataManager : public PersonalDataManager {
         PersonalDataManager::metric_logger());
   }
 
-  static void ResetHasLoggedProfileCount() {
-    PersonalDataManager::set_has_logged_profile_count(false);
+  void set_autofill_enabled(bool autofill_enabled) {
+    autofill_enabled_ = autofill_enabled;
+  }
+
+  virtual bool IsAutofillEnabled() const OVERRIDE {
+    return autofill_enabled_;
   }
 
   MOCK_METHOD1(SaveImportedCreditCard,
@@ -101,6 +110,8 @@ class TestPersonalDataManager : public PersonalDataManager {
     profiles->push_back(profile);
   }
 
+  bool autofill_enabled_;
+
   DISALLOW_COPY_AND_ASSIGN(TestPersonalDataManager);
 };
 
@@ -671,8 +682,6 @@ TEST_F(AutofillMetricsTest, QualityMetricsWithExperimentId) {
 
 // Test that the profile count is logged correctly.
 TEST_F(AutofillMetricsTest, StoredProfileCount) {
-  TestPersonalDataManager::ResetHasLoggedProfileCount();
-
   // The metric should be logged when the profiles are first loaded.
   EXPECT_CALL(*test_personal_data_->metric_logger(),
               LogStoredProfileCount(2)).Times(1);
@@ -684,6 +693,19 @@ TEST_F(AutofillMetricsTest, StoredProfileCount) {
   test_personal_data_->LoadProfiles();
 }
 
+// Test that we correctly log whether Autofill is enabled.
+TEST_F(AutofillMetricsTest, AutofillIsEnabledAtStartup) {
+  test_personal_data_->set_autofill_enabled(true);
+  EXPECT_CALL(*test_personal_data_->metric_logger(),
+              LogIsAutofillEnabledAtStartup(true)).Times(1);
+  test_personal_data_->Init(NULL);
+
+  test_personal_data_->set_autofill_enabled(false);
+  EXPECT_CALL(*test_personal_data_->metric_logger(),
+              LogIsAutofillEnabledAtStartup(false)).Times(1);
+  test_personal_data_->Init(NULL);
+}
+
 // Test that we log the number of Autofill suggestions when filling a form.
 TEST_F(AutofillMetricsTest, AddressSuggestionsCount) {
   // Set up our form data.
@@ -713,6 +735,7 @@ TEST_F(AutofillMetricsTest, AddressSuggestionsCount) {
   autofill_manager_->AddSeenForm(form_structure);
 
   // Establish our expectations.
+  ::testing::FLAGS_gmock_verbose = "error";
   ::testing::InSequence dummy;
   EXPECT_CALL(*autofill_manager_->metric_logger(),
               LogAddressSuggestionsCount(2)).Times(1);
@@ -753,6 +776,28 @@ TEST_F(AutofillMetricsTest, AddressSuggestionsCount) {
   autofill_manager_->OnQueryFormFieldAutofill(0, form, field);
 }
 
+// Test that we log whether Autofill is enabled when filling a form.
+TEST_F(AutofillMetricsTest, AutofillIsEnabledAtPageLoad) {
+  // Establish our expectations.
+  ::testing::FLAGS_gmock_verbose = "error";
+  ::testing::InSequence dummy;
+  EXPECT_CALL(*autofill_manager_->metric_logger(),
+              LogIsAutofillEnabledAtPageLoad(true)).Times(1);
+
+  autofill_manager_->set_autofill_enabled(true);
+  autofill_manager_->OnFormsSeen(std::vector<FormData>());
+
+  // Reset the autofill manager state.
+  autofill_manager_->Reset();
+
+  // Establish our expectations.
+  EXPECT_CALL(*autofill_manager_->metric_logger(),
+              LogIsAutofillEnabledAtPageLoad(false)).Times(1);
+
+  autofill_manager_->set_autofill_enabled(false);
+  autofill_manager_->OnFormsSeen(std::vector<FormData>());
+}
+
 // Test that credit card infobar metrics are logged correctly.
 TEST_F(AutofillMetricsTest, CreditCardInfoBar) {
   MockAutofillMetrics metric_logger;
diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc
index 184b7a9..25c9c57 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -102,9 +102,6 @@ bool IsMinimumAddress(const AutofillProfile& profile) {
          !profile.GetInfo(ADDRESS_HOME_ZIP).empty();
 }
 
-// Whether we have already logged the number of profiles this session.
-bool g_has_logged_profile_count = false;
-
 }  // namespace
 
 PersonalDataManager::~PersonalDataManager() {
@@ -644,26 +641,27 @@ void PersonalDataManager::Refresh() {
   LoadCreditCards();
 }
 
-// static
-void PersonalDataManager::set_has_logged_profile_count(
-    bool has_logged_profile_count) {
-  g_has_logged_profile_count = has_logged_profile_count;
-}
-
 PersonalDataManager::PersonalDataManager()
     : profile_(NULL),
       is_data_loaded_(false),
       pending_profiles_query_(0),
       pending_creditcards_query_(0),
-      metric_logger_(new AutofillMetrics) {
+      metric_logger_(new AutofillMetrics),
+      has_logged_profile_count_(false) {
 }
 
 void PersonalDataManager::Init(Profile* profile) {
   profile_ = profile;
+  metric_logger_->LogIsAutofillEnabledAtStartup(IsAutofillEnabled());
+
   LoadProfiles();
   LoadCreditCards();
 }
 
+bool PersonalDataManager::IsAutofillEnabled() const {
+  return profile_->GetPrefs()->GetBoolean(prefs::kAutofillEnabled);
+}
+
 // static
 bool PersonalDataManager::IsValidLearnableProfile(
     const AutofillProfile& profile) {
@@ -904,9 +902,9 @@ void PersonalDataManager::EmptyMigrationTrash() {
 }
 
 void PersonalDataManager::LogProfileCount() const {
-  if (!g_has_logged_profile_count) {
-    g_has_logged_profile_count = true;
+  if (!has_logged_profile_count_) {
     metric_logger_->LogStoredProfileCount(web_profiles_.size());
+    has_logged_profile_count_ = true;
   }
 }
 
diff --git a/chrome/browser/autofill/personal_data_manager.h b/chrome/browser/autofill/personal_data_manager.h
index 890dfd4..7b6299a 100644
--- a/chrome/browser/autofill/personal_data_manager.h
+++ b/chrome/browser/autofill/personal_data_manager.h
@@ -169,15 +169,9 @@ class PersonalDataManager
   friend class ProfileImpl;
   friend class ProfileSyncServiceAutofillTest;
 
-  // For tests.
-  static void set_has_logged_profile_count(bool has_logged_profile_count);
-
   PersonalDataManager();
   virtual ~PersonalDataManager();
 
-  // Returns the profile of the tab contents.
-  Profile* profile();
-
   // Loads the saved profiles from the web database.
   virtual void LoadProfiles();
 
@@ -211,6 +205,9 @@ class PersonalDataManager
   // profiles the user has. On subsequent calls, does nothing.
   void LogProfileCount() const;
 
+  // Returns the value of the AutofillEnabled pref.
+  virtual bool IsAutofillEnabled() const;
+
   // For tests.
   const AutofillMetrics* metric_logger() const;
   void set_metric_logger(const AutofillMetrics* metric_logger);
@@ -252,6 +249,9 @@ class PersonalDataManager
   // For logging UMA metrics. Overridden by metrics tests.
   scoped_ptr<const AutofillMetrics> metric_logger_;
 
+  // Whether we have already logged the number of profiles this session.
+  mutable bool has_logged_profile_count_;
+
   DISALLOW_COPY_AND_ASSIGN(PersonalDataManager);
 };
 
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index 97f0426..04c6f3a6 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -142,12 +142,6 @@ const char kPasswordManagerEnabled[] = "profile.password_manager_enabled";
 const char kPasswordManagerAllowShowPasswords[] =
     "profile.password_manager_allow_show_passwords";
 
-// OBSOLETE.  Boolean that is true if the form Autofill is on (will record
-// values entered in text inputs in forms and shows them in a popup when user
-// type in a text input with the same name later on).  This has been superseded
-// by kAutofillEnabled.
-const char kFormAutofillEnabled[] = "profile.form_autofill_enabled";
-
 // Boolean that is true when SafeBrowsing is enabled.
 const char kSafeBrowsingEnabled[] = "safebrowsing.enabled";
 
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index c10057b..32ad145 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -60,7 +60,6 @@ extern const char kWebKitJavaEnabled[];
 extern const char kWebkitTabsToLinks[];
 extern const char kPasswordManagerEnabled[];
 extern const char kPasswordManagerAllowShowPasswords[];
-extern const char kFormAutofillEnabled[];  // OBSOLETE
 extern const char kSafeBrowsingEnabled[];
 extern const char kSafeBrowsingReportingEnabled[];
 extern const char kIncognitoEnabled[];