V4Only: In V4Only mode use PVer4 for all SafeBrowsing operations

Setup V4Only mode, so it can be enabled via finch.

[Other modes are V3Only (or Control) and V4Hybrid.]

V4Only mode is when Chrome uses only PVer4 implementation for all SafeBrowsing operations.
V3Only (or Control): When Chrome uses the existing PVer3 implementation for all SafeBrowsing operations.
V4Hybrid: When Chrome uses PVer4 implementation for some SafeBrowsing operations. Namely, CheckBrowseUrl.

For details, see https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bGL0/edit#heading=h.c2drkrmajbam

This is 98% vakh's CL.

BUG=543161
patch from issue 2533993004 at patchset 140001 (http://crrev.com/2533993004#ps140001)

Committed: https://crrev.com/6ec02574cadea969029792e1cbbfbab3f8ae5368
Cr-Commit-Position: refs/heads/master@{#437412}

[Nathan Parker, 2016-12-08 05:19:26]

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

[commit-bot: I haz the power, 2016-12-08 05:19: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, 2016-12-08 05:43:31]

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

[Nathan Parker, 2016-12-08 05:43:32]

shess -- I added one patch beyond vakh's CL. PTAL.

[Nathan Parker, 2016-12-08 05:44:32]

Description was changed from

==========
Implements V4Only mode.
[Other modes are V3Only (or Control) and V4Hybrid.]

V4Only mode is when Chrome uses only PVer4 implementation for all SafeBrowsing
operations.
V3Only (or Control): When Chrome uses the existing PVer3 implementation for all
SafeBrowsing operations.
V4Hybrid: When Chrome uses PVer4 implementation for some SafeBrowsing
operations. Namely, CheckBrowseUrl.

For details, see
https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bG...

This is mostly vakh's CL.

BUG=543161

patch from issue 2533993004 at patchset 140001
(http://crrev.com/2533993004#ps140001)
==========

to

==========
Implements V4Only mode.
[Other modes are V3Only (or Control) and V4Hybrid.]

V4Only mode is when Chrome uses only PVer4 implementation for all SafeBrowsing
operations.
V3Only (or Control): When Chrome uses the existing PVer3 implementation for all
SafeBrowsing operations.
V4Hybrid: When Chrome uses PVer4 implementation for some SafeBrowsing
operations. Namely, CheckBrowseUrl.

For details, see
https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bG...

This is mostly vakh's CL.

BUG=543161

patch from issue 2533993004 at patchset 140001
(http://crrev.com/2533993004#ps140001)
==========

[Scott Hess - ex-Googler, 2016-12-08 06:02:55]

LGTM.  The ref comment can probably be ignored for now in the interests of not
rewriting the world.

I can't tell offhand, but are there any consequences of the various settings
other than setting up which manager(s) to use?  AFAICT, they seem independent,
though they sound discrete.  So like if you had metrics based on v4-only or
v4-hybrid, but both somehow were enabled, then the metrics might be dirty.

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

https://codereview.chromium.org/2561053002/diff/20001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/services_delegate_impl.cc:66:
V4LocalDatabaseManager::Create(SafeBrowsingService::GetBaseFilename());
This presumably needs to be here so that someone gets the right
database_manager() somewhere.  As such, perhaps it should be documented that it
needs to be above the rest.

https://codereview.chromium.org/2561053002/diff/20001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/services_delegate_impl.h (right):

https://codereview.chromium.org/2561053002/diff/20001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/services_delegate_impl.h:31: const
scoped_refptr<SafeBrowsingDatabaseManager>& v4_local_database_manager()
Wait - you can even do this?

I mean, of course you can, it compiles.  But I'm not even sure what it means.  I
guess that the implementation guarantees a value with storage to reference
(basically, it can only return a reference to a member variable or static)?

[commit-bot: I haz the power, 2016-12-08 06:40:48]

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

[commit-bot: I haz the power, 2016-12-08 06:40:49]

Dry run: This issue passed the CQ dry run.

[Nathan Parker, 2016-12-08 23:00:58]

Thanks Scott.

It's _possible_ we could accidentally put a user in V4Hybrid and V4Only, but
we'll plan to just not do that in the finch config.

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

https://codereview.chromium.org/2561053002/diff/20001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/services_delegate_impl.cc:66:
V4LocalDatabaseManager::Create(SafeBrowsingService::GetBaseFilename());
On 2016/12/08 06:02:55, Scott Hess wrote:
> This presumably needs to be here so that someone gets the right
> database_manager() somewhere.  As such, perhaps it should be documented that
it
> needs to be above the rest.

Done.

https://codereview.chromium.org/2561053002/diff/20001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/services_delegate_impl.h (right):

https://codereview.chromium.org/2561053002/diff/20001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/services_delegate_impl.h:31: const
scoped_refptr<SafeBrowsingDatabaseManager>& v4_local_database_manager()
On 2016/12/08 06:02:55, Scott Hess wrote:
> Wait - you can even do this?
> 
> I mean, of course you can, it compiles.  But I'm not even sure what it means. 
I
> guess that the implementation guarantees a value with storage to reference
> (basically, it can only return a reference to a member variable or static)?

Yea it works, but constrains it as you describe. I think it'd be a bit clear to
use a non-ref version, but I'm trying to keep this CL small so it's easier to
merge.

[Nathan Parker, 2016-12-08 23:02:11]

Description was changed from

==========
Implements V4Only mode.
[Other modes are V3Only (or Control) and V4Hybrid.]

V4Only mode is when Chrome uses only PVer4 implementation for all SafeBrowsing
operations.
V3Only (or Control): When Chrome uses the existing PVer3 implementation for all
SafeBrowsing operations.
V4Hybrid: When Chrome uses PVer4 implementation for some SafeBrowsing
operations. Namely, CheckBrowseUrl.

For details, see
https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bG...

This is mostly vakh's CL.

BUG=543161

patch from issue 2533993004 at patchset 140001
(http://crrev.com/2533993004#ps140001)
==========

to

==========
Setup V4Only mode, so it can be enabled via finch.

[Other modes are V3Only (or Control) and V4Hybrid.]

V4Only mode is when Chrome uses only PVer4 implementation for all SafeBrowsing
operations.
V3Only (or Control): When Chrome uses the existing PVer3 implementation for all
SafeBrowsing operations.
V4Hybrid: When Chrome uses PVer4 implementation for some SafeBrowsing
operations. Namely, CheckBrowseUrl.

For details, see
https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bG...

This is 98% vakh's CL.

BUG=543161

patch from issue 2533993004 at patchset 140001
(http://crrev.com/2533993004#ps140001)
==========

[Nathan Parker, 2016-12-08 23:03:05]

The CQ bit was checked by nparker@chromium.org

[commit-bot: I haz the power, 2016-12-08 23:03:38]

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-12-09 01:35:22]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481238185171680,
"parent_rev": "aff28f377d73eda4097a9d1442bbc5492e679897", "commit_rev":
"2802fae235c749d876894ba472202d65c4ee8901"}

[commit-bot: I haz the power, 2016-12-09 01:35:50]

Description was changed from

==========
Setup V4Only mode, so it can be enabled via finch.

[Other modes are V3Only (or Control) and V4Hybrid.]

V4Only mode is when Chrome uses only PVer4 implementation for all SafeBrowsing
operations.
V3Only (or Control): When Chrome uses the existing PVer3 implementation for all
SafeBrowsing operations.
V4Hybrid: When Chrome uses PVer4 implementation for some SafeBrowsing
operations. Namely, CheckBrowseUrl.

For details, see
https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bG...

This is 98% vakh's CL.

BUG=543161

patch from issue 2533993004 at patchset 140001
(http://crrev.com/2533993004#ps140001)
==========

to

==========
Setup V4Only mode, so it can be enabled via finch.

[Other modes are V3Only (or Control) and V4Hybrid.]

V4Only mode is when Chrome uses only PVer4 implementation for all SafeBrowsing
operations.
V3Only (or Control): When Chrome uses the existing PVer3 implementation for all
SafeBrowsing operations.
V4Hybrid: When Chrome uses PVer4 implementation for some SafeBrowsing
operations. Namely, CheckBrowseUrl.

For details, see
https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bG...

This is 98% vakh's CL.

BUG=543161

patch from issue 2533993004 at patchset 140001
(http://crrev.com/2533993004#ps140001)
==========

[commit-bot: I haz the power, 2016-12-09 01:35:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-09 01:37:42]

Description was changed from

==========
Setup V4Only mode, so it can be enabled via finch.

[Other modes are V3Only (or Control) and V4Hybrid.]

V4Only mode is when Chrome uses only PVer4 implementation for all SafeBrowsing
operations.
V3Only (or Control): When Chrome uses the existing PVer3 implementation for all
SafeBrowsing operations.
V4Hybrid: When Chrome uses PVer4 implementation for some SafeBrowsing
operations. Namely, CheckBrowseUrl.

For details, see
https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bG...

This is 98% vakh's CL.

BUG=543161

patch from issue 2533993004 at patchset 140001
(http://crrev.com/2533993004#ps140001)
==========

to

==========
Setup V4Only mode, so it can be enabled via finch.

[Other modes are V3Only (or Control) and V4Hybrid.]

V4Only mode is when Chrome uses only PVer4 implementation for all SafeBrowsing
operations.
V3Only (or Control): When Chrome uses the existing PVer3 implementation for all
SafeBrowsing operations.
V4Hybrid: When Chrome uses PVer4 implementation for some SafeBrowsing
operations. Namely, CheckBrowseUrl.

For details, see
https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bG...

This is 98% vakh's CL.

BUG=543161

patch from issue 2533993004 at patchset 140001
(http://crrev.com/2533993004#ps140001)

Committed: https://crrev.com/6ec02574cadea969029792e1cbbfbab3f8ae5368
Cr-Commit-Position: refs/heads/master@{#437412}
==========

[commit-bot: I haz the power, 2016-12-09 01:37:43]

Patchset 2 (id:??) landed as
https://crrev.com/6ec02574cadea969029792e1cbbfbab3f8ae5368
Cr-Commit-Position: refs/heads/master@{#437412}