Chromium Code Reviews
Help | Chromium Project | Sign in
(560)

Issue 2868030: Add switches and apis in safebrowsing code to allow tests. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 10 months ago by lzheng
Modified:
2 years, 11 months ago
Reviewers:
eroman
CC:
chromium-reviews_chromium.org, Paul Godavari, ben+cc_chromium.org
Visibility:
Public.

Description

Add switches and apis in safebrowsing protocol_manager that will allow end-to-end test later.

BUG=6787, 47318
TEST=protocol_manager_unittest.cc


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51177

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Patch Set 6 : '' #

Patch Set 7 : '' #

Total comments: 45

Patch Set 8 : '' #

Total comments: 19

Patch Set 9 : '' #

Patch Set 10 : '' #

Patch Set 11 : '' #

Patch Set 12 : '' #

Total comments: 2

Patch Set 13 : '' #

Patch Set 14 : '' #

Patch Set 15 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+343 lines, -83 lines) Lint Patch
M chrome/browser/safe_browsing/protocol_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 7 chunks +81 lines, -13 lines 0 comments 0 errors Download
M chrome/browser/safe_browsing/protocol_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 12 chunks +98 lines, -66 lines 0 comments 1 errors Download
M chrome/browser/safe_browsing/protocol_manager_unittest.cc View 2 3 4 5 6 7 8 9 10 11 12 4 chunks +119 lines, -3 lines 0 comments 1 errors Download
M chrome/browser/safe_browsing/safe_browsing_service.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -0 lines 0 comments 0 errors Download
M chrome/browser/safe_browsing/safe_browsing_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +30 lines, -1 line 0 comments 0 errors Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +3 lines, -0 lines 0 comments 0 errors Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +10 lines, -0 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 11
lzheng
First CL to add test for safebrowsing. This cl is to add necessary hooks so ...
3 years, 10 months ago #1
eroman
Overall design comment: I like to keep command-line processing out of core logic classes. And ...
3 years, 10 months ago #2
lzheng
Good points. I will make the change and ping you back. Lei On 2010/06/23 21:44:25, ...
3 years, 10 months ago #3
lzheng
I moved the switches up to safebrowsing service. Can you take another look? Thanks! Lei ...
3 years, 10 months ago #4
eroman
http://codereview.chromium.org/2868030/diff/36002/32003 File chrome/browser/safe_browsing/protocol_manager.cc (right): http://codereview.chromium.org/2868030/diff/36002/32003#newcode47 chrome/browser/safe_browsing/protocol_manager.cc:47: "http://%s/safebrowsing/newkey?client=%s&appver=%s&pver=2.2"; Is it intentional that you have changed the ...
3 years, 10 months ago #5
lzheng
Thanks for the careful review. I replace the hostname flag with two url_prefixes: one for ...
3 years, 10 months ago #6
eroman
http://codereview.chromium.org/2868030/diff/36002/32004 File chrome/browser/safe_browsing/protocol_manager.h (right): http://codereview.chromium.org/2868030/diff/36002/32004#newcode86 chrome/browser/safe_browsing/protocol_manager.h:86: void ForceScheduleNextUpdate(const int next_update_msec); On 2010/06/25 00:40:16, lzheng wrote: ...
3 years, 10 months ago #7
lzheng
http://codereview.chromium.org/2868030/diff/44001/34004 File chrome/browser/safe_browsing/protocol_manager.cc (right): http://codereview.chromium.org/2868030/diff/44001/34004#newcode557 chrome/browser/safe_browsing/protocol_manager.cc:557: GURL update_url(UpdateUrl(use_mac)); On 2010/06/25 01:33:09, eroman wrote: > Can ...
3 years, 10 months ago #8
eroman
lgtm after considering comment below. http://codereview.chromium.org/2868030/diff/47004/49002 File chrome/browser/safe_browsing/protocol_manager.cc (right): http://codereview.chromium.org/2868030/diff/47004/49002#newcode433 chrome/browser/safe_browsing/protocol_manager.cc:433: update_timer_.Stop(); Do we not ...
3 years, 10 months ago #9
lzheng
Eric: Thanks for the careful review. Comments below: http://codereview.chromium.org/2868030/diff/47004/49002 File chrome/browser/safe_browsing/protocol_manager.cc (right): http://codereview.chromium.org/2868030/diff/47004/49002#newcode433 chrome/browser/safe_browsing/protocol_manager.cc:433: update_timer_.Stop(); ...
3 years, 10 months ago #10
eroman
3 years, 10 months ago #11
lgtm
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6