Fixed bug where CHECKs don't fire if min_log_level > FATAL

Fixed bug where CHECKs don't fire if min_log_level > FATAL

BUG=61510
TEST=New unittests in base_unittests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=64883

[akalin, 2010-11-01 23:35:48]

+brettw for review

[brettw, 2010-11-03 00:35:43]

http://codereview.chromium.org/4262001/diff/1/3
File base/logging.h (right):

http://codereview.chromium.org/4262001/diff/1/3#newcode606
base/logging.h:606: const LogSeverity LOG_DCHECK = LOG_INFO;
I don't understand what this line does. I've stared at this for a while and
can't get my head around all the different conditions. Can you also comment
above this block what case this is handling?

[akalin, 2010-11-03 00:46:11]

On 2010/11/03 00:35:43, brettw wrote:
> http://codereview.chromium.org/4262001/diff/1/3
> File base/logging.h (right):
> 
> http://codereview.chromium.org/4262001/diff/1/3#newcode606
> base/logging.h:606: const LogSeverity LOG_DCHECK = LOG_INFO;
> I don't understand what this line does. I've stared at this for a while and
> can't get my head around all the different conditions. Can you also comment
> above this block what case this is handling?

Those are just dummy values since DCHECK_IS_ON() is set to false; those values
are unused, so I figured I'd set them to something "safe".

Added comment.

Please take another look.

[brettw, 2010-11-03 01:06:15]

LGTM

[akalin, 2010-11-03 02:34:05]

Fixed ipc_test that was relying on the old behavior.

Checking in as soon as trybots pass.

On 2010/11/03 01:06:15, brettw wrote:
> LGTM

[Timur Iskhodzhanov, 2010-11-03 12:57:07]

FYI, (regarding ipc/ipc_sync_channel_unittest.cc)

Last time I've tried temporary hot-swapping of the LogMessageHandler I was
"unLGTMed":
http://codereview.chromium.org/3394013/diff/9001/10001
+ see review comments from Pawel.

Also, this has introduced a data race report on TSan/Mac since
LOG/SetLogMessageHandler can be called on different threads. Please watch the
memory waterfall status after commiting.

[hansl_g, 2010-11-03 17:29:41]

There is a testing::LogDisabler that is broken with this change. We use it a lot
in CEEE.

I'm fixing it right now, but you might want to use it instead of setting
manually the LogAssertHandler in ipc_sync_channel_unittest.cc

On 2010/11/03 12:57:07, Timur Iskhodzhanov wrote:
> FYI, (regarding ipc/ipc_sync_channel_unittest.cc)
> 
> Last time I've tried temporary hot-swapping of the LogMessageHandler I was
> "unLGTMed":
> http://codereview.chromium.org/3394013/diff/9001/10001
> + see review comments from Pawel.
> 
> Also, this has introduced a data race report on TSan/Mac since
> LOG/SetLogMessageHandler can be called on different threads. Please watch the
> memory waterfall status after commiting.

[hansl, 2010-11-03 17:30:37]

Sorry I didn't use the right account :-(

On 2010/11/03 17:29:41, hansl_g wrote:
> There is a testing::LogDisabler that is broken with this change. We use it a
lot
> in CEEE.
> 
> I'm fixing it right now, but you might want to use it instead of setting
> manually the LogAssertHandler in ipc_sync_channel_unittest.cc
> 
> On 2010/11/03 12:57:07, Timur Iskhodzhanov wrote:
> > FYI, (regarding ipc/ipc_sync_channel_unittest.cc)
> > 
> > Last time I've tried temporary hot-swapping of the LogMessageHandler I was
> > "unLGTMed":
> > http://codereview.chromium.org/3394013/diff/9001/10001
> > + see review comments from Pawel.
> > 
> > Also, this has introduced a data race report on TSan/Mac since
> > LOG/SetLogMessageHandler can be called on different threads. Please watch
the
> > memory waterfall status after commiting.