Small: Implement CheckDownloadUrls -- not called yet.

Small: Implement CheckDownloadUrls -- not called yet.

The structure is very similar to CheckBrowseUrl so it should be an easy
review.

BUG=543161
Committed: https://crrev.com/e2fd18bc20db5dc5f99b14a2bb553ceaeff06d86
Cr-Commit-Position: refs/heads/master@{#427905}

[vakh (use Gerrit instead), 2016-10-25 18:48:08]

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

[commit-bot: I haz the power, 2016-10-25 18:48:41]

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), 2016-10-25 18:51:26]

Use the right store: GetUrlMalBinId()

[vakh (use Gerrit instead), 2016-10-25 18:51:27]

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

[commit-bot: I haz the power, 2016-10-25 18:52:18]

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), 2016-10-25 18:58:35]

Call OnCheckDownloadUrlResult when the result is available

[vakh (use Gerrit instead), 2016-10-25 18:58:37]

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

[commit-bot: I haz the power, 2016-10-25 18:58:40]

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), 2016-10-25 19:00:24]

Patchset #2 (id:20001) has been deleted

[vakh (use Gerrit instead), 2016-10-25 19:00:38]

Patchset #1 (id:1) has been deleted

[vakh (use Gerrit instead), 2016-10-25 19:45:02]

DRY: Don't repeat yourself

[vakh (use Gerrit instead), 2016-10-25 19:45:09]

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

[commit-bot: I haz the power, 2016-10-25 19:45:34]

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, 2016-10-25 21:20:22]

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

[commit-bot: I haz the power, 2016-10-25 21:20:23]

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

[vakh (use Gerrit instead), 2016-10-25 23:59:57]

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

[Scott Hess - ex-Googler, 2016-10-26 00:09:21]

lgtm

[Nathan Parker, 2016-10-26 00:45:45]

lgtm

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

https://codereview.chromium.org/2450633003/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:153:
std::vector<GURL>(1, url));
nit: Does {url} work here? I'm not sure if that will avoid a temporary or not.

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

https://codereview.chromium.org/2450633003/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:81: enum class
ClientCallbackType {
Nit: Could this and the PendingCheck go in the .cc file? It's an implenentation
detail, so it'd make the .h easier to read for clients calling this.

Looks like PendingCheck is only referenced by ptr.

https://codereview.chromium.org/2450633003/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:91: CHECK_MAX
Nit: you don't really need a max if you're going to have a specific branch of
code for each value, since you'll detect bad ones there.

https://codereview.chromium.org/2450633003/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:125: // The URL that is
being checked for being unsafe.
URL(s) that are...

[vakh (use Gerrit instead), 2016-10-26 19:52:46]

nparker@ review + some tests

[vakh (use Gerrit instead), 2016-10-26 19:52:48]

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

[vakh (use Gerrit instead), 2016-10-26 19:53:22]

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

https://codereview.chromium.org/2450633003/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:153:
std::vector<GURL>(1, url));
On 2016/10/26 00:45:45, Nathan Parker wrote:
> nit: Does {url} work here? I'm not sure if that will avoid a temporary or not.

Tried it. MakeUnique doesn't like it very much.

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

https://codereview.chromium.org/2450633003/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:81: enum class
ClientCallbackType {
On 2016/10/26 00:45:45, Nathan Parker wrote:
> Nit: Could this and the PendingCheck go in the .cc file? It's an
implenentation
> detail, so it'd make the .h easier to read for clients calling this.
> 
> Looks like PendingCheck is only referenced by ptr.

Done.

https://codereview.chromium.org/2450633003/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:91: CHECK_MAX
On 2016/10/26 00:45:45, Nathan Parker wrote:
> Nit: you don't really need a max if you're going to have a specific branch of
> code for each value, since you'll detect bad ones there.

Done.

https://codereview.chromium.org/2450633003/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:125: // The URL that is
being checked for being unsafe.
On 2016/10/26 00:45:45, Nathan Parker wrote:
> URL(s) that are...

Done.

[commit-bot: I haz the power, 2016-10-26 19:53:22]

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, 2016-10-26 20:04:33]

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

[commit-bot: I haz the power, 2016-10-26 20:04:34]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[vakh (use Gerrit instead), 2016-10-26 21:23:36]

git rebase

[vakh (use Gerrit instead), 2016-10-26 21:23:40]

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

[commit-bot: I haz the power, 2016-10-26 21:24:13]

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, 2016-10-26 21:55:13]

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

[commit-bot: I haz the power, 2016-10-26 21:55:14]

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

[vakh (use Gerrit instead), 2016-10-26 23:42:23]

Windows doesn't like struct in .h file. Reverted test changes.

[vakh (use Gerrit instead), 2016-10-26 23:42:25]

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

[commit-bot: I haz the power, 2016-10-26 23:42:52]

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), 2016-10-26 23:43:17]

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

[vakh (use Gerrit instead), 2016-10-26 23:43:18]

holte@ -- please review the one-line change in histograms.xml

[commit-bot: I haz the power, 2016-10-26 23:55:50]

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

[commit-bot: I haz the power, 2016-10-26 23:55:52]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[vakh (use Gerrit instead), 2016-10-27 00:17:26]

s/V4LocalDatabaseManager::PendingCheck/PendingCheck

[vakh (use Gerrit instead), 2016-10-27 00:17:28]

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

[commit-bot: I haz the power, 2016-10-27 00:18:00]

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), 2016-10-27 00:38:27]

Remove histograms change. Will send that as a separate CL. Remove an unused
variable.

[vakh (use Gerrit instead), 2016-10-27 00:38:29]

The CQ bit was checked by vakh@chromium.org

[vakh (use Gerrit instead), 2016-10-27 00:38:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, shess@chromium.org
Link to the patchset: https://codereview.chromium.org/2450633003/#ps160001
(title: "Remove histograms change. Will send that as a separate CL. Remove an
unused variable.")

[commit-bot: I haz the power, 2016-10-27 00:38:47]

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

[vakh (use Gerrit instead), 2016-10-27 00:39:33]

vakh@chromium.org changed reviewers:
- holte@chromium.org

[vakh (use Gerrit instead), 2016-10-27 00:39:34]

-R/+CC: holte@ -- Will send a separate CL including other related changes for
the histogram.

[commit-bot: I haz the power, 2016-10-27 01:25:07]

Committed patchset #7 (id:160001)

[commit-bot: I haz the power, 2016-10-27 01:26:46]

Description was changed from

==========
Small: Implement CheckDownloadUrls -- not called yet.

The structure is very similar to CheckBrowseUrl so it should be an easy
review.

BUG=543161
==========

to

==========
Small: Implement CheckDownloadUrls -- not called yet.

The structure is very similar to CheckBrowseUrl so it should be an easy
review.

BUG=543161

Committed: https://crrev.com/e2fd18bc20db5dc5f99b14a2bb553ceaeff06d86
Cr-Commit-Position: refs/heads/master@{#427905}
==========

[commit-bot: I haz the power, 2016-10-27 01:26:47]

Patchset 7 (id:??) landed as
https://crrev.com/e2fd18bc20db5dc5f99b14a2bb553ceaeff06d86
Cr-Commit-Position: refs/heads/master@{#427905}