Each DatabaseManager gets to decide which stores to track

Each DatabaseManager gets to decide which stores to track.
V4LocalDatabaseManager now tracks the same lists for updates as well as full hashes.
SafeBrowsingDatabaseManager tracks only API requests.

Also, since hash_tables.h is deprecated, moved all code from base::hash_set to std::unordered_set using sed:
$ sed -i 's/base::hash_set/std::unordered_set/g' *

BUG=543161, 576864
Committed: https://crrev.com/824ff459fd925601083cd9204ef6eebaa76d39c1
Cr-Commit-Position: refs/heads/master@{#419014}

[vakh (use Gerrit instead), 2016-09-14 19:30:47]

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

[commit-bot: I haz the power, 2016-09-14 19:31:30]

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-09-14 19:31:34]

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

[vakh (use Gerrit instead), 2016-09-14 19:40:17]

reset V4GetHashManager on StopOnIOThread in V4LDBM also

[vakh (use Gerrit instead), 2016-09-14 19:40:19]

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

[commit-bot: I haz the power, 2016-09-14 19:40:51]

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-09-14 20:33:35]

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

[commit-bot: I haz the power, 2016-09-14 20:33:36]

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_...)

[Nathan Parker, 2016-09-14 20:59:45]

re: the CL desc -- We'll still need the V4GetHashProtocolManager on Android,
where there's only a RemoteDatabaseManager.

An idea: Maybe the child class (remote and v4) should implement the creation of
the V4GHPM, so it can have different stores_to_look, and if there's common code
it could go in the parent class.

[vakh (use Gerrit instead), 2016-09-14 21:22:17]

On 2016/09/14 20:59:45, Nathan Parker wrote:
> re: the CL desc -- We'll still need the V4GetHashProtocolManager on Android,
> where there's only a RemoteDatabaseManager.

Do we? Doesn't the Android code deal with full hashes behind the Java API?

> 
> An idea: Maybe the child class (remote and v4) should implement the creation
of
> the V4GHPM, so it can have different stores_to_look, and if there's common
code
> it could go in the parent class.

Good idea. I'll add a virtual method in SBDBM that specifies what stores to use
and then override it in V4LDBM.

[vakh (use Gerrit instead), 2016-09-15 20:31:44]

Each DBmanager defines stores to look. base::hash_set -> std::unorderd_set.
StoreToFileNameMap -> StoreIdAndFilename struct

[vakh (use Gerrit instead), 2016-09-15 20:31:47]

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

[commit-bot: I haz the power, 2016-09-15 20:32:54]

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-09-15 20:40:53]

On 2016/09/14 21:22:17, vakh (Varun Khaneja) wrote:
> On 2016/09/14 20:59:45, Nathan Parker wrote:
> > re: the CL desc -- We'll still need the V4GetHashProtocolManager on Android,
> > where there's only a RemoteDatabaseManager.
> 
> Do we? Doesn't the Android code deal with full hashes behind the Java API?
> 
> > 
> > An idea: Maybe the child class (remote and v4) should implement the creation
> of
> > the V4GHPM, so it can have different stores_to_look, and if there's common
> code
> > it could go in the parent class.
> 
> Good idea. I'll add a virtual method in SBDBM that specifies what stores to
use
> and then override it in V4LDBM.

Done. PTAL.

[vakh (use Gerrit instead), 2016-09-15 20:41:26]

Description was changed from

==========
V4LocalDatabaseManager has its own instance of V4GetHashManager
It throws away the instance of V4GetHashManager that it inherits from
SafeBrowsingDatabaseManager. That instance will go away when PVer3 is
deprecated.

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

to

==========
V4LocalDatabaseManager has its own instance of V4GetHashManager
It throws away the instance of V4GetHashManager that it inherits from
SafeBrowsingDatabaseManager. That instance will go away when PVer3 is
deprecated.

BUG=543161, 576864
==========

[vakh (use Gerrit instead), 2016-09-15 20:47:11]

Description was changed from

==========
V4LocalDatabaseManager has its own instance of V4GetHashManager
It throws away the instance of V4GetHashManager that it inherits from
SafeBrowsingDatabaseManager. That instance will go away when PVer3 is
deprecated.

BUG=543161, 576864
==========

to

==========
Each DatabaseManager gets to decide which stores to track.
V4LocalDatabaseManager now tracks the same lists for updates as well as full
hashes.
SafeBrowsingDatabaseManager tracks only API requests.

Also, since hash_tables.h is deprecated, moved all code from base::hash_set to
std::unordered_set using sed:
$ sed -i 's/base::hash_set/std::unordered_set/g' *

BUG=543161, 576864
==========

[Nathan Parker, 2016-09-15 21:12:54]

lgtm

https://codereview.chromium.org/2345573002/diff/40001/components/safe_browsin...
File components/safe_browsing_db/database_manager.h (right):

https://codereview.chromium.org/2345573002/diff/40001/components/safe_browsin...
components/safe_browsing_db/database_manager.h:168:
GetStoresForFullHashRequests();
Could this be protected?

https://codereview.chromium.org/2345573002/diff/40001/components/safe_browsin...
File components/safe_browsing_db/v4_database.h (right):

https://codereview.chromium.org/2345573002/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_database.h:42: // disk could be named:
"uws_win_url.store"
nit: the example file doesn't match the naming style the actual files are using.

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

https://codereview.chromium.org/2345573002/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:60:
std::unordered_set<UpdateListIdentifier> GetStoresForFullHashRequests()
protected as well

[vakh (use Gerrit instead), 2016-09-15 21:22:37]

Incorporated nparker@ feedback

[vakh (use Gerrit instead), 2016-09-15 21:22:39]

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

[commit-bot: I haz the power, 2016-09-15 21:23:07]

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

[Scott Hess - ex-Googler, 2016-09-15 21:24:00]

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

[Scott Hess - ex-Googler, 2016-09-15 21:24:01]

lgtm

[vakh (use Gerrit instead), 2016-09-15 21:33:37]

https://codereview.chromium.org/2345573002/diff/40001/components/safe_browsin...
File components/safe_browsing_db/database_manager.h (right):

https://codereview.chromium.org/2345573002/diff/40001/components/safe_browsin...
components/safe_browsing_db/database_manager.h:168:
GetStoresForFullHashRequests();
On 2016/09/15 21:12:54, Nathan Parker wrote:
> Could this be protected?

Done.

https://codereview.chromium.org/2345573002/diff/40001/components/safe_browsin...
File components/safe_browsing_db/v4_database.h (right):

https://codereview.chromium.org/2345573002/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_database.h:42: // disk could be named:
"uws_win_url.store"
On 2016/09/15 21:12:54, Nathan Parker wrote:
> nit: the example file doesn't match the naming style the actual files are
using.

Done.

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

https://codereview.chromium.org/2345573002/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:60:
std::unordered_set<UpdateListIdentifier> GetStoresForFullHashRequests()
On 2016/09/15 21:12:54, Nathan Parker wrote:
> protected as well

Done.

[vakh (use Gerrit instead), 2016-09-15 21:33:45]

The CQ bit was checked by vakh@chromium.org

[vakh (use Gerrit instead), 2016-09-15 21:33:46]

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/2345573002/#ps60001
(title: "Incorporated nparker@ feedback")

[vakh (use Gerrit instead), 2016-09-15 21:33:46]

The CQ bit was unchecked by vakh@chromium.org

[vakh (use Gerrit instead), 2016-09-15 21:33:58]

The CQ bit was checked by vakh@chromium.org

[commit-bot: I haz the power, 2016-09-15 21:34:09]

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-09-15 22:23:24]

Description was changed from

==========
Each DatabaseManager gets to decide which stores to track.
V4LocalDatabaseManager now tracks the same lists for updates as well as full
hashes.
SafeBrowsingDatabaseManager tracks only API requests.

Also, since hash_tables.h is deprecated, moved all code from base::hash_set to
std::unordered_set using sed:
$ sed -i 's/base::hash_set/std::unordered_set/g' *

BUG=543161, 576864
==========

to

==========
Each DatabaseManager gets to decide which stores to track.
V4LocalDatabaseManager now tracks the same lists for updates as well as full
hashes.
SafeBrowsingDatabaseManager tracks only API requests.

Also, since hash_tables.h is deprecated, moved all code from base::hash_set to
std::unordered_set using sed:
$ sed -i 's/base::hash_set/std::unordered_set/g' *

BUG=543161, 576864
==========

[commit-bot: I haz the power, 2016-09-15 22:23:25]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-15 22:27:06]

Description was changed from

==========
Each DatabaseManager gets to decide which stores to track.
V4LocalDatabaseManager now tracks the same lists for updates as well as full
hashes.
SafeBrowsingDatabaseManager tracks only API requests.

Also, since hash_tables.h is deprecated, moved all code from base::hash_set to
std::unordered_set using sed:
$ sed -i 's/base::hash_set/std::unordered_set/g' *

BUG=543161, 576864
==========

to

==========
Each DatabaseManager gets to decide which stores to track.
V4LocalDatabaseManager now tracks the same lists for updates as well as full
hashes.
SafeBrowsingDatabaseManager tracks only API requests.

Also, since hash_tables.h is deprecated, moved all code from base::hash_set to
std::unordered_set using sed:
$ sed -i 's/base::hash_set/std::unordered_set/g' *

BUG=543161, 576864

Committed: https://crrev.com/824ff459fd925601083cd9204ef6eebaa76d39c1
Cr-Commit-Position: refs/heads/master@{#419014}
==========

[commit-bot: I haz the power, 2016-09-15 22:27:08]

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

[kcarattini, 2016-09-18 12:20:46]

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

[kcarattini, 2016-09-18 12:20:46]

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

https://codereview.chromium.org/2345573002/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:222:
DCHECK(!store_id_file_names_.empty());
Just checking that this list includes API checks as well?

[vakh (use Gerrit instead), 2016-09-19 17:03:06]

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

https://codereview.chromium.org/2345573002/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:222:
DCHECK(!store_id_file_names_.empty());
On 2016/09/18 12:20:46, kcarattini wrote:
> Just checking that this list includes API checks as well?

SafeBrowsingDatabaseManager includes API checks. V4LocalDatabaseManager doesn't
include API checks *yet*,  but it's straightforward to add it by adding a store
for it in GetStoreIdAndFileNames()

[kcarattini, 2016-09-26 00:23:34]

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

https://codereview.chromium.org/2345573002/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:222:
DCHECK(!store_id_file_names_.empty());
On 2016/09/19 17:03:05, vakh (Varun Khaneja) wrote:
> On 2016/09/18 12:20:46, kcarattini wrote:
> > Just checking that this list includes API checks as well?
> 
> SafeBrowsingDatabaseManager includes API checks. V4LocalDatabaseManager
doesn't
> include API checks *yet*,  but it's straightforward to add it by adding a
store
> for it in GetStoreIdAndFileNames()

So that means that until it's added to V$, API checks will still Use the V3
LocalDatabaseManager and RemoteDatabaseManager. Is that ok? Are other things
still using the v3 database manager?

[vakh (use Gerrit instead), 2016-09-26 06:09:55]

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

https://codereview.chromium.org/2345573002/diff/60001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.cc:222:
DCHECK(!store_id_file_names_.empty());
On 2016/09/26 00:23:34, kcarattini wrote:
> On 2016/09/19 17:03:05, vakh (Varun Khaneja) wrote:
> > On 2016/09/18 12:20:46, kcarattini wrote:
> > > Just checking that this list includes API checks as well?
> > 
> > SafeBrowsingDatabaseManager includes API checks. V4LocalDatabaseManager
> doesn't
> > include API checks *yet*,  but it's straightforward to add it by adding a
> store
> > for it in GetStoreIdAndFileNames()
> 
> So that means that until it's added to V$, API checks will still Use the V3
> LocalDatabaseManager and RemoteDatabaseManager. Is that ok? Are other things
> still using the v3 database manager?

That's OK. Other things are sill using PVer3 but I am going to additionally
enable V4 for Malware and Phishing this week.