summaryrefslogtreecommitdiffstats
path: root/chrome/browser/password_manager
diff options
context:
space:
mode:
authordpranke@chromium.org <dpranke@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-07-01 23:40:41 +0000
committerdpranke@chromium.org <dpranke@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-07-01 23:40:41 +0000
commit210aaa7d582d1315820b2c9321c5f7abe6bdb60b (patch)
tree49547329e8eafb4ec286b3018b2a3e908761e0ad /chrome/browser/password_manager
parent272ef554fb04254a04552a07a49a1923e5247300 (diff)
downloadchromium_src-210aaa7d582d1315820b2c9321c5f7abe6bdb60b.zip
chromium_src-210aaa7d582d1315820b2c9321c5f7abe6bdb60b.tar.gz
chromium_src-210aaa7d582d1315820b2c9321c5f7abe6bdb60b.tar.bz2
Implement UMA tracking for every form that the password manager sees.
We will track how the manager first interprets the form (match found or not, etc.), what action if any the user takes to change the values, and the result (form submitted or not, and did it succeed or fail). BUG=45946 R=jcivelli@chromium.org, jar@chromium.org TEST=manual testing using about:histograms/PasswordManager and verifying the stats Review URL: http://codereview.chromium.org/2831033 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51455 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/password_manager')
-rw-r--r--chrome/browser/password_manager/password_form_manager.cc61
-rw-r--r--chrome/browser/password_manager/password_form_manager.h60
-rw-r--r--chrome/browser/password_manager/password_manager.cc22
-rw-r--r--chrome/browser/password_manager/password_manager.h3
4 files changed, 124 insertions, 22 deletions
diff --git a/chrome/browser/password_manager/password_form_manager.cc b/chrome/browser/password_manager/password_form_manager.cc
index 72636bb..1d7f345 100644
--- a/chrome/browser/password_manager/password_form_manager.cc
+++ b/chrome/browser/password_manager/password_form_manager.cc
@@ -6,6 +6,7 @@
#include <algorithm>
+#include "base/histogram.h"
#include "base/string_util.h"
#include "chrome/browser/password_manager/password_manager.h"
#include "chrome/browser/profile.h"
@@ -26,7 +27,10 @@ PasswordFormManager::PasswordFormManager(Profile* profile,
pending_login_query_(0),
preferred_match_(NULL),
state_(PRE_MATCHING_PHASE),
- profile_(profile) {
+ profile_(profile),
+ manager_action_(kManagerActionNone),
+ user_action_(kUserActionNone),
+ submit_result_(kSubmitResultNotSubmitted) {
DCHECK(profile_);
if (observed_form_.origin.is_valid())
SplitString(observed_form_.origin.path(), '/', &form_path_tokens_);
@@ -35,8 +39,16 @@ PasswordFormManager::PasswordFormManager(Profile* profile,
PasswordFormManager::~PasswordFormManager() {
CancelLoginsQuery();
+ UMA_HISTOGRAM_ENUMERATION("PasswordManager.ActionsTaken",
+ GetActionsTaken(),
+ kMaxNumActionsTaken);
}
+int PasswordFormManager::GetActionsTaken() {
+ return user_action_ + kUserActionMax * (manager_action_ +
+ kManagerActionMax * submit_result_);
+};
+
// TODO(timsteele): use a hash of some sort in the future?
bool PasswordFormManager::DoesManage(const PasswordForm& form) const {
if (form.scheme != PasswordForm::SCHEME_HTML)
@@ -153,7 +165,13 @@ void PasswordFormManager::ProvisionallySave(const PasswordForm& credentials) {
// bless it with the action URL from the observed form. See bug 1107719.
if (pending_credentials_.action.is_empty())
pending_credentials_.action = observed_form_.action;
+
+ // Check to see if we're using a known username but a new password.
+ if (pending_credentials_.password_value != credentials.password_value)
+ user_action_ = kUserActionOverride;
} else {
+ // User typed in a new, unknown username.
+ user_action_ = kUserActionOverride;
pending_credentials_ = observed_form_;
pending_credentials_.username_value = credentials.username_value;
}
@@ -196,7 +214,7 @@ void PasswordFormManager::OnRequestDone(int handle,
// copies to a minimum here.
int best_score = 0;
- std::vector<PasswordForm> empties; // Empty-path matches in result set.
+ std::vector<PasswordForm> empties; // Empty-path matches in result set.
for (size_t i = 0; i < logins_result.size(); i++) {
if (IgnoreResult(*logins_result[i])) {
delete logins_result[i];
@@ -256,7 +274,7 @@ void PasswordFormManager::OnRequestDone(int handle,
best_matches_[it->username_value] = new PasswordForm(*it);
}
- // Its possible we have at least one match but have no preferred_match_,
+ // It is possible we have at least one match but have no preferred_match_,
// because a user may have chosen to 'Forget' the preferred match. So we
// just pick the first one and whichever the user selects for submit will
// be saved as preferred.
@@ -264,12 +282,22 @@ void PasswordFormManager::OnRequestDone(int handle,
if (!preferred_match_)
preferred_match_ = best_matches_.begin()->second;
- // Now we determine if the user told us to ignore this site in the past.
- // If they haven't, we proceed to auto-fill.
- if (!preferred_match_->blacklisted_by_user) {
- password_manager_->Autofill(observed_form_, best_matches_,
- preferred_match_);
+ // Check to see if the user told us to ignore this site in the past.
+ if (preferred_match_->blacklisted_by_user) {
+ manager_action_ = kManagerActionBlacklisted;
+ return;
}
+
+ // Proceed to autofill (note that we provide the choices but don't
+ // actually prefill a value if the ACTION paths don't match).
+ bool wait_for_username = observed_form_.action.GetWithEmptyPath() !=
+ preferred_match_->action.GetWithEmptyPath();
+ if (wait_for_username)
+ manager_action_ = kManagerActionNone;
+ else
+ manager_action_ = kManagerActionAutoFilled;
+ password_manager_->Autofill(observed_form_, best_matches_,
+ preferred_match_, wait_for_username);
}
void PasswordFormManager::OnPasswordStoreRequestDone(
@@ -333,6 +361,8 @@ void PasswordFormManager::UpdatePreferredLoginState(
iter->second->preferred) {
// This wasn't the selected login but it used to be preferred.
iter->second->preferred = false;
+ if (user_action_ == kUserActionNone)
+ user_action_ = kUserActionChoose;
password_store->UpdateLogin(*iter->second);
}
}
@@ -341,9 +371,10 @@ void PasswordFormManager::UpdatePreferredLoginState(
void PasswordFormManager::UpdateLogin() {
DCHECK_EQ(state_, POST_MATCHING_PHASE);
DCHECK(preferred_match_);
- // If we're doing an Update, its because we autofilled a form and the user
- // submitted it with a possibly new password value, page security, or selected
- // one of the non-preferred matches, thus requiring a swap of preferred bits.
+ // If we're doing an Update, we either autofilled correctly and need to
+ // update the stats, or the user typed in a new password for autofilled
+ // username, or the user selected one of the non-preferred matches,
+ // thus requiring a swap of preferred bits.
DCHECK(!IsNewLogin() && pending_credentials_.preferred);
DCHECK(!profile_->IsOffTheRecord());
@@ -442,3 +473,11 @@ int PasswordFormManager::ScoreResult(const PasswordForm& candidate) const {
return score;
}
+
+void PasswordFormManager::SubmitPassed() {
+ submit_result_ = kSubmitResultPassed;
+}
+
+void PasswordFormManager::SubmitFailed() {
+ submit_result_ = kSubmitResultFailed;
+}
diff --git a/chrome/browser/password_manager/password_form_manager.h b/chrome/browser/password_manager/password_form_manager.h
index 242e3ec..a96bffc 100644
--- a/chrome/browser/password_manager/password_form_manager.h
+++ b/chrome/browser/password_manager/password_form_manager.h
@@ -5,6 +5,9 @@
#ifndef CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_FORM_MANAGER_H_
#define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_FORM_MANAGER_H_
+#include <string>
+#include <vector>
+
#include "build/build_config.h"
#include "base/stl_util-inl.h"
@@ -84,8 +87,54 @@ class PasswordFormManager : public PasswordStoreConsumer {
// observed_form_ (e.g DoesManage(pending_credentials_) == true).
void Save();
+ // Call these if/when we know the form submission worked or failed.
+ // These routines are used to update internal statistics ("ActionsTaken").
+ void SubmitPassed();
+ void SubmitFailed();
+
private:
friend class PasswordFormManagerTest;
+
+ // ManagerAction - What does the manager do with this form? Either it
+ // fills it, or it doesn't. If it doesn't fill it, that's either
+ // because it has no match, or it is blacklisted, or it is disabled
+ // via the AUTOCOMPLETE=off attribute. Note that if we don't have
+ // an exact match, we still provide candidates that the user may
+ // end up choosing.
+ enum ManagerAction {
+ kManagerActionNone = 0,
+ kManagerActionAutoFilled,
+ kManagerActionBlacklisted,
+ kManagerActionDisabled,
+ kManagerActionMax
+ };
+
+ // UserAction - What does the user do with this form? If he or she
+ // does nothing (either by accepting what the password manager did, or
+ // by simply (not typing anything at all), you get None. If there were
+ // multiple choices and the user selects one other than the default,
+ // you get Choose, and if the user types in a new value, you get
+ // Override.
+ enum UserAction {
+ kUserActionNone = 0,
+ kUserActionChoose,
+ kUserActionOverride,
+ kUserActionMax
+ };
+
+ // Result - What happens to the form?
+ enum SubmitResult {
+ kSubmitResultNotSubmitted = 0,
+ kSubmitResultFailed,
+ kSubmitResultPassed,
+ kSubmitResultMax
+ };
+
+ // The maximum number of combinations of the three preceding enums.
+ // This is used when recording the actions taken by the form in UMA.
+ static const int kMaxNumActionsTaken = kManagerActionMax * kUserActionMax *
+ kSubmitResultMax;
+
// Called by destructor to ensure if this object is deleted, no potential
// outstanding callbacks can call OnPasswordStoreRequestDone.
void CancelLoginsQuery();
@@ -116,6 +165,10 @@ class PasswordFormManager : public PasswordStoreConsumer {
// |pending_credentials_|.
void UpdatePreferredLoginState(PasswordStore* password_store);
+ // Converts the "ActionsTaken" fields into an int so they can be logged to
+ // UMA.
+ int GetActionsTaken();
+
// Set of PasswordForms from the DB that best match the form
// being managed by this. Use a map instead of vector, because we most
// frequently require lookups by username value in IsNewLogin.
@@ -168,6 +221,13 @@ class PasswordFormManager : public PasswordStoreConsumer {
// The profile from which we get the PasswordStore.
Profile* profile_;
+ // These three fields record the "ActionsTaken" by the browser and
+ // the user with this form, and the result. They are combined and
+ // recorded in UMA when the manager is destroyed.
+ ManagerAction manager_action_;
+ UserAction user_action_;
+ SubmitResult submit_result_;
+
DISALLOW_COPY_AND_ASSIGN(PasswordFormManager);
};
#endif // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_FORM_MANAGER_H_
diff --git a/chrome/browser/password_manager/password_manager.cc b/chrome/browser/password_manager/password_manager.cc
index 09a75f2..fbebafc 100644
--- a/chrome/browser/password_manager/password_manager.cc
+++ b/chrome/browser/password_manager/password_manager.cc
@@ -30,8 +30,8 @@ void PasswordManager::RegisterUserPrefs(PrefService* prefs) {
// This routine is called when PasswordManagers are constructed.
//
-// Currently we report metrics only once at startup. We require
-// that this is only ever called from a single thread in order to
+// Currently we report metrics only once at startup. We require
+// that this is only ever called from a single thread in order to
// avoid needing to lock (a static boolean flag is then sufficient to
// guarantee running only once).
static void ReportMetrics(bool password_manager_enabled) {
@@ -42,13 +42,13 @@ static void ReportMetrics(bool password_manager_enabled) {
if (ran_once)
return;
ran_once = true;
-
+
if (password_manager_enabled)
UserMetrics::RecordAction(UserMetricsAction("PasswordManager_Enabled"));
else
UserMetrics::RecordAction(UserMetricsAction("PasswordManager_Disabled"));
-}
-
+}
+
PasswordManager::PasswordManager(Delegate* delegate)
: login_managers_deleter_(&pending_login_managers_),
delegate_(delegate),
@@ -56,7 +56,7 @@ PasswordManager::PasswordManager(Delegate* delegate)
DCHECK(delegate_);
password_manager_enabled_.Init(prefs::kPasswordManagerEnabled,
delegate_->GetProfileForPasswordManager()->GetPrefs(), NULL);
-
+
ReportMetrics(*password_manager_enabled_);
}
@@ -134,6 +134,8 @@ void PasswordManager::DidStopLoading() {
// Form is not completely valid - we do not support it.
if (!provisional_save_manager_->HasValidPasswordForm())
return;
+
+ provisional_save_manager_->SubmitPassed();
if (provisional_save_manager_->IsNewLogin()) {
delegate_->AddSavePasswordInfoBar(provisional_save_manager_.release());
} else {
@@ -176,6 +178,7 @@ void PasswordManager::PasswordFormsVisible(
// failure and abort this save, by clearing provisional_save_manager_.
// Don't delete the login managers since the user may try again
// and we want to be able to save in that case.
+ provisional_save_manager_->SubmitFailed();
ClearProvisionalSave();
break;
}
@@ -185,19 +188,18 @@ void PasswordManager::PasswordFormsVisible(
void PasswordManager::Autofill(
const PasswordForm& form_for_autofill,
const PasswordFormMap& best_matches,
- const PasswordForm* const preferred_match) const {
+ const PasswordForm* const preferred_match,
+ bool wait_for_username) const {
DCHECK(preferred_match);
switch (form_for_autofill.scheme) {
case PasswordForm::SCHEME_HTML: {
// Note the check above is required because the observer_ for a non-HTML
// schemed password form may have been freed, so we need to distinguish.
- bool action_mismatch = form_for_autofill.action.GetWithEmptyPath() !=
- preferred_match->action.GetWithEmptyPath();
webkit_glue::PasswordFormDomManager::FillData fill_data;
webkit_glue::PasswordFormDomManager::InitFillData(form_for_autofill,
best_matches,
preferred_match,
- action_mismatch,
+ wait_for_username,
&fill_data);
delegate_->FillPasswordForm(fill_data);
return;
diff --git a/chrome/browser/password_manager/password_manager.h b/chrome/browser/password_manager/password_manager.h
index 79bd332..a4eaafb 100644
--- a/chrome/browser/password_manager/password_manager.h
+++ b/chrome/browser/password_manager/password_manager.h
@@ -59,7 +59,8 @@ class PasswordManager : public LoginModel {
// on the page.
void Autofill(const webkit_glue::PasswordForm& form_for_autofill,
const webkit_glue::PasswordFormMap& best_matches,
- const webkit_glue::PasswordForm* const preferred_match) const;
+ const webkit_glue::PasswordForm* const preferred_match,
+ bool wait_for_username) const;
// Notification that the user navigated away from the current page.
// Unless this is a password form submission, for our purposes this