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

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

Created:
10 years, 5 months ago by lzheng
Modified:
9 years, 7 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

Messages

Total messages: 11 (0 generated)
lzheng
More will come. This change is mainly to add a file to the browser test. ...
10 years, 5 months 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 ? ...
10 years, 5 months ago (2010-07-07 23:02:06 UTC) #2
lzheng1
+ Scott Hey, Scott: Eric suggested that we should let you take a quick look ...
10 years, 5 months 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 ...
10 years, 5 months 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 ...
10 years, 5 months 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 ...
10 years, 5 months 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 ...
10 years, 5 months 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: > ...
10 years, 5 months 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 ...
10 years, 5 months 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: > ...
10 years, 5 months ago (2010-07-09 22:12:58 UTC) #10
eroman
10 years, 5 months ago (2010-07-13 22:22:27 UTC) #11
LGTM

Powered by Google App Engine
This is Rietveld 408576698