Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(15)

Issue 2720643003: Call CSD whitelist checking on UI thread and record UMA (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 3 weeks ago by Jialiu Lin
Modified:
5 months, 2 weeks ago
CC:
chromium-reviews, grt+watch_chromium.org, sdefresne+watchlist_chromium.org, asvitkine+watch_chromium.org, droger+watchlist_chromium.org, blundell+watchlist_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Call CSD whitelist checking on UI thread and record UMA This is the first part of CL to log non-whitelisted password-reuses to UMA. This CL creates a function on UI thread to trigger the checking of CSD whitelist (actual checking happens on IO thread), such that password_reuse_detection_manager can call. BUG=691103 Review-Url: https://codereview.chromium.org/2720643003 Cr-Commit-Position: refs/heads/master@{#454405} Committed: https://chromium.googlesource.com/chromium/src/+/8b7d485807e80a6fd022fe96c474ff97fb75fc03

Patch Set 1 #

Patch Set 2 : nit #

Patch Set 3 : Add TODO #

Total comments: 34

Patch Set 4 : address comments from nparker and vakh #

Patch Set 5 : nit #

Total comments: 4

Patch Set 6 : change source_set name #

Unified diffs Side-by-side diffs Delta from patch set Stats (+260 lines, -0 lines) Patch
M chrome/browser/BUILD.gn View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/safe_browsing/safe_browsing_service.h View 1 2 3 3 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/safe_browsing/safe_browsing_service.cc View 1 2 3 3 chunks +10 lines, -0 lines 0 comments Download
M components/BUILD.gn View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
A components/safe_browsing/password_protection/BUILD.gn View 1 2 3 4 5 1 chunk +35 lines, -0 lines 0 comments Download
A components/safe_browsing/password_protection/DEPS View 1 1 chunk +7 lines, -0 lines 0 comments Download
A components/safe_browsing/password_protection/password_protection_service.h View 1 2 3 4 1 chunk +47 lines, -0 lines 0 comments Download
A components/safe_browsing/password_protection/password_protection_service.cc View 1 2 3 4 1 chunk +49 lines, -0 lines 0 comments Download
A components/safe_browsing/password_protection/password_protection_service_unittest.cc View 1 2 3 4 1 chunk +88 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 chunk +15 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 98 (82 generated)
Jialiu Lin
nparker@ and vakh@, Please take a look. Thanks!
5 months, 3 weeks ago (2017-03-01 19:48:33 UTC) #67
Nathan Parker
lgtm https://codereview.chromium.org/2720643003/diff/270001/chrome/browser/safe_browsing/safe_browsing_service.cc File chrome/browser/safe_browsing/safe_browsing_service.cc (right): https://codereview.chromium.org/2720643003/diff/270001/chrome/browser/safe_browsing/safe_browsing_service.cc#newcode335 chrome/browser/safe_browsing/safe_browsing_service.cc:335: new PasswordProtectionService(database_manager()); nit: I think these and others ...
5 months, 3 weeks ago (2017-03-01 22:59:22 UTC) #68
vakh (use Gerrit instead)
Nice work and I really liked that you added the tests for histograms! https://codereview.chromium.org/2720643003/diff/270001/chrome/browser/safe_browsing/safe_browsing_service.cc File ...
5 months, 3 weeks ago (2017-03-01 23:25:20 UTC) #69
Jialiu Lin
Thanks! https://codereview.chromium.org/2720643003/diff/270001/chrome/browser/safe_browsing/safe_browsing_service.cc File chrome/browser/safe_browsing/safe_browsing_service.cc (right): https://codereview.chromium.org/2720643003/diff/270001/chrome/browser/safe_browsing/safe_browsing_service.cc#newcode335 chrome/browser/safe_browsing/safe_browsing_service.cc:335: new PasswordProtectionService(database_manager()); On 2017/03/01 22:59:22, Nathan Parker wrote: ...
5 months, 3 weeks ago (2017-03-02 00:53:07 UTC) #72
vakh (use Gerrit instead)
lgtm https://codereview.chromium.org/2720643003/diff/270001/components/safe_browsing/password_protection/BUILD.gn File components/safe_browsing/password_protection/BUILD.gn (right): https://codereview.chromium.org/2720643003/diff/270001/components/safe_browsing/password_protection/BUILD.gn#newcode5 components/safe_browsing/password_protection/BUILD.gn:5: source_set("password_protection_service") { On 2017/03/02 00:53:06, Jialiu Lin wrote: ...
5 months, 3 weeks ago (2017-03-02 01:01:01 UTC) #73
Nathan Parker
lgtm https://codereview.chromium.org/2720643003/diff/270001/components/safe_browsing/password_protection/password_protection_service.h File components/safe_browsing/password_protection/password_protection_service.h (right): https://codereview.chromium.org/2720643003/diff/270001/components/safe_browsing/password_protection/password_protection_service.h#newcode26 components/safe_browsing/password_protection/password_protection_service.h:26: void RecordPasswordReuse(const GURL& main_frame_url); On 2017/03/02 00:53:06, Jialiu ...
5 months, 3 weeks ago (2017-03-02 01:02:42 UTC) #74
Jialiu Lin
Some remaining comments. Thanks! https://codereview.chromium.org/2720643003/diff/270001/components/safe_browsing/password_protection/BUILD.gn File components/safe_browsing/password_protection/BUILD.gn (right): https://codereview.chromium.org/2720643003/diff/270001/components/safe_browsing/password_protection/BUILD.gn#newcode5 components/safe_browsing/password_protection/BUILD.gn:5: source_set("password_protection_service") { On 2017/03/02 01:01:00, ...
5 months, 3 weeks ago (2017-03-02 01:33:15 UTC) #75
Jialiu Lin
+holte@ for histograms.xml +alexmos@ because content/public/browser and content/public/test are added into components/safe_browsing/password_protection/DEPS Thanks!
5 months, 3 weeks ago (2017-03-02 03:38:41 UTC) #77
Jialiu Lin
+blundell@ for components/BUILD.gn
5 months, 3 weeks ago (2017-03-02 03:40:25 UTC) #79
blundell (OOO until Sept 4)
lgtm https://codereview.chromium.org/2720643003/diff/310001/components/safe_browsing/password_protection/BUILD.gn File components/safe_browsing/password_protection/BUILD.gn (right): https://codereview.chromium.org/2720643003/diff/310001/components/safe_browsing/password_protection/BUILD.gn#newcode5 components/safe_browsing/password_protection/BUILD.gn:5: source_set("pp_service") { nit: you should just call these ...
5 months, 2 weeks ago (2017-03-02 15:54:02 UTC) #80
blundell (OOO until Sept 4)
lgtm
5 months, 2 weeks ago (2017-03-02 15:54:03 UTC) #81
alexmos
LGTM
5 months, 2 weeks ago (2017-03-02 17:40:46 UTC) #82
Jialiu Lin
Thanks! https://codereview.chromium.org/2720643003/diff/310001/components/safe_browsing/password_protection/BUILD.gn File components/safe_browsing/password_protection/BUILD.gn (right): https://codereview.chromium.org/2720643003/diff/310001/components/safe_browsing/password_protection/BUILD.gn#newcode5 components/safe_browsing/password_protection/BUILD.gn:5: source_set("pp_service") { On 2017/03/02 15:54:02, blundell wrote: > ...
5 months, 2 weeks ago (2017-03-02 19:02:50 UTC) #88
Steven Holte
histograms lgtm
5 months, 2 weeks ago (2017-03-02 21:00:23 UTC) #89
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2720643003/350001
5 months, 2 weeks ago (2017-03-02 22:09:41 UTC) #94
commit-bot: I haz the power
5 months, 2 weeks ago (2017-03-02 22:23:04 UTC) #97
Message was sent while issue was closed.
Committed patchset #6 (id:350001) as
https://chromium.googlesource.com/chromium/src/+/8b7d485807e80a6fd022fe96c474...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b