Two chagnes to cast socket:

chrome/browser/extensions/api/cast_channel/cast_socket.cc

 // Copyright 2013 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 "chrome/browser/extensions/api/cast_channel/cast_socket.h"
 
 #include <string.h>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ skipping 53 matching lines @@
 // sizeof() for the fields.
 const uint32 kMessageHeaderSize = sizeof(uint32);
 
 CastSocket::CastSocket(const std::string& owner_extension_id,
                        const GURL& url,
                        CastSocket::Delegate* delegate,
                        net::NetLog* net_log) :
     ApiResource(owner_extension_id),
     channel_id_(0),
     url_(url),
+    url_log_str_("[URL: " + url.spec() + "] "),

[Wez, 2014/02/12 21:13:16]

nit: Is "URL:" redundant here?

[mark a. foltz, 2014/02/12 21:45:02]

This is really only for the purposes of logging and doesn't have to do with the
core functionality of the class.  WDYT of defining a macro like VLOG_WITH_URL
instead?

[Wez, 2014/02/12 21:52:14]

+1

[Munjal (Google), 2014/02/12 22:34:36]

Done.

Started doing that but thought I cannot do << syntax in that case. But that is
not the case - macro's text replacement semantics come to the rescue :).

[Munjal (Google), 2014/02/12 22:34:36]

Done.

     delegate_(delegate),
     auth_required_(false),
     current_message_size_(0),
+    current_message_(new CastMessage()),
     net_log_(net_log),
     connect_state_(CONN_STATE_NONE),
     write_state_(WRITE_STATE_NONE),
     read_state_(READ_STATE_NONE),
     error_state_(CHANNEL_ERROR_NONE),
     ready_state_(READY_STATE_NONE) {
   DCHECK(net_log_);
   net_log_source_.type = net::NetLog::SOURCE_SOCKET;
   net_log_source_.id = net_log_->NextID();
 
@@ skipping 51 matching lines @@
 
 bool CastSocket::ExtractPeerCert(std::string* cert) {
   DCHECK(cert);
   DCHECK(peer_cert_.empty());
   net::SSLInfo ssl_info;
   if (!socket_->GetSSLInfo(&ssl_info) || !ssl_info.cert.get())
     return false;
   bool result = net::X509Certificate::GetDEREncoded(
      ssl_info.cert->os_cert_handle(), cert);
   if (result)
-    VLOG(1) << "[URL: " << url_.spec()
-            << "] Successfully extracted peer certificate: " << *cert;
+    VLOG(1) << url_log_str_
+            << "Successfully extracted peer certificate: " << *cert;
   return result;
 }
 
 bool CastSocket::VerifyChallengeReply() {
   return AuthenticateChallengeReply(*challenge_reply_.get(), peer_cert_);
 }
 
 void CastSocket::Connect(const net::CompletionCallback& callback) {
   DCHECK(CalledOnValidThread());
-  VLOG(1) << "Connect readyState = " << ready_state_;
+  VLOG(1) << url_log_str_ << "Connect readyState = " << ready_state_;
   if (ready_state_ != READY_STATE_NONE) {
     callback.Run(net::ERR_CONNECTION_FAILED);
     return;
   }
   if (!ParseChannelUrl(url_)) {
     callback.Run(net::ERR_CONNECTION_FAILED);
     return;
   }
 
   ready_state_ = READY_STATE_CONNECTING;
@@ skipping 55 matching lines @@
   // Get out of the loop either when:
   // a. A network operation is pending, OR
   // b. The Do* method called did not change state
 
   // Connect loop is finished: if there is no pending IO invoke the callback.
   if (rv != net::ERR_IO_PENDING)
     DoConnectCallback(rv);
 }
 
 int CastSocket::DoTcpConnect() {
-  VLOG(1) << "DoTcpConnect";
+  VLOG(1) << url_log_str_ << "DoTcpConnect";
   connect_state_ = CONN_STATE_TCP_CONNECT_COMPLETE;
   tcp_socket_ = CreateTcpSocket();
   return tcp_socket_->Connect(
       base::Bind(&CastSocket::DoConnectLoop, AsWeakPtr()));
 }
 
 int CastSocket::DoTcpConnectComplete(int result) {
-  VLOG(1) << "DoTcpConnectComplete: " << result;
+  VLOG(1) << url_log_str_ << "DoTcpConnectComplete: " << result;
   if (result == net::OK) {
     // Enable TCP protocol-level keep-alive.
     bool result = tcp_socket_->SetKeepAlive(true, kTcpKeepAliveDelaySecs);
     LOG_IF(WARNING, !result) << "Failed to SetKeepAlive.";
     connect_state_ = CONN_STATE_SSL_CONNECT;
   }
   return result;
 }
 
 int CastSocket::DoSslConnect() {
-  VLOG(1) << "DoSslConnect";
+  VLOG(1) << url_log_str_ << "DoSslConnect";
   connect_state_ = CONN_STATE_SSL_CONNECT_COMPLETE;
   socket_ = CreateSslSocket(tcp_socket_.PassAs<net::StreamSocket>());
   return socket_->Connect(
       base::Bind(&CastSocket::DoConnectLoop, AsWeakPtr()));
 }
 
 int CastSocket::DoSslConnectComplete(int result) {
-  VLOG(1) << "DoSslConnectComplete: " << result;
+  VLOG(1) << url_log_str_ << "DoSslConnectComplete: " << result;
   if (result == net::ERR_CERT_AUTHORITY_INVALID &&
              peer_cert_.empty() &&
              ExtractPeerCert(&peer_cert_)) {
     connect_state_ = CONN_STATE_TCP_CONNECT;
   } else if (result == net::OK && auth_required_) {
     connect_state_ = CONN_STATE_AUTH_CHALLENGE_SEND;
   }
   return result;
 }
 
 int CastSocket::DoAuthChallengeSend() {
-  VLOG(1) << "DoAuthChallengeSend";
+  VLOG(1) << url_log_str_ << "DoAuthChallengeSend";
   connect_state_ = CONN_STATE_AUTH_CHALLENGE_SEND_COMPLETE;
   CastMessage challenge_message;
   CreateAuthChallengeMessage(&challenge_message);
-  VLOG(1) << "Sending challenge: " << CastMessageToString(challenge_message);
+  VLOG(1) << url_log_str_
+          << "Sending challenge: " << CastMessageToString(challenge_message);
   // Post a task to send auth challenge so that DoWriteLoop is not nested inside
   // DoConnectLoop. This is not strictly necessary but keeps the write loop
   // code decoupled from connect loop code.
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&CastSocket::SendCastMessageInternal, AsWeakPtr(),
                  challenge_message,
                  base::Bind(&CastSocket::DoConnectLoop, AsWeakPtr())));
   // Always return IO_PENDING since the result is always asynchronous.
   return net::ERR_IO_PENDING;
 }
 
 int CastSocket::DoAuthChallengeSendComplete(int result) {
-  VLOG(1) << "DoAuthChallengeSendComplete: " << result;
+  VLOG(1) << url_log_str_ << "DoAuthChallengeSendComplete: " << result;
   if (result < 0)
     return result;
   connect_state_ = CONN_STATE_AUTH_CHALLENGE_REPLY_COMPLETE;
   // Post a task to start read loop so that DoReadLoop is not nested inside
   // DoConnectLoop. This is not strictly necessary but keeps the read loop
   // code decoupled from connect loop code.
   PostTaskToStartReadLoop();
   // Always return IO_PENDING since the result is always asynchronous.
   return net::ERR_IO_PENDING;
 }
 
 int CastSocket::DoAuthChallengeReplyComplete(int result) {
-  VLOG(1) << "DoAuthChallengeReplyComplete: " << result;
+  VLOG(1) << url_log_str_ << "DoAuthChallengeReplyComplete: " << result;
   if (result < 0)
     return result;
   if (!VerifyChallengeReply())
     return net::ERR_FAILED;
-  VLOG(1) << "Auth challenge verification succeeded";
+  VLOG(1) << url_log_str_ << "Auth challenge verification succeeded";
   return net::OK;
 }
 
 void CastSocket::DoConnectCallback(int result) {
   ready_state_ = (result == net::OK) ? READY_STATE_OPEN : READY_STATE_CLOSED;
   error_state_ = (result == net::OK) ?
       CHANNEL_ERROR_NONE : CHANNEL_ERROR_CONNECT_ERROR;
   if (result == net::OK)  // Start the read loop
     PostTaskToStartReadLoop();
   base::ResetAndReturn(&connect_callback_).Run(result);
 }
 
 void CastSocket::Close(const net::CompletionCallback& callback) {
   DCHECK(CalledOnValidThread());
-  VLOG(1) << "Close ReadyState = " << ready_state_;
+  VLOG(1) << url_log_str_ << "Close ReadyState = " << ready_state_;
   tcp_socket_.reset();
   socket_.reset();
   cert_verifier_.reset();
   transport_security_state_.reset();
   ready_state_ = READY_STATE_CLOSED;
   callback.Run(net::OK);
   // |callback| can delete |this|
 }
 
 void CastSocket::SendMessage(const MessageInfo& message,
@@ skipping 23 matching lines @@
 
   write_queue_.push(write_request);
   if (write_state_ == WRITE_STATE_NONE) {
     write_state_ = WRITE_STATE_WRITE;
     DoWriteLoop(net::OK);
   }
 }
 
 void CastSocket::DoWriteLoop(int result) {
   DCHECK(CalledOnValidThread());
-  VLOG(1) << "WriteData q = " << write_queue_.size();
+  VLOG(1) << url_log_str_ << "DoWriteLoop queue size: " << write_queue_.size();
 
   if (write_queue_.empty()) {
     write_state_ = WRITE_STATE_NONE;
     return;
   }
 
   // Network operations can either finish synchronously or asynchronously.
   // This method executes the state machine transitions in a loop so that
   // write state transitions happen even when network operations finish
   // synchronously.
@@ skipping 29 matching lines @@
 
   // Write loop is done - if the result is ERR_FAILED then close with error.
   if (rv == net::ERR_FAILED)
     CloseWithError(error_state_);
 }
 
 int CastSocket::DoWrite() {
   DCHECK(!write_queue_.empty());
   WriteRequest& request = write_queue_.front();
 
-  VLOG(1) << "WriteData byte_count = " << request.io_buffer->size()
-           << " bytes_written " << request.io_buffer->BytesConsumed();
+  VLOG(2) << url_log_str_
+          << "WriteData byte_count = " << request.io_buffer->size()
+          << " bytes_written " << request.io_buffer->BytesConsumed();
 
   write_state_ = WRITE_STATE_WRITE_COMPLETE;
 
   return socket_->Write(
       request.io_buffer.get(),
       request.io_buffer->BytesRemaining(),
       base::Bind(&CastSocket::DoWriteLoop, AsWeakPtr()));
 }
 
 int CastSocket::DoWriteComplete(int result) {
@@ skipping 126 matching lines @@
   DCHECK_GT(num_bytes_to_read, 0U);
 
   // Read up to num_bytes_to_read into |current_read_buffer_|.
   return socket_->Read(
       current_read_buffer_.get(),
       num_bytes_to_read,
       base::Bind(&CastSocket::DoReadLoop, AsWeakPtr()));
 }
 
 int CastSocket::DoReadComplete(int result) {
-  VLOG(1) << "DoReadDataComplete result = " << result
+  VLOG(2) << url_log_str_ << "DoReadComplete result = " << result
           << " header offset = " << header_read_buffer_->offset()
           << " body offset = " << body_read_buffer_->offset();
   if (result <= 0) {  // 0 means EOF: the peer closed the socket
+    VLOG(1) << url_log_str_ << "Read error, peer closed the socket";
     error_state_ = CHANNEL_ERROR_SOCKET_ERROR;
     read_state_ = READ_STATE_ERROR;
     return result == 0 ? net::ERR_FAILED : result;
   }
 
   // Some data was read.  Move the offset in the current buffer forward.
   DCHECK_LE(current_read_buffer_->offset() + result,
             current_read_buffer_->capacity());
   current_read_buffer_->set_offset(current_read_buffer_->offset() + result);
   read_state_ = READ_STATE_READ;
@@ skipping 15 matching lines @@
       error_state_ = cast_channel::CHANNEL_ERROR_INVALID_MESSAGE;
       read_state_ = READ_STATE_ERROR;
     }
   }
 
   return net::OK;
 }
 
 int CastSocket::DoReadCallback() {
   read_state_ = READ_STATE_READ;
-  if (IsAuthMessage(current_message_)) {
+  const CastMessage& message = *(current_message_.get());
+  if (IsAuthMessage(message)) {
     // An auth message is received, check that connect flow is running.
     if (ready_state_ == READY_STATE_CONNECTING) {
-      challenge_reply_.reset(new CastMessage(current_message_));
+      challenge_reply_.reset(new CastMessage(message));
       PostTaskToStartConnectLoop(net::OK);
     } else {
       read_state_ = READ_STATE_ERROR;
     }
   } else if (delegate_) {
-    MessageInfo message;
-    if (CastMessageToMessageInfo(current_message_, &message))
-      delegate_->OnMessage(this, message);
+    MessageInfo message_info;
+    if (CastMessageToMessageInfo(message, &message_info))
+      delegate_->OnMessage(this, message_info);
     else
       read_state_ = READ_STATE_ERROR;
   }
-  current_message_.Clear();
+  current_message_->Clear();

[Wez, 2014/02/12 21:13:16]

Rather than clearing & re-using the CastMessage, why not .reset() it here, and
create a new one on each read?

[Munjal (Google), 2014/02/12 22:34:36]

So that we don't create many objects on the heap even though we only need one at
a time. Clear gets the object in the exact same state as creating a new one.

   return net::OK;
 }
 
 int CastSocket::DoReadError(int result) {
   DCHECK_LE(result, 0);
   // If inside connection flow, then get back to connect loop.
   if (ready_state_ == READY_STATE_CONNECTING) {
     PostTaskToStartConnectLoop(result);
     // does not try to report error also.
     return net::OK;
   }
   return net::ERR_FAILED;
 }
 
 bool CastSocket::ProcessHeader() {
   DCHECK_EQ(static_cast<uint32>(header_read_buffer_->offset()),
             kMessageHeaderSize);
   MessageHeader header;
   MessageHeader::ReadFromIOBuffer(header_read_buffer_.get(), &header);
   if (header.message_size > kMaxMessageSize)
     return false;
 
-  VLOG(1) << "Parsed header { message_size: " << header.message_size << " }";
+  VLOG(2) << url_log_str_
+          << "Parsed header { message_size: " << header.message_size << " }";
   current_message_size_ = header.message_size;
   return true;
 }
 
 bool CastSocket::ProcessBody() {
   DCHECK_EQ(static_cast<uint32>(body_read_buffer_->offset()),
             current_message_size_);
-  if (!current_message_.ParseFromArray(
+  if (!current_message_->ParseFromArray(
       body_read_buffer_->StartOfBuffer(), current_message_size_)) {
     return false;
   }
   current_message_size_ = 0;
   header_read_buffer_->set_offset(0);
   body_read_buffer_->set_offset(0);
   current_read_buffer_ = header_read_buffer_;
   return true;
 }
 
@@ skipping 16 matching lines @@
 void CastSocket::CloseWithError(ChannelError error) {
   DCHECK(CalledOnValidThread());
   socket_.reset(NULL);
   ready_state_ = READY_STATE_CLOSED;
   error_state_ = error;
   if (delegate_)
     delegate_->OnError(this, error);
 }
 
 bool CastSocket::ParseChannelUrl(const GURL& url) {
-  VLOG(1) << "url = " + url.spec();
+  VLOG(2) << url_log_str_ << "ParseChannelUrl";
   if (url.SchemeIs(kCastInsecureScheme)) {
     auth_required_ = false;
   } else if (url.SchemeIs(kCastSecureScheme)) {
     auth_required_ = true;
   } else {
     return false;
   }
   // TODO(mfoltz): Manual parsing, yech. Register cast[s] as standard schemes?
   // TODO(mfoltz): Test for IPv6 addresses.  Brackets or no brackets?
-  // TODO(mfoltz): Maybe enforce restriction to IPv4 private and IPv6 link-local
-  // networks

[Wez, 2014/02/12 21:13:16]

Did this need re-wrapping?

[Munjal (Google), 2014/02/12 22:34:36]

Yes, I think it was 81 without wrapping.

+  // TODO(mfoltz): Maybe enforce restriction to IPv4 private and IPv6
+  // link-local networks
   const std::string& path = url.path();
   // Shortest possible: //A:B
   if (path.size() < 5) {
     return false;
   }
   if (path.find("//") != 0) {
     return false;
   }
   size_t colon = path.find_last_of(':');
   if (colon == std::string::npos || colon < 3 || colon > path.size() - 2) {
     return false;
   }
   const std::string& ip_address_str = path.substr(2, colon - 2);
   const std::string& port_str = path.substr(colon + 1);
-  VLOG(1) << "addr " << ip_address_str << " port " << port_str;
+  VLOG(2) << url_log_str_ << "IP: " << ip_address_str << " Port: " << port_str;
   int port;
   if (!base::StringToInt(port_str, &port))
     return false;
   net::IPAddressNumber ip_address;
   if (!net::ParseIPLiteralToNumber(ip_address_str, &ip_address))
     return false;
   ip_endpoint_ = net::IPEndPoint(ip_address, port);
   return true;
 };
 
@@ skipping 46 matching lines @@
   io_buffer = new net::DrainableIOBuffer(new net::StringIOBuffer(message_data),
                                          message_data.size());
   return true;
 }
 
 CastSocket::WriteRequest::~WriteRequest() { }
 
 }  // namespace cast_channel
 }  // namespace api
 }  // namespace extensions