Small: Start checking URLs using PVer4. Verdict not returned to client yet.

Start checking URLs using PVer4. Verdict not returned to client yet.

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

[vakh (use Gerrit instead), 2016-09-27 18:56:32]

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

[commit-bot: I haz the power, 2016-09-27 18:57:11]

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-27 19:13:58]

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

[commit-bot: I haz the power, 2016-09-27 19:13:58]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  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-09-27 21:50:43]

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

[Scott Hess - ex-Googler, 2016-09-27 21:59:59]

Isn't using the same client in two separate sub-systems going to be racy?  I
mean once the v3 response is sent through, the client might be deleted without
regard to whether the v4 sub-system still has the client in progress.

[vakh (use Gerrit instead), 2016-09-27 22:03:34]

On 2016/09/27 21:59:59, Scott Hess wrote:
> Isn't using the same client in two separate sub-systems going to be racy?  I
> mean once the v3 response is sent through, the client might be deleted without
> regard to whether the v4 sub-system still has the client in progress.

Good observation. The V4* system doesn't respond to the client (yet) for this
exact reason:
https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_local_dat...

[Scott Hess - ex-Googler, 2016-09-27 22:18:05]

On 2016/09/27 22:03:34, vakh (Varun Khaneja) wrote:
> On 2016/09/27 21:59:59, Scott Hess wrote:
> > Isn't using the same client in two separate sub-systems going to be racy?  I
> > mean once the v3 response is sent through, the client might be deleted
without
> > regard to whether the v4 sub-system still has the client in progress.
> 
> Good observation. The V4* system doesn't respond to the client (yet) for this
> exact reason:
>
https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_local_dat...

That means that you won't get a use-after-free error sending a response to the
client.  But the client instances will still hang around in the v4 data
structures like pending_checks_.  Or is the idea that they will eventually
proceed through a parallel life and be removed?

OK.  I think so.  Almost wondering about making PendingCheck.client void* to
prevent accidental shenanigans, but that's probably overkill.

[vakh (use Gerrit instead), 2016-09-27 22:23:31]

On 2016/09/27 22:18:05, Scott Hess wrote:
> On 2016/09/27 22:03:34, vakh (Varun Khaneja) wrote:
> > On 2016/09/27 21:59:59, Scott Hess wrote:
> > > Isn't using the same client in two separate sub-systems going to be racy? 
I
> > > mean once the v3 response is sent through, the client might be deleted
> without
> > > regard to whether the v4 sub-system still has the client in progress.
> > 
> > Good observation. The V4* system doesn't respond to the client (yet) for
this
> > exact reason:
> >
>
https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_local_dat...
> 
> That means that you won't get a use-after-free error sending a response to the
> client.  But the client instances will still hang around in the v4 data
> structures like pending_checks_.  Or is the idea that they will eventually
> proceed through a parallel life and be removed?
At the moment, PendingChecks would have client pointing to invalid address but
that address won't be de-ref'd.
When the response from the server arrives, PendingCheck gets deleted and
everything gets cleaned up.

In the next few CLs, I'll try to make it so that if V4 is available, send the
request to V4 otherwise send it to V3 code.
That way, PendingCheck would have a valid client address and will start
responding to the client. This will be behind a Finch trial.

> 
> OK.  I think so.  Almost wondering about making PendingCheck.client void* to
> prevent accidental shenanigans, but that's probably overkill.

Not sure if it improves anything. I am the only person working on this code
right now and the client field will start getting used soon so there isn't much
value in doing this, IMO.

[Scott Hess - ex-Googler, 2016-09-27 22:24:54]

Yeah, I think LGTM.

The ordering points are mostly that you should keep the calls next to each other
just in case there's some weird interaction which later moves across the
intervening code.

That said, I think only the timeout case had side effects which would be
detectable from elsewhere.

https://codereview.chromium.org/2371043003/diff/1/chrome/browser/loader/safe_...
File chrome/browser/loader/safe_browsing_resource_throttle.cc (right):

https://codereview.chromium.org/2371043003/diff/1/chrome/browser/loader/safe_...
chrome/browser/loader/safe_browsing_resource_throttle.cc:115: }
Move this above the log event.

https://codereview.chromium.org/2371043003/diff/1/chrome/browser/loader/safe_...
chrome/browser/loader/safe_browsing_resource_throttle.cc:416: }
Probably move this above the response, so that both sub-systems cancel before
the callback.

[vakh (use Gerrit instead), 2016-09-28 01:23:53]

shess@'s review

[vakh (use Gerrit instead), 2016-09-28 01:23:54]

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

[vakh (use Gerrit instead), 2016-09-28 01:24:34]

https://codereview.chromium.org/2371043003/diff/1/chrome/browser/loader/safe_...
File chrome/browser/loader/safe_browsing_resource_throttle.cc (right):

https://codereview.chromium.org/2371043003/diff/1/chrome/browser/loader/safe_...
chrome/browser/loader/safe_browsing_resource_throttle.cc:115: }
On 2016/09/27 22:24:53, Scott Hess wrote:
> Move this above the log event.

Done.

https://codereview.chromium.org/2371043003/diff/1/chrome/browser/loader/safe_...
chrome/browser/loader/safe_browsing_resource_throttle.cc:416: }
On 2016/09/27 22:24:53, Scott Hess wrote:
> Probably move this above the response, so that both sub-systems cancel before
> the callback.

Done.

[commit-bot: I haz the power, 2016-09-28 01:24:35]

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-28 01:28:02]

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

[vakh (use Gerrit instead), 2016-09-28 01:28:03]

mmenke@ -- please review the changes in:
chrome/browser/loader/safe_browsing_resource_throttle.*

[commit-bot: I haz the power, 2016-09-28 01:42:11]

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

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

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_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[mmenke, 2016-09-28 01:55:59]

LGTM

https://codereview.chromium.org/2371043003/diff/20001/chrome/browser/loader/s...
File chrome/browser/loader/safe_browsing_resource_throttle.h (right):

https://codereview.chromium.org/2371043003/diff/20001/chrome/browser/loader/s...
chrome/browser/loader/safe_browsing_resource_throttle.h:178: GURL
url_being_checked_;
While you're here, should be include url.h.

[Nathan Parker, 2016-09-28 17:24:14]

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

https://codereview.chromium.org/2371043003/diff/20001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/services_delegate_impl.cc:197: return
base::FeatureList::IsEnabled(
Double checking the expected outcome here: All debug builds and 25% of
dev/canary will, in addition to loading the v4 DBs, start doing parallel checks
for all browse URLs.

I think this effectively merges the planned V4Parallel and V4JustUpdate
experiment groups into one, currently called V4JustUpdate.  If you want to go
that way, can you update the doc, and decide what to do with the experiment
group name? 
https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bG...

Alternately, to keep them separate, you'll want to check for the
"SafeBrowingV4ParallelCheck" feature and do parallel checking only if that's
set.

[vakh (use Gerrit instead), 2016-09-28 22:11:40]

feature_list file for v4. Add V4Parallel feature.

[vakh (use Gerrit instead), 2016-09-28 22:11:42]

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

[commit-bot: I haz the power, 2016-09-28 22:12:09]

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-28 22:13:36]

nparker@ -- I know you've LGTM'd it but could you please take another look?
Thanks.

https://codereview.chromium.org/2371043003/diff/20001/chrome/browser/loader/s...
File chrome/browser/loader/safe_browsing_resource_throttle.h (right):

https://codereview.chromium.org/2371043003/diff/20001/chrome/browser/loader/s...
chrome/browser/loader/safe_browsing_resource_throttle.h:178: GURL
url_being_checked_;
On 2016/09/28 01:55:59, mmenke wrote:
> While you're here, should be include url.h.

Done.

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

https://codereview.chromium.org/2371043003/diff/20001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/services_delegate_impl.cc:197: return
base::FeatureList::IsEnabled(
On 2016/09/28 17:24:14, Nathan Parker wrote:
> Double checking the expected outcome here: All debug builds and 25% of
> dev/canary will, in addition to loading the v4 DBs, start doing parallel
checks
> for all browse URLs.
> 
> I think this effectively merges the planned V4Parallel and V4JustUpdate
> experiment groups into one, currently called V4JustUpdate.  If you want to go
> that way, can you update the doc, and decide what to do with the experiment
> group name? 
>
https://docs.google.com/document/d/1fA3tvcTRtYOV5dNMst45OE6RXTr_cCtNsZijhB6bG...
> 
> Alternately, to keep them separate, you'll want to check for the
> "SafeBrowingV4ParallelCheck" feature and do parallel checking only if that's
> set.

Good catch. Changed.

[Nathan Parker, 2016-09-28 22:25:55]

https://codereview.chromium.org/2371043003/diff/40001/chrome/browser/loader/s...
File chrome/browser/loader/safe_browsing_resource_throttle.cc (right):

https://codereview.chromium.org/2371043003/diff/40001/chrome/browser/loader/s...
chrome/browser/loader/safe_browsing_resource_throttle.cc:111: if
(safe_browsing::V4FeatureList::IsParallelCheckEnabled()) {
Also check ve_local_database_manager is not null, in case we push an in
consistent finch config.  Same below.

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

https://codereview.chromium.org/2371043003/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_feature_list.cc:10: #ifdef NDEBUG
I'm wondering if we should not auto-enable this on debug -- that means the debug
build never tests the baseline functionality (w/o v4) and some of the  you're
adding in this CL.  Instead, you could rely on the chromium test .json config
that switches on the appropriate experiment/feature, and you could run your own
with the command line flag to force it enabled.

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

https://codereview.chromium.org/2371043003/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_feature_list.h:12: class V4FeatureList {
Rather than a class, you can just use a namespace.

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

https://codereview.chromium.org/2371043003/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:31: enum class
ClientCallbackType {
(unrelated nit: Does ClientCallbackType need to be public?)

https://codereview.chromium.org/2371043003/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:42: static
V4LocalDatabaseManager* Create(const base::FilePath& base_path);
return a unique_ptr

[commit-bot: I haz the power, 2016-09-28 22:27:12]

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

[commit-bot: I haz the power, 2016-09-28 22:27:13]

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-09-28 23:00:02]

https://codereview.chromium.org/2371043003/diff/40001/chrome/browser/loader/s...
File chrome/browser/loader/safe_browsing_resource_throttle.cc (right):

https://codereview.chromium.org/2371043003/diff/40001/chrome/browser/loader/s...
chrome/browser/loader/safe_browsing_resource_throttle.cc:111: if
(safe_browsing::V4FeatureList::IsParallelCheckEnabled()) {
On 2016/09/28 22:25:55, Nathan Parker wrote:
> Also check ve_local_database_manager is not null, in case we push an in
> consistent finch config.  Same below.

That's not possible.
If IsParallelCheckEnabled() is true, then IsLocalDatabaseManagerEnabled() will
return true, in which case v4_local_database_manager_ won't be NULL.

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

https://codereview.chromium.org/2371043003/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_feature_list.cc:10: #ifdef NDEBUG
On 2016/09/28 22:25:55, Nathan Parker wrote:
> I'm wondering if we should not auto-enable this on debug -- that means the
debug
> build never tests the baseline functionality (w/o v4) and some of the  you're
> adding in this CL.  Instead, you could rely on the chromium test .json config
> that switches on the appropriate experiment/feature, and you could run your
own
> with the command line flag to force it enabled.  

I agree. How about I test all that after submitting the CL and then remove the
ifdefs as a separate CL?
Reason: This CL won't be blocked by my lack of understanding of how to test
finch flags from the command line.

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

https://codereview.chromium.org/2371043003/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_feature_list.h:12: class V4FeatureList {
On 2016/09/28 22:25:55, Nathan Parker wrote:
> Rather than a class, you can just use a namespace.

Good idea. Didn't think of that. Done.

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

https://codereview.chromium.org/2371043003/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:31: enum class
ClientCallbackType {
On 2016/09/28 22:25:55, Nathan Parker wrote:
> (unrelated nit: Does ClientCallbackType need to be public?)

No. Made protected.

https://codereview.chromium.org/2371043003/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:42: static
V4LocalDatabaseManager* Create(const base::FilePath& base_path);
On 2016/09/28 22:25:55, Nathan Parker wrote:
> return a unique_ptr

It looks like the destructor of the class needs to be public for creating a
unique_ptr but the destructor of V4LocalDatabaseManager can't be made public
according to Chromium style guide.

../../components/safe_browsing_db/v4_local_database_manager.cc:87:1: error:
[chromium-style] Classes that are ref-counted should have destructors that are
declared protected or private.
V4LocalDatabaseManager::~V4LocalDatabaseManager() {
^
../../components/safe_browsing_db/v4_local_database_manager.h:29:32: note:
[chromium-style] 'V4LocalDatabaseManager' inherits from
'safe_browsing::SafeBrowsingDatabaseManager' here
class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
                               ^
../../components/safe_browsing_db/database_manager.h:36:7: note:
[chromium-style] 'SafeBrowsingDatabaseManager' inherits from
'base::RefCountedThreadSafe<SafeBrowsingDatabaseManager>' here
    : public base::RefCountedThreadSafe<SafeBrowsingDatabaseManager> {

[vakh (use Gerrit instead), 2016-09-28 23:01:10]

nparker@ review

[vakh (use Gerrit instead), 2016-09-28 23:01:13]

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

[commit-bot: I haz the power, 2016-09-28 23:02:06]

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-28 23:03:28]

fixed typo

[vakh (use Gerrit instead), 2016-09-28 23:03:31]

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

[commit-bot: I haz the power, 2016-09-28 23:03: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-28 23:06:37]

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

[commit-bot: I haz the power, 2016-09-28 23:06:38]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[vakh (use Gerrit instead), 2016-09-28 23:41:38]

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

[commit-bot: I haz the power, 2016-09-28 23:41:55]

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-28 23:43:40]

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

[commit-bot: I haz the power, 2016-09-28 23:44:01]

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-28 23:45:03]

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

[commit-bot: I haz the power, 2016-09-28 23:45:04]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Nathan Parker, 2016-09-28 23:50:14]

lgtm

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

https://codereview.chromium.org/2371043003/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_feature_list.cc:10: #ifdef NDEBUG
On 2016/09/28 23:00:01, vakh (Varun Khaneja) wrote:
> On 2016/09/28 22:25:55, Nathan Parker wrote:
> > I'm wondering if we should not auto-enable this on debug -- that means the
> debug
> > build never tests the baseline functionality (w/o v4) and some of the 
you're
> > adding in this CL.  Instead, you could rely on the chromium test .json
config
> > that switches on the appropriate experiment/feature, and you could run your
> own
> > with the command line flag to force it enabled.  
> 
> I agree. How about I test all that after submitting the CL and then remove the
> ifdefs as a separate CL?
> Reason: This CL won't be blocked by my lack of understanding of how to test
> finch flags from the command line.

Sure, though the flags look simple:
https://g3doc.corp.google.com/analysis/uma/g3doc/finch/feature-api.md?cl=head

That doc describes a way to add an entry in chrome://flags -- let's do that. 
That'll make it easier to enable it in a sticky way.  I'm sure we'll run into
cases where we need to walk a walk a googler through enabling/testing pver4 on
their mac/windows/chromeos where command line flags are more annoying.

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

https://codereview.chromium.org/2371043003/diff/40001/components/safe_browsin...
components/safe_browsing_db/v4_local_database_manager.h:42: static
V4LocalDatabaseManager* Create(const base::FilePath& base_path);
On 2016/09/28 23:00:02, vakh (Varun Khaneja) wrote:
> On 2016/09/28 22:25:55, Nathan Parker wrote:
> > return a unique_ptr
> 
> It looks like the destructor of the class needs to be public for creating a
> unique_ptr but the destructor of V4LocalDatabaseManager can't be made public
> according to Chromium style guide.
> 
> ../../components/safe_browsing_db/v4_local_database_manager.cc:87:1: error:
> [chromium-style] Classes that are ref-counted should have destructors that are
> declared protected or private.
> V4LocalDatabaseManager::~V4LocalDatabaseManager() {
> ^
> ../../components/safe_browsing_db/v4_local_database_manager.h:29:32: note:
> [chromium-style] 'V4LocalDatabaseManager' inherits from
> 'safe_browsing::SafeBrowsingDatabaseManager' here
> class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
>                                ^
> ../../components/safe_browsing_db/database_manager.h:36:7: note:
> [chromium-style] 'SafeBrowsingDatabaseManager' inherits from
> 'base::RefCountedThreadSafe<SafeBrowsingDatabaseManager>' here
>     : public base::RefCountedThreadSafe<SafeBrowsingDatabaseManager> {

Ah yes, it's a base::RefCountedThreadSafe<> decendent, so maybe it should return
a scoped_refptr<v4ldm> (via make_scoped_refptr(..)).  Or maybe it'd need to be a
scoped_refptr<sbdm>, which might be annoying for tests since you'd need to
upcast it.  

If none of that works, this is probably ok.

[vakh (use Gerrit instead), 2016-09-28 23:51:06]

git pull

[vakh (use Gerrit instead), 2016-09-28 23:51:10]

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

[commit-bot: I haz the power, 2016-09-28 23:51:25]

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-29 00:14:15]

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

[commit-bot: I haz the power, 2016-09-29 00:14:16]

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), 2016-09-29 00:52:02]

include ref_counted.h and use scoped_refptr

[vakh (use Gerrit instead), 2016-09-29 00:52:04]

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

[commit-bot: I haz the power, 2016-09-29 00:52:30]

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-29 01:13:50]

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

[commit-bot: I haz the power, 2016-09-29 01:13:51]

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-09-29 01:31:23]

Define v4_local_database_manager that returns nullptr for ServicesDelegateStub

[vakh (use Gerrit instead), 2016-09-29 01:31:26]

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

[commit-bot: I haz the power, 2016-09-29 01:32:08]

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-29 01:53:59]

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

[commit-bot: I haz the power, 2016-09-29 01:54:00]

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), 2016-09-29 03:17:27]

Define v4_local_database_manager_ in ServicesDelegateStub

[vakh (use Gerrit instead), 2016-09-29 03:17:30]

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

[commit-bot: I haz the power, 2016-09-29 03:17:42]

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-29 03:17:56]

Patchset #8 (id:140001) has been deleted

[commit-bot: I haz the power, 2016-09-29 04:46:59]

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

[commit-bot: I haz the power, 2016-09-29 04:47:00]

Dry run: Exceeded global retry quota

[vakh (use Gerrit instead), 2016-09-29 05:03:20]

The CQ bit was checked by vakh@chromium.org

[vakh (use Gerrit instead), 2016-09-29 05:03:21]

The patchset sent to the CQ was uploaded after l-g-t-m from shess@chromium.org,
mmenke@chromium.org, nparker@chromium.org
Link to the patchset: https://codereview.chromium.org/2371043003/#ps160001
(title: "Define v4_local_database_manager_ in ServicesDelegateStub")

[commit-bot: I haz the power, 2016-09-29 05:03:35]

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-29 06:29:59]

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

[commit-bot: I haz the power, 2016-09-29 06:30:00]

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), 2016-09-29 16:31:26]

The CQ bit was checked by vakh@chromium.org

[commit-bot: I haz the power, 2016-09-29 16:32:10]

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-29 17:55:51]

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

[commit-bot: I haz the power, 2016-09-29 17:55:52]

Exceeded global retry quota

[vakh (use Gerrit instead), 2016-09-29 18:13:32]

Remove NOTREACHED

[vakh (use Gerrit instead), 2016-09-29 18:13:35]

The CQ bit was checked by vakh@chromium.org

[vakh (use Gerrit instead), 2016-09-29 18:13:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, shess@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2371043003/#ps180001
(title: "Remove NOTREACHED")

[commit-bot: I haz the power, 2016-09-29 18:14:11]

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-29 19:06:21]

Committed patchset #9 (id:180001)

[commit-bot: I haz the power, 2016-09-29 19:09:11]

Description was changed from

==========
Start checking URLs using PVer4. Verdict not returned to client yet.

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

to

==========
Start checking URLs using PVer4. Verdict not returned to client yet.

BUG=543161

Committed: https://crrev.com/17fb38d372c77f2dcbae3da61da22d20b0d98af9
Cr-Commit-Position: refs/heads/master@{#421893}
==========

[commit-bot: I haz the power, 2016-09-29 19:09:13]

Patchset 9 (id:??) landed as
https://crrev.com/17fb38d372c77f2dcbae3da61da22d20b0d98af9
Cr-Commit-Position: refs/heads/master@{#421893}

[vakh (use Gerrit instead), 2016-09-29 22:14:58]

A revert of this CL (patchset #9 id:180001) has been created in
https://codereview.chromium.org/2381963003/ by vakh@chromium.org.

The reason for reverting is: Reported by dfalcantara@

 09-30 14:35:21.701 27694 27767 E cr_SafeBrowsingApi: Failed to init handler:
Attempt to invoke virtual method 'java.lang.Object
java.lang.Class.newInstance()' on a null object reference
 09-30 14:35:21.741 27694 27767 F chromium: [FATAL:ref_counted.h(322)] Assert
failed: ptr_ != __null. 
.