Add the missing callback to client for CheckResourceUrl.

Add the missing callback to client for CheckResourceUrl.

Also updated the test to check for the matching full hash in that case.

BUG=679906
Review-Url: https://codereview.chromium.org/2622063002
Cr-Commit-Position: refs/heads/master@{#443096}
Committed: https://chromium.googlesource.com/chromium/src/+/ff1a6112240cb285eab6a15a62419c2359b84ff5

[vakh (use Gerrit instead), 2017-01-10 23:36:24]

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

[commit-bot: I haz the power, 2017-01-10 23:37:32]

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

[vakh (use Gerrit instead), 2017-01-10 23:38:26]

Add the missing callback to client for CheckResourceUrl

[vakh (use Gerrit instead), 2017-01-10 23:38:30]

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

[vakh (use Gerrit instead), 2017-01-10 23:38:34]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-01-10 23:39:02]

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

[vakh (use Gerrit instead), 2017-01-10 23:51:46]

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

[commit-bot: I haz the power, 2017-01-10 23:56:22]

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

[commit-bot: I haz the power, 2017-01-10 23:56:22]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[vakh (use Gerrit instead), 2017-01-11 00:08:57]

rebase

[vakh (use Gerrit instead), 2017-01-11 00:08:59]

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

[commit-bot: I haz the power, 2017-01-11 00:09:33]

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

[vakh (use Gerrit instead), 2017-01-11 00:12:11]

nit: fix mistake in rebase

[vakh (use Gerrit instead), 2017-01-11 00:12:12]

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

[commit-bot: I haz the power, 2017-01-11 00:12:45]

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

[Nathan Parker, 2017-01-11 00:13:35]

https://codereview.chromium.org/2622063002/diff/40001/components/safe_browsin...
File components/safe_browsing_db/v4_local_database_manager.cc (right):

https://codereview.chromium.org/2622063002/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:656: if
(check->client_callback_type == ClientCallbackType::CHECK_BROWSE_URL) {
How about a switch statement, so the compiler will enforce that you've dealt
with all the cases?  CHECK_OTHER could call NOTREACHED still.

https://codereview.chromium.org/2622063002/diff/60001/components/safe_browsin...
File components/safe_browsing_db/v4_local_database_manager.cc (right):

https://codereview.chromium.org/2622063002/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:242: bool
V4LocalDatabaseManager::CheckResourceUrl(const GURL& url, Client* client) {
How about a unit test for this?

[commit-bot: I haz the power, 2017-01-11 01:36:48]

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

[commit-bot: I haz the power, 2017-01-11 01:36:50]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[vakh (use Gerrit instead), 2017-01-11 20:53:45]

Add test, incorporate nparker@ feedback

[vakh (use Gerrit instead), 2017-01-11 20:53:46]

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

[commit-bot: I haz the power, 2017-01-11 20:54:25]

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

[vakh (use Gerrit instead), 2017-01-11 20:54:34]

PTAL. Thanks.

https://codereview.chromium.org/2622063002/diff/40001/components/safe_browsin...
File components/safe_browsing_db/v4_local_database_manager.cc (right):

https://codereview.chromium.org/2622063002/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:656: if
(check->client_callback_type == ClientCallbackType::CHECK_BROWSE_URL) {
On 2017/01/11 00:13:35, Nathan Parker wrote:
> How about a switch statement, so the compiler will enforce that you've dealt
> with all the cases?  CHECK_OTHER could call NOTREACHED still.

Done.

https://codereview.chromium.org/2622063002/diff/60001/components/safe_browsin...
File components/safe_browsing_db/v4_local_database_manager.cc (right):

https://codereview.chromium.org/2622063002/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:242: bool
V4LocalDatabaseManager::CheckResourceUrl(const GURL& url, Client* client) {
On 2017/01/11 00:13:35, Nathan Parker wrote:
> How about a unit test for this?

Done.

[commit-bot: I haz the power, 2017-01-11 21:44:38]

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

[commit-bot: I haz the power, 2017-01-11 21:44:39]

Dry run: This issue passed the CQ dry run.

[Nathan Parker, 2017-01-11 22:59:59]

lgtm

LGTM w/ one suggestion and one nit

https://codereview.chromium.org/2622063002/diff/80001/components/safe_browsin...
File components/safe_browsing_db/v4_local_database_manager.cc (right):

https://codereview.chromium.org/2622063002/diff/80001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:657: case
ClientCallbackType::CHECK_BROWSE_URL: {
nit: You don't need the {}'s, unless you're using them to scope something (just
on the last one)

https://codereview.chromium.org/2622063002/diff/80001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:683: 
Don't include default here (instead list the _OTHER). That way when someone adds
a new callbackType, the compiler will complain here.

[vakh (use Gerrit instead), 2017-01-11 23:22:37]

Incorporate nparker@ feedback

[vakh (use Gerrit instead), 2017-01-11 23:22:40]

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

[vakh (use Gerrit instead), 2017-01-11 23:22:47]

The CQ bit was checked by vakh@chromium.org

[vakh (use Gerrit instead), 2017-01-11 23:22:47]

The patchset sent to the CQ was uploaded after l-g-t-m from nparker@chromium.org
Link to the patchset: https://codereview.chromium.org/2622063002/#ps100001
(title: "Incorporate nparker@ feedback")

[vakh (use Gerrit instead), 2017-01-11 23:23:18]

https://codereview.chromium.org/2622063002/diff/80001/components/safe_browsin...
File components/safe_browsing_db/v4_local_database_manager.cc (right):

https://codereview.chromium.org/2622063002/diff/80001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:657: case
ClientCallbackType::CHECK_BROWSE_URL: {
On 2017/01/11 22:59:59, Nathan Parker wrote:
> nit: You don't need the {}'s, unless you're using them to scope something
(just
> on the last one)

Done.

https://codereview.chromium.org/2622063002/diff/80001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:683: 
On 2017/01/11 22:59:59, Nathan Parker wrote:
> Don't include default here (instead list the _OTHER). That way when someone
adds
> a new callbackType, the compiler will complain here.

Done.

https://codereview.chromium.org/2622063002/diff/80001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:683: 
On 2017/01/11 22:59:59, Nathan Parker wrote:
> Don't include default here (instead list the _OTHER). That way when someone
adds
> a new callbackType, the compiler will complain here.

Done.

[commit-bot: I haz the power, 2017-01-11 23:23:59]

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-01-12 01:00:20]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484176967476710,
"parent_rev": "fc0c5aaa4f06389720901ff12af43fe6738515ac", "commit_rev":
"ff1a6112240cb285eab6a15a62419c2359b84ff5"}

[commit-bot: I haz the power, 2017-01-12 01:01:07]

Description was changed from

==========
Add the missing callback to client for CheckResourceUrl.

Also updated the test to check for the matching full hash in that case.

BUG=679906
==========

to

==========
Add the missing callback to client for CheckResourceUrl.

Also updated the test to check for the matching full hash in that case.

BUG=679906

Review-Url: https://codereview.chromium.org/2622063002
Cr-Commit-Position: refs/heads/master@{#443096}
Committed:
https://chromium.googlesource.com/chromium/src/+/ff1a6112240cb285eab6a15a6241...
==========

[commit-bot: I haz the power, 2017-01-12 01:01:08]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/ff1a6112240cb285eab6a15a6241...