Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org, qinmin@chromium.org, marq@chromium.org, rockot@chromium.org, blundell@chromium.org, oshima@chromium.org, slan@chromium.org, boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Original-Commit-Position: refs/heads/master@{#450321}
Committed: https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d601298bd114a9d237
Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450384}
Committed: https://chromium.googlesource.com/chromium/src/+/70b7c5d796420107930c72d4ad82e0c850166b1a
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450903}
Committed: https://chromium.googlesource.com/chromium/src/+/0c2fcf56d2df0bb7b31ccb1509662255b5852451

[tzik, 2017-02-10 09:03:02]

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

[commit-bot: I haz the power, 2017-02-10 09:03:20]

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-02-10 09:16:28]

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

[commit-bot: I haz the power, 2017-02-10 09:16:29]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[tzik, 2017-02-11 02:50:04]

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

[commit-bot: I haz the power, 2017-02-11 02:50:31]

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-02-11 03:12:24]

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

[commit-bot: I haz the power, 2017-02-11 03:12:25]

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

[tzik, 2017-02-11 15:41:56]

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

[commit-bot: I haz the power, 2017-02-11 15:42:07]

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-02-11 15:59:57]

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

[commit-bot: I haz the power, 2017-02-11 15:59:58]

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

[tzik, 2017-02-11 16:06:29]

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

[commit-bot: I haz the power, 2017-02-11 16:06: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-02-11 16:18:54]

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

[commit-bot: I haz the power, 2017-02-11 16:18:55]

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

[tzik, 2017-02-11 16:32:35]

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

[commit-bot: I haz the power, 2017-02-11 16:32:45]

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-02-11 16:47:35]

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

[commit-bot: I haz the power, 2017-02-11 16:47:36]

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

[tzik, 2017-02-11 16:52:05]

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

[commit-bot: I haz the power, 2017-02-11 16:52:15]

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-02-11 17:06:26]

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

[commit-bot: I haz the power, 2017-02-11 17:06:27]

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

[tzik, 2017-02-11 18:04:38]

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

[commit-bot: I haz the power, 2017-02-11 18:04:46]

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-02-11 18:19:35]

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

[commit-bot: I haz the power, 2017-02-11 18:19:36]

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

[tzik, 2017-02-11 18:20:37]

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

[commit-bot: I haz the power, 2017-02-11 18:20:46]

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-02-11 18:37:11]

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

[commit-bot: I haz the power, 2017-02-11 18:37:12]

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

[tzik, 2017-02-11 18:46:48]

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

[commit-bot: I haz the power, 2017-02-11 18:46:52]

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-02-11 19:09:12]

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

[commit-bot: I haz the power, 2017-02-11 19:09:13]

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

[tzik, 2017-02-12 05:58:30]

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

[commit-bot: I haz the power, 2017-02-12 05:58:43]

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-02-12 06:24:00]

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

[commit-bot: I haz the power, 2017-02-12 06:24:01]

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

[tzik, 2017-02-12 06:37:31]

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

[commit-bot: I haz the power, 2017-02-12 06:37:41]

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-02-12 07:01:31]

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

[commit-bot: I haz the power, 2017-02-12 07:01:32]

Dry run: 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_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tzik, 2017-02-12 07:11:53]

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

[commit-bot: I haz the power, 2017-02-12 07:11:57]

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-02-12 07:34:22]

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

[commit-bot: I haz the power, 2017-02-12 07:34:23]

Dry run: 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_...)

[tzik, 2017-02-12 08:43:21]

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

[commit-bot: I haz the power, 2017-02-12 08:43:27]

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

[tzik, 2017-02-12 08:45:14]

Patchset #2 (id:20001) has been deleted

[tzik, 2017-02-12 08:45:31]

Patchset #2 (id:40001) has been deleted

[tzik, 2017-02-12 08:45:42]

Patchset #2 (id:60001) has been deleted

[tzik, 2017-02-12 08:46:30]

Patchset #2 (id:60002) has been deleted

[tzik, 2017-02-12 08:46:42]

Patchset #2 (id:90001) has been deleted

[tzik, 2017-02-12 08:47:00]

Patchset #2 (id:110001) has been deleted

[tzik, 2017-02-12 08:47:43]

Patchset #2 (id:130001) has been deleted

[tzik, 2017-02-12 08:47:55]

Patchset #2 (id:150001) has been deleted

[tzik, 2017-02-12 08:48:44]

Patchset #2 (id:170001) has been deleted

[tzik, 2017-02-12 08:49:33]

Patchset #2 (id:190001) has been deleted

[tzik, 2017-02-12 08:50:06]

Patchset #2 (id:210001) has been deleted

[tzik, 2017-02-12 08:50:41]

Patchset #2 (id:230001) has been deleted

[tzik, 2017-02-12 09:14:46]

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

[commit-bot: I haz the power, 2017-02-12 09:14: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-02-12 10:12:57]

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

[commit-bot: I haz the power, 2017-02-12 10:12:58]

Dry run: 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_...)

[tzik, 2017-02-12 10:17:25]

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

[commit-bot: I haz the power, 2017-02-12 10:17:36]

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-02-12 11:14:28]

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

[commit-bot: I haz the power, 2017-02-12 11:14:30]

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

[tzik, 2017-02-12 13:04:35]

Patchset #2 (id:240001) has been deleted

[tzik, 2017-02-12 13:04:53]

Patchset #2 (id:260001) has been deleted

[tzik, 2017-02-13 03:58:12]

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

[commit-bot: I haz the power, 2017-02-13 03:58:31]

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

[tzik, 2017-02-13 04:15:18]

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

[commit-bot: I haz the power, 2017-02-13 04:15: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-02-13 05:17:07]

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

[commit-bot: I haz the power, 2017-02-13 05:17:09]

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

[tzik, 2017-02-13 06:15:27]

Description was changed from

==========
Reduce header dependencies from base/sequence_checker.h
==========

to

==========
Reduce header dependencies from base/sequence_checker.h
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[tzik, 2017-02-13 06:15:34]

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

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

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

[tzik, 2017-02-13 06:23:42]

Patchset #3 (id:300001) has been deleted

[tzik, 2017-02-13 06:32:23]

Description was changed from

==========
Reduce header dependencies from base/sequence_checker.h
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_cherker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are sequence_cherker.{h,cc} only.

The header dependency from sequence_cherker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[tzik, 2017-02-13 06:32:24]

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

[tzik, 2017-02-13 06:45:15]

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

[commit-bot: I haz the power, 2017-02-13 06:45:37]

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

[tzik, 2017-02-13 07:14:15]

Patchset #2 (id:280001) has been deleted

[tzik, 2017-02-13 07:15:06]

Patchset #2 (id:320001) has been deleted

[tzik, 2017-02-13 07:26:26]

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

[commit-bot: I haz the power, 2017-02-13 07:26:46]

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

[tzik, 2017-02-13 07:31:30]

Patchset #2 (id:340001) has been deleted

[commit-bot: I haz the power, 2017-02-13 08:30:56]

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

[commit-bot: I haz the power, 2017-02-13 08:30:57]

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

[tzik, 2017-02-13 11:18:51]

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

[commit-bot: I haz the power, 2017-02-13 11:19:03]

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

[tzik, 2017-02-13 11:21:36]

Patchset #2 (id:360001) has been deleted

[tzik, 2017-02-13 11:21:56]

Patchset #2 (id:380001) has been deleted

[tzik, 2017-02-13 12:11:36]

PTAL

[tzik, 2017-02-13 13:29:42]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_cherker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are sequence_cherker.{h,cc} only.

The header dependency from sequence_cherker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_cherker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_cherker_impl.{h,cc} only.

The header dependency from sequence_cherker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[gab, 2017-02-13 15:26:39]

This is great! Also have had my share of trouble with that cyclic dependency in
the past (but was banking on upcoming obliteration of SequencedWorkerPool to
address it -- thanks for finding a solution in the meantime and fixing the IWYU
mess!)

//base lgtm with comments (didn't double-check all includes fixing...)

https://codereview.chromium.org/2690533002/diff/340002/base/sequence_checker_...
File base/sequence_checker_impl.cc (right):

https://codereview.chromium.org/2690533002/diff/340002/base/sequence_checker_...
base/sequence_checker_impl.cc:41: // sequence token.
nit: wrap

https://codereview.chromium.org/2690533002/diff/340002/base/sequence_checker_...
base/sequence_checker_impl.cc:46: mutable SequenceToken sequence_token_;
No longer needs to be mutable since |core_| is what's modified from the const
checkers now :)

https://codereview.chromium.org/2690533002/diff/340002/base/sequence_checker_...
base/sequence_checker_impl.cc:50: mutable SequencedWorkerPool::SequenceToken
sequenced_worker_pool_token_;
ditto

https://codereview.chromium.org/2690533002/diff/340002/base/sequence_checker_...
base/sequence_checker_impl.cc:74: void
SequenceCheckerImpl::EnsureSequenceTokenAssigned() const {
Since this is now simple and only called from two places:

Move |core_|'s initialization into the constructor's initializer list (no longer
requires to grab lock :))

And move if (!core) core_ = ... inline in CalledOnValidSequence()

[commit-bot: I haz the power, 2017-02-13 16:17:18]

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

[commit-bot: I haz the power, 2017-02-13 16:17:20]

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

[tzik, 2017-02-14 06:55:11]

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

[commit-bot: I haz the power, 2017-02-14 06:55:29]

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

[tzik, 2017-02-14 07:00:25]

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

[commit-bot: I haz the power, 2017-02-14 07:00:46]

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

[tzik, 2017-02-14 07:24:00]

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

[commit-bot: I haz the power, 2017-02-14 07:24:11]

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

[tzik, 2017-02-14 09:14:37]

https://codereview.chromium.org/2690533002/diff/340002/base/sequence_checker_...
File base/sequence_checker_impl.cc (right):

https://codereview.chromium.org/2690533002/diff/340002/base/sequence_checker_...
base/sequence_checker_impl.cc:41: // sequence token.
On 2017/02/13 15:26:39, gab wrote:
> nit: wrap

Done.

https://codereview.chromium.org/2690533002/diff/340002/base/sequence_checker_...
base/sequence_checker_impl.cc:46: mutable SequenceToken sequence_token_;
On 2017/02/13 15:26:39, gab wrote:
> No longer needs to be mutable since |core_| is what's modified from the const
> checkers now :)

Done.

https://codereview.chromium.org/2690533002/diff/340002/base/sequence_checker_...
base/sequence_checker_impl.cc:50: mutable SequencedWorkerPool::SequenceToken
sequenced_worker_pool_token_;
On 2017/02/13 15:26:39, gab wrote:
> ditto

Done.

https://codereview.chromium.org/2690533002/diff/340002/base/sequence_checker_...
base/sequence_checker_impl.cc:74: void
SequenceCheckerImpl::EnsureSequenceTokenAssigned() const {
On 2017/02/13 15:26:39, gab wrote:
> Since this is now simple and only called from two places:
> 
> Move |core_|'s initialization into the constructor's initializer list (no
longer
> requires to grab lock :))
> 
> And move if (!core) core_ = ... inline in CalledOnValidSequence()

Done.

[tzik, 2017-02-14 09:15:33]

tzik@chromium.org changed reviewers:
+ sky@chromium.org

[tzik, 2017-02-14 09:25:56]

tzik@chromium.org changed reviewers:
+ blundell@chromium.org, boliu@chromium.org, kinuko@chromium.org,
marq@chromium.org, oshima@chromium.org, qinmin@chromium.org,
rdsmith@chromium.org, rockot@chromium.org, slan@chromium.org

[tzik, 2017-02-14 09:28:47]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_cherker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_cherker_impl.{h,cc} only.

The header dependency from sequence_cherker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_cherker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_cherker_impl.{h,cc} only.

The header dependency from sequence_cherker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[tzik, 2017-02-14 09:34:55]

Adding OWNERS for non-//base file review to TBR. The diffs are mechanical
forward decl addition or #include addition.

PTAL later to:
sky: //ui, //services/service_manager, //chrome
kinuko: //storage, //content
rdsmith: //net
qinmin: //media/base/android
marq: //ios
rockot: //extensions, //device
blundell: //components,
oshima: //chromeos, //ash
slan: //chromecast
boliu: //android_webview

[marq (ping after 24h), 2017-02-14 10:09:37]

ios/ LGTM

Please correct the typos ("sequence_cherker" for "sequence_checker", several
times) in the CL description.

[tzik, 2017-02-14 10:24:28]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_cherker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_cherker_impl.{h,cc} only.

The header dependency from sequence_cherker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[tzik, 2017-02-14 10:33:37]

On 2017/02/14 10:09:37, marq wrote:
> ios/ LGTM
> 
> Please correct the typos ("sequence_cherker" for "sequence_checker", several
> times) in the CL description.

Thanks! Done.

[tzik, 2017-02-14 11:08:35]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org
==========

[tzik, 2017-02-14 11:09:03]

The CQ bit was unchecked by tzik@chromium.org

[tzik, 2017-02-14 11:09:24]

The CQ bit was checked by tzik@chromium.org

[tzik, 2017-02-14 11:09:25]

The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org
Link to the patchset: https://codereview.chromium.org/2690533002/#ps450001
(title: "inline EnsureSequenceTokenAssigned")

[commit-bot: I haz the power, 2017-02-14 11:09:51]

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-02-14 11:18:00]

CQ is committing da patch.

Bot data: {"patchset_id": 450001, "attempt_start_ts": 1487070564921630,
"parent_rev": "0a762e43411be0bd7426da0d749b7e49d4de47b3", "commit_rev":
"d502a3f2fa0b673a5082c1d601298bd114a9d237"}

[commit-bot: I haz the power, 2017-02-14 11:18:44]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
==========

[commit-bot: I haz the power, 2017-02-14 11:18:46]

Committed patchset #5 (id:450001) as
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...

[tzik, 2017-02-14 11:35:40]

A revert of this CL (patchset #5 id:450001) has been created in
https://codereview.chromium.org/2691263002/ by tzik@chromium.org.

The reason for reverting is: This caused a compile error on a mac bot:
https://build.chromium.org/p/chromium.mac/buildstatus?builder=ios-device&numb....

[sdefresne, 2017-02-14 11:36:04]

A revert of this CL (patchset #5 id:450001) has been created in
https://codereview.chromium.org/2696583003/ by sdefresne@chromium.org.

The reason for reverting is: Broke compilation on all iOS bots:

../../components/favicon/ios/web_favicon_driver.mm:134:7: error: no matching
constructor for initialization of 'image_fetcher::IOSImageDataFetcherWrapper'
      image_fetcher_(web_state->GetBrowserState()->GetRequestContext(),
      ^              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h:37:3: note:
candidate constructor not viable: cannot convert argument of incomplete type
'base::SequencedWorkerPool *' to 'const scoped_refptr<base::TaskRunner>' for 2nd
argument
  IOSImageDataFetcherWrapper(
  ^
../../components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h:70:28: note:
candidate constructor not viable: requires 1 argument, but 2 were provided
  DISALLOW_COPY_AND_ASSIGN(IOSImageDataFetcherWrapper);
.

[msramek, 2017-02-14 12:07:47]

msramek@chromium.org changed reviewers:
+ msramek@chromium.org

[msramek, 2017-02-14 12:07:48]

Another compilation error appeared on Mac:
https://uberchromegw.corp.google.com/i/chromium.chrome/builders/Google%20Chro...

======================================================================

../../chrome/browser/component_updater/pepper_flash_component_installer.cc:315:35:
error: member access into incomplete type 'base::SequencedWorkerPool'
  BrowserThread::GetBlockingPool()->PostTask(
                                  ^
../../base/threading/thread_restrictions.h:93:7: note: forward declaration of
'base::SequencedWorkerPool'
class SequencedWorkerPool;
      ^
1 error generated.

======================================================================

Just FYI, as this had been already reverted.

[tzik, 2017-02-14 13:32:28]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
==========

[tzik, 2017-02-14 13:44:04]

On 2017/02/14 12:07:48, msramek wrote:
> Another compilation error appeared on Mac:
>
https://uberchromegw.corp.google.com/i/chromium.chrome/builders/Google%20Chro...
> 
> ======================================================================
> 
>
../../chrome/browser/component_updater/pepper_flash_component_installer.cc:315:35:
> error: member access into incomplete type 'base::SequencedWorkerPool'
>   BrowserThread::GetBlockingPool()->PostTask(
>                                   ^
> ../../base/threading/thread_restrictions.h:93:7: note: forward declaration of
> 'base::SequencedWorkerPool'
> class SequencedWorkerPool;
>       ^
> 1 error generated.
> 
> ======================================================================
> 
> Just FYI, as this had been already reverted.

Ah... Thanks for noticing me.
There seem at least 3 author who are writing conflicting CL to this CL...

[tzik, 2017-02-14 13:45:17]

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

[commit-bot: I haz the power, 2017-02-14 13:45:41]

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-02-14 13:56:28]

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

[commit-bot: I haz the power, 2017-02-14 13:56:30]

Dry run: 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...)

[tzik, 2017-02-14 13:58:20]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[tzik, 2017-02-14 13:58:28]

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

[commit-bot: I haz the power, 2017-02-14 13:58:50]

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

[tzik, 2017-02-14 16:09:12]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
==========

[tzik, 2017-02-14 16:31:07]

The CQ bit was unchecked by tzik@chromium.org

[Randy Smith (Not in Mondays), 2017-02-14 16:44:33]

/net LGTM.

[tzik, 2017-02-14 16:52:04]

The CQ bit was checked by tzik@chromium.org

[tzik, 2017-02-14 16:52:05]

The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org,
marq@chromium.org
Link to the patchset: https://codereview.chromium.org/2690533002/#ps470001
(title: "resolve conflict")

[commit-bot: I haz the power, 2017-02-14 16:52:33]

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-02-14 17:01:46]

CQ is committing da patch.

Bot data: {"patchset_id": 470001, "attempt_start_ts": 1487091124162570,
"parent_rev": "309c0997afe19ab971686abf5cca1dd7fd4f7d8b", "commit_rev":
"70b7c5d796420107930c72d4ad82e0c850166b1a"}

[commit-bot: I haz the power, 2017-02-14 17:03:26]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Original-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450384}
Committed:
https://chromium.googlesource.com/chromium/src/+/70b7c5d796420107930c72d4ad82...
==========

[commit-bot: I haz the power, 2017-02-14 17:03:28]

Committed patchset #6 (id:470001) as
https://chromium.googlesource.com/chromium/src/+/70b7c5d796420107930c72d4ad82...

[Garrett Casto, 2017-02-14 18:01:17]

A revert of this CL (patchset #6 id:470001) has been created in
https://codereview.chromium.org/2694813005/ by gcasto@chromium.org.

The reason for reverting is: This patch is causing failures on the CrOS build
bot:

../../chrome/browser/chromeos/login/wizard_controller.cc:225:42: error: member
access into incomplete type 'base::SequencedWorkerPool'
  DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
                                         ^
../../base/threading/thread_restrictions.h:93:7: note: forward declaration of
'base::SequencedWorkerPool'
class SequencedWorkerPool;
      ^
1 error generated..

[tzik, 2017-02-14 18:02:42]

On 2017/02/14 18:01:17, Garrett Casto wrote:
> A revert of this CL (patchset #6 id:470001) has been created in
> https://codereview.chromium.org/2694813005/ by mailto:gcasto@chromium.org.
> 
> The reason for reverting is: This patch is causing failures on the CrOS build
> bot:
> 
> ../../chrome/browser/chromeos/login/wizard_controller.cc:225:42: error: member
> access into incomplete type 'base::SequencedWorkerPool'
>   DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
>                                          ^
> ../../base/threading/thread_restrictions.h:93:7: note: forward declaration of
> 'base::SequencedWorkerPool'
> class SequencedWorkerPool;
>       ^
> 1 error generated..

Thanks for taking care!

[tzik, 2017-02-14 18:05:43]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Original-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450384}
Committed:
https://chromium.googlesource.com/chromium/src/+/70b7c5d796420107930c72d4ad82...
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Original-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450384}
Committed:
https://chromium.googlesource.com/chromium/src/+/70b7c5d796420107930c72d4ad82...
==========

[Garrett Casto, 2017-02-14 18:14:46]

On 2017/02/14 18:02:42, tzik wrote:
> On 2017/02/14 18:01:17, Garrett Casto wrote:
> > A revert of this CL (patchset #6 id:470001) has been created in
> > https://codereview.chromium.org/2694813005/ by mailto:gcasto@chromium.org.
> > 
> > The reason for reverting is: This patch is causing failures on the CrOS
build
> > bot:
> > 
> > ../../chrome/browser/chromeos/login/wizard_controller.cc:225:42: error:
member
> > access into incomplete type 'base::SequencedWorkerPool'
> >   DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
> >                                          ^
> > ../../base/threading/thread_restrictions.h:93:7: note: forward declaration
of
> > 'base::SequencedWorkerPool'
> > class SequencedWorkerPool;
> >       ^
> > 1 error generated..
> 
> Thanks for taking care

I posted this in the revert CL, but putting it here as well for visibility.

This also caused failures on the Windows bot
(https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/b...)

FAILED: obj/chrome/browser/ui/ui_1/version_updater_win.obj 
ninja -t msvc -e environment.x86 -- C:\b\c\cipd\goma/gomacc.exe
"C:\b\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\VC\bin\amd64_x86/cl.exe"
/nologo /showIncludes /FC
@obj/chrome/browser/ui/ui_1/version_updater_win.obj.rsp /c
../../chrome/browser/ui/webui/help/version_updater_win.cc
/Foobj/chrome/browser/ui/ui_1/version_updater_win.obj
/Fd"obj/chrome/browser/ui/ui_1_cc.pdb"
c:\b\c\b\win_chrome\src\chrome\browser\ui\webui\help\version_updater_win.cc(58):
error C2664: 'bool base::PostTaskAndReplyWithResult<R,bool>(base::TaskRunner
*,const tracked_objects::Location &,base::Callback<R
(void),base::internal::CopyMode::Copyable,base::internal::RepeatMode::Repeating>,base::Callback<void
(bool),base::internal::CopyMode::Copyable,base::internal::RepeatMode::Repeating>)':
cannot convert argument 1 from 'base::SequencedWorkerPool *' to
'base::TaskRunner *'
        with
        [
            R=bool
        ]
c:\b\c\b\win_chrome\src\chrome\browser\ui\webui\help\version_updater_win.cc(58):
note: Types pointed to are unrelated; conversion requires reinterpret_cast,
C-style cast or function-style cast

[tzik, 2017-02-14 18:30:14]

On 2017/02/14 18:14:46, Garrett Casto wrote:
> On 2017/02/14 18:02:42, tzik wrote:
> > On 2017/02/14 18:01:17, Garrett Casto wrote:
> > > A revert of this CL (patchset #6 id:470001) has been created in
> > > https://codereview.chromium.org/2694813005/ by mailto:gcasto@chromium.org.
> > > 
> > > The reason for reverting is: This patch is causing failures on the CrOS
> build
> > > bot:
> > > 
> > > ../../chrome/browser/chromeos/login/wizard_controller.cc:225:42: error:
> member
> > > access into incomplete type 'base::SequencedWorkerPool'
> > >   DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
> > >                                          ^
> > > ../../base/threading/thread_restrictions.h:93:7: note: forward declaration
> of
> > > 'base::SequencedWorkerPool'
> > > class SequencedWorkerPool;
> > >       ^
> > > 1 error generated..
> > 
> > Thanks for taking care
> 
> I posted this in the revert CL, but putting it here as well for visibility.
> 
> This also caused failures on the Windows bot
>
(https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/b...)
> 
> FAILED: obj/chrome/browser/ui/ui_1/version_updater_win.obj 
> ninja -t msvc -e environment.x86 -- C:\b\c\cipd\goma/gomacc.exe
>
"C:\b\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\VC\bin\amd64_x86/cl.exe"
> /nologo /showIncludes /FC
> @obj/chrome/browser/ui/ui_1/version_updater_win.obj.rsp /c
> ../../chrome/browser/ui/webui/help/version_updater_win.cc
> /Foobj/chrome/browser/ui/ui_1/version_updater_win.obj
> /Fd"obj/chrome/browser/ui/ui_1_cc.pdb"
>
c:\b\c\b\win_chrome\src\chrome\browser\ui\webui\help\version_updater_win.cc(58):
> error C2664: 'bool base::PostTaskAndReplyWithResult<R,bool>(base::TaskRunner
> *,const tracked_objects::Location &,base::Callback<R
>
(void),base::internal::CopyMode::Copyable,base::internal::RepeatMode::Repeating>,base::Callback<void
>
(bool),base::internal::CopyMode::Copyable,base::internal::RepeatMode::Repeating>)':
> cannot convert argument 1 from 'base::SequencedWorkerPool *' to
> 'base::TaskRunner *'
>         with
>         [
>             R=bool
>         ]
>
c:\b\c\b\win_chrome\src\chrome\browser\ui\webui\help\version_updater_win.cc(58):
> note: Types pointed to are unrelated; conversion requires reinterpret_cast,
> C-style cast or function-style cast

That's specific to the official build on Windows. :(

folks: This CL touches handreds of files and was reverted twice.
Rather than relanding this CL as is, let me break this down to pieces and land
one by one.

[gab, 2017-02-14 18:47:53]

On 2017/02/14 18:30:14, tzik wrote:
> On 2017/02/14 18:14:46, Garrett Casto wrote:
> > On 2017/02/14 18:02:42, tzik wrote:
> > > On 2017/02/14 18:01:17, Garrett Casto wrote:
> > > > A revert of this CL (patchset #6 id:470001) has been created in
> > > > https://codereview.chromium.org/2694813005/ by
mailto:gcasto@chromium.org.
> > > > 
> > > > The reason for reverting is: This patch is causing failures on the CrOS
> > build
> > > > bot:
> > > > 
> > > > ../../chrome/browser/chromeos/login/wizard_controller.cc:225:42: error:
> > member
> > > > access into incomplete type 'base::SequencedWorkerPool'
> > > >   DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
> > > >                                          ^
> > > > ../../base/threading/thread_restrictions.h:93:7: note: forward
declaration
> > of
> > > > 'base::SequencedWorkerPool'
> > > > class SequencedWorkerPool;
> > > >       ^
> > > > 1 error generated..
> > > 
> > > Thanks for taking care
> > 
> > I posted this in the revert CL, but putting it here as well for visibility.
> > 
> > This also caused failures on the Windows bot
> >
>
(https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/b...)
> > 
> > FAILED: obj/chrome/browser/ui/ui_1/version_updater_win.obj 
> > ninja -t msvc -e environment.x86 -- C:\b\c\cipd\goma/gomacc.exe
> >
>
"C:\b\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\VC\bin\amd64_x86/cl.exe"
> > /nologo /showIncludes /FC
> > @obj/chrome/browser/ui/ui_1/version_updater_win.obj.rsp /c
> > ../../chrome/browser/ui/webui/help/version_updater_win.cc
> > /Foobj/chrome/browser/ui/ui_1/version_updater_win.obj
> > /Fd"obj/chrome/browser/ui/ui_1_cc.pdb"
> >
>
c:\b\c\b\win_chrome\src\chrome\browser\ui\webui\help\version_updater_win.cc(58):
> > error C2664: 'bool base::PostTaskAndReplyWithResult<R,bool>(base::TaskRunner
> > *,const tracked_objects::Location &,base::Callback<R
> >
>
(void),base::internal::CopyMode::Copyable,base::internal::RepeatMode::Repeating>,base::Callback<void
> >
>
(bool),base::internal::CopyMode::Copyable,base::internal::RepeatMode::Repeating>)':
> > cannot convert argument 1 from 'base::SequencedWorkerPool *' to
> > 'base::TaskRunner *'
> >         with
> >         [
> >             R=bool
> >         ]
> >
>
c:\b\c\b\win_chrome\src\chrome\browser\ui\webui\help\version_updater_win.cc(58):
> > note: Types pointed to are unrelated; conversion requires reinterpret_cast,
> > C-style cast or function-style cast
> 
> That's specific to the official build on Windows. :(
> 
> folks: This CL touches handreds of files and was reverted twice.
> Rather than relanding this CL as is, let me break this down to pieces and land
> one by one.

@tizk: what I usually do for such changes is land all the prerequisite IWYU
changes first (they're the right thing on their own) and then repeatedly try to
land the core change until it sticks (making another precursor CL when it
doesn't)

[tzik, 2017-02-15 08:44:38]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Original-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450384}
Committed:
https://chromium.googlesource.com/chromium/src/+/70b7c5d796420107930c72d4ad82...
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Original-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450384}
Committed:
https://chromium.googlesource.com/chromium/src/+/70b7c5d796420107930c72d4ad82...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[tzik, 2017-02-15 11:08:42]

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

[commit-bot: I haz the power, 2017-02-15 11:08:56]

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

[tzik, 2017-02-15 11:40:07]

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

[commit-bot: I haz the power, 2017-02-15 11:40:33]

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

[tzik, 2017-02-15 12:08:56]

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

[commit-bot: I haz the power, 2017-02-15 12:09:04]

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-02-15 14:04:27]

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

[commit-bot: I haz the power, 2017-02-15 14:04:28]

Dry run: This issue passed the CQ dry run.

[tzik, 2017-02-16 08:39:17]

The CQ bit was checked by tzik@chromium.org

[tzik, 2017-02-16 08:39:18]

The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org,
rdsmith@chromium.org, marq@chromium.org
Link to the patchset: https://codereview.chromium.org/2690533002/#ps550001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-16 08:40:13]

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-02-16 08:52:35]

CQ is committing da patch.

Bot data: {"patchset_id": 550001, "attempt_start_ts": 1487234357688170,
"parent_rev": "de938a8e39930cca0d2a1f8c96f2cdc9b15192de", "commit_rev":
"0c2fcf56d2df0bb7b31ccb1509662255b5852451"}

[commit-bot: I haz the power, 2017-02-16 08:53:13]

Description was changed from

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Original-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450384}
Committed:
https://chromium.googlesource.com/chromium/src/+/70b7c5d796420107930c72d4ad82...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Remove header dependencies from sequence_checker.h to sequenced_worker_pool.h

This CL removes #include to sequenced_worker_pool.h in sequence_checker.h
by hiding its members into a nested class, and adds #include and forward
decls to other files as needed.

Note that the non trivial diffs are in sequence_checker_impl.{h,cc} only.

The header dependency from sequence_checker.h to sequenced_worker_pool.h
prevents other headers to use sequence_checker.h due to a dependency
cycle.

TBR=sky@chromium.org, kinuko@chromium.org, rdsmith@chromium.org,
qinmin@chromium.org, marq@chromium.org, rockot@chromium.org,
blundell@chromium.org, oshima@chromium.org, slan@chromium.org,
boliu@chromium.org

Review-Url: https://codereview.chromium.org/2690533002
Cr-Original-Commit-Position: refs/heads/master@{#450321}
Committed:
https://chromium.googlesource.com/chromium/src/+/d502a3f2fa0b673a5082c1d60129...
Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450384}
Committed:
https://chromium.googlesource.com/chromium/src/+/70b7c5d796420107930c72d4ad82...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2690533002
Cr-Commit-Position: refs/heads/master@{#450903}
Committed:
https://chromium.googlesource.com/chromium/src/+/0c2fcf56d2df0bb7b31ccb150966...
==========

[commit-bot: I haz the power, 2017-02-16 08:53:15]

Committed patchset #10 (id:550001) as
https://chromium.googlesource.com/chromium/src/+/0c2fcf56d2df0bb7b31ccb150966...