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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 months ago by melandory
Modified:
5 months, 3 weeks 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
6 months 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 ...
6 months 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 ...
6 months ago (2017-02-16 20:04:43 UTC) #17
melandory
mattm@chromium.org: Please review changes in this CL.
6 months 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 ...
6 months 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 ...
5 months, 3 weeks ago (2017-02-27 15:01:48 UTC) #30
melandory
5 months, 3 weeks 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 ...
5 months, 3 weeks 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
5 months, 3 weeks ago (2017-02-27 22:40:23 UTC) #35
commit-bot: I haz the power
5 months, 3 weeks 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 b40b6558b