C++ readability review from original change https://chromiumcodereview.appspot.com/14263024/

chrome/test/chromedriver/chrome/console_logger.cc

Index: chrome/test/chromedriver/chrome/console_logger.cc
diff --git a/chrome/test/chromedriver/chrome/console_logger.cc b/chrome/test/chromedriver/chrome/console_logger.cc
index 71b1a798c3dd69cd87270ebc28b42b914b21c350..f6d9decc2452187501f462d9d2b689cc8c8c1f2e 100644
--- a/chrome/test/chromedriver/chrome/console_logger.cc
+++ b/chrome/test/chromedriver/chrome/console_logger.cc
@@ -4,10 +4,9 @@
 
 #include "chrome/test/chromedriver/chrome/console_logger.h"
 
-#include <sstream>
-
 #include "base/json/json_writer.h"
-#include "base/string_util.h"
+#include "base/logging.h"
+#include "base/stringprintf.h"
 #include "base/values.h"
 #include "chrome/test/chromedriver/chrome/devtools_client.h"
 #include "chrome/test/chromedriver/chrome/log.h"
@@ -15,8 +14,9 @@
 
 namespace {
 
+// Translates Console.messageAdded.message.level into Log::Level.
 bool ConsoleLevelToLogLevel(const std::string& name, Log::Level *out_level) {
-  const char* kConsoleLevelNames[] = {
+  const char* const kConsoleLevelNames[] = {
     "debug", "log", "warning", "error"
   };
 
@@ -44,48 +44,47 @@ void ConsoleLogger::OnEvent(
     DevToolsClient* client,
     const std::string& method,
     const base::DictionaryValue& params) {
-  if (!StartsWithASCII(method, "Console.messageAdded", true))
+  if (method != "Console.messageAdded")
     return;
 
   // If the event has proper structure and fields, log formatted.
   // Else it's a weird message that we don't know how to format, log full JSON.
   const base::DictionaryValue *message_dict = NULL;
   if (params.GetDictionary("message", &message_dict)) {
-    std::ostringstream message;
-    std::string origin;
-    if (message_dict->GetString("url", &origin) && !origin.empty()) {
-      message << origin;
-    } else if (message_dict->GetString("source", &origin) && !origin.empty()) {
-      message << origin;
-    } else {
-      message << "unknown";
-    }
+    std::string text;
+    std::string level_name;
+    Log::Level level = Log::kLog;
+    if (message_dict->GetString("text", &text) && !text.empty() &&
+        message_dict->GetString("level", &level_name) &&
+        ConsoleLevelToLogLevel(level_name, &level)) {
 
-    int line = -1;
-    if (message_dict->GetInteger("line", &line)) {
-      message << " " << line;
-      int column = -1;
-      if (message_dict->GetInteger("column", &column)) {
-        message << ":" << column;
+      const char* origin_cstr = "unknown";
+      std::string origin;
+      if ((message_dict->GetString("url", &origin) && !origin.empty()) ||
+          (message_dict->GetString("source", &origin) && !origin.empty())) {
+        origin_cstr = origin.c_str();
       }
-    } else {
-      // No line number, but print anyway, just to maintain the number of
-      // fields in the formatted message in case someone wants to parse it.
-      message << " -";
-    }
 
-    std::string text;
-    if (message_dict->GetString("text", &text)) {
-      message << " " << text;
-
-      std::string level_name;
-      Log::Level level = Log::kLog;
-      if (message_dict->GetString("level", &level_name)) {
-        if (ConsoleLevelToLogLevel(level_name, &level)) {
-          log_->AddEntry(level, message.str());  // Found all expected fields.
-          return;
+      std::string line_column;
+      int line = -1;
+      if (message_dict->GetInteger("line", &line)) {
+        int column = -1;
+        if (message_dict->GetInteger("column", &column)) {
+          base::SStringPrintf(&line_column, "%d:%d", line, column);
+        } else {
+          base::SStringPrintf(&line_column, "%d", line);
         }
+      } else {
+        // No line number, but print anyway, just to maintain the number of
+        // fields in the formatted message in case someone wants to parse it.
+        line_column = "-";
       }
+
+      log_->AddEntry(level, base::StringPrintf("%s %s %s",
+                                               origin_cstr,
+                                               line_column.c_str(),
+                                               text.c_str()));
+      return;
     }
   }