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

Unified Diff: chrome/browser/ui/passwords/manage_passwords_bubble_model.cc

Issue 1304573004: Added UMA statistics for change passwords in the Password Manager (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 4 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: chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
diff --git a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc b/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
index 3f50dede3343639a6125c16a3fb397ade03e3f60..847110ae865e3a37316e0c2b4d45a83ab0194967 100644
--- a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
+++ b/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
@@ -90,7 +90,8 @@ ManagePasswordsBubbleModel::ManagePasswordsBubbleModel(
: content::WebContentsObserver(web_contents),
never_save_passwords_(false),
display_disposition_(metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING),
- dismissal_reason_(metrics_util::NOT_DISPLAYED) {
+ dismissal_reason_(metrics_util::NOT_DISPLAYED),
+ update_password_submission_event_(metrics_util::NO_UPDATE_SUBMISSION) {
ManagePasswordsUIController* controller =
ManagePasswordsUIController::FromWebContents(web_contents);
@@ -160,23 +161,44 @@ ManagePasswordsBubbleModel::~ManagePasswordsBubbleModel() {}
void ManagePasswordsBubbleModel::OnBubbleShown(
ManagePasswordsBubble::DisplayReason reason) {
if (reason == ManagePasswordsBubble::USER_ACTION) {
- if (state_ == password_manager::ui::PENDING_PASSWORD_STATE) {
+ switch (state_) {
+ case password_manager::ui::PENDING_PASSWORD_STATE:
dvadym 2015/08/19 11:47:56 It's added display disposition calculation for upd
display_disposition_ = metrics_util::MANUAL_WITH_PASSWORD_PENDING;
vasilii 2015/08/19 13:01:18 indent 2 spaces
dvadym 2015/08/19 16:49:54 Done.
- } else if (state_ == password_manager::ui::BLACKLIST_STATE) {
- display_disposition_ = metrics_util::MANUAL_BLACKLISTED;
- } else {
- display_disposition_ = metrics_util::MANUAL_MANAGE_PASSWORDS;
+ break;
+ case password_manager::ui::PENDING_PASSWORD_UPDATE_STATE:
+ display_disposition_ =
+ metrics_util::MANUAL_WITH_PASSWORD_PENDING_UPDATE;
+ break;
+ case password_manager::ui::BLACKLIST_STATE:
+ display_disposition_ = metrics_util::MANUAL_BLACKLISTED;
+ break;
+ case password_manager::ui::MANAGE_STATE:
+ display_disposition_ = metrics_util::MANUAL_MANAGE_PASSWORDS;
+ break;
+ default:
+ NOTREACHED();
}
} else {
- if (state_ == password_manager::ui::CONFIRMATION_STATE) {
- display_disposition_ =
- metrics_util::AUTOMATIC_GENERATED_PASSWORD_CONFIRMATION;
- } else if (state_ == password_manager::ui::CREDENTIAL_REQUEST_STATE) {
- display_disposition_ = metrics_util::AUTOMATIC_CREDENTIAL_REQUEST;
- } else if (state_ == password_manager::ui::AUTO_SIGNIN_STATE) {
- display_disposition_ = metrics_util::AUTOMATIC_SIGNIN_TOAST;
- } else {
- display_disposition_ = metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING;
+ switch (state_) {
+ case password_manager::ui::PENDING_PASSWORD_STATE:
+ display_disposition_ = metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING;
+ break;
+ case password_manager::ui::PENDING_PASSWORD_UPDATE_STATE:
+ display_disposition_ =
+ metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING_UPDATE;
+ break;
+ case password_manager::ui::CONFIRMATION_STATE:
+ display_disposition_ =
+ metrics_util::AUTOMATIC_GENERATED_PASSWORD_CONFIRMATION;
+ break;
+ case password_manager::ui::CREDENTIAL_REQUEST_STATE:
+ display_disposition_ = metrics_util::AUTOMATIC_CREDENTIAL_REQUEST;
+ break;
+ case password_manager::ui::AUTO_SIGNIN_STATE:
+ display_disposition_ = metrics_util::AUTOMATIC_SIGNIN_TOAST;
+ break;
+ default:
+ NOTREACHED();
}
}
metrics_util::LogUIDisplayDisposition(display_disposition_);
@@ -206,15 +228,43 @@ void ManagePasswordsBubbleModel::OnBubbleHidden() {
if (state_ == password_manager::ui::PENDING_PASSWORD_STATE &&
dismissal_reason_ == metrics_util::NO_DIRECT_INTERACTION)
RecordExperimentStatistics(web_contents(), dismissal_reason_);
+ // Check if this was update password and record update statistics.
+ if (update_password_submission_event_ == metrics_util::NO_UPDATE_SUBMISSION) {
+ if (state_ == password_manager::ui::PENDING_PASSWORD_UPDATE_STATE) {
+ update_password_submission_event_ =
+ password_overridden_
+ ? metrics_util::PASSWORD_OVERRIDEN_NO_INTERACTION
+ : (ShouldShowMultipleAccountUpdateUI()
+ ? metrics_util::MULTIPLE_ACCOUNTS_NO_INTERACTION
+ : metrics_util::ONE_ACCOUNT_NO_INTERACTION);
+ } else if (state_ == password_manager::ui::PENDING_PASSWORD_UPDATE_STATE ||
vasilii 2015/08/19 13:01:17 It's the same condition as above.
dvadym 2015/08/19 16:49:54 Thanks, PENDING_PASSWORD_UPDATE_STATE ->PENDING_PA
+ pending_password_
+ .IsPossibleChangePasswordFormWithoutUsername()) {
+ update_password_submission_event_ =
+ metrics_util::NO_ACCOUNTS_NO_INTERACTION;
+ }
+ }
+ if (update_password_submission_event_ != metrics_util::NO_UPDATE_SUBMISSION)
+ LogUpdatePasswordSubmissionEvent(update_password_submission_event_);
}
void ManagePasswordsBubbleModel::OnNopeClicked() {
dismissal_reason_ = metrics_util::CLICKED_NOPE;
RecordExperimentStatistics(web_contents(), dismissal_reason_);
+ if (pending_password_.IsPossibleChangePasswordFormWithoutUsername())
vasilii 2015/08/19 13:01:18 How is it possible to get into that function from
dvadym 2015/08/19 16:49:54 We want to log not only UpdateBubble, but any even
+ update_password_submission_event_ = metrics_util::NO_ACCOUNTS_CLICKED_NOPE;
if (state_ != password_manager::ui::CREDENTIAL_REQUEST_STATE)
state_ = password_manager::ui::PENDING_PASSWORD_STATE;
}
+void ManagePasswordsBubbleModel::OnNopeUpdateClicked() {
+ update_password_submission_event_ =
vasilii 2015/08/19 13:01:17 optional: "if () else if () else" seems more reada
dvadym 2015/08/19 16:49:54 At first I tried with If else, but it was awful, w
+ password_overridden_ ? metrics_util::PASSWORD_OVERRIDEN_CLICKED_NOPE
+ : (ShouldShowMultipleAccountUpdateUI()
+ ? metrics_util::MULTIPLE_ACCOUNTS_CLICKED_NOPE
+ : metrics_util::ONE_ACCOUNT_CLICKED_NOPE);
+}
+
void ManagePasswordsBubbleModel::OnConfirmationForNeverForThisSite() {
never_save_passwords_ = true;
UpdatePendingStateTitle();
@@ -245,6 +295,10 @@ void ManagePasswordsBubbleModel::OnUnblacklistClicked() {
void ManagePasswordsBubbleModel::OnSaveClicked() {
dismissal_reason_ = metrics_util::CLICKED_SAVE;
RecordExperimentStatistics(web_contents(), dismissal_reason_);
+ if (pending_password_.IsPossibleChangePasswordFormWithoutUsername()) {
vasilii 2015/08/19 13:01:17 How is it possible to get into that function from
dvadym 2015/08/19 16:49:53 We want to log not only UpdateBubble, but any even
+ update_password_submission_event_ =
+ metrics_util::NO_ACCOUNTS_CLICKED_UPDATE;
+ }
ManagePasswordsUIController* manage_passwords_ui_controller =
ManagePasswordsUIController::FromWebContents(web_contents());
manage_passwords_ui_controller->SavePassword();
@@ -253,6 +307,12 @@ void ManagePasswordsBubbleModel::OnSaveClicked() {
void ManagePasswordsBubbleModel::OnUpdateClicked(
const autofill::PasswordForm& password_form) {
+ update_password_submission_event_ =
+ password_overridden_
+ ? metrics_util::PASSWORD_OVERRIDEN_CLICKED_UPDATE
+ : (ShouldShowMultipleAccountUpdateUI()
+ ? metrics_util::MULTIPLE_ACCOUNTS_CLICKED_UPDATE
+ : metrics_util::ONE_ACCOUNT_CLICKED_UPDATE);
ManagePasswordsUIController* manage_passwords_ui_controller =
ManagePasswordsUIController::FromWebContents(web_contents());
manage_passwords_ui_controller->UpdatePassword(password_form);

Powered by Google App Engine
This is Rietveld 408576698