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

chrome/test/chromedriver/chrome/performance_logger_unittest.cc

Index: chrome/test/chromedriver/chrome/performance_logger_unittest.cc
diff --git a/chrome/test/chromedriver/chrome/performance_logger_unittest.cc b/chrome/test/chromedriver/chrome/performance_logger_unittest.cc
index 6206e19e4177469942de66e58042e031a36e9ffb..9e0a37e73a34466ebead6ea0395c3b3c13dd2fe7 100644
--- a/chrome/test/chromedriver/chrome/performance_logger_unittest.cc
+++ b/chrome/test/chromedriver/chrome/performance_logger_unittest.cc
@@ -98,7 +98,7 @@ scoped_ptr<DictionaryValue> ParseDictionary(const std::string& json) {
     ADD_FAILURE();
     return scoped_ptr<DictionaryValue>(NULL);
   }
-  DictionaryValue* dict = 0;
+  DictionaryValue* dict = NULL;
   if (!value->GetAsDictionary(&dict)) {
     SCOPED_TRACE("JSON object is not a dictionary");
     ADD_FAILURE();
@@ -108,28 +108,28 @@ scoped_ptr<DictionaryValue> ParseDictionary(const std::string& json) {
 }
 
 void ValidateLogEntry(LogEntry *entry,
-                      const char* expect_webview,
-                      const char* expect_method) {
+                      const char* expected_webview,
+                      const char* expected_method) {

[jdennett, 2013/05/23 03:44:24]

Optional: In general I'd recommend just passing std::string here, unless you
have some particular reason for wanting to play with pointers.

[klm, 2013/05/23 15:21:39]

Done.

   EXPECT_EQ(Log::kLog, entry->level);
   EXPECT_LT(0, entry->timestamp.ToTimeT());
 
   scoped_ptr<base::DictionaryValue> message(ParseDictionary(entry->message));
   std::string webview;
   EXPECT_TRUE(message->GetString("webview", &webview));
-  EXPECT_STREQ(expect_webview, webview.c_str());
+  EXPECT_EQ(expected_webview, webview);
   std::string method;
   EXPECT_TRUE(message->GetString("message.method", &method));
-  EXPECT_STREQ(expect_method, method.c_str());
+  EXPECT_EQ(expected_method, method);
   DictionaryValue* params;
   EXPECT_TRUE(message->GetDictionary("message.params", &params));
   EXPECT_EQ(0u, params->size());
 }
 
 void ExpectEnableDomains(FakeDevToolsClient& client) {
-  EXPECT_STREQ("Network.enable", client.PopSentCommand().c_str());
-  EXPECT_STREQ("Page.enable", client.PopSentCommand().c_str());
-  EXPECT_STREQ("Timeline.start", client.PopSentCommand().c_str());
-  EXPECT_STREQ("", client.PopSentCommand().c_str());
+  EXPECT_EQ("Network.enable", client.PopSentCommand());
+  EXPECT_EQ("Page.enable", client.PopSentCommand());
+  EXPECT_EQ("Timeline.start", client.PopSentCommand());
+  EXPECT_TRUE(client.PopSentCommand().empty());
 }
 
 }  // namespace
@@ -166,7 +166,7 @@ TEST(PerformanceLogger, TwoWebViews) {
   // OnConnected sends the enable command only to that client, not others.
   client1.ConnectIfNecessary();
   ExpectEnableDomains(client1);
-  EXPECT_STREQ("", client2.PopSentCommand().c_str());
+  EXPECT_TRUE(client2.PopSentCommand().empty());
 
   client1.TriggerEvent("Page.gaga1");
   client2.TriggerEvent("Timeline.gaga2");