From 8a2bc4a5300bdcd37221ab634a1e1143c1b02f79 Mon Sep 17 00:00:00 2001 From: gcasto Date: Tue, 18 Nov 2014 16:29:37 -0800 Subject: [PATCH] [Password Generation] Breakout UMA stats for generated passwords The main impetus of this change is to make it possible to calculate the percentage of generated passwords overriden by non-generated passwords, for which we need the total number of generated passwords BUG=432407 Review URL: https://codereview.chromium.org/715193002 Cr-Commit-Position: refs/heads/master@{#304716} --- .../core/browser/login_database.cc | 50 ++++++++++----- .../core/browser/login_database_unittest.cc | 74 ++++++++++++++++++++++ tools/metrics/histograms/histograms.xml | 16 +++++ 3 files changed, 123 insertions(+), 17 deletions(-) diff --git a/components/password_manager/core/browser/login_database.cc b/components/password_manager/core/browser/login_database.cc index f2ab2e4bad57..60fa6fdb1f99 100644 --- a/components/password_manager/core/browser/login_database.cc +++ b/components/password_manager/core/browser/login_database.cc @@ -324,8 +324,9 @@ void LoginDatabase::ReportMetrics(const std::string& sync_username, bool custom_passphrase_sync_enabled) { sql::Statement s(db_.GetCachedStatement( SQL_FROM_HERE, - "SELECT signon_realm, blacklisted_by_user, COUNT(username_value) " - "FROM logins GROUP BY signon_realm, blacklisted_by_user")); + "SELECT signon_realm, password_type, blacklisted_by_user," + "COUNT(username_value) FROM logins GROUP BY " + "signon_realm, password_type, blacklisted_by_user")); if (!s.is_valid()) return; @@ -335,23 +336,38 @@ void LoginDatabase::ReportMetrics(const std::string& sync_username, custom_passphrase = "WithCustomPassphrase"; } - int total_accounts = 0; + int total_user_created_accounts = 0; + int total_generated_accounts = 0; int blacklisted_sites = 0; while (s.Step()) { - int blacklisted = s.ColumnInt(1); - int accounts_per_site = s.ColumnInt(2); + PasswordForm::Type password_type = + static_cast(s.ColumnInt(1)); + int blacklisted = s.ColumnInt(2); + int accounts_per_site = s.ColumnInt(3); if (blacklisted) { ++blacklisted_sites; + } else if (password_type == PasswordForm::TYPE_GENERATED) { + total_generated_accounts += accounts_per_site; + LogAccountStat( + base::StringPrintf("PasswordManager.AccountsPerSite.AutoGenerated.%s", + custom_passphrase.c_str()), + accounts_per_site); } else { - total_accounts += accounts_per_site; - LogAccountStat(base::StringPrintf("PasswordManager.AccountsPerSite.%s", - custom_passphrase.c_str()), - accounts_per_site); + total_user_created_accounts += accounts_per_site; + LogAccountStat( + base::StringPrintf("PasswordManager.AccountsPerSite.UserCreated.%s", + custom_passphrase.c_str()), + accounts_per_site); } } - LogAccountStat(base::StringPrintf("PasswordManager.TotalAccounts.%s", - custom_passphrase.c_str()), - total_accounts); + LogAccountStat( + base::StringPrintf("PasswordManager.TotalAccounts.UserCreated.%s", + custom_passphrase.c_str()), + total_user_created_accounts); + LogAccountStat( + base::StringPrintf("PasswordManager.TotalAccounts.AutoGenerated.%s", + custom_passphrase.c_str()), + total_generated_accounts); LogAccountStat(base::StringPrintf("PasswordManager.BlacklistedSites.%s", custom_passphrase.c_str()), blacklisted_sites); @@ -368,13 +384,13 @@ void LoginDatabase::ReportMetrics(const std::string& sync_username, usage_statement.ColumnInt(0)); if (type == PasswordForm::TYPE_GENERATED) { - LogTimesUsedStat( - base::StringPrintf("PasswordManager.TimesGeneratedPasswordUsed.%s", - custom_passphrase.c_str()), - usage_statement.ColumnInt(1)); + LogTimesUsedStat(base::StringPrintf( + "PasswordManager.TimesPasswordUsed.AutoGenerated.%s", + custom_passphrase.c_str()), + usage_statement.ColumnInt(1)); } else { LogTimesUsedStat( - base::StringPrintf("PasswordManager.TimesPasswordUsed.%s", + base::StringPrintf("PasswordManager.TimesPasswordUsed.UserCreated.%s", custom_passphrase.c_str()), usage_statement.ColumnInt(1)); } diff --git a/components/password_manager/core/browser/login_database_unittest.cc b/components/password_manager/core/browser/login_database_unittest.cc index 4faaa8c96835..5ec0e2ed88c3 100644 --- a/components/password_manager/core/browser/login_database_unittest.cc +++ b/components/password_manager/core/browser/login_database_unittest.cc @@ -11,6 +11,7 @@ #include "base/path_service.h" #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" +#include "base/test/histogram_tester.h" #include "base/time/time.h" #include "components/autofill/core/common/password_form.h" #include "components/password_manager/core/browser/psl_matching_helper.h" @@ -951,6 +952,79 @@ TEST_F(LoginDatabaseTest, UpdateLogin) { EXPECT_EQ(form, *result[0]); } +TEST_F(LoginDatabaseTest, ReportMetricsTest) { + PasswordForm password_form; + password_form.origin = GURL("http://example.com"); + password_form.username_value = ASCIIToUTF16("test1@gmail.com"); + password_form.password_value = ASCIIToUTF16("test"); + password_form.signon_realm = "http://example.com/"; + password_form.times_used = 0; + EXPECT_EQ(AddChangeForForm(password_form), db_.AddLogin(password_form)); + + password_form.username_value = ASCIIToUTF16("test2@gmail.com"); + password_form.times_used = 1; + EXPECT_EQ(AddChangeForForm(password_form), db_.AddLogin(password_form)); + + password_form.origin = GURL("http://second.example.com"); + password_form.signon_realm = "http://second.example.com"; + password_form.times_used = 3; + EXPECT_EQ(AddChangeForForm(password_form), db_.AddLogin(password_form)); + + password_form.username_value = ASCIIToUTF16("test3@gmail.com"); + password_form.type = PasswordForm::TYPE_GENERATED; + password_form.times_used = 2; + EXPECT_EQ(AddChangeForForm(password_form), db_.AddLogin(password_form)); + + password_form.origin = GURL("http://third.example.com/"); + password_form.signon_realm = "http://third.example.com/"; + password_form.times_used = 4; + EXPECT_EQ(AddChangeForForm(password_form), db_.AddLogin(password_form)); + + base::HistogramTester histogram_tester; + db_.ReportMetrics("", false); + + histogram_tester.ExpectUniqueSample( + "PasswordManager.TotalAccounts.UserCreated.WithoutCustomPassphrase", + 3, + 1); + histogram_tester.ExpectBucketCount( + "PasswordManager.AccountsPerSite.UserCreated.WithoutCustomPassphrase", + 1, + 1); + histogram_tester.ExpectBucketCount( + "PasswordManager.AccountsPerSite.UserCreated.WithoutCustomPassphrase", + 2, + 1); + histogram_tester.ExpectBucketCount( + "PasswordManager.TimesPasswordUsed.UserCreated.WithoutCustomPassphrase", + 0, + 1); + histogram_tester.ExpectBucketCount( + "PasswordManager.TimesPasswordUsed.UserCreated.WithoutCustomPassphrase", + 1, + 1); + histogram_tester.ExpectBucketCount( + "PasswordManager.TimesPasswordUsed.UserCreated.WithoutCustomPassphrase", + 3, + 1); + histogram_tester.ExpectUniqueSample( + "PasswordManager.TotalAccounts.AutoGenerated.WithoutCustomPassphrase", + 2, + 1); + histogram_tester.ExpectUniqueSample( + "PasswordManager.AccountsPerSite.AutoGenerated.WithoutCustomPassphrase", + 1, + 2); + histogram_tester.ExpectBucketCount( + "PasswordManager.TimesPasswordUsed.AutoGenerated.WithoutCustomPassphrase", + 2, + 1); + histogram_tester.ExpectBucketCount( + "PasswordManager.TimesPasswordUsed.AutoGenerated.WithoutCustomPassphrase", + 4, + 1); +} + #if defined(OS_POSIX) // Only the current user has permission to read the database. // diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index b727ef4172c3..926467c5df07 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -22752,6 +22752,10 @@ Therefore, the affected-histogram name has to have at least one dot in it. gcasto@chromium.org vabr@chromium.org + + Deprecated as of 11/11/14. New statistic is + PasswordManager.TimesPasswordUsed.AutoGenerated. + The number of times each generated password has been used to log in. Recorded by iterating over stored passwords once per run. This information @@ -59395,8 +59399,20 @@ To add a new entry, add it with any value and run test to compute valid value. + + + + + + + + + + + + -- 2.11.4.GIT