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

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, 3 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
Project "None" does not have a commit queue.

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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month ago (2010-07-09 22:12:58 UTC) #10
eroman
5 years, 1 month 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 c33a7a4