Replace deprecated base::NonThreadSafe in extensions in favor of SequenceChecker.

Replace deprecated base::NonThreadSafe in extensions in favor of SequenceChecker.

Note to crash team: This CL is a refactor and has no intended behavior change.

This change was scripted by https://crbug.com/676387#c8.

Note-worthy for the reviewer:
 * SequenceChecker enforces thread-safety but not thread-affinity!
   If the classes that were updated are thread-affine (use thread local
   storage or a third-party API that does) they should be migrated to
   ThreadChecker instead.
 * ~NonThreadSafe() used to implicitly check in its destructor
   ~Sequence/ThreadChecker() doesn't by design. To keep this CL a
   no-op, an explicit check was added to the destructor of migrated
   classes.
 * NonThreadSafe used to provide access to subclasses, as such
   the |sequence_checker_| member was made protected rather than
   private where necessary.

BUG=676387
This CL was uploaded by git cl split.

R=finnur@chromium.org

Review-Url: https://codereview.chromium.org/2915523002
Cr-Commit-Position: refs/heads/master@{#476280}
Committed: https://chromium.googlesource.com/chromium/src/+/370841f24900462d5f74d2b4f96b7125d44eae37

[gab, 2017-05-30 02:58:54]

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

[gab, 2017-05-30 02:58:55]

Hi, this is an automated change made by https://crbug.com/676387#c8 and sharded
across directories for OWNERS.

No human has looked at this specific output yet, see cl description for things
to watch for.

If it LGTY, please CQ.

If it didn't pass CQ dry-run it may be a simple patch failure or an IWYU failure
from having this being a partial CL... please review anyways and a human will
look at the failure (and ping for re-review if anything major is required..). It
is known to compile all targets on Windows at r474679...  manual tweaks will
likely be required to smoothen things off, bear with me!

Thanks!
Gab

[commit-bot: I haz the power, 2017-05-30 02:59:16]

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, 2017-05-30 03:54:57]

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

[commit-bot: I haz the power, 2017-05-30 03:54:58]

Dry run: This issue passed the CQ dry run.

[Finnur, 2017-05-30 13:31:47]

finnur@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Finnur, 2017-05-30 13:31:48]

Devlin is probably more appropriate for this.

[Devlin, 2017-05-30 19:06:47]

cl description typo: implcitly -> implicitly

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/quota_se...
File extensions/browser/quota_service.h (right):

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/quota_se...
extensions/browser/quota_service.h:46: // and IO threads.
This class is probably better suited for ThreadChecker.

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/value_st...
File extensions/browser/value_store/value_store_frontend.h (right):

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/value_st...
extensions/browser/value_store/value_store_frontend.h:23: // A frontend for a
LeveldbValueStore, for use on the UI thread.
Since this is only designed to be used on the UI thread, probably ThreadChecker.

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/warning_...
File extensions/browser/warning_service.h (right):

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/warning_...
extensions/browser/warning_service.h:32: // them. This class must be used on the
UI thread only.
I'm not sure that SequenceChecker is right for this class (and not sure that
base::NonThreadSafe was entirely right, either).  This is designed to be used
specifically on the UI thread *only*, and uses UI-thread concepts
(BrowserContext and related).  I think what we want instead is to replace
CalledOnValidThread() or CALLED_ON_VALID_SEQUENCE with
DCHECK_CURRENTLY_ON(content::BrowserThread::UI) or ThreadChecker.

[gab, 2017-05-31 17:47:43]

Description was changed from

==========
Replace deprecated base::NonThreadSafe in extensions in favor of
SequenceChecker.

Note to crash team: This CL is a refactor and has no intended behavior change.

This change was scripted by https://crbug.com/676387#c8.

Note-worthy for the reviewer:
 * SequenceChecker enforces thread-safety but not thread-affinity!
   If the classes that were updated are thread-affine (use thread local
   storage or a third-party API that does) they should be migrated to
   ThreadChecker instead.
 * ~NonThreadSafe() used to implcitly check in its destructor
   ~Sequence/ThreadChecker() doesn't by design. To keep this CL a
   no-op, an explicit check was added to the destructor of migrated
   classes.
 * NonThreadSafe used to provide access to subclasses, as such
   the |sequence_checker_| member was made protected rather than
   private where necessary.

BUG=676387
This CL was uploaded by git cl split.

R=finnur@chromium.org
==========

to

==========
Replace deprecated base::NonThreadSafe in extensions in favor of
SequenceChecker.

Note to crash team: This CL is a refactor and has no intended behavior change.

This change was scripted by https://crbug.com/676387#c8.

Note-worthy for the reviewer:
 * SequenceChecker enforces thread-safety but not thread-affinity!
   If the classes that were updated are thread-affine (use thread local
   storage or a third-party API that does) they should be migrated to
   ThreadChecker instead.
 * ~NonThreadSafe() used to implicitly check in its destructor
   ~Sequence/ThreadChecker() doesn't by design. To keep this CL a
   no-op, an explicit check was added to the destructor of migrated
   classes.
 * NonThreadSafe used to provide access to subclasses, as such
   the |sequence_checker_| member was made protected rather than
   private where necessary.

BUG=676387
This CL was uploaded by git cl split.

R=finnur@chromium.org
==========

[gab, 2017-05-31 17:50:39]

Patchset #2 (id:20001) has been deleted

[gab, 2017-05-31 17:51:09]

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

[commit-bot: I haz the power, 2017-05-31 17:51:44]

Dry run: CQ is trying da patch.

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

[gab, 2017-05-31 17:52:28]

Comments addressed, please CQ if LGTYs

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/quota_se...
File extensions/browser/quota_service.h (right):

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/quota_se...
extensions/browser/quota_service.h:46: // and IO threads.
On 2017/05/30 19:06:47, Devlin wrote:
> This class is probably better suited for ThreadChecker.

Done.

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/value_st...
File extensions/browser/value_store/value_store_frontend.h (right):

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/value_st...
extensions/browser/value_store/value_store_frontend.h:23: // A frontend for a
LeveldbValueStore, for use on the UI thread.
On 2017/05/30 19:06:47, Devlin wrote:
> Since this is only designed to be used on the UI thread, probably
ThreadChecker.

I don't see thread-affinity requirements in ValueStoreFrontend's design?
Anyways, added BrowserThread::UI checks for now to match current API comment.

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/warning_...
File extensions/browser/warning_service.h (right):

https://codereview.chromium.org/2915523002/diff/1/extensions/browser/warning_...
extensions/browser/warning_service.h:32: // them. This class must be used on the
UI thread only.
On 2017/05/30 19:06:47, Devlin wrote:
> I'm not sure that SequenceChecker is right for this class (and not sure that
> base::NonThreadSafe was entirely right, either).  This is designed to be used
> specifically on the UI thread *only*, and uses UI-thread concepts
> (BrowserContext and related).  I think what we want instead is to replace
> CalledOnValidThread() or CALLED_ON_VALID_SEQUENCE with
> DCHECK_CURRENTLY_ON(content::BrowserThread::UI) or ThreadChecker.

Done.

[commit-bot: I haz the power, 2017-05-31 18:39:08]

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

[commit-bot: I haz the power, 2017-05-31 18:39:09]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Devlin, 2017-05-31 18:45:27]

LGTM, but looks like a few tests are failing (I think we just need to add mock
threads).

[gab, 2017-05-31 19:06:29]

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

[gab, 2017-05-31 19:06:33]

The CQ bit was checked by gab@chromium.org

[gab, 2017-05-31 19:06:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2915523002/#ps60001
(title: "Add TestBrowserThreadBundle")

[commit-bot: I haz the power, 2017-05-31 19:07:14]

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, 2017-05-31 21:39:29]

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

[commit-bot: I haz the power, 2017-05-31 21:39:31]

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

[gab, 2017-06-01 13:51:11]

On 2017/05/31 21:39:31, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
> (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

The failing printer tests were fixed in
https://codereview.chromium.org/2912003002/, retrying.

[gab, 2017-06-01 13:51:16]

The CQ bit was checked by gab@chromium.org

[commit-bot: I haz the power, 2017-06-01 13:51:23]

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, 2017-06-01 14:38:28]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1496325076725890,
"parent_rev": "bf77513ab512c29a2f27b71dbd0510b7ef9695f1", "commit_rev":
"370841f24900462d5f74d2b4f96b7125d44eae37"}

[commit-bot: I haz the power, 2017-06-01 14:38:36]

Description was changed from

==========
Replace deprecated base::NonThreadSafe in extensions in favor of
SequenceChecker.

Note to crash team: This CL is a refactor and has no intended behavior change.

This change was scripted by https://crbug.com/676387#c8.

Note-worthy for the reviewer:
 * SequenceChecker enforces thread-safety but not thread-affinity!
   If the classes that were updated are thread-affine (use thread local
   storage or a third-party API that does) they should be migrated to
   ThreadChecker instead.
 * ~NonThreadSafe() used to implicitly check in its destructor
   ~Sequence/ThreadChecker() doesn't by design. To keep this CL a
   no-op, an explicit check was added to the destructor of migrated
   classes.
 * NonThreadSafe used to provide access to subclasses, as such
   the |sequence_checker_| member was made protected rather than
   private where necessary.

BUG=676387
This CL was uploaded by git cl split.

R=finnur@chromium.org
==========

to

==========
Replace deprecated base::NonThreadSafe in extensions in favor of
SequenceChecker.

Note to crash team: This CL is a refactor and has no intended behavior change.

This change was scripted by https://crbug.com/676387#c8.

Note-worthy for the reviewer:
 * SequenceChecker enforces thread-safety but not thread-affinity!
   If the classes that were updated are thread-affine (use thread local
   storage or a third-party API that does) they should be migrated to
   ThreadChecker instead.
 * ~NonThreadSafe() used to implicitly check in its destructor
   ~Sequence/ThreadChecker() doesn't by design. To keep this CL a
   no-op, an explicit check was added to the destructor of migrated
   classes.
 * NonThreadSafe used to provide access to subclasses, as such
   the |sequence_checker_| member was made protected rather than
   private where necessary.

BUG=676387
This CL was uploaded by git cl split.

R=finnur@chromium.org

Review-Url: https://codereview.chromium.org/2915523002
Cr-Commit-Position: refs/heads/master@{#476280}
Committed:
https://chromium.googlesource.com/chromium/src/+/370841f24900462d5f74d2b4f96b...
==========

[commit-bot: I haz the power, 2017-06-01 14:38:37]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/370841f24900462d5f74d2b4f96b...