Deprecate NonThreadSafe in net/ in favor of SequenceChecker/ThreadChecker.

Deprecate NonThreadSafe in net/ in favor of ThreadChecker.

This change was scripted by the ThreadChecker equivalent of https://crbug.com/676387#c8.

Noteworthy:
 *  ~NonThreadSafe() used to implicitly check, ~ThreadChecker() doesn't so explicit 
    checks were added in relevant destructors to maintain status quo
 * NonThreadSafe provided a public interface for CalledOnValidThread() and protected
   for DetachFromThread(). The ThreadChecker was exposed as protected where required
   and public AssertCalledOnValidThread() methods added as appropriate.

BUG=676387
TBR=alokp,benwells

Review-Url: https://codereview.chromium.org/2910473005
Cr-Commit-Position: refs/heads/master@{#476681}
Committed: https://chromium.googlesource.com/chromium/src/+/47aa7da88b306da3da8cefa5180ddcad753107f5

[gab, 2017-05-26 00:26:42]

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

[gab, 2017-05-26 00:27:39]

gab@chromium.org changed reviewers:
+ rsleevi@chromium.org

[gab, 2017-05-26 00:27:40]

Ryan PTaL, thanks

[commit-bot: I haz the power, 2017-05-26 00:28: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, 2017-05-26 00:42:13]

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

[commit-bot: I haz the power, 2017-05-26 00:42:14]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[gab, 2017-05-26 00:45:41]

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

[commit-bot: I haz the power, 2017-05-26 00:46:58]

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-26 00:59:58]

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

[commit-bot: I haz the power, 2017-05-26 00:59:59]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[gab, 2017-05-26 02:40:56]

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

[commit-bot: I haz the power, 2017-05-26 02:41:32]

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-26 02:41:45]

Patchset #3 (id:40001) has been deleted

[gab, 2017-05-26 02:45:18]

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

[commit-bot: I haz the power, 2017-05-26 02:45:38]

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-26 02:56:53]

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

[commit-bot: I haz the power, 2017-05-26 02:56:54]

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

[gab, 2017-05-26 03:58:45]

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

[commit-bot: I haz the power, 2017-05-26 03:59:27]

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-26 04:11:33]

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

[commit-bot: I haz the power, 2017-05-26 04:11:34]

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

[gab, 2017-05-26 05:39:05]

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

[commit-bot: I haz the power, 2017-05-26 05:39: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, 2017-05-26 05:53:26]

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

[commit-bot: I haz the power, 2017-05-26 05:53:28]

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

[Ryan Sleevi, 2017-05-26 11:54:49]

rsleevi@chromium.org changed reviewers:
+ eroman@chromium.org, mmenke@chromium.org

[Ryan Sleevi, 2017-05-26 11:54:50]

Another one where I'm out today, so asking Matt and Eric to check out and/or
delegate as appropriate.

The ones I'm not sure about are naturally in the _platform implementations (the
HttpAuthController calls into platform impls), and so I'm not sure. It may be OK
for the auth controller, since we don't impersonate ourselves in Chrome, not
sure for the NCN.

https://codereview.chromium.org/2910473005/diff/100001/net/base/network_chang...
File net/base/network_change_notifier_win.cc (right):

https://codereview.chromium.org/2910473005/diff/100001/net/base/network_chang...
net/base/network_change_notifier_win.cc:70:
CancelIPChangeNotify(&addr_overlapped_);
This is why I'm not sure

https://codereview.chromium.org/2910473005/diff/100001/net/base/network_chang...
File net/base/network_change_notifier_win.h (left):

https://codereview.chromium.org/2910473005/diff/100001/net/base/network_chang...
net/base/network_change_notifier_win.h:29: NON_EXPORTED_BASE(public
base::NonThreadSafe) {
I'm not sure on this one

https://codereview.chromium.org/2910473005/diff/100001/net/dns/dns_config_ser...
File net/dns/dns_config_service_win.cc (right):

https://codereview.chromium.org/2910473005/diff/100001/net/dns/dns_config_ser...
net/dns/dns_config_service_win.cc:310: base::Unretained(this)));
Not sure because of this

https://codereview.chromium.org/2910473005/diff/100001/net/http/http_auth_con...
File net/http/http_auth_controller.cc (right):

https://codereview.chromium.org/2910473005/diff/100001/net/http/http_auth_con...
net/http/http_auth_controller.cc:158: int rv = handler_->GenerateAuthToken(
Here, I'm not sure because I believe some //content embedders are relying on
providing a thread impersonation token on the IO thread, expecting these calls
to use that thread-level impersonation. There was at least one discussion of
this on net-dev@

I'm not sure if there are other embedders relying on this behaviour, but it
would mean it quietly breaks if we downgraded sequence protections to thread
protections

[gab, 2017-05-26 14:55:20]

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

[commit-bot: I haz the power, 2017-05-26 14:55:35]

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-26 15:21:44]

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

[commit-bot: I haz the power, 2017-05-26 15:21:46]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[gab, 2017-05-26 16:51:30]

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

[commit-bot: I haz the power, 2017-05-26 16:51:55]

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-26 18:54:00]

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

[commit-bot: I haz the power, 2017-05-26 18:54:01]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[mmenke, 2017-05-26 20:20:03]

https://codereview.chromium.org/2910473005/diff/140001/net/base/network_chang...
File net/base/network_change_notifier_win.cc (right):

https://codereview.chromium.org/2910473005/diff/140001/net/base/network_chang...
net/base/network_change_notifier_win.cc:70:
CancelIPChangeNotify(&addr_overlapped_);
This class uses an overlapped structure, which I don't believe is safe to
teardown on threads other than the one on which it was created.

Do we have a standard way to account for that?

[gab, 2017-05-30 14:33:36]

On 2017/05/26 20:20:03, mmenke wrote:
>
https://codereview.chromium.org/2910473005/diff/140001/net/base/network_chang...
> File net/base/network_change_notifier_win.cc (right):
> 
>
https://codereview.chromium.org/2910473005/diff/140001/net/base/network_chang...
> net/base/network_change_notifier_win.cc:70:
> CancelIPChangeNotify(&addr_overlapped_);
> This class uses an overlapped structure, which I don't believe is safe to
> teardown on threads other than the one on which it was created.
> 
> Do we have a standard way to account for that?

ThreadChecker instead of SequenceChecker is the standard way to flag a class as
thread-affine.

Shall I switch to ThreadChecker for this? And for the ones flagged by rsleevi?

Overall the only harm done by using ThreadChecker over SequenceChecker where
unecessary is that it prevents components using those classes to run on
SequencedTaskRunners (forced to use SingleThreadTaskRunners). For any class that
isn't intended to be used widely this isn't much of an issue (and we can come
back to discuss a specific one with net-dev if moving some code to a sequence
eventually becomes blocked on such a strict requirement in net).

[mmenke, 2017-05-30 16:30:03]

On 2017/05/30 14:33:36, gab wrote:
> On 2017/05/26 20:20:03, mmenke wrote:
> >
>
https://codereview.chromium.org/2910473005/diff/140001/net/base/network_chang...
> > File net/base/network_change_notifier_win.cc (right):
> > 
> >
>
https://codereview.chromium.org/2910473005/diff/140001/net/base/network_chang...
> > net/base/network_change_notifier_win.cc:70:
> > CancelIPChangeNotify(&addr_overlapped_);
> > This class uses an overlapped structure, which I don't believe is safe to
> > teardown on threads other than the one on which it was created.
> > 
> > Do we have a standard way to account for that?
> 
> ThreadChecker instead of SequenceChecker is the standard way to flag a class
as
> thread-affine.
> 
> Shall I switch to ThreadChecker for this? And for the ones flagged by rsleevi?
> 
> Overall the only harm done by using ThreadChecker over SequenceChecker where
> unecessary is that it prevents components using those classes to run on
> SequencedTaskRunners (forced to use SingleThreadTaskRunners). For any class
that
> isn't intended to be used widely this isn't much of an issue (and we can come
> back to discuss a specific one with net-dev if moving some code to a sequence
> eventually becomes blocked on such a strict requirement in net).

Using ThreadChecker instead SGTM

[mmenke, 2017-05-30 16:37:18]

On 2017/05/30 16:30:03, mmenke wrote:
> On 2017/05/30 14:33:36, gab wrote:
> > On 2017/05/26 20:20:03, mmenke wrote:
> > >
> >
>
https://codereview.chromium.org/2910473005/diff/140001/net/base/network_chang...
> > > File net/base/network_change_notifier_win.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2910473005/diff/140001/net/base/network_chang...
> > > net/base/network_change_notifier_win.cc:70:
> > > CancelIPChangeNotify(&addr_overlapped_);
> > > This class uses an overlapped structure, which I don't believe is safe to
> > > teardown on threads other than the one on which it was created.
> > > 
> > > Do we have a standard way to account for that?
> > 
> > ThreadChecker instead of SequenceChecker is the standard way to flag a class
> as
> > thread-affine.
> > 
> > Shall I switch to ThreadChecker for this? And for the ones flagged by
rsleevi?
> > 
> > Overall the only harm done by using ThreadChecker over SequenceChecker where
> > unecessary is that it prevents components using those classes to run on
> > SequencedTaskRunners (forced to use SingleThreadTaskRunners). For any class
> that
> > isn't intended to be used widely this isn't much of an issue (and we can
come
> > back to discuss a specific one with net-dev if moving some code to a
sequence
> > eventually becomes blocked on such a strict requirement in net).
> 
> Using ThreadChecker instead SGTM

Also note that using a thread checker for sockets basically mandates it also be
used for all classes that directly or indirectly use sockets - don't want to
have the ThreadChecker fail at the bottom of the stack when sequence checkers at
all other layers of the stack passed, since it's the net stack's consumer that's
creating the problem.

[gab, 2017-05-30 18:32:47]

Description was changed from

==========
Deprecate NonThreadSafe in net/ in favor of SequenceChecker.

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

The script does a few things:
 * Most importantly it switches from NonThreadSafe -> SequenceChecker
 * It adds a check in the destructor as ~NonThreadSafe() used to check
   but ~SequenceChecker() doesn't by design.
 * It renames a bunch of stuff to be coherent with the udpate.

The implications (the reviewer of this CL **needs** to make sure of this,
                  the script has *no* idea!):
 * IMPORTANT: SequenceChecker checks for thread-safety but not
   thread-affinity if a class truly required thread-affinity it needs
   to be changed to use ThreadChecker. See sequence_checker.h for details.

BUG=676387
==========

to

==========
Deprecate NonThreadSafe in net/ in favor of SequenceChecker/ThreadChecker.

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

And then manually amended to make some SequenceChecker into ThreadCheckers.

The script does a few things:
 * Most importantly it switches from NonThreadSafe -> SequenceChecker
 * It adds a check in the destructor as ~NonThreadSafe() used to check
   but ~SequenceChecker() doesn't by design.
 * It renames a bunch of stuff to be coherent with the udpate.

The implications (the reviewer of this CL **needs** to make sure of this,
                  the script has *no* idea!):
 * IMPORTANT: SequenceChecker checks for thread-safety but not
   thread-affinity if a class truly required thread-affinity it needs
   to be changed to use ThreadChecker. See sequence_checker.h for details.

BUG=676387
==========

[gab, 2017-05-30 21:08:02]

Description was changed from

==========
Deprecate NonThreadSafe in net/ in favor of SequenceChecker/ThreadChecker.

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

And then manually amended to make some SequenceChecker into ThreadCheckers.

The script does a few things:
 * Most importantly it switches from NonThreadSafe -> SequenceChecker
 * It adds a check in the destructor as ~NonThreadSafe() used to check
   but ~SequenceChecker() doesn't by design.
 * It renames a bunch of stuff to be coherent with the udpate.

The implications (the reviewer of this CL **needs** to make sure of this,
                  the script has *no* idea!):
 * IMPORTANT: SequenceChecker checks for thread-safety but not
   thread-affinity if a class truly required thread-affinity it needs
   to be changed to use ThreadChecker. See sequence_checker.h for details.

BUG=676387
==========

to

==========
Deprecate NonThreadSafe in net/ in favor of ThreadChecker.

This change was scripted by the ThreadChecker equivalent of
https://crbug.com/676387#c8.

Noteworthy:
 *  ~NonThreadSafe() used to implicitly check, ~ThreadChecker() doesn't so
explicit 
    checks were added in relevant destructors to maintain status quo
 * NonThreadSafe provided a public interface for CalledOnValidThread() and
protected
   for DetachFromThread(). The ThreadChecker was exposed as protected where
required
   and public AssertCalledOnValidThread() methods added as appropriate.

BUG=676387
==========

[gab, 2017-05-30 21:08:43]

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

[commit-bot: I haz the power, 2017-05-30 21:09:17]

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-30 21:15:52]

gab@chromium.org changed reviewers:
- eroman@chromium.org, rsleevi@chromium.org

[gab, 2017-05-30 21:15:53]

Migrated to ThreadChecker instead as discussed offline PTanL

@mmenke as official reviewer in an attempt to avoid having 3 people redundantly
look at this..

[mmenke, 2017-05-30 21:23:59]

On 2017/05/30 21:15:53, gab wrote:
> Migrated to ThreadChecker instead as discussed offline PTanL
> 
> @mmenke as official reviewer in an attempt to avoid having 3 people
redundantly
> look at this..

I'll take a look at it tomorrow.

[commit-bot: I haz the power, 2017-05-30 21:30:27]

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

[commit-bot: I haz the power, 2017-05-30 21:30:28]

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

[mmenke, 2017-05-31 15:41:40]

On 2017/05/30 21:23:59, mmenke wrote:
> On 2017/05/30 21:15:53, gab wrote:
> > Migrated to ThreadChecker instead as discussed offline PTanL
> > 
> > @mmenke as official reviewer in an attempt to avoid having 3 people
> redundantly
> > look at this..
> 
> I'll take a look at it tomorrow.

Mind fixing the build failures before I review this?

[gab, 2017-05-31 16:31:45]

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

[commit-bot: I haz the power, 2017-05-31 16:32: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, 2017-05-31 17:57:39]

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

[commit-bot: I haz the power, 2017-05-31 17:57:40]

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

[gab, 2017-05-31 18:03:10]

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

[commit-bot: I haz the power, 2017-05-31 18:03:56]

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

On 2017/05/31 15:41:40, mmenke wrote:
> On 2017/05/30 21:23:59, mmenke wrote:
> > On 2017/05/30 21:15:53, gab wrote:
> > > Migrated to ThreadChecker instead as discussed offline PTanL
> > > 
> > > @mmenke as official reviewer in an attempt to avoid having 3 people
> > redundantly
> > > look at this..
> > 
> > I'll take a look at it tomorrow.
> 
> Mind fixing the build failures before I review this?

Done, was a typo, sorry about that.

[commit-bot: I haz the power, 2017-05-31 19:12:56]

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

[commit-bot: I haz the power, 2017-05-31 19:12:58]

Dry run: This issue passed the CQ dry run.

[mmenke, 2017-05-31 19:28:08]

LGTM, modulo comments.

https://codereview.chromium.org/2910473005/diff/180001/net/base/backoff_entry.cc
File net/base/backoff_entry.cc (right):

https://codereview.chromium.org/2910473005/diff/180001/net/base/backoff_entry...
net/base/backoff_entry.cc:31: DETACH_FROM_THREAD(thread_checker_);
Is this needed?  thread_checker_ doesn't check on destruction, right?  Also need
to update the TODO to be about adding
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_)

https://codereview.chromium.org/2910473005/diff/180001/net/url_request/url_re...
File net/url_request/url_request_context.h (right):

https://codereview.chromium.org/2910473005/diff/180001/net/url_request/url_re...
net/url_request/url_request_context.h:285: }
De-inline this?

[gab, 2017-05-31 20:46:36]

https://codereview.chromium.org/2910473005/diff/180001/net/base/backoff_entry.cc
File net/base/backoff_entry.cc (right):

https://codereview.chromium.org/2910473005/diff/180001/net/base/backoff_entry...
net/base/backoff_entry.cc:31: DETACH_FROM_THREAD(thread_checker_);
On 2017/05/31 19:28:08, mmenke wrote:
> Is this needed?  thread_checker_ doesn't check on destruction, right?  Also
need
> to update the TODO to be about adding
> DCHECK_CALLED_ON_VALID_THREAD(thread_checker_)

Done.

https://codereview.chromium.org/2910473005/diff/180001/net/url_request/url_re...
File net/url_request/url_request_context.h (right):

https://codereview.chromium.org/2910473005/diff/180001/net/url_request/url_re...
net/url_request/url_request_context.h:285: }
On 2017/05/31 19:28:08, mmenke wrote:
> De-inline this?

If inlined the compiler can know to compile it out. If not it goes to LTO which
I forget if we do these days (it was disabled for a while because Visual Studio
couldn't handle our DLL size).

[gab, 2017-05-31 20:47:13]

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

[commit-bot: I haz the power, 2017-05-31 20:48:19]

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-31 22:45:42]

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

[commit-bot: I haz the power, 2017-05-31 22:45:43]

Dry run: This issue passed the CQ dry run.

[gab, 2017-06-01 18:54:06]

The CQ bit was checked by gab@chromium.org

[gab, 2017-06-01 18:54:07]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2910473005/#ps200001
(title: "s/detach/dcheck TODO")

[commit-bot: I haz the power, 2017-06-01 18:54:56]

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 19:04:47]

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

[commit-bot: I haz the power, 2017-06-01 19:04:48]

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

[gab, 2017-06-01 19:15:37]

Description was changed from

==========
Deprecate NonThreadSafe in net/ in favor of ThreadChecker.

This change was scripted by the ThreadChecker equivalent of
https://crbug.com/676387#c8.

Noteworthy:
 *  ~NonThreadSafe() used to implicitly check, ~ThreadChecker() doesn't so
explicit 
    checks were added in relevant destructors to maintain status quo
 * NonThreadSafe provided a public interface for CalledOnValidThread() and
protected
   for DetachFromThread(). The ThreadChecker was exposed as protected where
required
   and public AssertCalledOnValidThread() methods added as appropriate.

BUG=676387
==========

to

==========
Deprecate NonThreadSafe in net/ in favor of ThreadChecker.

This change was scripted by the ThreadChecker equivalent of
https://crbug.com/676387#c8.

Noteworthy:
 *  ~NonThreadSafe() used to implicitly check, ~ThreadChecker() doesn't so
explicit 
    checks were added in relevant destructors to maintain status quo
 * NonThreadSafe provided a public interface for CalledOnValidThread() and
protected
   for DetachFromThread(). The ThreadChecker was exposed as protected where
required
   and public AssertCalledOnValidThread() methods added as appropriate.

BUG=676387
TBR=alokp,benwells
==========

[gab, 2017-06-01 19:16:12]

gab@chromium.org changed reviewers:
+ alokp@chromium.org, benwells@chromium.org

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

TBR alokp for chromecast side-effects
TBR benwells for c/b/custom_handlers side-effects

[gab, 2017-06-01 19:16:19]

The CQ bit was checked by gab@chromium.org

[commit-bot: I haz the power, 2017-06-01 19:17:00]

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

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

[commit-bot: I haz the power, 2017-06-01 19:29:48]

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

[gab, 2017-06-01 20:00:11]

The CQ bit was checked by gab@chromium.org

[gab, 2017-06-01 20:00:12]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2910473005/#ps220001
(title: "rebase on r476354")

[commit-bot: I haz the power, 2017-06-01 20:01: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-06-01 21:37:24]

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

[commit-bot: I haz the power, 2017-06-01 21:37:26]

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

[gab, 2017-06-01 21:39:24]

The CQ bit was checked by gab@chromium.org

[commit-bot: I haz the power, 2017-06-01 21:40:08]

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 23:16:10]

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

[commit-bot: I haz the power, 2017-06-01 23:16:12]

Failed to apply patch for net/http/http_server_properties_impl.h:
While running git apply --index -3 -p1;
  error: patch failed: net/http/http_server_properties_impl.h:16
  Falling back to three-way merge...
  Applied patch to 'net/http/http_server_properties_impl.h' with conflicts.
  U net/http/http_server_properties_impl.h

Patch:       net/http/http_server_properties_impl.h
Index: net/http/http_server_properties_impl.h
diff --git a/net/http/http_server_properties_impl.h
b/net/http/http_server_properties_impl.h
index
598b4474e88d2ed590dd46978f44283ee610da38..c5cb4127c3893859aefd72d4220324db2bcbbbca
100644
--- a/net/http/http_server_properties_impl.h
+++ b/net/http/http_server_properties_impl.h
@@ -16,7 +16,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "base/threading/non_thread_safe.h"
+#include "base/threading/thread_checker.h"
 #include "base/values.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/ip_address.h"
@@ -37,9 +37,7 @@ struct AlternativeServiceHash {
 };
 
 // The implementation for setting/retrieving the HTTP server properties.
-class NET_EXPORT HttpServerPropertiesImpl
-    : public HttpServerProperties,
-      NON_EXPORTED_BASE(public base::NonThreadSafe) {
+class NET_EXPORT HttpServerPropertiesImpl : public HttpServerProperties {
  public:
   HttpServerPropertiesImpl();
   ~HttpServerPropertiesImpl() override;
@@ -175,6 +173,8 @@ class NET_EXPORT HttpServerPropertiesImpl
   QuicServerInfoMap quic_server_info_map_;
   size_t max_server_configs_stored_in_properties_;
 
+  THREAD_CHECKER(thread_checker_);
+
   base::WeakPtrFactory<HttpServerPropertiesImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(HttpServerPropertiesImpl);

[gab, 2017-06-02 14:25:52]

The CQ bit was checked by gab@chromium.org

[gab, 2017-06-02 14:25:52]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2910473005/#ps240001
(title: "rebase on r476634")

[commit-bot: I haz the power, 2017-06-02 14:26: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, 2017-06-02 16:09:46]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1496413552002070,
"parent_rev": "6df637aba4c300a651cd8934f59bf8d48760f8ef", "commit_rev":
"47aa7da88b306da3da8cefa5180ddcad753107f5"}

[commit-bot: I haz the power, 2017-06-02 16:09:53]

Description was changed from

==========
Deprecate NonThreadSafe in net/ in favor of ThreadChecker.

This change was scripted by the ThreadChecker equivalent of
https://crbug.com/676387#c8.

Noteworthy:
 *  ~NonThreadSafe() used to implicitly check, ~ThreadChecker() doesn't so
explicit 
    checks were added in relevant destructors to maintain status quo
 * NonThreadSafe provided a public interface for CalledOnValidThread() and
protected
   for DetachFromThread(). The ThreadChecker was exposed as protected where
required
   and public AssertCalledOnValidThread() methods added as appropriate.

BUG=676387
TBR=alokp,benwells
==========

to

==========
Deprecate NonThreadSafe in net/ in favor of ThreadChecker.

This change was scripted by the ThreadChecker equivalent of
https://crbug.com/676387#c8.

Noteworthy:
 *  ~NonThreadSafe() used to implicitly check, ~ThreadChecker() doesn't so
explicit 
    checks were added in relevant destructors to maintain status quo
 * NonThreadSafe provided a public interface for CalledOnValidThread() and
protected
   for DetachFromThread(). The ThreadChecker was exposed as protected where
required
   and public AssertCalledOnValidThread() methods added as appropriate.

BUG=676387
TBR=alokp,benwells

Review-Url: https://codereview.chromium.org/2910473005
Cr-Commit-Position: refs/heads/master@{#476681}
Committed:
https://chromium.googlesource.com/chromium/src/+/47aa7da88b306da3da8cefa5180d...
==========

[commit-bot: I haz the power, 2017-06-02 16:09:56]

Committed patchset #12 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/47aa7da88b306da3da8cefa5180d...