New issue
Advanced search Search tips

Issue 866794 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 8
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Remove blacklist de-duplication code

Project Member Reported by gemene@google.com, Jul 24

Issue description

https://crrev.com/c/1140158 introduced BlacklistedCredentialsCleaner::RemoveDuplicates() method to delete blacklisted duplicates from PasswordStore.

Histogram named PasswordManager.BlacklistedSites.NeedRemoveBlacklistDuplicates is added in https://crrev.com/c/1164783.
This histogram has the role to show when blacklisted duplicates don't exist in PasswordStore. When this happens the code that made de-duplication and report metric will not be useful anymore.

More information:  https://crbug.com/862930 

 
Description: Show this description
Description: Show this description
Components: UI>Browser>Passwords
Labels: -Type-Bug Hotlist-TechnicalDebt Hotlist-Refactoring Type-Task
Status: Available (was: Untriaged)
Marking as available, but whoever ends up implementing this, please note that this is blocked by PasswordManager.BlacklistedSites.NeedRemoveBlacklistDuplicates showing consistently no duplicates for some time.
Owner: jdoerrie@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 8

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

commit f0eaba271750726683c5ccd07317a98a4d251358
Author: jdoerrie <jdoerrie@chromium.org>
Date: Tue Jan 08 07:40:24 2019

[Passwords] Remove blacklist de-duplication logic

This change removes the blacklist de-duplication logic that was
introduced in M70. In particular, this change deletes the
BlacklistedDuplicatesCleaner class, and gets rid of / obsoletes the
related PasswordManager.BlacklistedDuplicates and
PasswordManager.BlacklistedSites.NeedRemoveBlacklistDuplicates UMA
histograms and "profile.duplicated_blacklisted_credentials_removed"
preference.

Bug: 915901,  866794 
Change-Id: I9d934d61d1e7ed0ccbc950ef429437605661bc5c
Reviewed-on: https://chromium-review.googlesource.com/c/1397628
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620646}
[modify] https://crrev.com/f0eaba271750726683c5ccd07317a98a4d251358/components/password_manager/core/browser/BUILD.gn
[delete] https://crrev.com/16d659bed5f91f6e9b020fb0c052e1e5eef39530/components/password_manager/core/browser/blacklisted_duplicates_cleaner.cc
[delete] https://crrev.com/16d659bed5f91f6e9b020fb0c052e1e5eef39530/components/password_manager/core/browser/blacklisted_duplicates_cleaner.h
[delete] https://crrev.com/16d659bed5f91f6e9b020fb0c052e1e5eef39530/components/password_manager/core/browser/blacklisted_duplicates_cleaner_unittest.cc
[modify] https://crrev.com/f0eaba271750726683c5ccd07317a98a4d251358/components/password_manager/core/browser/invalid_realm_credential_cleaner_unittest.cc
[modify] https://crrev.com/f0eaba271750726683c5ccd07317a98a4d251358/components/password_manager/core/browser/login_database.cc
[modify] https://crrev.com/f0eaba271750726683c5ccd07317a98a4d251358/components/password_manager/core/browser/login_database_unittest.cc
[modify] https://crrev.com/f0eaba271750726683c5ccd07317a98a4d251358/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/f0eaba271750726683c5ccd07317a98a4d251358/components/password_manager/core/browser/password_manager_util.cc
[modify] https://crrev.com/f0eaba271750726683c5ccd07317a98a4d251358/components/password_manager/core/browser/password_manager_util_unittest.cc
[modify] https://crrev.com/f0eaba271750726683c5ccd07317a98a4d251358/components/password_manager/core/common/password_manager_pref_names.cc
[modify] https://crrev.com/f0eaba271750726683c5ccd07317a98a4d251358/components/password_manager/core/common/password_manager_pref_names.h
[modify] https://crrev.com/f0eaba271750726683c5ccd07317a98a4d251358/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Assigned)
Wrt #c4: We were observing an average of 0.06 in the PasswordManager.BlacklistedDuplicates histogram, i.e. very close to 0. Furthermore, PasswordManager.BlacklistedSites.NeedRemoveBlacklistDuplicates was below 2% (going further down only very very slowly), which we believe is caused by the fact that the preference was recorded before performing the actual clean-up.

Sign in to add a comment