Mojo Log service and a thread-safe client library.

services/log/log_impl.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "services/log/log_impl.h"
+
+#include <stdio.h>
+
+#include "mojo/public/cpp/application/application_connection.h"
+#include "mojo/public/cpp/bindings/interface_request.h"

[viettrungluu, 2015/11/20 23:21:51]

Probably don't need this include (already included by log_impl.h).

[vardhan, 2015/12/02 00:06:14]

Done.

+#include "mojo/public/cpp/system/functions.h"
+#include "mojo/services/log/interfaces/entry.mojom.h"
+
+namespace mojo {
+namespace log {
+
+namespace {

[viettrungluu, 2015/11/20 23:21:51]

blank line after

[vardhan, 2015/12/02 00:06:14]

Done.

+std::string LogLevelToString(int32_t log_level) {
+  if (log_level <= kLogLevelVerbose - 3)
+    return "VERBOSE4+";
+  switch (log_level) {
+    case kLogLevelVerbose - 2:
+      return "VERBOSE3";
+    case kLogLevelVerbose - 1:
+      return "VERBOSE2";
+    case kLogLevelVerbose:
+      return "VERBOSE1";
+    case kLogLevelInfo:
+      return "INFO";
+    case kLogLevelWarning:
+      return "WARNING";
+    case kLogLevelError:
+      return "ERROR";
+  }
+  return "FATAL";
+}
+}  // namespace

[viettrungluu, 2015/11/20 23:21:51]

blank line before

[vardhan, 2015/12/02 00:06:14]

Done.

+
+LogImpl::LogImpl(const std::string& remote_url, InterfaceRequest<Log> request)
+    : remote_url_(remote_url), binding_(this, request.Pass()) {}

[viettrungluu, 2015/11/20 23:21:51]

nit: should probably use std::move instead of .Pass() now (don't forget to
include <utility>)

[vardhan, 2015/12/02 00:06:14]

Done.

+
+LogImpl::~LogImpl() {}
+
+// static
+void LogImpl::Create(ApplicationConnection* connection,
+                     InterfaceRequest<Log> request) {
+  MOJO_DCHECK(connection);

[viettrungluu, 2015/11/20 23:21:51]

Probably need to include some header for MOJO_DCHECK, etc.

(Note that since this is a service impl, it's actually OK to use //base, in
particular plain DCHECK, etc., from base/logging.h.)

[vardhan, 2015/12/02 00:06:14]

Done.  do you recommend using DCHECK or MOJO_DCHECK for things under //services
(unclear to me from your comment)?

[viettrungluu, 2015/12/02 00:08:01]

Probably, if you're using //base anyway, you should just use DCHECK, LOG, etc.

+  const std::string& remote_url = connection->GetRemoteApplicationURL();
+  if (remote_url.empty()) {
+    MOJO_LOG(ERROR) << "No remote URL";
+    return;
+  }
+
+  new LogImpl(remote_url, request.Pass());
+}
+
+void LogImpl::AddEntry(EntryPtr entry) {
+  MOJO_DCHECK(entry);
+
+  // In order to keep LogImpl thread-safe (for the future), we should only print
+  // one thing here (otherwise, it could interleave with other prints).
+  fprintf(stderr, "%s\n", FormatEntry(entry).c_str());
+}
+
+std::string LogImpl::FormatEntry(const EntryPtr& entry) {
+  std::string rv = "<";

[viettrungluu, 2015/11/20 23:21:51]

This would all be more readable if you just used StringPrintf (from
//base/strings/string_printf.h).

[vardhan, 2015/12/02 00:06:14]

Done.

+  rv += remote_url_;
+  rv += "> [";
+  rv += LogLevelToString(entry->log_level);
+  rv += "] ";
+  if (entry->source_file) {
+    rv += entry->source_file.To<std::string>();
+    if (entry->source_line) {
+      char line_str[20];
+      int chars_written = snprintf(line_str, sizeof(line_str), ":%u",
+                                   static_cast<uint32_t>(entry->source_line));
+      MOJO_DCHECK(chars_written > 0);
+      MOJO_DCHECK(chars_written <= static_cast<int>(sizeof(line_str) - 1));
+
+      rv += line_str;
+    }
+    rv += ": ";
+  }
+  rv += entry->message ? entry->message.To<std::string>() : "<no message>";
+  return rv;
+}
+
+}  // namespace log
+}  // namespace mojo