Skip to content

Commit

Permalink
[Passwords] Record authentication time in the management bubble
Browse files Browse the repository at this point in the history
This CL is recording the time the user takes to complete the
authentication flow when navigating to the details view in the new
password management bubble.

Bug: 1427951, 1297513
Change-Id: I66370ed178535dd047b1f8a22fc34b5accd4626d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4370684
Reviewed-by: Viktor Semeniuk <vsemeniuk@google.com>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1122325}
  • Loading branch information
mohamedamir authored and Chromium LUCI CQ committed Mar 27, 2023
1 parent 38d0695 commit 1868f29
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,14 @@ void ItemsBubbleController::AuthenticateUserAndDisplayDetailsOf(
// bubble is closed (and controller is destructed) while the reauth flow is
// running, no callback will be invoked upon the conclusion of the
// authentication flow.
delegate_->AuthenticateUserWithMessage(
message,
auto on_reath_complete =
base::BindOnce(&ItemsBubbleController::OnUserAuthenticationCompleted,
weak_ptr_factory_.GetWeakPtr(), std::move(password_form),
std::move(completion)));
std::move(completion));
delegate_->AuthenticateUserWithMessage(
message, metrics_util::TimeCallback(
std::move(on_reath_complete),
"PasswordManager.ManagementBubble.AuthenticationTime"));
}

bool ItemsBubbleController::UsernameExists(const std::u16string& username) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,11 @@ class ItemsBubbleControllerTest : public ::testing::Test {
GetCurrentForms() const;
void DestroyController();

base::test::TaskEnvironment& task_environment() { return task_environment_; }

private:
content::BrowserTaskEnvironment task_environment_;
content::BrowserTaskEnvironment task_environment_{
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
content::RenderViewHostTestEnabler rvh_enabler_;
std::unique_ptr<TestingProfile> profile_;
raw_ptr<syncer::TestSyncService> test_sync_service_;
Expand Down Expand Up @@ -310,19 +313,28 @@ TEST_F(ItemsBubbleControllerTest, OnUpdateUsernameAndPasswordNote) {

TEST_F(ItemsBubbleControllerTest,
ShouldChangeSelectedPasswordOnSuccessfulOsAuth) {
// The time it takes the user to complete the authentication flow in seconds.
const int kTimeToAuth = 10;
base::HistogramTester histogram_tester;
Init();
password_manager::PasswordForm selected_form = CreateTestForm();

EXPECT_CALL(*delegate(), AuthenticateUserWithMessage)
.WillOnce(testing::WithArg<1>(testing::Invoke(
[&](PasswordsModelDelegate::AvailabilityCallback callback) {
// Waiting for kTimeToAuth seconds to simulate the time user will
// need to authenticate
task_environment().FastForwardBy(base::Seconds(kTimeToAuth));
// Respond with true to simulate a successful user reauth.
std::move(callback).Run(true);
})));
base::MockCallback<base::OnceCallback<void(bool)>> mock_callback;
EXPECT_CALL(mock_callback, Run(true));
controller()->AuthenticateUserAndDisplayDetailsOf(selected_form,
mock_callback.Get());
histogram_tester.ExpectUniqueTimeSample(
"PasswordManager.ManagementBubble.AuthenticationTime",
base::Seconds(kTimeToAuth), 1);
EXPECT_EQ(controller()->get_currently_selected_password(), selected_form);
}

Expand Down
4 changes: 3 additions & 1 deletion tools/metrics/histograms/metadata/password/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3761,11 +3761,13 @@ chromium-metrics-reviews@google.com.
</histogram>

<histogram name="PasswordManager.{Location}.AuthenticationTime" units="ms"
expires_after="M115">
expires_after="M120">
<owner>vsemeniuk@google.com</owner>
<owner>vasilii@chromium.org</owner>
<summary>Records the time it takes user to authenticate {Location}.</summary>
<token key="Location">
<variant name="ManagementBubble"
summary="in management bubble when navigating to details view"/>
<variant name="PasswordFilling"
summary="on webpage after selecting a suggestion to fill"/>
<variant name="Settings"
Expand Down

0 comments on commit 1868f29

Please sign in to comment.