Safeguard against crash-looping if subresource filter rule indexing fails.

Safeguard against crash-looping if subresource filter rule indexing fails.

A sentinel file is placed in the ruleset version directory just before indexing
commences, and removed afterwards. Therefore, if a sentinel file is found on
next start-up, it is an indication that the previous indexing operation may
have crashed, and indexing will not be attempted again.

This CL also slightly refactors unit tests so that we no longer have specific
tests verifying histograms values, but instead we have tests exercising error
scenarios, and verifying histogram values is performed as part of that.

TBR=rkaplow@chromium.org
BUG=609747
Committed: https://crrev.com/6ab601217b9c7ff2123ae262d2eada5e1b1557ec
Cr-Commit-Position: refs/heads/master@{#414235}

[engedy, 2016-08-23 10:01:59]

Description was changed from

==========
Safeguard against crash-looping if subresource filter rule indexing fails.

A sentinel file is placed in the ruleset version directory just before indexing
commences, and removed afterwards. Therefore, if a sentinel file is found on
next start-up, it is an indication that the previous indexing operation may
have crashed, and indexing will not be attempted again.

BUG=609747
==========

to

==========
Safeguard against crash-looping if subresource filter rule indexing fails.

A sentinel file is placed in the ruleset version directory just before indexing
commences, and removed afterwards. Therefore, if a sentinel file is found on
next start-up, it is an indication that the previous indexing operation may
have crashed, and indexing will not be attempted again.

This CL also slightly refactors unit tests so that we no longer have specific
tests verifying histograms values, but instead we have tests exercising error
scenarios, and verifying histogram values is performed as part of that.

TBR=rkaplow@chromium.org
BUG=609747
==========

[engedy, 2016-08-23 10:01:59]

engedy@chromium.org changed reviewers:
+ pkalinnikov@chromium.org, rkaplow@chromium.org

[engedy, 2016-08-23 10:02:57]

@Pavel, please review.  The fault injection is a bit ugly, simplification ideas
are very welcome!

@Robert: TBR'ing you for trivial change in histograms.xml (adding more enum
values).

[engedy, 2016-08-23 10:37:09]

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

[commit-bot: I haz the power, 2016-08-23 10:37:24]

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-08-23 10:52:30]

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

[commit-bot: I haz the power, 2016-08-23 10:52:31]

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

[engedy, 2016-08-24 09:33:29]

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

[commit-bot: I haz the power, 2016-08-24 09:33:53]

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

[engedy, 2016-08-24 09:42:55]

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

[commit-bot: I haz the power, 2016-08-24 09:43:17]

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-08-24 09:57:45]

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

[commit-bot: I haz the power, 2016-08-24 09:57:46]

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

[engedy, 2016-08-24 11:38:26]

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

[commit-bot: I haz the power, 2016-08-24 11:38:40]

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-08-24 13:04:20]

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

[commit-bot: I haz the power, 2016-08-24 13:04:21]

Dry run: This issue passed the CQ dry run.

[engedy, 2016-08-24 13:05:13]

Test failures are fixed. Pavel, PTAL.

[rkaplow, 2016-08-24 14:25:22]

lgtm

[pkalinnikov, 2016-08-24 15:13:01]

LGTM % couple of nits.

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
File components/subresource_filter/core/browser/ruleset_service.h (right):

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
components/subresource_filter/core/browser/ruleset_service.h:201: private:
Is double private section allowed by styleguide? Quite ambiguous.

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
File components/subresource_filter/core/browser/ruleset_service_unittest.cc
(right):

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
components/subresource_filter/core/browser/ruleset_service_unittest.cc:62:
DISALLOW_COPY_AND_ASSIGN(ScopedFunctionOverride);
include "base/macros.h"?

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
File components/subresource_filter/core/browser/subresource_filter_constants.h
(right):

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
components/subresource_filter/core/browser/subresource_filter_constants.h:37: //
The name of the sentinel file that is temporarily stored the indicate that
s/the/to/

[engedy, 2016-08-24 15:23:09]

The CQ bit was checked by engedy@chromium.org

[engedy, 2016-08-24 15:23:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
rkaplow@chromium.org, pkalinnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2272593002/#ps100001
(title: "Addressed comments by pkalinnikov@.")

[commit-bot: I haz the power, 2016-08-24 15:23:25]

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

[engedy, 2016-08-24 15:23:25]

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
File components/subresource_filter/core/browser/ruleset_service.h (right):

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
components/subresource_filter/core/browser/ruleset_service.h:201: private:
On 2016/08/24 15:13:01, pkalinnikov wrote:
> Is double private section allowed by styleguide? Quite ambiguous.

Removed.

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
File components/subresource_filter/core/browser/ruleset_service_unittest.cc
(right):

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
components/subresource_filter/core/browser/ruleset_service_unittest.cc:62:
DISALLOW_COPY_AND_ASSIGN(ScopedFunctionOverride);
On 2016/08/24 15:13:01, pkalinnikov wrote:
> include "base/macros.h"?

Done.

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
File components/subresource_filter/core/browser/subresource_filter_constants.h
(right):

https://codereview.chromium.org/2272593002/diff/80001/components/subresource_...
components/subresource_filter/core/browser/subresource_filter_constants.h:37: //
The name of the sentinel file that is temporarily stored the indicate that
On 2016/08/24 15:13:01, pkalinnikov wrote:
> s/the/to/

Done.

[commit-bot: I haz the power, 2016-08-24 16:51:07]

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

[commit-bot: I haz the power, 2016-08-24 16:51:08]

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

[engedy, 2016-08-24 18:28:15]

The CQ bit was checked by engedy@chromium.org

[commit-bot: I haz the power, 2016-08-24 18:29:15]

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-08-24 21:08:44]

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

[commit-bot: I haz the power, 2016-08-24 21:08:45]

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

[engedy, 2016-08-24 22:23:29]

The CQ bit was checked by engedy@chromium.org

[commit-bot: I haz the power, 2016-08-24 22:25: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-08-25 00:56:58]

Description was changed from

==========
Safeguard against crash-looping if subresource filter rule indexing fails.

A sentinel file is placed in the ruleset version directory just before indexing
commences, and removed afterwards. Therefore, if a sentinel file is found on
next start-up, it is an indication that the previous indexing operation may
have crashed, and indexing will not be attempted again.

This CL also slightly refactors unit tests so that we no longer have specific
tests verifying histograms values, but instead we have tests exercising error
scenarios, and verifying histogram values is performed as part of that.

TBR=rkaplow@chromium.org
BUG=609747
==========

to

==========
Safeguard against crash-looping if subresource filter rule indexing fails.

A sentinel file is placed in the ruleset version directory just before indexing
commences, and removed afterwards. Therefore, if a sentinel file is found on
next start-up, it is an indication that the previous indexing operation may
have crashed, and indexing will not be attempted again.

This CL also slightly refactors unit tests so that we no longer have specific
tests verifying histograms values, but instead we have tests exercising error
scenarios, and verifying histogram values is performed as part of that.

TBR=rkaplow@chromium.org
BUG=609747
==========

[commit-bot: I haz the power, 2016-08-25 00:56:59]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-25 00:59:01]

Description was changed from

==========
Safeguard against crash-looping if subresource filter rule indexing fails.

A sentinel file is placed in the ruleset version directory just before indexing
commences, and removed afterwards. Therefore, if a sentinel file is found on
next start-up, it is an indication that the previous indexing operation may
have crashed, and indexing will not be attempted again.

This CL also slightly refactors unit tests so that we no longer have specific
tests verifying histograms values, but instead we have tests exercising error
scenarios, and verifying histogram values is performed as part of that.

TBR=rkaplow@chromium.org
BUG=609747
==========

to

==========
Safeguard against crash-looping if subresource filter rule indexing fails.

A sentinel file is placed in the ruleset version directory just before indexing
commences, and removed afterwards. Therefore, if a sentinel file is found on
next start-up, it is an indication that the previous indexing operation may
have crashed, and indexing will not be attempted again.

This CL also slightly refactors unit tests so that we no longer have specific
tests verifying histograms values, but instead we have tests exercising error
scenarios, and verifying histogram values is performed as part of that.

TBR=rkaplow@chromium.org
BUG=609747

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

[commit-bot: I haz the power, 2016-08-25 00:59:02]

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