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

Issue 2845035: First change to add safebrowsing service test. ... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 1 month ago by lzheng
Modified:
4 years, 2 months ago
Reviewers:
eroman
CC:
chromium-reviews, Paweł Hajdan Jr., Paul Godavari, ben+cc_chromium.org
Visibility:
Public.

Description

First change to add safebrowsing service test. Right now, the test only launch the browser and makes sure no auto update is scheduled. Later, the test suite server will be launched within the test and requests will be issued, BUG=47318 TEST=this is the test. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=52719

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Patch Set 6 : '' #

Total comments: 13

Patch Set 7 : '' #

Patch Set 8 : '' #

Patch Set 9 : '' #

Patch Set 10 : '' #

Total comments: 30

Patch Set 11 : '' #

Patch Set 12 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+199 lines, -0 lines) Patch
M chrome/browser/safe_browsing/protocol_manager.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
A chrome/browser/safe_browsing/safe_browsing_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +196 lines, -0 lines 0 comments Download
M chrome/browser/safe_browsing/safe_browsing_service.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
Commit: CQ not working?

Messages

Total messages: 11 (0 generated)
lzheng
More will come. This change is mainly to add a file to the browser test. ...
5 years, 1 month ago (2010-07-02 19:47:10 UTC) #1
eroman
http://codereview.chromium.org/2845035/diff/20001/21001 File chrome/browser/safe_browsing/protocol_manager.h (right): http://codereview.chromium.org/2845035/diff/20001/21001#newcode57 chrome/browser/safe_browsing/protocol_manager.h:57: friend class SafeBrowsingServiceTest; Can you not use FRIEND_TEST ? ...
5 years ago (2010-07-07 23:02:06 UTC) #2
lzheng1
+ Scott Hey, Scott: Eric suggested that we should let you take a quick look ...
5 years ago (2010-07-08 00:55:01 UTC) #3
sky
On Wed, Jul 7, 2010 at 5:54 PM, Lei Zheng <lzheng@google.com> wrote: > + Scott ...
5 years ago (2010-07-08 16:13:52 UTC) #4
lzheng1
Scott: Thanks for the response. A couple of comments inline: On Thu, Jul 8, 2010 ...
5 years ago (2010-07-08 16:22:20 UTC) #5
sky
Thu, Jul 8, 2010 at 9:22 AM, Lei Zheng <lzheng@google.com> wrote: > Scott: > Thanks ...
5 years ago (2010-07-08 16:38:23 UTC) #6
lzheng1
On Thu, Jul 8, 2010 at 9:38 AM, Scott Violet <sky@chromium.org> wrote: > Thu, Jul ...
5 years ago (2010-07-09 00:19:19 UTC) #7
lzheng
http://codereview.chromium.org/2845035/diff/20001/21001 File chrome/browser/safe_browsing/protocol_manager.h (right): http://codereview.chromium.org/2845035/diff/20001/21001#newcode57 chrome/browser/safe_browsing/protocol_manager.h:57: friend class SafeBrowsingServiceTest; On 2010/07/07 23:02:06, eroman wrote: > ...
5 years ago (2010-07-09 00:27:50 UTC) #8
eroman
Overall LGTM, some nits: http://codereview.chromium.org/2845035/diff/39002/46002 File chrome/browser/safe_browsing/safe_browsing_browsertest.cc (right): http://codereview.chromium.org/2845035/diff/39002/46002#newcode37 chrome/browser/safe_browsing/safe_browsing_browsertest.cc:37: } nit: in some cases ...
5 years ago (2010-07-09 18:23:23 UTC) #9
lzheng
Another look? http://codereview.chromium.org/2845035/diff/39002/46002 File chrome/browser/safe_browsing/safe_browsing_browsertest.cc (right): http://codereview.chromium.org/2845035/diff/39002/46002#newcode37 chrome/browser/safe_browsing/safe_browsing_browsertest.cc:37: } On 2010/07/09 18:23:23, eroman wrote: > ...
5 years ago (2010-07-09 22:12:58 UTC) #10
eroman
5 years ago (2010-07-13 22:22:27 UTC) #11
LGTM
Sign in to reply to this message.

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