Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(7)

Issue 2280003003: Clean up some assertion code in WebKit. (Closed)

Created:
1 year, 2 months ago by Wez
Modified:
1 year, 2 months ago
Reviewers:
tkent, Stephen White
CC:
tkent, ajuma+watch_chromium.org, blink-reviews, blink-reviews-platform-graphics_chromium.org, blink-reviews-wtf_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, f(malita), jbroman, Justin Novosad, Mikhail, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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}

Patch Set 1 : Try to fix patchset deps again #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -10 lines) Patch
M third_party/WebKit/Source/platform/graphics/GraphicsContext.h View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp View 4 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/wtf/Assertions.h View 3 chunks +2 lines, -3 lines 2 comments Download
Trybot results:  linux_chromium_asan_rel_ng   linux_android_rel_ng   mac_chromium_rel_ng   chromium_presubmit   android_compile_dbg   linux_android_rel_ng   win_clang   win_chromium_compile_dbg_ng   win_chromium_x64_rel_ng   win_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-device   mac_chromium_rel_ng   ios-simulator   linux_android_rel_ng   android_clang_dbg_recipe   cast_shell_android   android_arm64_dbg_recipe   android_compile_dbg   linux_chromium_chromeos_compile_dbg_ng   linux_chromium_asan_rel_ng   linux_chromium_chromeos_rel_ng   chromeos_x86-generic_chromium_compile_only_ng   chromium_presubmit   linux_chromium_compile_dbg_ng   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_linux   linux_chromium_rel_ng   chromeos_daisy_chromium_compile_only_ng   linux_layout_tests_slimming_paint_v2   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_clobber_rel_ng   chromium_presubmit   chromium_presubmit   win_clang   win_chromium_compile_dbg_ng   win_chromium_x64_rel_ng   win_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-device   ios-simulator   mac_chromium_rel_ng   linux_android_rel_ng   android_clang_dbg_recipe   cast_shell_android   android_arm64_dbg_recipe   android_compile_dbg   linux_chromium_chromeos_compile_dbg_ng   linux_chromium_asan_rel_ng   linux_chromium_chromeos_rel_ng   chromeos_x86-generic_chromium_compile_only_ng   chromium_presubmit   linux_chromium_compile_dbg_ng   chromeos_amd64-generic_chromium_compile_only_ng   linux_chromium_rel_ng   cast_shell_linux   chromeos_daisy_chromium_compile_only_ng   linux_chromium_chromeos_ozone_rel_ng   linux_layout_tests_slimming_paint_v2   linux_chromium_clobber_rel_ng 

Dependent Patchsets:

Messages

Total messages: 38 (25 generated)
Wez
PTAL - thanks!
1 year, 2 months ago (2016-08-27 20:12:54 UTC) #13
Stephen White
+tkent FYI https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Source/wtf/Assertions.h File third_party/WebKit/Source/wtf/Assertions.h (right): https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Source/wtf/Assertions.h#newcode189 third_party/WebKit/Source/wtf/Assertions.h:189: #define DCHECK_AT(assertion, file, line) LAZY_STREAM(logging::LogMessage(file, line, #assertion).stream(), ...
1 year, 2 months ago (2016-08-27 23:20:21 UTC) #19
Wez
https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Source/wtf/Assertions.h File third_party/WebKit/Source/wtf/Assertions.h (right): https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Source/wtf/Assertions.h#newcode189 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() ? ...
1 year, 2 months ago (2016-08-28 01:11:36 UTC) #20
Stephen White
On 2016/08/28 01:11:36, Wez wrote: > https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Source/wtf/Assertions.h > File third_party/WebKit/Source/wtf/Assertions.h (right): > > https://codereview.chromium.org/2280003003/diff/40001/third_party/WebKit/Source/wtf/Assertions.h#newcode189 > ...
1 year, 2 months ago (2016-08-28 01:32:40 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2280003003/40001
1 year, 2 months ago (2016-08-28 04:31:03 UTC) #23
commit-bot: I haz the power
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_presubmit/builds/247847)
1 year, 2 months ago (2016-08-28 04:36:33 UTC) #25
Wez
+tkent for Assertions.h OWNERS
1 year, 2 months ago (2016-08-28 05:24:58 UTC) #27
tkent
lgtm
1 year, 2 months ago (2016-08-29 00:09:09 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2280003003/40001
1 year, 2 months ago (2016-08-29 00:09:23 UTC) #30
commit-bot: I haz the power
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_ng/builds/286262)
1 year, 2 months ago (2016-08-29 01:13:58 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2280003003/40001
1 year, 2 months ago (2016-08-29 01:16:58 UTC) #34
commit-bot: I haz the power
Committed patchset #1 (id:40001)
1 year, 2 months ago (2016-08-29 03:13:46 UTC) #36
commit-bot: I haz the power
1 year, 2 months ago (2016-08-29 03:16:54 UTC) #38
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/67a60716623af6f7e4571a206ce0df2e05e904f0
Cr-Commit-Position: refs/heads/master@{#414968}

Powered by Google App Engine
This is Rietveld efc10ee0f