Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(9)

Issue 2690413002: Refactor safe browsing service tests. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months, 2 weeks ago by melandory
Modified:
4 months 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 Review-Url: https://codereview.chromium.org/2690413002 Cr-Commit-Position: refs/heads/master@{#453372} Committed: https://chromium.googlesource.com/chromium/src/+/994815ee0b5bbf09286abeb06ae25fda6aa50e07

Patch Set 1 : . #

Patch Set 2 : rename #

Total comments: 6

Patch Set 3 : addressed comments #

Messages

Total messages: 38 (28 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
4 months, 2 weeks ago (2017-02-14 17:57:50 UTC) #7
vakh (use Gerrit instead)
LGTM. Modulo one nit but I don't feel super strongly about it: It might be ...
4 months, 1 week ago (2017-02-16 20:03:38 UTC) #16
vakh (use Gerrit instead)
Still lgtm. One more thing, please revise the CL description to make it clearer to ...
4 months, 1 week ago (2017-02-16 20:04:43 UTC) #17
melandory
mattm@chromium.org: Please review changes in this CL.
4 months, 1 week ago (2017-02-17 09:55:17 UTC) #22
mattm
https://codereview.chromium.org/2690413002/diff/110001/chrome/browser/safe_browsing/v4_test_utils.cc File chrome/browser/safe_browsing/v4_test_utils.cc (right): https://codereview.chromium.org/2690413002/diff/110001/chrome/browser/safe_browsing/v4_test_utils.cc#newcode10 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 ...
4 months, 1 week ago (2017-02-17 21:49:53 UTC) #25
melandory
https://codereview.chromium.org/2690413002/diff/110001/chrome/browser/safe_browsing/v4_test_utils.cc File chrome/browser/safe_browsing/v4_test_utils.cc (right): https://codereview.chromium.org/2690413002/diff/110001/chrome/browser/safe_browsing/v4_test_utils.cc#newcode10 chrome/browser/safe_browsing/v4_test_utils.cc:10: #include "components/safe_browsing_db/v4_get_hash_protocol_manager.h" On 2017/02/17 21:49:53, mattm wrote: > Don't ...
4 months ago (2017-02-27 15:01:48 UTC) #30
melandory
4 months ago (2017-02-27 15:01:51 UTC) #31
mattm
note for future reference: when updating a codereview it's generally appreciated to upload the rebase ...
4 months ago (2017-02-27 22:03:30 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2690413002/130001
4 months ago (2017-02-27 22:40:23 UTC) #35
commit-bot: I haz the power
4 months ago (2017-02-27 22:54:56 UTC) #38
Message was sent while issue was closed.
Committed patchset #3 (id:130001) as
https://chromium.googlesource.com/chromium/src/+/994815ee0b5bbf09286abeb06ae2...
Sign in to reply to this message.

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