Don't evaluate args to LOG(INFO) and LOG(WARNING) in release builds.

Don't evaluate args to LOG(INFO) and LOG(WARNING) in release builds.

We were using the LAZY_STREAM macro to do this based on --log-level (which
defaults to INFO), but not for LoggingSettings::logging_dest == LOG_NONE.

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

[skobes, 2015-12-03 18:09:27]

Description was changed from

==========
Don't evaluate args to LOG(INFO) and LOG(WARNING) in release builds.

We were using the LAZY_STREAM macro to do this based on --log-level (which
defaults to INFO), but not for LoggingSettings::logging_dest == LOG_NONE.
==========

to

==========
Don't evaluate args to LOG(INFO) and LOG(WARNING) in release builds.

We were using the LAZY_STREAM macro to do this based on --log-level (which
defaults to INFO), but not for LoggingSettings::logging_dest == LOG_NONE.

TESTED="LOG(INFO) << foo()" doesn't call foo
==========

[skobes, 2015-12-03 18:09:49]

Description was changed from

==========
Don't evaluate args to LOG(INFO) and LOG(WARNING) in release builds.

We were using the LAZY_STREAM macro to do this based on --log-level (which
defaults to INFO), but not for LoggingSettings::logging_dest == LOG_NONE.

TESTED="LOG(INFO) << foo()" doesn't call foo
==========

to

==========
Don't evaluate args to LOG(INFO) and LOG(WARNING) in release builds.

We were using the LAZY_STREAM macro to do this based on --log-level (which
defaults to INFO), but not for LoggingSettings::logging_dest == LOG_NONE.
==========

[skobes, 2015-12-03 18:30:48]

Patchset #1 (id:1) has been deleted

[skobes, 2015-12-03 18:35:07]

Patchset #1 (id:20001) has been deleted

[skobes, 2015-12-03 18:45:19]

Patchset #1 (id:40001) has been deleted

[skobes, 2015-12-03 19:44:26]

Patchset #1 (id:60001) has been deleted

[skobes, 2015-12-03 21:14:41]

Patchset #1 (id:80001) has been deleted

[skobes, 2015-12-03 23:29:42]

skobes@chromium.org changed reviewers:
+ brettw@chromium.org

[skobes, 2015-12-03 23:38:25]

Alternatively we could make InitChromeLogging call SetMinLogLevel(2).  But this
seems better.

[brettw, 2015-12-03 23:52:14]

https://codereview.chromium.org/1499693002/diff/100001/base/logging.cc
File base/logging.cc (right):

https://codereview.chromium.org/1499693002/diff/100001/base/logging.cc#newcod...
base/logging.cc:399: severity >= kAlwaysPrintErrorLevel;
Do ew need the kAlwaysPrintErrorLevel check here? If there is no destinatoin and
no message handler, can even LOG_FATAL go anywhere? If this is needed, we should
mention in the comment why.

[skobes, 2015-12-03 23:57:26]

https://codereview.chromium.org/1499693002/diff/100001/base/logging.cc
File base/logging.cc (right):

https://codereview.chromium.org/1499693002/diff/100001/base/logging.cc#newcod...
base/logging.cc:399: severity >= kAlwaysPrintErrorLevel;
On 2015/12/03 23:52:14, brettw wrote:
> Do ew need the kAlwaysPrintErrorLevel check here? If there is no destinatoin
and
> no message handler, can even LOG_FATAL go anywhere? If this is needed, we
should
> mention in the comment why.

Yeah, ~LogMessage writes to stderr if severity_ >= kAlwaysPrintErrorLevel, even
when g_logging_destination is LOG_NONE.

I tried to capture this in the comment... is there another way of wording it
that you'd prefer?

[brettw, 2015-12-04 21:18:06]

On 2015/12/03 23:57:26, skobes wrote:
> Yeah, ~LogMessage writes to stderr if severity_ >= kAlwaysPrintErrorLevel,
even
> when g_logging_destination is LOG_NONE.

^^ This thing you just wrote seems very clear to me in a way that "These are the
cases for which ~LogMessage does something." doesn't

[skobes, 2015-12-04 21:23:08]

On 2015/12/04 21:18:06, brettw wrote:
> On 2015/12/03 23:57:26, skobes wrote:
> > Yeah, ~LogMessage writes to stderr if severity_ >= kAlwaysPrintErrorLevel,
> even
> > when g_logging_destination is LOG_NONE.
> 
> ^^ This thing you just wrote seems very clear to me in a way that "These are
the
> cases for which ~LogMessage does something." doesn't

Done.

[brettw, 2015-12-04 21:24:44]

lgtm

[skobes, 2015-12-04 21:44:18]

The CQ bit was checked by skobes@chromium.org

[commit-bot: I haz the power, 2015-12-04 21:45:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1499693002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1499693002/120001

[commit-bot: I haz the power, 2015-12-05 03:24:31]

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

[commit-bot: I haz the power, 2015-12-05 03:24:32]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[skobes, 2015-12-05 04:30:34]

The CQ bit was checked by skobes@chromium.org

[commit-bot: I haz the power, 2015-12-05 04:33:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1499693002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1499693002/120001

[commit-bot: I haz the power, 2015-12-05 06:35:49]

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

[commit-bot: I haz the power, 2015-12-05 06:35:49]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build URL)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build URL)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)

[skobes, 2015-12-07 16:54:36]

The CQ bit was checked by skobes@chromium.org

[commit-bot: I haz the power, 2015-12-07 16:54:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1499693002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1499693002/120001

[commit-bot: I haz the power, 2015-12-07 20:21:31]

Description was changed from

==========
Don't evaluate args to LOG(INFO) and LOG(WARNING) in release builds.

We were using the LAZY_STREAM macro to do this based on --log-level (which
defaults to INFO), but not for LoggingSettings::logging_dest == LOG_NONE.
==========

to

==========
Don't evaluate args to LOG(INFO) and LOG(WARNING) in release builds.

We were using the LAZY_STREAM macro to do this based on --log-level (which
defaults to INFO), but not for LoggingSettings::logging_dest == LOG_NONE.
==========

[commit-bot: I haz the power, 2015-12-07 20:21:32]

Committed patchset #2 (id:120001)

[commit-bot: I haz the power, 2015-12-07 20:22:19]

Description was changed from

==========
Don't evaluate args to LOG(INFO) and LOG(WARNING) in release builds.

We were using the LAZY_STREAM macro to do this based on --log-level (which
defaults to INFO), but not for LoggingSettings::logging_dest == LOG_NONE.
==========

to

==========
Don't evaluate args to LOG(INFO) and LOG(WARNING) in release builds.

We were using the LAZY_STREAM macro to do this based on --log-level (which
defaults to INFO), but not for LoggingSettings::logging_dest == LOG_NONE.

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

[commit-bot: I haz the power, 2015-12-07 20:22:20]

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