Chromium Code Reviews
Help | Chromium Project | Sign in

Issue 2690413002: Refactor safe browsing service tests.

Can't Edit
Can't Publish+Mail
Start Review
1 week, 3 days ago by melandory
1 week ago
chromium-reviews,,, Nathan Parker
Target Ref:


Refactor safe browsing service tests. Extracts code which helps to mock communication with the V4 database. This code will be reused in the browser tests for the Subrsource Filter. Part 1, (this one) Part 2, BUG=671962

Patch Set 1 : . #

Patch Set 2 : rename #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+222 lines, -143 lines) Patch
M chrome/browser/ View 1 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/safe_browsing/ View 1 4 chunks +15 lines, -143 lines 0 comments Download
A chrome/browser/safe_browsing/v4_test_utils.h View 1 1 chunk +99 lines, -0 lines 2 comments Download
A chrome/browser/safe_browsing/ View 1 1 chunk +106 lines, -0 lines 1 comment Download
Commit queue not available (can’t edit this change).

Dependent Patchsets:


Total messages: 25 (20 generated)
Hey, PTAL. Here (in te second comment) are more details why this is needed:
1 week, 3 days ago (2017-02-14 17:57:50 UTC) #7
vakh (Varun Khaneja)
LGTM. Modulo one nit but I don't feel super strongly about it: It might be ...
1 week, 1 day ago (2017-02-16 20:03:38 UTC) #16
vakh (Varun Khaneja)
Still lgtm. One more thing, please revise the CL description to make it clearer to ...
1 week, 1 day ago (2017-02-16 20:04:43 UTC) #17
melandory Please review changes in this CL.
1 week ago (2017-02-17 09:55:17 UTC) #22
1 week ago (2017-02-17 21:49:53 UTC) #25
File chrome/browser/safe_browsing/ (right):
chrome/browser/safe_browsing/ #include
Don't need to duplicate includes that are in the v4_test_utils.h file
File chrome/browser/safe_browsing/v4_test_utils.h (right):
chrome/browser/safe_browsing/v4_test_utils.h:8: #include
Is this needed?
chrome/browser/safe_browsing/v4_test_utils.h:50: // Owned by
V4LocalDatabaseManager. Each test in the V4SafeBrowsingServiceTest
update comment (no longer specific to V4SafeBrowsingServiceTest)
Sign in to reply to this message.

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