Mojo: Add basic logging facilities to environment.

Mojo: Add basic logging facilities to environment.

I'll add nicer logging macros separately.

Also to do: We should also build mojo_public_environment_unittests
against mojo_environment_chromium (to test the "chromium"
implementation). I tried this manually (on a shared library build), and
the LoggerTests pass but the AsyncWaiterTests blow up.

R=darin@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277189

[darin (slow to review), 2014-06-14 06:08:24]

LGTM w/ some suggestions:

https://codereview.chromium.org/330933004/diff/1/mojo/environment/default_log...
File mojo/environment/default_logger_impl.cc (right):

https://codereview.chromium.org/330933004/diff/1/mojo/environment/default_log...
mojo/environment/default_logger_impl.cc:31: logging::LogMessage(__FILE__,
__LINE__,
it seems like MojoLogger::LogMessage should probably take file and lineno
parameters.

https://codereview.chromium.org/330933004/diff/1/mojo/public/c/environment/lo...
File mojo/public/c/environment/logger.h (right):

https://codereview.chromium.org/330933004/diff/1/mojo/public/c/environment/lo...
mojo/public/c/environment/logger.h:22: #define MOJO_LOG_LEVEL_INFO
((MojoLogLevel) 0)
MOJO_LOG_LEVEL_VERBOSE needed for the C case too?

https://codereview.chromium.org/330933004/diff/1/mojo/public/cpp/environment/...
File mojo/public/cpp/environment/lib/default_logger.cc (right):

https://codereview.chromium.org/330933004/diff/1/mojo/public/cpp/environment/...
mojo/public/cpp/environment/lib/default_logger.cc:32: fprintf(stderr, "%s:%s\n",
GetLogLevelString(log_level), message);
nit: it might be nice to add a space after the colon for readability.

ERROR: Foo
WARNING: Bar

[viettrungluu, 2014-06-14 07:24:38]

Committed patchset #1 manually as r277189.

[darin (slow to review), 2014-06-17 00:00:55]

On 2014/06/14 at 07:24:38, viettrungluu wrote:
> Committed patchset #1 manually as r277189.

Looks like what was committed didn't reflect any of the suggestions from above.
Was that intended? Maybe the MOJO_LOG_LEVEL_VERBOSE #define is needed at
minimum.

[viettrungluu, 2014-06-17 00:26:06]

On 2014/06/17 00:00:55, darin wrote:
> On 2014/06/14 at 07:24:38, viettrungluu wrote:
> > Committed patchset #1 manually as r277189.
> 
> Looks like what was committed didn't reflect any of the suggestions from
above.
> Was that intended? Maybe the MOJO_LOG_LEVEL_VERBOSE #define is needed at
> minimum.

Gah, I totally didn't see your comments. Sorry about that. Replies to what you
wrote:

1) I don't want to add file/line number parameters because I think this should
be a potential frontend to a general-purpose logging facility (for which
file/lineno often don't make sense); e.g., at some point, it'd be nice to be
able to hook this logging library through to a Mojo logging service.

So in a sense it's the "lowest-level" thing needed for a logger. It should be
more JS's console.log or Android's android.util.Log. Having to plumb through an
extra string + int is a bit odd. (My plan is to have the logging headers, which
define macros, etc., deal with the file/lineno. This is, admittedly,
inconvenient for plumbing through to Chromium's logging facility.)

2) Probably, I should remove MOJO_LOG_LEVEL_VERBOSE.... (The Chromium-like thing
to do is to access them via VLOG, etc. You can't do, e.g., LOG(VERBOSE), so I
won't need the constants/defines for token pasting purposes.)

3) I'll add the space.

[viettrungluu, 2014-06-17 16:47:11]

On 2014/06/17 00:26:06, viettrungluu wrote:
> On 2014/06/17 00:00:55, darin wrote:
> > On 2014/06/14 at 07:24:38, viettrungluu wrote:
> > > Committed patchset #1 manually as r277189.
> > 
> > Looks like what was committed didn't reflect any of the suggestions from
> above.
> > Was that intended? Maybe the MOJO_LOG_LEVEL_VERBOSE #define is needed at
> > minimum.
> 
> Gah, I totally didn't see your comments. Sorry about that. Replies to what you
> wrote:
> 
> 1) I don't want to add file/line number parameters because I think this should
> be a potential frontend to a general-purpose logging facility (for which
> file/lineno often don't make sense); e.g., at some point, it'd be nice to be
> able to hook this logging library through to a Mojo logging service.
> 
> So in a sense it's the "lowest-level" thing needed for a logger. It should be
> more JS's console.log or Android's android.util.Log. Having to plumb through
an
> extra string + int is a bit odd. (My plan is to have the logging headers,
which
> define macros, etc., deal with the file/lineno. This is, admittedly,
> inconvenient for plumbing through to Chromium's logging facility.)
> 
> 2) Probably, I should remove MOJO_LOG_LEVEL_VERBOSE.... (The Chromium-like
thing
> to do is to access them via VLOG, etc. You can't do, e.g., LOG(VERBOSE), so I
> won't need the constants/defines for token pasting purposes.)

Ha, I'm totally wrong. base/logging.h *does* have "verbose". I guess I should
add it to the C side then.

> 
> 3) I'll add the space.