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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 9 months ago by lzheng
Modified:
2 years, 11 months ago
Reviewers:
eroman
CC:
chromium-reviews_chromium.org, 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) Lint 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 0 errors 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 1 errors 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 0 errors 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 0 errors Download
Commit:

Messages

Total messages: 11
lzheng
More will come. This change is mainly to add a file to the browser test. ...
3 years, 9 months ago #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 ? ...
3 years, 9 months ago #2
lzheng1
+ Scott Hey, Scott: Eric suggested that we should let you take a quick look ...
3 years, 9 months ago #3
sky
On Wed, Jul 7, 2010 at 5:54 PM, Lei Zheng <lzheng@google.com> wrote: > + Scott ...
3 years, 9 months ago #4
lzheng1
Scott: Thanks for the response. A couple of comments inline: On Thu, Jul 8, 2010 ...
3 years, 9 months ago #5
sky
Thu, Jul 8, 2010 at 9:22 AM, Lei Zheng <lzheng@google.com> wrote: > Scott: > Thanks ...
3 years, 9 months ago #6
lzheng1
On Thu, Jul 8, 2010 at 9:38 AM, Scott Violet <sky@chromium.org> wrote: > Thu, Jul ...
3 years, 9 months ago #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: > ...
3 years, 9 months ago #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 ...
3 years, 9 months ago #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: > ...
3 years, 9 months ago #10
eroman
3 years, 9 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