Clean up some assertion code in WebKit.

Clean up some assertion code in WebKit.

- Redefine DCHECK_AT to always reference line and file; the compiler
  should still optimize them out if !DCHECK_IS_ON().
- Fix inconsistent use of DCHECK_IS_ON() vs ENABLE(ASSERT) checks.

These changes are prerequisites for the dump-on-DCHECK experiment.

BUG=596231
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/67a60716623af6f7e4571a206ce0df2e05e904f0
Cr-Commit-Position: refs/heads/master@{#414968}

[Wez, 2016-08-27 02:48:54]

Description was changed from

==========
Clean up some assertion code in WebKit.

- Redefine DCHECK_AT to always reference line and file; the compiler
  should still optimize them out if !DCHECK_IS_ON().
- Fix inconsistent use of DCHECK_IS_ON() vs ENABLE(ASSERT) checks.

These changes are prerequisites for the dump-on-DCHECK experiment.

BUG=596231
==========

to

==========
Clean up some assertion code in WebKit.

- Redefine DCHECK_AT to always reference line and file; the compiler
  should still optimize them out if !DCHECK_IS_ON().
- Fix inconsistent use of DCHECK_IS_ON() vs ENABLE(ASSERT) checks.

These changes are prerequisites for the dump-on-DCHECK experiment.

BUG=596231
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Wez, 2016-08-27 02:49:11]

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

[commit-bot: I haz the power, 2016-08-27 02:49:42]

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

[commit-bot: I haz the power, 2016-08-27 03:02:31]

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

[commit-bot: I haz the power, 2016-08-27 03:02:32]

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

[Wez, 2016-08-27 08:38:41]

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

[commit-bot: I haz the power, 2016-08-27 08:39: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, 2016-08-27 12:01:16]

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

[commit-bot: I haz the power, 2016-08-27 12:01:17]

Dry run: This issue passed the CQ dry run.

[Wez, 2016-08-27 20:08:45]

Patchset #2 (id:20001) has been deleted

[Wez, 2016-08-27 20:09:06]

Patchset #1 (id:1) has been deleted

[Wez, 2016-08-27 20:12:53]

wez@chromium.org changed reviewers:
+ senorblanco@chromium.org

[Wez, 2016-08-27 20:12:54]

PTAL - thanks!

[Wez, 2016-08-27 20:13:25]

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

[commit-bot: I haz the power, 2016-08-27 20:13:34]

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

[commit-bot: I haz the power, 2016-08-27 21:49:47]

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

[commit-bot: I haz the power, 2016-08-27 21:49:48]

Dry run: This issue passed the CQ dry run.

[Stephen White, 2016-08-27 23:20:09]

Description was changed from

==========
Clean up some assertion code in WebKit.

- Redefine DCHECK_AT to always reference line and file; the compiler
  should still optimize them out if !DCHECK_IS_ON().
- Fix inconsistent use of DCHECK_IS_ON() vs ENABLE(ASSERT) checks.

These changes are prerequisites for the dump-on-DCHECK experiment.

BUG=596231
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Clean up some assertion code in WebKit.

- Redefine DCHECK_AT to always reference line and file; the compiler
  should still optimize them out if !DCHECK_IS_ON().
- Fix inconsistent use of DCHECK_IS_ON() vs ENABLE(ASSERT) checks.

These changes are prerequisites for the dump-on-DCHECK experiment.

BUG=596231
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Stephen White, 2016-08-27 23:20:21]

+tkent FYI

https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/Assertions.h (right):

https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/Assertions.h:189: #define DCHECK_AT(assertion,
file, line) LAZY_STREAM(logging::LogMessage(file, line, #assertion).stream(),
DCHECK_IS_ON() ? !(assertion) : false)
Wouldn't this be clearer as

#if DCHECK_IS_ON()
#define DCHECK_AT(assertion, file, line) LAZY_STREAM(logging::LogMessage(file,
line, #assertion).stream(), !(assertion))
#else
#define DCHECK_AT(assertion, file, line) EAT_STREAM_PARAMETERS
#endif

i.e., same as before, but putting it behind #if DHECK_IS_ON() instead of #if
ENABLE(ASSERT) ? Or am I missing something?

[Wez, 2016-08-28 01:11:36]

https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/Assertions.h (right):

https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/Assertions.h:189: #define DCHECK_AT(assertion,
file, line) LAZY_STREAM(logging::LogMessage(file, line, #assertion).stream(),
DCHECK_IS_ON() ? !(assertion) : false)
On 2016/08/27 23:20:21, Stephen White wrote:
> Wouldn't this be clearer as
> 
> #if DCHECK_IS_ON()
> #define DCHECK_AT(assertion, file, line) LAZY_STREAM(logging::LogMessage(file,
> line, #assertion).stream(), !(assertion))
> #else
> #define DCHECK_AT(assertion, file, line) EAT_STREAM_PARAMETERS
> #endif
> 
> i.e., same as before, but putting it behind #if DHECK_IS_ON() instead of #if
> ENABLE(ASSERT) ? Or am I missing something?

The definition of DCHECK is specifically designed to ensure that |condition|
(aka |assertion| in this version) is referenced even if DCHECK is off, so the
compiler won't complain about unused variables that the condition would
otherwise reference. We then rely on the compiler optimizing the condition out
when it sees that DCHECK_IS_ON() is always true.

[Stephen White, 2016-08-28 01:32:40]

On 2016/08/28 01:11:36, Wez wrote:
>
https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/wtf/Assertions.h (right):
> 
>
https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/wtf/Assertions.h:189: #define DCHECK_AT(assertion,
> file, line) LAZY_STREAM(logging::LogMessage(file, line, #assertion).stream(),
> DCHECK_IS_ON() ? !(assertion) : false)
> On 2016/08/27 23:20:21, Stephen White wrote:
> > Wouldn't this be clearer as
> > 
> > #if DCHECK_IS_ON()
> > #define DCHECK_AT(assertion, file, line)
LAZY_STREAM(logging::LogMessage(file,
> > line, #assertion).stream(), !(assertion))
> > #else
> > #define DCHECK_AT(assertion, file, line) EAT_STREAM_PARAMETERS
> > #endif
> > 
> > i.e., same as before, but putting it behind #if DHECK_IS_ON() instead of #if
> > ENABLE(ASSERT) ? Or am I missing something?
> 
> The definition of DCHECK is specifically designed to ensure that |condition|
> (aka |assertion| in this version) is referenced even if DCHECK is off, so the
> compiler won't complain about unused variables that the condition would
> otherwise reference. We then rely on the compiler optimizing the condition out
> when it sees that DCHECK_IS_ON() is always true.

Got it.

LGTM

[Wez, 2016-08-28 04:30:58]

The CQ bit was checked by wez@chromium.org

[commit-bot: I haz the power, 2016-08-28 04:31:03]

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-28 04:36:31]

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

[commit-bot: I haz the power, 2016-08-28 04:36:33]

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

[Wez, 2016-08-28 05:24:58]

wez@chromium.org changed reviewers:
+ tkent@chromium.org

[Wez, 2016-08-28 05:24:58]

+tkent for Assertions.h OWNERS

[tkent, 2016-08-29 00:09:08]

The CQ bit was checked by tkent@chromium.org

[tkent, 2016-08-29 00:09:09]

lgtm

[commit-bot: I haz the power, 2016-08-29 00:09:23]

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

[commit-bot: I haz the power, 2016-08-29 01:13:56]

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

[commit-bot: I haz the power, 2016-08-29 01:13:58]

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

[tkent, 2016-08-29 01:16:39]

The CQ bit was checked by tkent@chromium.org

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

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-29 03:13:44]

Description was changed from

==========
Clean up some assertion code in WebKit.

- Redefine DCHECK_AT to always reference line and file; the compiler
  should still optimize them out if !DCHECK_IS_ON().
- Fix inconsistent use of DCHECK_IS_ON() vs ENABLE(ASSERT) checks.

These changes are prerequisites for the dump-on-DCHECK experiment.

BUG=596231
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Clean up some assertion code in WebKit.

- Redefine DCHECK_AT to always reference line and file; the compiler
  should still optimize them out if !DCHECK_IS_ON().
- Fix inconsistent use of DCHECK_IS_ON() vs ENABLE(ASSERT) checks.

These changes are prerequisites for the dump-on-DCHECK experiment.

BUG=596231
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[commit-bot: I haz the power, 2016-08-29 03:13:46]

Committed patchset #1 (id:40001)

[commit-bot: I haz the power, 2016-08-29 03:16:53]

Description was changed from

==========
Clean up some assertion code in WebKit.

- Redefine DCHECK_AT to always reference line and file; the compiler
  should still optimize them out if !DCHECK_IS_ON().
- Fix inconsistent use of DCHECK_IS_ON() vs ENABLE(ASSERT) checks.

These changes are prerequisites for the dump-on-DCHECK experiment.

BUG=596231
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Clean up some assertion code in WebKit.

- Redefine DCHECK_AT to always reference line and file; the compiler
  should still optimize them out if !DCHECK_IS_ON().
- Fix inconsistent use of DCHECK_IS_ON() vs ENABLE(ASSERT) checks.

These changes are prerequisites for the dump-on-DCHECK experiment.

BUG=596231
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/67a60716623af6f7e4571a206ce0df2e05e904f0
Cr-Commit-Position: refs/heads/master@{#414968}
==========

[commit-bot: I haz the power, 2016-08-29 03:16:54]

Patchset 1 (id:??) landed as
https://crrev.com/67a60716623af6f7e4571a206ce0df2e05e904f0
Cr-Commit-Position: refs/heads/master@{#414968}