NetLogEventParameter to Callback refactoring 8.

net/websockets/websocket_job.cc

Index: net/websockets/websocket_job.cc
===================================================================
--- net/websockets/websocket_job.cc	(revision 141995)
+++ net/websockets/websocket_job.cc	(working copy)
@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/lazy_instance.h"
 #include "base/string_tokenizer.h"
+#include "base/values.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
@@ -55,6 +56,13 @@
 static base::LazyInstance<WebSocketJobInitSingleton> g_websocket_job_init =
     LAZY_INSTANCE_INITIALIZER;
 
+Value* NetLogWebSocketHandshakeResponseHandlerCallback(
+    const net::WebSocketHandshakeResponseHandler* handler,
+    net::NetLog::LogLevel log_level) {
+  std::string headers = handler->GetRawResponse();
+  return net::NetLogWebSocketHandshakeCallback(&headers, log_level);
+}
+
 }  // anonymous namespace
 
 namespace net {
@@ -397,8 +405,7 @@
     handshake_request_sent_ = 0;
     socket_->net_log()->AddEvent(
         NetLog::TYPE_WEB_SOCKET_SEND_REQUEST_HEADERS,
-        make_scoped_refptr(
-            new NetLogWebSocketHandshakeParameter(handshake_request)));
+        base::Bind(&NetLogWebSocketHandshakeCallback, &handshake_request));

[eroman, 2012/06/14 17:19:40]

This is confusing. There are two identically named functions, one in the header
takes a string, and one in this file which takes a
WebSocketHandshakeResponseHandler*.

Perhaps the one in this file should be deleted and all just use string version.

[mmenke, 2012/06/14 17:30:06]

They actually have different, but confusingly similar, names (Bind doesn't work
with overloaded functions, at least on Windows).

     socket_->SendData(handshake_request.data(),
                       handshake_request.size());
   }
@@ -439,10 +446,11 @@
     return;
   }
   // handshake message is completed.
+  std::string raw_response = handshake_response_->GetRawResponse();

[eroman, 2012/06/14 17:19:40]

This is unused. See comment above, perhaps you meant to change the line below to
pass &raw_response rather than handshake_response_.get()

[mmenke, 2012/06/14 17:30:06]

Done.  That's what I did initially, actually but thought you'd prefer this. 
Just forgot to remove this line.

[eroman, 2012/06/14 18:12:21]

I generally like deferring construction of the string, but like you pointed out
since GetRawResponse() is non-const, best not to gamble with fate :)

   socket_->net_log()->AddEvent(
       NetLog::TYPE_WEB_SOCKET_READ_RESPONSE_HEADERS,
-      make_scoped_refptr(new NetLogWebSocketHandshakeParameter(
-          handshake_response_->GetRawResponse())));
+      base::Bind(&NetLogWebSocketHandshakeResponseHandlerCallback,
+                 handshake_response_.get()));
   if (len - response_length > 0) {
     // If we received extra data, it should be frame data.
     DCHECK(received_data_after_handshake_.empty());