Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(79)

Issue 6374017: Add caching to phishing client side detection. (Closed)

Created:
9 years, 10 months ago by gcasto (DO NOT USE)
Modified:
9 years, 10 months ago
Reviewers:
noelutz, Brian Ryner
CC:
chromium-reviews, Paweł Hajdan Jr., chrome-anti-phishing_googlegroups.com
Visibility:
Public.

Description

Add caching to phishing client side detection. BUG=None TEST=Ran client_side_detection_service_unittest Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=74874

Patch Set 1 #

Patch Set 2 : Updated header comments #

Total comments: 18

Patch Set 3 : Address comments #

Patch Set 4 : Add testing for cache refresh. Also make interval that we keep track of requests configurable. #

Total comments: 5

Patch Set 5 : Change kMaxReports to kMaxReportsPerInterval and change a few comments. #

Patch Set 6 : Add UMA stat for not being able to serialize request. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+224 lines, -22 lines) Patch
M chrome/browser/safe_browsing/client_side_detection_service.h View 1 2 3 4 4 chunks +31 lines, -4 lines 0 comments Download
M chrome/browser/safe_browsing/client_side_detection_service.cc View 1 2 3 4 5 6 chunks +84 lines, -12 lines 0 comments Download
M chrome/browser/safe_browsing/client_side_detection_service_unittest.cc View 1 2 3 3 chunks +109 lines, -6 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
gcasto (DO NOT USE)
9 years, 10 months ago (2011-02-07 21:41:20 UTC) #1
noelutz
codereview.chromium.org doesn't seem to accept comments at the moment. client_side_detection_service.h: - add a comment that ...
9 years, 10 months ago (2011-02-08 02:14:55 UTC) #2
gcasto (DO NOT USE)
On Mon, Feb 7, 2011 at 5:50 PM, Noé Lutz <noelutz@google.com> wrote: > codereview.chromium.org doesn't ...
9 years, 10 months ago (2011-02-08 19:02:21 UTC) #3
noelutz
LGTM. On Tue, Feb 8, 2011 at 11:01 AM, Garrett Casto <gcasto@google.com> wrote: > > ...
9 years, 10 months ago (2011-02-08 19:35:25 UTC) #4
Brian Ryner
http://codereview.chromium.org/6374017/diff/4001/chrome/browser/safe_browsing/client_side_detection_service.cc File chrome/browser/safe_browsing/client_side_detection_service.cc (right): http://codereview.chromium.org/6374017/diff/4001/chrome/browser/safe_browsing/client_side_detection_service.cc#newcode236 chrome/browser/safe_browsing/client_side_detection_service.cc:236: LOG(INFO) << "Satisfying request for " << phishing_url << ...
9 years, 10 months ago (2011-02-08 20:16:48 UTC) #5
gcasto (DO NOT USE)
http://codereview.chromium.org/6374017/diff/4001/chrome/browser/safe_browsing/client_side_detection_service.cc File chrome/browser/safe_browsing/client_side_detection_service.cc (right): http://codereview.chromium.org/6374017/diff/4001/chrome/browser/safe_browsing/client_side_detection_service.cc#newcode236 chrome/browser/safe_browsing/client_side_detection_service.cc:236: LOG(INFO) << "Satisfying request for " << phishing_url << ...
9 years, 10 months ago (2011-02-09 00:10:52 UTC) #6
gcasto (DO NOT USE)
I just uploaded a new patch which adds testing for cache refresh and makes the ...
9 years, 10 months ago (2011-02-09 19:56:33 UTC) #7
Brian Ryner
LGTM Just a couple of small things. http://codereview.chromium.org/6374017/diff/11001/chrome/browser/safe_browsing/client_side_detection_service.cc File chrome/browser/safe_browsing/client_side_detection_service.cc (right): http://codereview.chromium.org/6374017/diff/11001/chrome/browser/safe_browsing/client_side_detection_service.cc#newcode354 chrome/browser/safe_browsing/client_side_detection_service.cc:354: UpdateCache(); If ...
9 years, 10 months ago (2011-02-09 23:46:42 UTC) #8
gcasto (DO NOT USE)
http://codereview.chromium.org/6374017/diff/11001/chrome/browser/safe_browsing/client_side_detection_service.h File chrome/browser/safe_browsing/client_side_detection_service.h (right): http://codereview.chromium.org/6374017/diff/11001/chrome/browser/safe_browsing/client_side_detection_service.h#newcode115 chrome/browser/safe_browsing/client_side_detection_service.h:115: static const int kMaxReports; On 2011/02/09 23:46:42, Brian Ryner ...
9 years, 10 months ago (2011-02-10 19:28:33 UTC) #9
gcasto (DO NOT USE)
Added an UMA stat for issues with serializing the request, as Noe requested out of ...
9 years, 10 months ago (2011-02-10 21:08:41 UTC) #10
Brian Ryner
9 years, 10 months ago (2011-02-10 21:22:33 UTC) #11
LGTM

Powered by Google App Engine
This is Rietveld 408576698