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

Issue 2690413002: Refactor safe browsing service tests.

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 3 days ago by melandory
Modified:
1 week ago
CC:
chromium-reviews, grt+watch_chromium.org, vakh+watch_chromium.org, Nathan Parker
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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, https://codereview.chromium.org/2690413002 (this one) Part 2, https://codereview.chromium.org/2645283007 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/BUILD.gn View 1 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc 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/v4_test_utils.cc View 1 1 chunk +106 lines, -0 lines 1 comment Download
Commit queue not available (can’t edit this change).

Dependent Patchsets:

Messages

Total messages: 25 (20 generated)
melandory
Hey, PTAL. Here (in te second comment) are more details why this is needed: https://codereview.chromium.org/2645283007/diff/100001/chrome/browser/safe_browsing/test_safe_browsing_service.cc
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
mattm@chromium.org: Please review changes in this CL.
1 week ago (2017-02-17 09:55:17 UTC) #22
mattm
1 week ago (2017-02-17 21:49:53 UTC) #25
https://codereview.chromium.org/2690413002/diff/110001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/v4_test_utils.cc (right):

https://codereview.chromium.org/2690413002/diff/110001/chrome/browser/safe_br...
chrome/browser/safe_browsing/v4_test_utils.cc:10: #include
"components/safe_browsing_db/v4_get_hash_protocol_manager.h"
Don't need to duplicate includes that are in the v4_test_utils.h file

https://codereview.chromium.org/2690413002/diff/110001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/v4_test_utils.h (right):

https://codereview.chromium.org/2690413002/diff/110001/chrome/browser/safe_br...
chrome/browser/safe_browsing/v4_test_utils.h:8: #include
"chrome/browser/safe_browsing/test_safe_browsing_service.h"
Is this needed?

https://codereview.chromium.org/2690413002/diff/110001/chrome/browser/safe_br...
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