Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(70)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 189603007: Let DCHECK in non-official-release build be opt-in with dcheck_always_on=1 only (Closed)

Created:
6 years, 9 months ago by Xianzhu
Modified:
6 years, 9 months ago
Reviewers:
Nico, brettw
CC:
chromium-reviews, viettrungluu+watch_chromium.org, nasko+codewatch_chromium.org, ben+mojo_chromium.org, craigdh+watch_chromium.org, jam, abarth-chromium, joi+watch-content_chromium.org, darin-cc_chromium.org, yfriedman+watch_chromium.org, erikwright+watch_chromium.org, creis+watch_chromium.org, fischman+watch_chromium.org, feature-media-reviews_chromium.org, bulach+watch_chromium.org, ilevy-cc_chromium.org, piman+watch_chromium.org, darin (slow to review), mcasas+watch_chromium.org, Aaron Boodman, robertshield, klundberg+watch_chromium.org, wjia+watch_chromium.org, sehr (please use chromium)
Visibility:
Public.

Description

Let DCHECK in non-official-release build be opt-in with dcheck_always_on=1 only - Remove DCHECK in non-official-release build by default - Gyp variable dcheck_always_on=1 (existing) forces to enable DCHECK in release build - Remove flag --enable-dcheck Other effects/notes: - Now allow "buildtype=Official dcheck_always_on=1" (which will enable DCHECK in official build) combination. - Gyp variable logging_like_official_build no longer has an effect - Leave DCHECK_IS_ON() unchanged. May deal with it in a later change if needed. This won't affect bots which use dcheck_always_on=1. BUG=350462 TEST=LoggingTest.Dcheck R=thakis@chromium.org TBR=darin,sehr (command line changes in components/nacl and mojo) Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255987

Patch Set 1 : #

Total comments: 9

Patch Set 2 : #

Total comments: 2

Patch Set 3 : For landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -165 lines) Patch
M base/base_switches.h View 1 chunk +0 lines, -1 line 0 comments Download
M base/base_switches.cc View 1 chunk +0 lines, -3 lines 0 comments Download
M base/i18n/streaming_utf8_validator.cc View 1 1 chunk +1 line, -4 lines 0 comments Download
M base/logging.h View 7 chunks +8 lines, -68 lines 0 comments Download
M base/logging.cc View 2 chunks +1 line, -9 lines 0 comments Download
M base/logging_unittest.cc View 1 3 chunks +18 lines, -24 lines 0 comments Download
M build/common.gypi View 3 chunks +1 line, -11 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/common/logging_chrome.cc View 3 chunks +1 line, -14 lines 0 comments Download
M chrome/test/automation/proxy_launcher.h View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/test/automation/proxy_launcher.cc View 2 chunks +0 lines, -4 lines 0 comments Download
M components/nacl/common/nacl_cmd_line.cc View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M content/app/android/library_loader_hooks.cc View 1 chunk +0 lines, -7 lines 0 comments Download
M content/browser/browser_child_process_host_impl.cc View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 chunk +0 lines, -1 line 0 comments Download
M content/common/gpu/media/vaapi_h264_decoder_unittest.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M content/common/gpu/media/video_decode_accelerator_unittest.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M content/common/gpu/media/video_encode_accelerator_unittest.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M mojo/shell/init.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M tools/android/forwarder2/daemon.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M tools/set_default_handler/set_default_handler_main.cc View 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 14 (0 generated)
Xianzhu
https://codereview.chromium.org/189603007/diff/60001/base/i18n/streaming_utf8_validator.cc File base/i18n/streaming_utf8_validator.cc (right): https://codereview.chromium.org/189603007/diff/60001/base/i18n/streaming_utf8_validator.cc#newcode19 base/i18n/streaming_utf8_validator.cc:19: if (logging::DEBUG_MODE) I'm not sure about this. The original ...
6 years, 9 months ago (2014-03-07 22:40:38 UTC) #1
Nico
https://codereview.chromium.org/189603007/diff/60001/base/i18n/streaming_utf8_validator.cc File base/i18n/streaming_utf8_validator.cc (right): https://codereview.chromium.org/189603007/diff/60001/base/i18n/streaming_utf8_validator.cc#newcode19 base/i18n/streaming_utf8_validator.cc:19: if (logging::DEBUG_MODE) On 2014/03/07 22:40:38, Xianzhu wrote: > I'm ...
6 years, 9 months ago (2014-03-07 22:58:35 UTC) #2
Xianzhu
https://codereview.chromium.org/189603007/diff/60001/base/i18n/streaming_utf8_validator.cc File base/i18n/streaming_utf8_validator.cc (right): https://codereview.chromium.org/189603007/diff/60001/base/i18n/streaming_utf8_validator.cc#newcode19 base/i18n/streaming_utf8_validator.cc:19: if (logging::DEBUG_MODE) On 2014/03/07 22:58:35, Nico wrote: > On ...
6 years, 9 months ago (2014-03-07 23:29:36 UTC) #3
Nico
lgtm https://codereview.chromium.org/189603007/diff/10019/components/nacl/common/nacl_cmd_line.cc File components/nacl/common/nacl_cmd_line.cc (right): https://codereview.chromium.org/189603007/diff/10019/components/nacl/common/nacl_cmd_line.cc#newcode25 components/nacl/common/nacl_cmd_line.cc:25: switches::kNoErrorDialogs, alphabetize (i.e. put it back in the ...
6 years, 9 months ago (2014-03-08 00:11:06 UTC) #4
Xianzhu
https://codereview.chromium.org/189603007/diff/10019/components/nacl/common/nacl_cmd_line.cc File components/nacl/common/nacl_cmd_line.cc (right): https://codereview.chromium.org/189603007/diff/10019/components/nacl/common/nacl_cmd_line.cc#newcode25 components/nacl/common/nacl_cmd_line.cc:25: switches::kNoErrorDialogs, On 2014/03/08 00:11:06, Nico wrote: > alphabetize (i.e. ...
6 years, 9 months ago (2014-03-08 00:16:40 UTC) #5
Xianzhu
The CQ bit was checked by wangxianzhu@chromium.org
6 years, 9 months ago (2014-03-08 00:18:12 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/189603007/90001
6 years, 9 months ago (2014-03-08 00:19:45 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/189603007/90001
6 years, 9 months ago (2014-03-08 11:13:06 UTC) #8
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-08 14:47:18 UTC) #9
commit-bot: I haz the power
Retried try job too often on mac_rel for step(s) telemetry_perf_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&number=234380
6 years, 9 months ago (2014-03-08 14:47:19 UTC) #10
Xianzhu
The CQ bit was checked by wangxianzhu@chromium.org
6 years, 9 months ago (2014-03-10 16:55:53 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/189603007/90001
6 years, 9 months ago (2014-03-10 17:06:22 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/189603007/90001
6 years, 9 months ago (2014-03-10 17:43:31 UTC) #13
commit-bot: I haz the power
6 years, 9 months ago (2014-03-10 18:23:50 UTC) #14
Message was sent while issue was closed. 
Change committed as 255987

Powered by Google App Engine
This is Rietveld 408576698