New issue
Advanced search Search tips

Issue 827110 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Password Manager metrics are incorrect when credentials already saved

Project Member Reported by dvadym@chromium.org, Mar 29 2018

Issue description

On many sites, metrics about password submission success are not sent when credentials are already saved. That skews our metrics and make them unreliable. 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a28a1a3605ebeb89beac600ea8f844b3a594314

commit 6a28a1a3605ebeb89beac600ea8f844b3a594314
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Thu Mar 29 15:58:57 2018

[Password Manager] Submission metrics fix.

The submission metrics are recorded when Password Manager knows whether
submission was successful or not. But it turned out that sometimes
information about submitted form is cleared before the moment when
success of the submission is known. Actually clearing form information
is wrong. This CL makes that form information is left.

Bug:  827110 

Change-Id: I53d3de461ae6f7dd53afb07f8c084e36f05f4767
Reviewed-on: https://chromium-review.googlesource.com/986134
Commit-Queue: Dominic Battré <battre@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546850}
[modify] https://crrev.com/6a28a1a3605ebeb89beac600ea8f844b3a594314/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/6a28a1a3605ebeb89beac600ea8f844b3a594314/components/password_manager/core/browser/password_manager_unittest.cc

Labels: Merge-Request-66
It's very simple and non-risky fix for Password Manager metrics. Without it, out metrics are significantly broken.
Project Member

Comment 3 by sheriffbot@chromium.org, Apr 3 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-66 Merge-Approved-66
Approving merge to M66. Branch:3359
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 4 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c22d88a457fa7bb634467fcc69133bf1fd69ada

commit 3c22d88a457fa7bb634467fcc69133bf1fd69ada
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Apr 04 11:03:21 2018

[Merge to M-66][Password Manager] Submission metrics fix.

The submission metrics are recorded when Password Manager knows whether
submission was successful or not. But it turned out that sometimes
information about submitted form is cleared before the moment when
success of the submission is known. Actually clearing form information
is wrong. This CL makes that form information is left.

Bug:  827110 

TBR=dvadym@chromium.org

(cherry picked from commit 6a28a1a3605ebeb89beac600ea8f844b3a594314)

Change-Id: I53d3de461ae6f7dd53afb07f8c084e36f05f4767
Reviewed-on: https://chromium-review.googlesource.com/986134
Commit-Queue: Dominic Battré <battre@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#546850}
Reviewed-on: https://chromium-review.googlesource.com/995276
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#575}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/3c22d88a457fa7bb634467fcc69133bf1fd69ada/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/3c22d88a457fa7bb634467fcc69133bf1fd69ada/components/password_manager/core/browser/password_manager_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/065efd2fc6b5400b371f4fc3511e67833c2d23be

commit 065efd2fc6b5400b371f4fc3511e67833c2d23be
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Apr 04 13:27:21 2018

[Password Manager] Test for metric fix.

On CL Password Manager metrics were fixed. This CL implements proper
test for it. The idea of this test, is to reproduce exact call to
PasswordManager, namely:

OnPasswordFormsParsed
OnPasswordFormSubmitted
OnPasswordFormsParsed
ShowManualFallbackForSaving  <- here before fix, provisional_save_manager_ was dropped
OnPasswordFormsParsed

Bug:  827110 
Change-Id: I4f7c850ba20078b1182d4eebf1701b39f113f74e
Reviewed-on: https://chromium-review.googlesource.com/995193
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548035}
[modify] https://crrev.com/065efd2fc6b5400b371f4fc3511e67833c2d23be/components/password_manager/core/browser/password_manager_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment