Cleanup: Address some of the todos in net_log.h

 - Get rid of the AddString() and AddStringLiteral() methods.
 - Make EventParameters able to serialize to JSON, instead of a string.

BUG=37421

Review URL: http://codereview.chromium.org/1716007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@45750 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 42 insertions, 42 deletions

diff --git a/net/proxy/init_proxy_resolver.cc b/net/proxy/init_proxy_resolver.cc
index 0ca39d4..e4bd51b 100644
--- a/net/proxy/init_proxy_resolver.cc
+++ b/net/proxy/init_proxy_resolver.cc
@@ -122,17 +122,15 @@ void InitProxyResolver::DoCallback(int result) {
 int InitProxyResolver::DoFetchPacScript() {
   DCHECK(resolver_->expects_pac_bytes());
 
-  net_log_.BeginEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT);
-
   next_state_ = STATE_FETCH_PAC_SCRIPT_COMPLETE;
 
   const GURL& pac_url = current_pac_url();
 
-  net_log_.AddString(pac_url.spec());
+  net_log_.BeginEventWithString(
+      NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT, "url", pac_url.spec());
 
   if (!proxy_script_fetcher_) {
-    net_log_.AddStringLiteral(
-        "Can't download PAC script, because no fetcher was specified");
+    net_log_.AddEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_HAS_NO_FETCHER);
     return ERR_UNEXPECTED;
   }
 
@@ -142,15 +140,14 @@ int InitProxyResolver::DoFetchPacScript() {
 int InitProxyResolver::DoFetchPacScriptComplete(int result) {
   DCHECK(resolver_->expects_pac_bytes());
 
-  net_log_.AddString(StringPrintf(
-          "Completed fetch with result %s. Received %" PRIuS " bytes",
-          ErrorToString(result),
-          pac_bytes_.size()));
-
-  net_log_.EndEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT);
-
-  if (result != OK)
+  if (result == OK) {
+    net_log_.EndEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT);
+  } else {
+    net_log_.EndEventWithInteger(
+        NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT,
+        "net_error", result);
     return TryToFallbackPacUrl(result);
+  }
 
   next_state_ = STATE_SET_PAC_SCRIPT;
   return result;
@@ -170,15 +167,12 @@ int InitProxyResolver::DoSetPacScript() {
 
 int InitProxyResolver::DoSetPacScriptComplete(int result) {
   if (result != OK) {
-    net_log_.AddString(
-        StringPrintf("Failed initializing the PAC script with error: %s",
-                     ErrorToString(result)));
-    net_log_.EndEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT);
+    net_log_.EndEventWithInteger(
+        NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT,
+        "net_error", result);
     return TryToFallbackPacUrl(result);
   }
 
-  net_log_.AddStringLiteral("Successfully initialized PAC script.");
-
   net_log_.EndEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT);
   return result;
 }
@@ -194,7 +188,8 @@ int InitProxyResolver::TryToFallbackPacUrl(int error) {
   // Advance to next URL in our list.
   ++current_pac_url_index_;
 
-  net_log_.AddStringLiteral("Falling back to next PAC URL...");
+  net_log_.AddEvent(
+      NetLog::TYPE_INIT_PROXY_RESOLVER_FALLING_BACK_TO_NEXT_PAC_URL);
 
   next_state_ = GetStartState();
 
diff --git a/net/proxy/init_proxy_resolver_unittest.cc b/net/proxy/init_proxy_resolver_unittest.cc
index d6ca4af..234e5b0 100644
--- a/net/proxy/init_proxy_resolver_unittest.cc
+++ b/net/proxy/init_proxy_resolver_unittest.cc
@@ -177,19 +177,19 @@ TEST(InitProxyResolverTest, CustomPacSucceeds) {
   EXPECT_EQ(rule.bytes(), resolver.pac_bytes());
 
   // Check the NetLog was filled correctly.
-  EXPECT_EQ(9u, log.entries().size());
+  EXPECT_EQ(6u, log.entries().size());
   EXPECT_TRUE(LogContainsBeginEvent(
       log.entries(), 0, NetLog::TYPE_INIT_PROXY_RESOLVER));
   EXPECT_TRUE(LogContainsBeginEvent(
       log.entries(), 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsEndEvent(
-      log.entries(), 4, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
+      log.entries(), 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsBeginEvent(
-      log.entries(), 5, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
+      log.entries(), 3, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsEndEvent(
-      log.entries(), 7, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
+      log.entries(), 4, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsEndEvent(
-      log.entries(), 8, NetLog::TYPE_INIT_PROXY_RESOLVER));
+      log.entries(), 5, NetLog::TYPE_INIT_PROXY_RESOLVER));
 }
 
 // Fail downloading the custom PAC script.
@@ -210,15 +210,15 @@ TEST(InitProxyResolverTest, CustomPacFails1) {
   EXPECT_EQ("", resolver.pac_bytes());
 
   // Check the NetLog was filled correctly.
-  EXPECT_EQ(6u, log.entries().size());
+  EXPECT_EQ(4u, log.entries().size());
   EXPECT_TRUE(LogContainsBeginEvent(
       log.entries(), 0, NetLog::TYPE_INIT_PROXY_RESOLVER));
   EXPECT_TRUE(LogContainsBeginEvent(
       log.entries(), 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsEndEvent(
-      log.entries(), 4, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
+      log.entries(), 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsEndEvent(
-      log.entries(), 5, NetLog::TYPE_INIT_PROXY_RESOLVER));
+      log.entries(), 3, NetLog::TYPE_INIT_PROXY_RESOLVER));
 }
 
 // Fail parsing the custom PAC script.
@@ -310,27 +310,31 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess2) {
   // Check the NetLog was filled correctly.
   // (Note that the Fetch and Set states are repeated since both WPAD and custom
   // PAC scripts are tried).
-  EXPECT_EQ(17u, log.entries().size());
+  EXPECT_EQ(11u, log.entries().size());
   EXPECT_TRUE(LogContainsBeginEvent(
       log.entries(), 0, NetLog::TYPE_INIT_PROXY_RESOLVER));
   EXPECT_TRUE(LogContainsBeginEvent(
       log.entries(), 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsEndEvent(
-      log.entries(), 4, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
+      log.entries(), 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsBeginEvent(
-      log.entries(), 5, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
+      log.entries(), 3, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsEndEvent(
-      log.entries(), 7, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
+      log.entries(), 4, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
+  EXPECT_TRUE(LogContainsEvent(
+      log.entries(), 5,
+      NetLog::TYPE_INIT_PROXY_RESOLVER_FALLING_BACK_TO_NEXT_PAC_URL,
+      NetLog::PHASE_NONE));
   EXPECT_TRUE(LogContainsBeginEvent(
-      log.entries(), 9, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
+      log.entries(), 6, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsEndEvent(
-      log.entries(), 12, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
+      log.entries(), 7, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsBeginEvent(
-      log.entries(), 13, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
+      log.entries(), 8, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsEndEvent(
-      log.entries(), 15, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
+      log.entries(), 9, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT));
   EXPECT_TRUE(LogContainsEndEvent(
-      log.entries(), 16, NetLog::TYPE_INIT_PROXY_RESOLVER));
+      log.entries(), 10, NetLog::TYPE_INIT_PROXY_RESOLVER));
 }
 
 // Fails at WPAD (downloading), and fails at custom PAC (downloading).
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index e3778b9..588b240 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -469,14 +469,15 @@ int ProxyService::DidFinishResolvingProxy(ProxyInfo* result,
   if (result_code == OK) {
     // When full logging is enabled, dump the proxy list.
     if (net_log.HasListener()) {
-      net_log.AddString(
-          std::string("Resolved proxy list: ") + result->ToPacString());
+      net_log.AddEventWithString(
+          NetLog::TYPE_PROXY_SERVICE_RESOLVED_PROXY_LIST,
+          "pac_string", result->ToPacString());
     }
     result->DeprioritizeBadProxies(proxy_retry_info_);
   } else {
-    net_log.AddString(StringPrintf(
-        "Got an error from proxy resolver (%d), falling-back to DIRECT.",
-        result_code));
+    net_log.AddEventWithInteger(
+        NetLog::TYPE_PROXY_SERVICE_RESOLVED_PROXY_LIST,
+        "net_error", result_code);
 
     // Fall-back to direct when the proxy resolver fails. This corresponds
     // with a javascript runtime error in the PAC script.