Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/ tests

Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/ tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

TODO(gab): Try to enable death tests on OS_ANDROID in a follow-up CL and document what breaks.

BUG=622400
(relation to that bug is the attempt to open as many tests as possible
to Release+DCHECK_ALWAYS_ON started in https://codereview.chromium.org/2163023002/
as a safety measure for the impl for 622400)

Committed: https://crrev.com/c964a852c558c9876e49b1bcea0d98c720b5b300
Cr-Commit-Position: refs/heads/master@{#408978}

[gab, 2016-07-20 16:02:15]

Description was changed from

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

BUG=622400
==========

to

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK builds
-- resulting in extra coverage (though sometimes mostly a no-op).

BUG=622400
==========

[gab, 2016-07-20 16:02:31]

Description was changed from

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK builds
-- resulting in extra coverage (though sometimes mostly a no-op).

BUG=622400
==========

to

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK 
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

BUG=622400
==========

[gab, 2016-07-20 16:03:06]

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

[commit-bot: I haz the power, 2016-07-20 16:03:30]

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

[gab, 2016-07-20 16:10:14]

Patchset #2 (id:20001) has been deleted

[gab, 2016-07-20 16:10:22]

Patchset #1 (id:1) has been deleted

[gab, 2016-07-20 16:11:05]

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

[commit-bot: I haz the power, 2016-07-20 16:11:33]

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

[gab, 2016-07-20 16:16:24]

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

[gab, 2016-07-20 16:16:25]

Dana PTAL, extracted from https://codereview.chromium.org/2163023002/ for the
death tests cleanup part of it.

[commit-bot: I haz the power, 2016-07-20 16:17:44]

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

[commit-bot: I haz the power, 2016-07-20 16:17:44]

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

[gab, 2016-07-20 16:17:48]

Description was changed from

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK 
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

BUG=622400
==========

to

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK 
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

BUG=622400
(relation to that bug is the attempt to open as many tests as possible
to Release+DCHECK_ALWAYS_ON started in
https://codereview.chromium.org/2163023002/)
==========

[gab, 2016-07-20 16:18:19]

Description was changed from

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK 
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

BUG=622400
(relation to that bug is the attempt to open as many tests as possible
to Release+DCHECK_ALWAYS_ON started in
https://codereview.chromium.org/2163023002/)
==========

to

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK 
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

BUG=622400
(relation to that bug is the attempt to open as many tests as possible
to Release+DCHECK_ALWAYS_ON started in
https://codereview.chromium.org/2163023002/
as a safety measure for the impl for 622400)
==========

[danakj, 2016-07-20 19:33:46]

https://codereview.chromium.org/2162053006/diff/40001/base/base.gyp
File base/base.gyp (right):

https://codereview.chromium.org/2162053006/diff/40001/base/base.gyp#newcode574
base/base.gyp:574: 'threading/sequenced_task_runner_handle_unittest.cc',
Thanks for the sorting

https://codereview.chromium.org/2162053006/diff/40001/base/numerics/safe_nume...
File base/numerics/safe_numerics_unittest.cc (right):

https://codereview.chromium.org/2162053006/diff/40001/base/numerics/safe_nume...
base/numerics/safe_numerics_unittest.cc:658: EXPECT_DCHECK_DEATH(
This is a CHECK not a DCHECK

https://codereview.chromium.org/2162053006/diff/40001/base/synchronization/ca...
File base/synchronization/cancellation_flag_unittest.cc (left):

https://codereview.chromium.org/2162053006/diff/40001/base/synchronization/ca...
base/synchronization/cancellation_flag_unittest.cc:63: }  // namespace
I think it is usual to wrap a unit test file an a nested anonymous namespace,
and I'd leave it here. Other random example:
https://cs.chromium.org/chromium/src/cc/layers/layer_unittest.cc?rcl=0&l=2542

https://codereview.chromium.org/2162053006/diff/40001/base/test/gtest_util.h
File base/test/gtest_util.h (right):

https://codereview.chromium.org/2162053006/diff/40001/base/test/gtest_util.h#...
base/test/gtest_util.h:22: // Death tests misbehave on Android.
Can you point to a bug or write something more about android? Most of the places
youve stiched to this did not check for android.

https://codereview.chromium.org/2162053006/diff/40001/base/threading/non_thre...
File base/threading/non_thread_safe_unittest.cc (right):

https://codereview.chromium.org/2162053006/diff/40001/base/threading/non_thre...
base/threading/non_thread_safe_unittest.cc:151: #else
For the same reasons as you state in the comment, running the methods that
should dcheck and verifying they dont in release is very O_O. IMO we should drop
the #else for all these tests.

[gab, 2016-07-20 21:12:02]

Description was changed from

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK 
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

BUG=622400
(relation to that bug is the attempt to open as many tests as possible
to Release+DCHECK_ALWAYS_ON started in
https://codereview.chromium.org/2163023002/
as a safety measure for the impl for 622400)
==========

to

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK 
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

TODO(gab): Try to enable death tests on OS_ANDROID in a follow-up CL and
document what breaks.

BUG=622400
(relation to that bug is the attempt to open as many tests as possible
to Release+DCHECK_ALWAYS_ON started in
https://codereview.chromium.org/2163023002/
as a safety measure for the impl for 622400)
==========

[gab, 2016-07-20 21:21:08]

https://codereview.chromium.org/2162053006/diff/40001/base/base.gyp
File base/base.gyp (right):

https://codereview.chromium.org/2162053006/diff/40001/base/base.gyp#newcode574
base/base.gyp:574: 'threading/sequenced_task_runner_handle_unittest.cc',
On 2016/07/20 19:33:45, danakj wrote:
> Thanks for the sorting

Acknowledged.

https://codereview.chromium.org/2162053006/diff/40001/base/numerics/safe_nume...
File base/numerics/safe_numerics_unittest.cc (right):

https://codereview.chromium.org/2162053006/diff/40001/base/numerics/safe_nume...
base/numerics/safe_numerics_unittest.cc:658: EXPECT_DCHECK_DEATH(
On 2016/07/20 19:33:45, danakj wrote:
> This is a CHECK not a DCHECK

Ah right, then previous form was correct (always do a death test if gtest
supports it).

https://codereview.chromium.org/2162053006/diff/40001/base/synchronization/ca...
File base/synchronization/cancellation_flag_unittest.cc (left):

https://codereview.chromium.org/2162053006/diff/40001/base/synchronization/ca...
base/synchronization/cancellation_flag_unittest.cc:63: }  // namespace
On 2016/07/20 19:33:45, danakj wrote:
> I think it is usual to wrap a unit test file an a nested anonymous namespace,
> and I'd leave it here. Other random example:
> https://cs.chromium.org/chromium/src/cc/layers/layer_unittest.cc?rcl=0&l=2542

Done.

https://codereview.chromium.org/2162053006/diff/40001/base/test/gtest_util.h
File base/test/gtest_util.h (right):

https://codereview.chromium.org/2162053006/diff/40001/base/test/gtest_util.h#...
base/test/gtest_util.h:22: // Death tests misbehave on Android.
On 2016/07/20 19:33:45, danakj wrote:
> Can you point to a bug or write something more about android? Most of the
places
> youve stiched to this did not check for android.

I don't know either, but a few did mentioned this. How about I get the bots
green and land this and then try to enable on Android, see what fails and
document it in a bug (with follow-up CL to add bug comment here)?

Added TODO in this CL's description to that effect.

https://codereview.chromium.org/2162053006/diff/40001/base/threading/non_thre...
File base/threading/non_thread_safe_unittest.cc (right):

https://codereview.chromium.org/2162053006/diff/40001/base/threading/non_thre...
base/threading/non_thread_safe_unittest.cc:151: #else
On 2016/07/20 19:33:45, danakj wrote:
> For the same reasons as you state in the comment, running the methods that
> should dcheck and verifying they dont in release is very O_O. IMO we should
drop
> the #else for all these tests.

True, except that these tests don't have undefined behavior when doing so (they
verify that the assertion doesn't trigger in Release but the check itself isn't
what's undefined, it's whatever the typical user of the checker would do based
on this assumption that is).
So I can see an argument for keeping them... and since it's not what I'm aiming
to change I'd rather not remove previously tested things in this CL.

https://codereview.chromium.org/2162053006/diff/60001/base/test/gtest_util.h
File base/test/gtest_util.h (right):

https://codereview.chromium.org/2162053006/diff/60001/base/test/gtest_util.h#...
base/test/gtest_util.h:31: GTEST_UNSUPPORTED_DEATH_TEST(statement, regex,
return)
This won't compile, locally I moved this from
testing/gtest/include/gtest/internal/gtest-death-test-internal.h to
gtest-death-test.h as it was only defined for !GTEST_HAS_DEATH_TEST and we need
it in a broader context (and I think it makes sense for gtest to support our use
case of wanting a different "death scope").

But I don't see how to make the change in gtest/ -- it's a separate git tree but
git cl upload doesn't work.

I don't need this macro per se but I need a way to refer to |statement| so that
compilations without DCHECKs at least don't complain about unreferenced
variables and ideally compile the code without running it so that people can
write valid code even in configurations DCHECK_DEATH isn't running.

[gab, 2016-07-20 21:21:49]

gab@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[gab, 2016-07-20 21:21:50]

+phajdan.jr@ to advise on base/test (see inline self-comment)

[gab, 2016-07-22 14:16:33]

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

[commit-bot: I haz the power, 2016-07-22 14:16:52]

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

[gab, 2016-07-22 14:19:26]

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

[gab, 2016-07-22 14:19:29]

Copied required internal gtest macro for now (while I'm working on exposing it
in the upstream repo).

@Pawel PTAL at base/test/gtest_util.h 
@Dana PTAnL, thanks!

[commit-bot: I haz the power, 2016-07-22 14:19:49]

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

[commit-bot: I haz the power, 2016-07-22 14:26:26]

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

[commit-bot: I haz the power, 2016-07-22 14:26:27]

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

[danakj, 2016-07-22 21:15:47]

https://codereview.chromium.org/2162053006/diff/110001/base/test/gtest_util.h
File base/test/gtest_util.h (right):

https://codereview.chromium.org/2162053006/diff/110001/base/test/gtest_util.h...
base/test/gtest_util.h:38: GTEST_AMBIGUOUS_ELSE_BLOCKER_ \
While I understand you copy/pasted this out of gtest, I'd prefer we drop
references to more internal gtest things if possible (and change "copied from"
to "inspired by" i guess).

Do we need this ELSE_BLOCKER_ for our tests?

Can we get away without the call to PartialMatch?

Can we make our own AlwaysTrue/AlwaysFalse and use them (also the
SUPPRESS_UNREACHABLE is just another AlwaysTrue).

https://codereview.chromium.org/2162053006/diff/110001/base/test/gtest_util.h...
base/test/gtest_util.h:47: } else \
nit: {}

[gab, 2016-07-25 15:43:37]

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

[commit-bot: I haz the power, 2016-07-25 15:43:45]

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

[gab, 2016-07-25 15:44:41]

Patchset #7 (id:150001) has been deleted

[gab, 2016-07-25 15:44:45]

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

[commit-bot: I haz the power, 2016-07-25 15:44:58]

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

[gab, 2016-07-25 15:46:21]

https://codereview.chromium.org/2162053006/diff/110001/base/test/gtest_util.h
File base/test/gtest_util.h (right):

https://codereview.chromium.org/2162053006/diff/110001/base/test/gtest_util.h...
base/test/gtest_util.h:38: GTEST_AMBIGUOUS_ELSE_BLOCKER_ \
On 2016/07/22 21:15:46, danakj wrote:
> While I understand you copy/pasted this out of gtest, I'd prefer we drop
> references to more internal gtest things if possible (and change "copied from"
> to "inspired by" i guess).
> 
> Do we need this ELSE_BLOCKER_ for our tests?
> 
> Can we get away without the call to PartialMatch?
> 
> Can we make our own AlwaysTrue/AlwaysFalse and use them (also the
> SUPPRESS_UNREACHABLE is just another AlwaysTrue).

I haven't looked into this macro closely so the answer to most of the above is
"I don't know"... I'd much rather copy it as-is and replace it with the gtest
one soon (already working on making this method public in gtest
https://critique.corp.google.com/#review/128201478 as well as working on rolling
gtest http://crbug.com/630705) then try to make our own version.

https://codereview.chromium.org/2162053006/diff/110001/base/test/gtest_util.h...
base/test/gtest_util.h:47: } else \
On 2016/07/22 21:15:46, danakj wrote:
> nit: {}

This appears to be intentional. It doesn't compile with {} as this is this
macro's way of forcing a semi-colon after it... We could instead wrap the whole
thing in do {...} while(0), but as mentioned above, I hate advanced macros and
would much rather copy this one as-is than try to out fancy it...

[commit-bot: I haz the power, 2016-07-25 16:42:28]

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

[commit-bot: I haz the power, 2016-07-25 16:42:29]

Dry run: This issue passed the CQ dry run.

[danakj, 2016-07-25 18:45:59]

https://codereview.chromium.org/2162053006/diff/110001/base/test/gtest_util.h
File base/test/gtest_util.h (right):

https://codereview.chromium.org/2162053006/diff/110001/base/test/gtest_util.h...
base/test/gtest_util.h:38: GTEST_AMBIGUOUS_ELSE_BLOCKER_ \
On 2016/07/25 15:46:21, gab wrote:
> On 2016/07/22 21:15:46, danakj wrote:
> > While I understand you copy/pasted this out of gtest, I'd prefer we drop
> > references to more internal gtest things if possible (and change "copied
from"
> > to "inspired by" i guess).
> > 
> > Do we need this ELSE_BLOCKER_ for our tests?
> > 
> > Can we get away without the call to PartialMatch?
> > 
> > Can we make our own AlwaysTrue/AlwaysFalse and use them (also the
> > SUPPRESS_UNREACHABLE is just another AlwaysTrue).
> 
> I haven't looked into this macro closely so the answer to most of the above is
> "I don't know"... I'd much rather copy it as-is and replace it with the gtest
> one soon (already working on making this method public in gtest
> https://critique.corp.google.com/#review/128201478 as well as working on
rolling
> gtest http://crbug.com/630705) then try to make our own version.

In the event that it all goes to ashes, I just prefer we do this as simply as
possible, and not depend on more internal gtest things.

AlwaysTrue is simply a function that returns true:
https://cs.chromium.org/chromium/src/testing/gtest/src/gtest.cc?rcl=146944535...

AlwaysFalse() is !AlwaysTrue():
https://cs.chromium.org/chromium/src/testing/gtest/include/gtest/internal/gte...

You could probably do these with lambdas inside the macro if u wanted to (scoped
inside a do {} while(0))

The macro is not that complicated when you break it down. I don't think we ever
write if (asda) EXPECT_DCHECK_DEATH in a test, so we can drop the ELSE_BLOCKER.

No one has written EXPECT_DCHECK_DEATH << "stuff", which would do nothing with
the stuff anyways, so we could drop the last else and use do{}while(0).

GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_ is just if (AlwaysTrue()) {
statement }:
https://cs.chromium.org/chromium/src/testing/gtest/include/gtest/internal/gte...

Or we could wait a bit and see how cl/128201478 goes.

[gab, 2016-07-27 17:00:50]

https://codereview.chromium.org/2162053006/diff/110001/base/test/gtest_util.h
File base/test/gtest_util.h (right):

https://codereview.chromium.org/2162053006/diff/110001/base/test/gtest_util.h...
base/test/gtest_util.h:38: GTEST_AMBIGUOUS_ELSE_BLOCKER_ \
On 2016/07/25 18:45:59, danakj wrote:
> On 2016/07/25 15:46:21, gab wrote:
> > On 2016/07/22 21:15:46, danakj wrote:
> > > While I understand you copy/pasted this out of gtest, I'd prefer we drop
> > > references to more internal gtest things if possible (and change "copied
> from"
> > > to "inspired by" i guess).
> > > 
> > > Do we need this ELSE_BLOCKER_ for our tests?
> > > 
> > > Can we get away without the call to PartialMatch?
> > > 
> > > Can we make our own AlwaysTrue/AlwaysFalse and use them (also the
> > > SUPPRESS_UNREACHABLE is just another AlwaysTrue).
> > 
> > I haven't looked into this macro closely so the answer to most of the above
is
> > "I don't know"... I'd much rather copy it as-is and replace it with the
gtest
> > one soon (already working on making this method public in gtest
> > https://critique.corp.google.com/#review/128201478 as well as working on
> rolling
> > gtest http://crbug.com/630705) then try to make our own version.
> 
> In the event that it all goes to ashes, I just prefer we do this as simply as
> possible, and not depend on more internal gtest things.
> 
> AlwaysTrue is simply a function that returns true:
>
https://cs.chromium.org/chromium/src/testing/gtest/src/gtest.cc?rcl=146944535...
> 
> AlwaysFalse() is !AlwaysTrue():
>
https://cs.chromium.org/chromium/src/testing/gtest/include/gtest/internal/gte...
> 
> You could probably do these with lambdas inside the macro if u wanted to
(scoped
> inside a do {} while(0))
> 
> The macro is not that complicated when you break it down. I don't think we
ever
> write if (asda) EXPECT_DCHECK_DEATH in a test, so we can drop the
ELSE_BLOCKER.
> 
> No one has written EXPECT_DCHECK_DEATH << "stuff", which would do nothing with
> the stuff anyways, so we could drop the last else and use do{}while(0).
> 
> GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_ is just if (AlwaysTrue()) {
> statement }:
>
https://cs.chromium.org/chromium/src/testing/gtest/include/gtest/internal/gte...
> 
> Or we could wait a bit and see how cl/128201478 goes.

https://cl/128201478 is approved and we have consensus on
http://crbug.com/630705 on how to perform the gtest roll. And even if it somehow
goes to ashes, I guess the TODO above puts me on the hook for fixing it (but I
don't see how it's possible that this would break without my gtest CL being
around as it would require rolling gtest to a version that doesn't support these
internal methods but also doesn't have my CL -- the current version of gtest
still has the same copy of this macro as our DEPS from a year ago).

[danakj, 2016-07-27 21:39:36]

ok LGTM thanks!

[gab, 2016-07-28 14:06:04]

The CQ bit was checked by gab@chromium.org

[commit-bot: I haz the power, 2016-07-28 14:06:28]

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

[commit-bot: I haz the power, 2016-07-28 14:08:35]

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

[commit-bot: I haz the power, 2016-07-28 14:08:37]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gyp_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gyp_...)

[gab, 2016-08-01 15:42:00]

The CQ bit was checked by gab@chromium.org

[gab, 2016-08-01 15:42:01]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/2162053006/#ps190001
(title: "merge up to r408965")

[commit-bot: I haz the power, 2016-08-01 15:42:17]

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

[commit-bot: I haz the power, 2016-08-01 16:40:28]

Description was changed from

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK 
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

TODO(gab): Try to enable death tests on OS_ANDROID in a follow-up CL and
document what breaks.

BUG=622400
(relation to that bug is the attempt to open as many tests as possible
to Release+DCHECK_ALWAYS_ON started in
https://codereview.chromium.org/2163023002/
as a safety measure for the impl for 622400)
==========

to

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK 
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

TODO(gab): Try to enable death tests on OS_ANDROID in a follow-up CL and
document what breaks.

BUG=622400
(relation to that bug is the attempt to open as many tests as possible
to Release+DCHECK_ALWAYS_ON started in
https://codereview.chromium.org/2163023002/
as a safety measure for the impl for 622400)
==========

[commit-bot: I haz the power, 2016-08-01 16:40:29]

Committed patchset #8 (id:190001)

[commit-bot: I haz the power, 2016-08-01 16:42:16]

Description was changed from

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK 
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

TODO(gab): Try to enable death tests on OS_ANDROID in a follow-up CL and
document what breaks.

BUG=622400
(relation to that bug is the attempt to open as many tests as possible
to Release+DCHECK_ALWAYS_ON started in
https://codereview.chromium.org/2163023002/
as a safety measure for the impl for 622400)
==========

to

==========
Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/
tests

Extracted from https://codereview.chromium.org/2163023002/

This has the following advantages:
 - Reduces inconsistent usage of #if preprocessors macros for death tests.
 - Enables some tests when DCHECK_ALWAYS_ON in Release builds
   that were previously not (per aforementioned inconsistency)
 - Runs those tests anyways (modulo call expected to die) in non-DCHECK
   builds -- resulting in extra coverage (though sometimes mostly a no-op).

TODO(gab): Try to enable death tests on OS_ANDROID in a follow-up CL and
document what breaks.

BUG=622400
(relation to that bug is the attempt to open as many tests as possible
to Release+DCHECK_ALWAYS_ON started in
https://codereview.chromium.org/2163023002/
as a safety measure for the impl for 622400)

Committed: https://crrev.com/c964a852c558c9876e49b1bcea0d98c720b5b300
Cr-Commit-Position: refs/heads/master@{#408978}
==========

[commit-bot: I haz the power, 2016-08-01 16:42:17]

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