New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 593295 link

Starred by 0 users
Status: Fixed
Owner:
█ vabr@chromium.org
hobby only
Closed: Mar 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[Password manager] Remove the code to assign website groups for UMA reporting

Project Member Reported by vabr@chromium.org, Mar 9 2016 
MonitoredDomainGroupId in components/password_manager/core/browser/password_manager_metrics_util.cc has been broken for some time (attempting to create a GURL from only a host, which resulted in an empty GURL), so the reporting of histograms like PasswordManager.ProvisionalSaveFailure_group_1 has not been working for quite some time.

Nobody noticed this, because sadly these histograms were not looked at. They were initially an attempt to allow us monitor password manager breakages on interesting sites while preserving user privacy [1], but the approach does not scale beyond a handful of sites, and we went with other ways of monitoring in the meantime.

Therefore, the related code should be removed, to simplify the code and get rid of unused preferences.

[1] https://docs.google.com/document/d/1mhg3fP44tG9GyZraBoKwzW_RzX27jm9vo91Z86OlXwg/edit?usp=sharing
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 9 2016 

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

commit 0090ffb1c2878f85bd94750db07c09448741172b
Author: vabr <vabr@chromium.org>
Date: Wed Mar 09 14:07:04 2016

Use password_manager::metrics_util::ResponseType on iOS

The passwords infobar delegate had a separate copy of the ResponseType enum. So
far it had a subset of values of the desktop version of ResponseType (matching
the positions), so it was not a problem, but to avoid diverging in the future,
this CL switches the iOS code to use the component-defined ResponseType (which
also matches histograms.xml).

R=melandory@chromium.org

BUG= 593295 

Review URL: https://codereview.chromium.org/1777473004

Cr-Commit-Position: refs/heads/master@{#380132}

[modify] https://crrev.com/0090ffb1c2878f85bd94750db07c09448741172b/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.h
[modify] https://crrev.com/0090ffb1c2878f85bd94750db07c09448741172b/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.mm
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 10 2016 

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

commit 229e744a728a91ab3753882fa4413f10c28678cd
Author: vabr <vabr@chromium.org>
Date: Thu Mar 10 09:58:39 2016

Remove groups handling from password manager metrics util

BUG= 593295 

Review URL: https://codereview.chromium.org/1776143003

Cr-Commit-Position: refs/heads/master@{#380384}

[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/chrome/browser/password_manager/save_password_infobar_delegate.cc
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/chrome/browser/password_manager/save_password_infobar_delegate.h
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/chrome/browser/password_manager/save_password_infobar_delegate_unittest.cc
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/components/components_tests.gyp
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/components/password_manager/core/browser/BUILD.gn
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/components/password_manager/core/browser/password_manager_metrics_util.cc
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/components/password_manager/core/browser/password_manager_metrics_util.h
[delete] https://crrev.com/7a5ee02cddcee37ccf3b86a253d64a53afe9ff75/components/password_manager/core/browser/password_manager_metrics_util_unittest.cc
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/components/password_manager/core/common/password_manager_pref_names.cc
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/components/password_manager/core/common/password_manager_pref_names.h
[modify] https://crrev.com/229e744a728a91ab3753882fa4413f10c28678cd/tools/metrics/histograms/histograms.xml

Comment 4 by vabr@chromium.org, Mar 10 2016

Status: Fixed (was: Started)

Comment 5 by vabr@chromium.org, Jun 6 2016

Labels: -refactoring Hotlist-Refactoring
►

Sign in to add a comment