Commit b0bb4724 authored by Ioana Pandele's avatar Ioana Pandele Committed by Commit Bot

Add metrics for password generation on Android

Bug: 879486, 835234
Change-Id: Ife5ece610ec7f3173caf368681e3f3338c96354b
Reviewed-on: https://chromium-review.googlesource.com/1195483
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587963}
parent aeafd48e
......@@ -54,8 +54,7 @@ void PasswordGenerationDialogViewAndroid::PasswordAccepted(
void PasswordGenerationDialogViewAndroid::PasswordRejected(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) {
// TODO(ioanap): Send message to controller. This will probably be used
// mainly for metrics.
controller_->GeneratedPasswordRejected();
}
// static
......
......@@ -101,6 +101,7 @@
#include "extensions/common/constants.h"
#endif
using autofill::password_generation::PasswordGenerationUserEvent;
using password_manager::metrics_util::PasswordType;
using password_manager::BadMessageReason;
using password_manager::ContentPasswordManagerDriverFactory;
......@@ -822,6 +823,13 @@ void ChromePasswordManagerClient::PresaveGeneratedPassword(
if (popup_controller_)
popup_controller_->UpdatePassword(password_form.password_value);
#if defined(OS_ANDROID)
PasswordAccessoryController* password_accessory =
PasswordAccessoryController::FromWebContents(web_contents());
if (password_accessory)
password_accessory->MaybeGeneratedPasswordChanged(
password_form.password_value);
#endif
password_manager_.OnPresaveGeneratedPassword(password_form);
}
......@@ -839,6 +847,12 @@ void ChromePasswordManagerClient::PasswordNoLongerGenerated(
PasswordGenerationPopupController::kEditGeneratedPassword) {
HidePasswordGenerationPopup();
}
#if defined(OS_ANDROID)
PasswordAccessoryController* password_accessory =
PasswordAccessoryController::FromWebContents(web_contents());
if (password_accessory)
password_accessory->GeneratedPasswordDeleted();
#endif
}
const GURL& ChromePasswordManagerClient::GetMainFrameURL() const {
......
......@@ -20,6 +20,7 @@
#include "components/autofill/content/browser/content_autofill_driver.h"
#include "components/autofill/content/browser/content_autofill_driver_factory.h"
#include "components/autofill/core/common/password_form.h"
#include "components/autofill/core/common/password_generation_util.h"
#include "components/favicon/core/favicon_service.h"
#include "components/password_manager/content/browser/content_password_manager_driver.h"
#include "components/password_manager/content/browser/content_password_manager_driver_factory.h"
......@@ -31,6 +32,7 @@
#include "ui/base/ui_base_features.h"
using autofill::PasswordForm;
using autofill::password_generation::PasswordGenerationUserEvent;
using Item = PasswordAccessoryViewInterface::AccessoryItem;
struct PasswordAccessoryController::GenerationElementData {
......@@ -80,6 +82,53 @@ struct PasswordAccessoryController::FaviconRequestData {
gfx::Image cached_icon;
};
class PasswordAccessoryController::GeneratedPasswordMetricsRecorder {
public:
explicit GeneratedPasswordMetricsRecorder(const base::string16& password)
: initial_password_(password), was_edited_(false) {}
void PasswordAccepted() {
autofill::password_generation::LogPasswordGenerationUserEvent(
PasswordGenerationUserEvent::kPasswordAccepted);
}
// Called when a generated password was presaved, which signals a possible
// change in the password.
void MaybePasswordChanged(const base::string16& new_password) {
if (was_edited_)
return;
// Check if the password is different from the accepted password, as this
// method is also called on the first presave after the user accepts the
// generated password.
if (new_password != initial_password_) {
was_edited_ = true;
autofill::password_generation::LogPasswordGenerationUserEvent(
PasswordGenerationUserEvent::kPasswordEdited);
}
}
void PasswordDeleted() {
autofill::password_generation::LogPasswordGenerationUserEvent(
PasswordGenerationUserEvent::kPasswordDeleted);
}
void PasswordRejected() {
autofill::password_generation::LogPasswordGenerationUserEvent(
PasswordGenerationUserEvent::kPasswordRejectedInDialog);
}
private:
// The initial password that was accepted by the user. Used to detect
// that the password was edited.
base::string16 initial_password_;
// Whether or not the password was edited. Used to prevent logging the same
// event multiple times.
bool was_edited_;
DISALLOW_COPY_AND_ASSIGN(GeneratedPasswordMetricsRecorder);
};
PasswordAccessoryController::PasswordAccessoryController(
content::WebContents* web_contents)
: web_contents_(web_contents),
......@@ -169,6 +218,16 @@ void PasswordAccessoryController::OnAutomaticGenerationStatusChanged(
view_->OnAutomaticGenerationStatusChanged(available);
}
void PasswordAccessoryController::MaybeGeneratedPasswordChanged(
const base::string16& changed_password) {
generated_password_metrics_recorder_->MaybePasswordChanged(changed_password);
}
void PasswordAccessoryController::GeneratedPasswordDeleted() {
generated_password_metrics_recorder_->PasswordDeleted();
generated_password_metrics_recorder_.reset();
}
void PasswordAccessoryController::OnFilledIntoFocusedField(
autofill::FillingStatus status) {
if (status != autofill::FillingStatus::SUCCESS)
......@@ -281,6 +340,8 @@ void PasswordAccessoryController::OnGenerationRequested() {
->ReportSpecPriorityForGeneratedPassword(generation_element_data_->form,
spec_priority);
}
generated_password_metrics_recorder_ =
std::make_unique<GeneratedPasswordMetricsRecorder>(password);
dialog_view_->Show(password);
}
......@@ -289,10 +350,13 @@ void PasswordAccessoryController::GeneratedPasswordAccepted(
if (!target_frame_driver_)
return;
target_frame_driver_->GeneratedPasswordAccepted(password);
generated_password_metrics_recorder_->PasswordAccepted();
dialog_view_.reset();
}
void PasswordAccessoryController::GeneratedPasswordRejected() {
generated_password_metrics_recorder_->PasswordRejected();
generated_password_metrics_recorder_.reset();
dialog_view_.reset();
}
......
......@@ -80,6 +80,14 @@ class PasswordAccessoryController
autofill::password_generation::PasswordGenerationUIData>& ui_data,
const base::WeakPtr<password_manager::PasswordManagerDriver>& driver);
// Notifies the controller that the generated password has potentially
// changed. Currently only used for recording metrics.
void MaybeGeneratedPasswordChanged(const base::string16& changed_password);
// Notifies the controller that the generated password was deleted. Used for
// recording metrics.
void GeneratedPasswordDeleted();
// Completes a filling attempt by recording metrics, giving feedback to the
// user and dismissing the accessory sheet.
void OnFilledIntoFocusedField(autofill::FillingStatus status);
......@@ -154,6 +162,10 @@ class PasswordAccessoryController
// Data allowing to cache favicons and favicon-related requests.
struct FaviconRequestData;
// Created when generation is requested the user. Used for recording metrics
// for the lifetime of the generated password.
class GeneratedPasswordMetricsRecorder;
// Required for construction via |CreateForWebContents|:
explicit PasswordAccessoryController(content::WebContents* contents);
friend class content::WebContentsUserData<PasswordAccessoryController>;
......@@ -220,6 +232,13 @@ class PasswordAccessoryController
// The favicon service used to make retrieve icons for a given origin.
favicon::FaviconService* favicon_service_;
// Used during the lifetime of a generated password to record metrics.
// The lifetime of a generated password starts when a user requests generation
// and ends when the password is rejected, deleted or when another password
// is generated.
std::unique_ptr<GeneratedPasswordMetricsRecorder>
generated_password_metrics_recorder_;
base::WeakPtrFactory<PasswordAccessoryController> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(PasswordAccessoryController);
......
......@@ -58,6 +58,10 @@ void LogPasswordGenerationEvent(PasswordGenerationEvent event) {
event, EVENT_ENUM_COUNT);
}
void LogPasswordGenerationUserEvent(PasswordGenerationUserEvent event) {
UMA_HISTOGRAM_ENUMERATION("PasswordGeneration.UserEvent", event);
}
bool IsPasswordGenerationEnabled() {
if (base::FeatureList::IsEnabled(
autofill::features::kAutomaticPasswordGeneration))
......
......@@ -75,6 +75,25 @@ enum PasswordGenerationEvent {
EVENT_ENUM_COUNT
};
// These values are used for metrics. Entries should not be renumbered and
// numeric values should never be reused.
// Metric: PasswordGeneration.UserEvent
enum class PasswordGenerationUserEvent {
// The generated password was accepted by the user.
kPasswordAccepted = 0,
// The generated password was edited by the user in the field in which
// it was filled after being accepted.
kPasswordEdited = 1,
// The generated password was deleted by the user from the field
// in which it was filled after being accepted.
kPasswordDeleted = 2,
// The generated password was rejected by the user from the modal
// dialog, either by pressing "Cancel" in the dialog or by dismissing it.
// Only used on Android.
kPasswordRejectedInDialog = 3,
kMaxValue = kPasswordRejectedInDialog
};
// Wrapper to store the user interactions with the password generation bubble.
struct PasswordGenerationActions {
// Whether the user has clicked on the learn more link.
......@@ -124,6 +143,8 @@ void LogUserActions(PasswordGenerationActions actions);
void LogPasswordGenerationEvent(PasswordGenerationEvent event);
void LogPasswordGenerationUserEvent(PasswordGenerationUserEvent event);
// Enumerates user actions after password generation bubble is shown.
// These are visible for testing purposes.
enum UserAction {
......
......@@ -37172,6 +37172,13 @@ Called by update_net_trust_anchors.py.-->
<int value="14" label="Generation item in the context menu shown"/>
</enum>
<enum name="PasswordGenerationUserEvent">
<int value="0" label="Password accepted"/>
<int value="1" label="Password edited"/>
<int value="2" label="Password deleted"/>
<int value="3" label="Password rejected in modal dialog"/>
</enum>
<enum name="PasswordImportFromCSVResult">
<int value="0" label="Password import succeeded"/>
<int value="1" label="Password import failed due to IO error"/>
......@@ -70898,6 +70898,15 @@ uploading your change for review.
</summary>
</histogram>
<histogram name="PasswordGeneration.UserEvent"
enum="PasswordGenerationUserEvent">
<owner>ioanap@chromium.org</owner>
<summary>
Records user-triggered events related to a generated password. Each event is
logged at most once per lifetime of the generated password.
</summary>
</histogram>
<histogram name="PasswordHash.CreateTime">
<owner>mlerman@chromium.org</owner>
<summary>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment