Make NetLog take in callbacks that return Values

content/browser/debugger/devtools_netlog_observer.cc

Index: content/browser/debugger/devtools_netlog_observer.cc
===================================================================
--- content/browser/debugger/devtools_netlog_observer.cc	(revision 140753)
+++ content/browser/debugger/devtools_netlog_observer.cc	(working copy)
@@ -11,7 +11,6 @@
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/common/resource_response.h"
 #include "net/base/load_flags.h"
-#include "net/http/http_net_log_params.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_util.h"
 #include "net/url_request/url_request.h"
@@ -38,37 +37,35 @@
   return NULL;
 }
 
-void DevToolsNetLogObserver::OnAddEntry(net::NetLog::EventType type,
-                                        const base::TimeTicks& time,
-                                        const net::NetLog::Source& source,
-                                        net::NetLog::EventPhase phase,
-                                        net::NetLog::EventParameters* params) {
+void DevToolsNetLogObserver::OnAddEntry(const net::NetLog::Entry& entry) {
   // The events that the Observer is interested in only occur on the IO thread.
   if (!BrowserThread::CurrentlyOn(BrowserThread::IO))
     return;
-  if (source.type == net::NetLog::SOURCE_URL_REQUEST)
-    OnAddURLRequestEntry(type, time, source, phase, params);
-  else if (source.type == net::NetLog::SOURCE_HTTP_STREAM_JOB)
-    OnAddHTTPStreamJobEntry(type, time, source, phase, params);
-  else if (source.type == net::NetLog::SOURCE_SOCKET)
-    OnAddSocketEntry(type, time, source, phase, params);
+
+  if (entry.source().type == net::NetLog::SOURCE_URL_REQUEST)
+    OnAddURLRequestEntry(entry);
+  else if (entry.source().type == net::NetLog::SOURCE_HTTP_STREAM_JOB)
+    OnAddHTTPStreamJobEntry(entry);
+  else if (entry.source().type == net::NetLog::SOURCE_SOCKET)
+    OnAddSocketEntry(entry);
 }
 
 void DevToolsNetLogObserver::OnAddURLRequestEntry(
-    net::NetLog::EventType type,
-    const base::TimeTicks& time,
-    const net::NetLog::Source& source,
-    net::NetLog::EventPhase phase,
-    net::NetLog::EventParameters* params) {
+    const net::NetLog::Entry& entry) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
-  bool is_begin = phase == net::NetLog::PHASE_BEGIN;
-  bool is_end = phase == net::NetLog::PHASE_END;
+  bool is_begin = entry.phase() == net::NetLog::PHASE_BEGIN;
+  bool is_end = entry.phase() == net::NetLog::PHASE_END;
 
-  if (type == net::NetLog::TYPE_URL_REQUEST_START_JOB) {
+  if (entry.type() == net::NetLog::TYPE_URL_REQUEST_START_JOB) {
     if (is_begin) {
-      int load_flags = static_cast<
-          net::URLRequestStartEventParameters*>(params)->load_flags();
+      int load_flags;
+      scoped_ptr<Value> event_param(entry.ParametersToValue());
+      if (!net::StartEventLoadFlagsFromEventParams(event_param.get(),
+                                                   &load_flags)) {
+        return;
+      }
+
       if (!(load_flags & net::LOAD_REPORT_RAW_HEADERS))
         return;
 
@@ -78,7 +75,7 @@
         request_to_info_.clear();
       }
 
-      request_to_info_[source.id] = new ResourceInfo();
+      request_to_info_[entry.source().id] = new ResourceInfo();
 
       if (request_to_encoded_data_length_.size() > kMaxNumEntries) {
         LOG(WARNING) << "The encoded data length observer url request count "
@@ -86,48 +83,58 @@
         request_to_encoded_data_length_.clear();
       }
 
-      request_to_encoded_data_length_[source.id] = 0;
+      request_to_encoded_data_length_[entry.source().id] = 0;
     }
     return;
-  } else if (type == net::NetLog::TYPE_REQUEST_ALIVE) {
+  } else if (entry.type() == net::NetLog::TYPE_REQUEST_ALIVE) {
     // Cleanup records based on the TYPE_REQUEST_ALIVE entry.
     if (is_end) {
-      request_to_info_.erase(source.id);
-      request_to_encoded_data_length_.erase(source.id);
+      request_to_info_.erase(entry.source().id);
+      request_to_encoded_data_length_.erase(entry.source().id);
     }
     return;
   }
 
-  ResourceInfo* info = GetResourceInfo(source.id);
+  ResourceInfo* info = GetResourceInfo(entry.source().id);
   if (!info)
     return;
 
-  switch (type) {
+  switch (entry.type()) {
     case net::NetLog::TYPE_HTTP_TRANSACTION_SEND_REQUEST_HEADERS: {
-      net::NetLogHttpRequestParameter* request_parameter =
-          static_cast<net::NetLogHttpRequestParameter*>(params);
-      const net::HttpRequestHeaders &request_headers =
-          request_parameter->GetHeaders();
+      scoped_ptr<Value> event_params(entry.ParametersToValue());
+      std::string request_line;
+      net::HttpRequestHeaders request_headers;
 
+      // This will also clear headers in the case the same url_request is reused
+      // for several http requests (e.g. see http://crbug.com/80157).

[eroman, 2012/06/06 21:27:11]

Is this a new comment or moved from somewhere?

[mmenke, 2012/06/07 19:20:46]

It was moved from below, but then I brought it back below, and forgot to delete
it here.  Removed.

+      if (!net::HttpRequestHeaders::FromNetLogParam(event_params.get(),
+                                                    &request_headers,
+                                                    &request_line)) {
+        NOTREACHED();
+      }
+
       // We need to clear headers in case the same url_request is reused for
       // several http requests (e.g. see http://crbug.com/80157).
       info->request_headers.clear();
 
       for (net::HttpRequestHeaders::Iterator it(request_headers);
            it.GetNext();) {
-        info->request_headers.push_back(std::make_pair(it.name(),
-                                                       it.value()));
+        info->request_headers.push_back(std::make_pair(it.name(), it.value()));
       }
-      info->request_headers_text =
-          request_parameter->GetLine() +
-          request_parameter->GetHeaders().ToString();
+      info->request_headers_text = request_line + request_headers.ToString();
       break;
     }
     case net::NetLog::TYPE_HTTP_TRANSACTION_READ_RESPONSE_HEADERS: {
-      const net::HttpResponseHeaders& response_headers =
-          static_cast<net::NetLogHttpResponseParameter*>(params)->GetHeaders();
-      info->http_status_code = response_headers.response_code();
-      info->http_status_text = response_headers.GetStatusText();
+      scoped_ptr<Value> event_params(entry.ParametersToValue());
+
+      scoped_refptr<net::HttpResponseHeaders> response_headers(
+          net::HttpResponseHeaders::FromNetLogParam(event_params.get()));
+
+      if (!response_headers)
+        NOTREACHED();

[eroman, 2012/06/06 21:27:11]

Please put curly braces.

Perhaps this works as is, but I have seen some scary stuff involving macro
expansions leading to multiple statements and consequently weirdness in the no
curly brace case.

[mmenke, 2012/06/07 19:20:46]

Done.  It does work, but your concern is reasonable.

+
+      info->http_status_code = response_headers->response_code();
+      info->http_status_text = response_headers->GetStatusText();
       std::string name, value;
 
       // We need to clear headers in case the same url_request is reused for
@@ -135,17 +142,24 @@
       info->response_headers.clear();
 
       for (void* it = NULL;
-           response_headers.EnumerateHeaderLines(&it, &name, &value); ) {
+           response_headers->EnumerateHeaderLines(&it, &name, &value); ) {
         info->response_headers.push_back(std::make_pair(name, value));
       }
       info->response_headers_text =
           net::HttpUtil::ConvertHeadersBackToHTTPResponse(
-              response_headers.raw_headers());
+              response_headers->raw_headers());
       break;
     }
     case net::NetLog::TYPE_HTTP_STREAM_REQUEST_BOUND_TO_JOB: {
-      uint32 http_stream_job_id = static_cast<net::NetLogSourceParameter*>(
-          params)->value().id;
+      scoped_ptr<Value> event_params(entry.ParametersToValue());
+      net::NetLog::Source http_stream_job_source;
+      if (!net::NetLog::Source::FromEventParameters(event_params.get(),
+                                                    &http_stream_job_source)) {
+        NOTREACHED();
+        break;
+      }
+
+      uint32 http_stream_job_id = http_stream_job_source.id;
       HTTPStreamJobToSocketMap::iterator it =
           http_stream_job_to_socket_.find(http_stream_job_id);
       if (it == http_stream_job_to_socket_.end())
@@ -158,7 +172,7 @@
         socket_to_request_.clear();
       }
 
-      socket_to_request_[socket_id] = source.id;
+      socket_to_request_[socket_id] = entry.source().id;
       http_stream_job_to_socket_.erase(http_stream_job_id);
       break;
     }
@@ -168,16 +182,17 @@
 }
 
 void DevToolsNetLogObserver::OnAddHTTPStreamJobEntry(
-    net::NetLog::EventType type,
-    const base::TimeTicks& time,
-    const net::NetLog::Source& source,
-    net::NetLog::EventPhase phase,
-    net::NetLog::EventParameters* params) {
+    const net::NetLog::Entry& entry) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
-  if (type == net::NetLog::TYPE_SOCKET_POOL_BOUND_TO_SOCKET) {
-    uint32 socket_id = static_cast<net::NetLogSourceParameter*>(
-      params)->value().id;
+  if (entry.type() == net::NetLog::TYPE_SOCKET_POOL_BOUND_TO_SOCKET) {
+    scoped_ptr<Value> event_params(entry.ParametersToValue());
+    net::NetLog::Source socket_source;
+    if (!net::NetLog::Source::FromEventParameters(event_params.get(),
+                                                  &socket_source)) {
+      NOTREACHED();
+      return;
+    }
 
     // Prevents us from passively growing the memory unbounded in
     // case something went wrong. Should not happen.
@@ -186,28 +201,25 @@
                       "has grown larger than expected, resetting";
       http_stream_job_to_socket_.clear();
     }
-    http_stream_job_to_socket_[source.id] = socket_id;
+    http_stream_job_to_socket_[entry.source().id] = socket_source.id;
   }
 }
 
 void DevToolsNetLogObserver::OnAddSocketEntry(
-    net::NetLog::EventType type,
-    const base::TimeTicks& time,
-    const net::NetLog::Source& source,
-    net::NetLog::EventPhase phase,
-    net::NetLog::EventParameters* params) {
+    const net::NetLog::Entry& entry) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
-  bool is_end = phase == net::NetLog::PHASE_END;
+  bool is_end = entry.phase() == net::NetLog::PHASE_END;
 
-  SocketToRequestMap::iterator it = socket_to_request_.find(source.id);
+  SocketToRequestMap::iterator it =
+      socket_to_request_.find(entry.source().id);
   if (it == socket_to_request_.end())
     return;
   uint32 request_id = it->second;
 
-  if (type == net::NetLog::TYPE_SOCKET_IN_USE) {
+  if (entry.type() == net::NetLog::TYPE_SOCKET_IN_USE) {
     if (is_end)
-      socket_to_request_.erase(source.id);
+      socket_to_request_.erase(entry.source().id);
     return;
   }
 
@@ -216,9 +228,9 @@
   if (encoded_data_length_it == request_to_encoded_data_length_.end())
     return;
 
-  if (net::NetLog::TYPE_SOCKET_BYTES_RECEIVED == type) {
+  if (net::NetLog::TYPE_SOCKET_BYTES_RECEIVED == entry.type()) {
     int byte_count = 0;
-    scoped_ptr<Value> value(params->ToValue());
+    scoped_ptr<Value> value(entry.ParametersToValue());
     if (!value->IsType(Value::TYPE_DICTIONARY))
       return;