NetLogEventParameter to Callback refactoring 8.

net/websockets/websocket_job.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/websockets/websocket_job.h"
 
 #include <algorithm>
 
 #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"
 #include "net/cookies/cookie_store.h"
 #include "net/base/io_buffer.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_transaction_factory.h"
 #include "net/http/http_util.h"
 #include "net/spdy/spdy_session.h"
 #include "net/spdy/spdy_session_pool.h"
@@ skipping 26 matching lines @@
   friend struct base::DefaultLazyInstanceTraits<WebSocketJobInitSingleton>;
   WebSocketJobInitSingleton() {
     net::SocketStreamJob::RegisterProtocolFactory("ws", WebSocketJobFactory);
     net::SocketStreamJob::RegisterProtocolFactory("wss", WebSocketJobFactory);
   }
 };
 
 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 {
 
 bool WebSocketJob::websocket_over_spdy_enabled_ = false;
 
 // static
 void WebSocketJob::EnsureInit() {
   g_websocket_job_init.Get();
 }
@@ skipping 318 matching lines @@
 }
 
 void WebSocketJob::DoSendData() {
   if (spdy_websocket_stream_.get()) {
     linked_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
     handshake_request_->GetRequestHeaderBlock(
         socket_->url(), headers.get(), &challenge_);
     spdy_websocket_stream_->SendRequest(headers);
   } else {
     const std::string& handshake_request =
         handshake_request_->GetRawRequest();

[mmenke, 2012/06/14 14:18:02]

This mutates the handshake_request and thus is non-const, weirdly, so not
wrapping it in a callback.

     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());
   }
   // Just buffered in |handshake_request_|.
   started_to_send_handshake_request_ = true;
 }
 
 void WebSocketJob::OnSentHandshakeRequest(
     SocketStream* socket, int amount_sent) {
   DCHECK_EQ(state_, CONNECTING);
@@ skipping 20 matching lines @@
         received_data_after_handshake_.end(), data, data + len);
     return;
   }
 
   size_t response_length = handshake_response_->ParseRawResponse(data, len);
   if (!handshake_response_->HasResponse()) {
     // not yet. we need more data.
     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());
     received_data_after_handshake_.assign(data + response_length, data + len);
   }
   SaveCookiesAndNotifyHeaderComplete();
 }
 
 void WebSocketJob::SaveCookiesAndNotifyHeaderComplete() {
   // handshake message is completed.
@@ skipping 195 matching lines @@
 
   scoped_refptr<IOBufferWithSize> next_buffer = send_buffer_queue_.front();
   send_buffer_queue_.pop_front();
   current_send_buffer_ = new DrainableIOBuffer(next_buffer,
                                                next_buffer->size());
   SendDataInternal(current_send_buffer_->data(),
                    current_send_buffer_->BytesRemaining());
 }
 
 }  // namespace net