Refactor safe browsing service tests.

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

[melandory, 2017-02-14 17:43:03]

The CQ bit was checked by melandory@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-14 17:43:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[melandory, 2017-02-14 17:54:47]

Patchset #2 (id:10001) has been deleted

[melandory, 2017-02-14 17:54:50]

Patchset #1 (id:1) has been deleted

[melandory, 2017-02-14 17:55:08]

Patchset #2 (id:50001) has been deleted

[melandory, 2017-02-14 17:57:49]

melandory@chromium.org changed reviewers:
+ vakh@chromium.org

[melandory, 2017-02-14 17:57:50]

Hey,

PTAL. Here (in te second comment) are more details why this is needed:
https://codereview.chromium.org/2645283007/diff/100001/chrome/browser/safe_br...

[melandory, 2017-02-15 09:30:45]

Description was changed from

==========
Extract some code from the Safe BRowsing tests, so it can be used in browser
tests for the Subrsource Filter.

BUG=671962
==========

to

==========
Refactor safe browsing service tests.

Extracts some code from the Safe Browsing service tests, so it can be used in
browser tests for the Subrsource Filter.

BUG=671962
==========

[melandory, 2017-02-15 09:30:56]

Patchset #1 (id:30001) has been deleted

[melandory, 2017-02-15 09:52:03]

The CQ bit was checked by melandory@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 09:52:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[melandory, 2017-02-15 09:57:49]

Patchset #1 (id:70001) has been deleted

[commit-bot: I haz the power, 2017-02-15 10:54:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 10:54:55]

Dry run: This issue passed the CQ dry run.

[melandory, 2017-02-16 09:54:21]

Description was changed from

==========
Refactor safe browsing service tests.

Extracts some code from the Safe Browsing service tests, so it can be used in
browser tests for the Subrsource Filter.

BUG=671962
==========

to

==========
Refactor safe browsing service tests.

Extracts some code from the Safe Browsing service tests, so it can be used in
browser tests for the Subrsource Filter.

Part 1, https://codereview.chromium.org/2690413002 (this one)
Part 2, https://codereview.chromium.org/2645283007


BUG=671962
==========

[vakh (use Gerrit instead), 2017-02-16 20:03:38]

LGTM.

Modulo one nit but I don't feel super strongly about it: It might be better to
call the file as comp/safe_browsing_db/v4_test_utils.* -- that would make them
easier to discover since they'll be right next to the classes that they are
overriding.

[vakh (use Gerrit instead), 2017-02-16 20:04:43]

Still lgtm.

One more thing, please revise the CL description to make it clearer to
understand what you are changing.

[melandory, 2017-02-17 09:22:35]

Description was changed from

==========
Refactor safe browsing service tests.

Extracts some code from the Safe Browsing service tests, so it can be used in
browser tests for the Subrsource Filter.

Part 1, https://codereview.chromium.org/2690413002 (this one)
Part 2, https://codereview.chromium.org/2645283007


BUG=671962
==========

to

==========
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
==========

[melandory, 2017-02-17 09:47:12]

The CQ bit was checked by melandory@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 09:47:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[melandory, 2017-02-17 09:55:16]

melandory@chromium.org changed reviewers:
+ mattm@chromium.org

[melandory, 2017-02-17 09:55:17]

mattm@chromium.org: Please review changes in this CL.

[commit-bot: I haz the power, 2017-02-17 11:31:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 11:31:09]

Dry run: This issue passed the CQ dry run.

[mattm, 2017-02-17 21:49:53]

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)

[melandory, 2017-02-27 13:37:35]

The CQ bit was checked by melandory@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-27 13:37:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-27 14:53:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-27 14:53:44]

Dry run: This issue passed the CQ dry run.

[melandory, 2017-02-27 15:01:48]

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"
On 2017/02/17 21:49:53, mattm wrote:
> Don't need to duplicate includes that are in the v4_test_utils.h file

Done.

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"
On 2017/02/17 21:49:53, mattm wrote:
> Is this needed?

Done.

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
On 2017/02/17 21:49:53, mattm wrote:
> update comment (no longer specific to V4SafeBrowsingServiceTest)

Done.

[mattm, 2017-02-27 22:03:30]

note for future reference: when updating a codereview it's generally appreciated
to upload the rebase as one patchset (with no changes other than the rebase) and
then the local changes in another patchset, keeps things easier to review.

lgtm

[melandory, 2017-02-27 22:39:09]

The CQ bit was checked by melandory@chromium.org

[melandory, 2017-02-27 22:39:10]

The patchset sent to the CQ was uploaded after l-g-t-m from vakh@chromium.org
Link to the patchset: https://codereview.chromium.org/2690413002/#ps130001
(title: "addressed comments")

[commit-bot: I haz the power, 2017-02-27 22:40:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-27 22:54:16]

CQ is committing da patch.

Bot data: {"patchset_id": 130001, "attempt_start_ts": 1488235149691050,
"parent_rev": "7085aa755cacbf3906608d930347a6b214428f76", "commit_rev":
"994815ee0b5bbf09286abeb06ae25fda6aa50e07"}

[commit-bot: I haz the power, 2017-02-27 22:54:55]

Description was changed from

==========
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
==========

to

==========
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/+/994815ee0b5bbf09286abeb06ae2...
==========

[commit-bot: I haz the power, 2017-02-27 22:54:56]

Committed patchset #3 (id:130001) as
https://chromium.googlesource.com/chromium/src/+/994815ee0b5bbf09286abeb06ae2...