Only call UrlToFullHashes once from LocalSafeBrowsingDatabaseManager::CheckBrowseUrl

Only call UrlToFullHashes once from LocalSafeBrowsingDatabaseManager::CheckBrowseUrl

Add ContainsBrowseHashes and ContainsUnwantedSoftwareHashes to
SafeBrowsingDatabase, to parallel ContainsBrowseUrl and
ContainsUnwantedSoftwareUrl. These functions take the full hash set instead of
the url, so that the caller can look up both "browse" and "unwanted software"
data for the same url without incurring the overhead of calling UrlToFullHashes
twice.

BUG=576750
Committed: https://crrev.com/1fbfd5fca8cc6270032b3c8ea9d1df6402af4e8c
Cr-Commit-Position: refs/heads/master@{#369755}

[Joe Mason, 2016-01-12 18:30:48]

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

[Joe Mason, 2016-01-14 15:44:26]

Ping? I would like to get this committed for the M49 branch tomorrow.

[chromium-reviews, 2016-01-14 18:16:59]

I'm ooo today, will try to get to this tonight.  Varun -- can you take a
first look?  I think the overall plan is fine.

On Thu, Jan 14, 2016 at 7:44 AM, <joenotcharles@chromium.org> wrote:

> Ping? I would like to get this committed for the M49 branch tomorrow.
>
> https://codereview.chromium.org/1579183003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vakh (use Gerrit instead), 2016-01-14 18:57:47]

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

[vakh (use Gerrit instead), 2016-01-14 18:57:48]

lgtm

https://codereview.chromium.org/1579183003/diff/20001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_database.cc (right):

https://codereview.chromium.org/1579183003/diff/20001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_database.cc:804: bool
SafeBrowsingDatabaseNew::ContainsUnwantedSoftwareUrl(
We could completely remove these Contains.*Url methods if we remove them from
here also:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/saf...

[Joe Mason, 2016-01-14 19:24:08]

https://codereview.chromium.org/1579183003/diff/20001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_database.cc (right):

https://codereview.chromium.org/1579183003/diff/20001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_database.cc:804: bool
SafeBrowsingDatabaseNew::ContainsUnwantedSoftwareUrl(
On 2016/01/14 18:57:47, vakh wrote:
> We could completely remove these Contains.*Url methods if we remove them from
> here also:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/saf...

Ack, browser_tests won't even build since the test stub doesn't implement
Contains*Hashes. Sorry, thought I ran that test suite but I must have forgot.

[Joe Mason, 2016-01-14 20:19:58]

On 2016/01/14 18:57:48, vakh wrote:
>
https://codereview.chromium.org/1579183003/diff/20001/chrome/browser/safe_bro...
> chrome/browser/safe_browsing/safe_browsing_database.cc:804: bool
> SafeBrowsingDatabaseNew::ContainsUnwantedSoftwareUrl(
> We could completely remove these Contains.*Url methods if we remove them from
> here also:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/saf...

Fixed the SafeBrowsingService test. Thanks for pointing that out. I don't want
to remove the existing functions without more review of the API. I'd prefer to
do that as a followup after M49.

[vakh (old account. dont use), 2016-01-14 21:34:02]

vakh@google.com changed reviewers:
+ vakh@google.com

[vakh (old account. dont use), 2016-01-14 21:34:02]

lgtm

[vakh (use Gerrit instead), 2016-01-14 21:35:05]

lgtm

LGTM.
Sorry, using the correct account now.

[Nathan Parker, 2016-01-15 01:08:50]

lgtm

+1 to removing the *ByUrl() functions since they have no value now. I'm fine if
it's in a follow-up CL.

[Joe Mason, 2016-01-15 05:16:45]

The CQ bit was checked by joenotcharles@chromium.org

[commit-bot: I haz the power, 2016-01-15 05:17:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1579183003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1579183003/40001

[commit-bot: I haz the power, 2016-01-15 05:20:00]

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

[commit-bot: I haz the power, 2016-01-15 05:20:00]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Joe Mason, 2016-01-15 15:32:06]

The CQ bit was checked by joenotcharles@chromium.org

[Joe Mason, 2016-01-15 15:32:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, vakh@chromium.org, vakh@google.com
Link to the patchset: https://codereview.chromium.org/1579183003/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-01-15 15:32:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1579183003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1579183003/60001

[commit-bot: I haz the power, 2016-01-15 16:10:46]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-15 16:11:57]

Description was changed from

==========
Only call UrlToFullHashes once from
LocalSafeBrowsingDatabaseManager::CheckBrowseUrl

Add ContainsBrowseHashes and ContainsUnwantedSoftwareHashes to
SafeBrowsingDatabase, to parallel ContainsBrowseUrl and
ContainsUnwantedSoftwareUrl. These functions take the full hash set instead of
the url, so that the caller can look up both "browse" and "unwanted software"
data for the same url without incurring the overhead of calling UrlToFullHashes
twice.

BUG=576750
==========

to

==========
Only call UrlToFullHashes once from
LocalSafeBrowsingDatabaseManager::CheckBrowseUrl

Add ContainsBrowseHashes and ContainsUnwantedSoftwareHashes to
SafeBrowsingDatabase, to parallel ContainsBrowseUrl and
ContainsUnwantedSoftwareUrl. These functions take the full hash set instead of
the url, so that the caller can look up both "browse" and "unwanted software"
data for the same url without incurring the overhead of calling UrlToFullHashes
twice.

BUG=576750

Committed: https://crrev.com/1fbfd5fca8cc6270032b3c8ea9d1df6402af4e8c
Cr-Commit-Position: refs/heads/master@{#369755}
==========

[commit-bot: I haz the power, 2016-01-15 16:11:58]

Patchset 4 (id:??) landed as
https://crrev.com/1fbfd5fca8cc6270032b3c8ea9d1df6402af4e8c
Cr-Commit-Position: refs/heads/master@{#369755}