Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(10017)

Unified Diff: components/password_manager/core/browser/login_database.cc

Issue 644053003: [Password Manager] Add UMA stats for custom passphrase users. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebase Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/password_manager/core/browser/login_database.cc
diff --git a/components/password_manager/core/browser/login_database.cc b/components/password_manager/core/browser/login_database.cc
index e2ebfc5ea0a3e904db9ebd4907aac44ee39deb9d..278d740235a01aea19abcea1b5e6ffcbe8c2ac14 100644
--- a/components/password_manager/core/browser/login_database.cc
+++ b/components/password_manager/core/browser/login_database.cc
@@ -13,8 +13,10 @@
#include "base/metrics/histogram.h"
#include "base/pickle.h"
#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
#include "base/time/time.h"
#include "components/autofill/core/common/password_form.h"
+#include "components/password_manager/core/browser/password_manager_client.h"
#include "google_apis/gaia/gaia_auth_util.h"
#include "google_apis/gaia/gaia_urls.h"
#include "sql/connection.h"
@@ -117,6 +119,22 @@ void AddCallback(int err, sql::Statement* /*stmt*/) {
DLOG(WARNING) << "LoginDatabase::AddLogin updated an existing form";
}
+// UMA_* macros assume that the name never changes. This is a helper function
+// where this assumption doesn't hold.
+void LogDynamicUMAStat(const std::string& name,
+ int sample,
+ int min,
+ int max,
+ int bucket_size) {
+ base::HistogramBase* counter = base::Histogram::FactoryGet(
+ name,
+ min,
+ max,
+ bucket_size,
+ base::HistogramBase::kUmaTargetedHistogramFlag);
+ counter->Add(sample);
+}
+
} // namespace
LoginDatabase::LoginDatabase() {
@@ -277,7 +295,7 @@ bool LoginDatabase::InitLoginsTable() {
return true;
}
-void LoginDatabase::ReportMetrics(const std::string& sync_username) {
+void LoginDatabase::ReportMetrics(PasswordManagerClient* client) {
sql::Statement s(db_.GetCachedStatement(
SQL_FROM_HERE,
"SELECT signon_realm, blacklisted_by_user, COUNT(username_value) "
@@ -286,6 +304,11 @@ void LoginDatabase::ReportMetrics(const std::string& sync_username) {
if (!s.is_valid())
return;
+ std::string custom_passphrase;
vabr (Chromium) 2014/10/13 09:24:18 Should we have a non-empty value describing the si
Garrett Casto 2014/10/13 20:57:56 Yeah, seems reasonable. I was assuming that there
+ if (client->IsPasswordSyncEnabled(ONLY_CUSTOM_PASSPHRASE_USERS)) {
+ custom_passphrase = "WithCustomPassphrase";
+ }
+
int total_accounts = 0;
int blacklisted_sites = 0;
while (s.Step()) {
@@ -295,14 +318,26 @@ void LoginDatabase::ReportMetrics(const std::string& sync_username) {
++blacklisted_sites;
} else {
total_accounts += accounts_per_site;
- UMA_HISTOGRAM_CUSTOM_COUNTS("PasswordManager.AccountsPerSite",
- accounts_per_site, 0, 32, 6);
+ LogDynamicUMAStat(base::StringPrintf("PasswordManager.AccountsPerSite%s",
+ custom_passphrase.c_str()),
+ accounts_per_site,
+ 0,
+ 32,
+ 6);
}
}
- UMA_HISTOGRAM_CUSTOM_COUNTS("PasswordManager.TotalAccounts",
- total_accounts, 0, 32, 6);
- UMA_HISTOGRAM_CUSTOM_COUNTS("PasswordManager.BlacklistedSites",
- blacklisted_sites, 0, 32, 6);
+ LogDynamicUMAStat(base::StringPrintf("PasswordManager.TotalAccounts%s",
+ custom_passphrase.c_str()),
+ total_accounts,
+ 0,
+ 32,
+ 6);
+ LogDynamicUMAStat(base::StringPrintf("PasswordManager.BlacklistedSites%s",
+ custom_passphrase.c_str()),
+ blacklisted_sites,
+ 0,
+ 32,
+ 6);
sql::Statement usage_statement(db_.GetCachedStatement(
SQL_FROM_HERE,
@@ -316,17 +351,26 @@ void LoginDatabase::ReportMetrics(const std::string& sync_username) {
usage_statement.ColumnInt(0));
if (type == PasswordForm::TYPE_GENERATED) {
- UMA_HISTOGRAM_CUSTOM_COUNTS(
- "PasswordManager.TimesGeneratedPasswordUsed",
- usage_statement.ColumnInt(1), 0, 100, 10);
+ LogDynamicUMAStat(
+ base::StringPrintf("PasswordManager.TimesGeneratedPasswordUsed%s",
+ custom_passphrase.c_str()),
+ usage_statement.ColumnInt(1),
+ 0,
+ 100,
+ 10);
} else {
- UMA_HISTOGRAM_CUSTOM_COUNTS(
- "PasswordManager.TimesPasswordUsed",
- usage_statement.ColumnInt(1), 0, 100, 10);
+ LogDynamicUMAStat(
+ base::StringPrintf("PasswordManager.TimesPasswordUsed%s",
+ custom_passphrase.c_str()),
+ usage_statement.ColumnInt(1),
+ 0,
+ 100,
+ 10);
}
}
bool syncing_account_saved = false;
+ std::string sync_username = client->GetSyncUsername();
if (!sync_username.empty()) {
sql::Statement sync_statement(db_.GetCachedStatement(
SQL_FROM_HERE,

Powered by Google App Engine
This is Rietveld 408576698