Change WebSocketChannel to pass data via IOBuffer

This avoids having to copy the data between std::vector<char> and IOBuffer within the implementation of WebSocketChannel.

As a follow-up, I plan to see about eliminating a similar copy at the Mojo bindings layer in content/browser/websockets/.

Review-Url: https://codereview.chromium.org/2267233002
Cr-Commit-Position: refs/heads/master@{#422846}

content/browser/websockets/websocket_impl.cc

@@ -24,6 +24,7 @@
 #include "content/browser/ssl/ssl_manager.h"
 #include "content/public/browser/storage_partition.h"
 #include "ipc/ipc_message.h"
+#include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_response_headers.h"
@@ -93,7 +94,8 @@ class WebSocketImpl::WebSocketEventHandler final
                                     const std::string& extensions) override;
   ChannelState OnDataFrame(bool fin,
                            WebSocketMessageType type,
-                           const std::vector<char>& data) override;
+                           scoped_refptr<net::IOBuffer> buffer,
+                           size_t buffer_size) override;
   ChannelState OnClosingHandshake() override;
   ChannelState OnFlowControl(int64_t quota) override;
   ChannelState OnDropChannel(bool was_clean,
@@ -167,15 +169,19 @@ ChannelState WebSocketImpl::WebSocketEventHandler::OnAddChannelResponse(
 ChannelState WebSocketImpl::WebSocketEventHandler::OnDataFrame(
     bool fin,
     net::WebSocketFrameHeader::OpCode type,
-    const std::vector<char>& data) {
+    scoped_refptr<net::IOBuffer> buffer,
+    size_t buffer_size) {
   DVLOG(3) << "WebSocketEventHandler::OnDataFrame @"
            << reinterpret_cast<void*>(this)
            << " fin=" << fin
-           << " type=" << type << " data is " << data.size() << " bytes";
+           << " type=" << type << " data is " << buffer_size << " bytes";
 
   // TODO(darin): Avoid this copy.
-  std::vector<uint8_t> data_to_pass(data.size());
-  std::copy(data.begin(), data.end(), data_to_pass.begin());
+  std::vector<uint8_t> data_to_pass(buffer_size);
+  if (buffer_size > 0) {
+    std::copy(buffer->data(), buffer->data() + buffer_size,
+              data_to_pass.begin());
+  }
 
   impl_->client_->OnDataFrame(fin, OpCodeToMessageType(type), data_to_pass);
 
@@ -435,10 +441,11 @@ void WebSocketImpl::SendFrame(bool fin,
   }
 
   // TODO(darin): Avoid this copy.
-  std::vector<char> data_to_pass(data.size());
-  std::copy(data.begin(), data.end(), data_to_pass.begin());
+  scoped_refptr<net::IOBuffer> data_to_pass(new net::IOBuffer(data.size()));
+  std::copy(data.begin(), data.end(), data_to_pass->data());
 
-  channel_->SendFrame(fin, MessageTypeToOpCode(type), data_to_pass);
+  channel_->SendFrame(fin, MessageTypeToOpCode(type), std::move(data_to_pass),
+                      data.size());
 }
 
 void WebSocketImpl::SendFlowControl(int64_t quota) {

net/websockets/websocket_channel.cc

@@ -128,6 +128,16 @@ void GetFrameTypeForOpcode(WebSocketFrameHeader::OpCode opcode,
   return;
 }
 
+class DependentIOBuffer : public WrappedIOBuffer {
+ public:
+  DependentIOBuffer(scoped_refptr<IOBuffer> buffer, size_t offset)
+      : WrappedIOBuffer(buffer->data() + offset), buffer_(std::move(buffer)) {}
+
+ private:
+  ~DependentIOBuffer() override {}
+  scoped_refptr<net::IOBuffer> buffer_;
+};
+
 }  // namespace
 
 // A class to encapsulate a set of frames and information about the size of
@@ -279,12 +289,12 @@ ChannelState WebSocketChannel::HandshakeNotificationSender::SendImmediately(
 WebSocketChannel::PendingReceivedFrame::PendingReceivedFrame(
     bool final,
     WebSocketFrameHeader::OpCode opcode,
-    const scoped_refptr<IOBuffer>& data,
+    scoped_refptr<IOBuffer> data,
     uint64_t offset,
     uint64_t size)
     : final_(final),
       opcode_(opcode),
-      data_(data),
+      data_(std::move(data)),
       offset_(offset),
       size_(size) {}
 
@@ -370,15 +380,16 @@ bool WebSocketChannel::InClosingState() const {
 WebSocketChannel::ChannelState WebSocketChannel::SendFrame(
     bool fin,
     WebSocketFrameHeader::OpCode op_code,
-    const std::vector<char>& data) {
-  if (data.size() > INT_MAX) {
+    scoped_refptr<IOBuffer> buffer,
+    size_t buffer_size) {
+  if (buffer_size > INT_MAX) {
     NOTREACHED() << "Frame size sanity check failed";
     return CHANNEL_ALIVE;
   }
   if (stream_ == NULL) {
     LOG(DFATAL) << "Got SendFrame without a connection established; "
                 << "misbehaving renderer? fin=" << fin << " op_code=" << op_code
-                << " data.size()=" << data.size();
+                << " buffer_size=" << buffer_size;
     return CHANNEL_ALIVE;
   }
   if (InClosingState()) {
@@ -390,7 +401,7 @@ WebSocketChannel::ChannelState WebSocketChannel::SendFrame(
     NOTREACHED() << "SendFrame() called in state " << state_;
     return CHANNEL_ALIVE;
   }
-  if (data.size() > base::checked_cast<size_t>(current_send_quota_)) {
+  if (buffer_size > base::checked_cast<size_t>(current_send_quota_)) {
     // TODO(ricea): Kill renderer.
     return FailChannel("Send quota exceeded", kWebSocketErrorGoingAway, "");
     // |this| has been deleted.
@@ -398,14 +409,14 @@ WebSocketChannel::ChannelState WebSocketChannel::SendFrame(
   if (!WebSocketFrameHeader::IsKnownDataOpCode(op_code)) {
     LOG(DFATAL) << "Got SendFrame with bogus op_code " << op_code
                 << "; misbehaving renderer? fin=" << fin
-                << " data.size()=" << data.size();
+                << " buffer_size=" << buffer_size;
     return CHANNEL_ALIVE;
   }
   if (op_code == WebSocketFrameHeader::kOpCodeText ||
       (op_code == WebSocketFrameHeader::kOpCodeContinuation &&
        sending_text_message_)) {
     StreamingUtf8Validator::State state =
-        outgoing_utf8_validator_.AddBytes(data.data(), data.size());
+        outgoing_utf8_validator_.AddBytes(buffer->data(), buffer_size);
     if (state == StreamingUtf8Validator::INVALID ||
         (state == StreamingUtf8Validator::VALID_MIDPOINT && fin)) {
       // TODO(ricea): Kill renderer.
@@ -416,14 +427,12 @@ WebSocketChannel::ChannelState WebSocketChannel::SendFrame(
     sending_text_message_ = !fin;
     DCHECK(!fin || state == StreamingUtf8Validator::VALID_ENDPOINT);
   }
-  current_send_quota_ -= data.size();
+  current_send_quota_ -= buffer_size;
   // TODO(ricea): If current_send_quota_ has dropped below
   // send_quota_low_water_mark_, it might be good to increase the "low
   // water mark" and "high water mark", but only if the link to the WebSocket
   // server is not saturated.
-  scoped_refptr<IOBuffer> buffer(new IOBuffer(data.size()));
-  std::copy(data.begin(), data.end(), buffer->data());
-  return SendFrameFromIOBuffer(fin, op_code, buffer, data.size());
+  return SendFrameInternal(fin, op_code, std::move(buffer), buffer_size);
   // |this| may have been deleted.
 }
 
@@ -443,15 +452,18 @@ ChannelState WebSocketChannel::SendFlowControl(int64_t quota) {
     const uint64_t bytes_to_send =
         std::min(base::checked_cast<uint64_t>(quota), data_size);
     const bool final = front.final() && data_size == bytes_to_send;
-    const char* data =
-        front.data().get() ? front.data()->data() + front.offset() : NULL;
-    DCHECK(!bytes_to_send || data) << "Non empty data should not be null.";
-    const std::vector<char> data_vector(data, data + bytes_to_send);
+    scoped_refptr<IOBuffer> buffer_to_pass;
+    if (front.data()) {
+      buffer_to_pass = new DependentIOBuffer(front.data(), front.offset());
+    } else {
+      DCHECK(!bytes_to_send) << "Non empty data should not be null.";
+    }
     DVLOG(3) << "Sending frame previously split due to quota to the "
              << "renderer: quota=" << quota << " data_size=" << data_size
              << " bytes_to_send=" << bytes_to_send;
-    if (event_interface_->OnDataFrame(final, front.opcode(), data_vector) ==
-        CHANNEL_DELETED)
+    if (event_interface_->OnDataFrame(final, front.opcode(),
+                                      std::move(buffer_to_pass),
+                                      bytes_to_send) == CHANNEL_DELETED)
       return CHANNEL_DELETED;
     if (bytes_to_send < data_size) {
       front.DidConsume(bytes_to_send);
@@ -832,14 +844,14 @@ ChannelState WebSocketChannel::HandleFrame(
   }
 
   // Respond to the frame appropriately to its type.
-  return HandleFrameByState(
-      opcode, frame->header.final, frame->data, frame->header.payload_length);
+  return HandleFrameByState(opcode, frame->header.final, std::move(frame->data),
+                            frame->header.payload_length);
 }
 
 ChannelState WebSocketChannel::HandleFrameByState(
     const WebSocketFrameHeader::OpCode opcode,
     bool final,
-    const scoped_refptr<IOBuffer>& data_buffer,
+    scoped_refptr<IOBuffer> data_buffer,
     uint64_t size) {
   DCHECK_NE(RECV_CLOSED, state_)
       << "HandleFrame() does not support being called re-entrantly from within "
@@ -857,13 +869,13 @@ ChannelState WebSocketChannel::HandleFrameByState(
     case WebSocketFrameHeader::kOpCodeText:  // fall-thru
     case WebSocketFrameHeader::kOpCodeBinary:
     case WebSocketFrameHeader::kOpCodeContinuation:
-      return HandleDataFrame(opcode, final, data_buffer, size);
+      return HandleDataFrame(opcode, final, std::move(data_buffer), size);
 
     case WebSocketFrameHeader::kOpCodePing:
       DVLOG(1) << "Got Ping of size " << size;
       if (state_ == CONNECTED)
-        return SendFrameFromIOBuffer(
-            true, WebSocketFrameHeader::kOpCodePong, data_buffer, size);
+        return SendFrameInternal(true, WebSocketFrameHeader::kOpCodePong,
+                                 std::move(data_buffer), size);
       DVLOG(3) << "Ignored ping in state " << state_;
       return CHANNEL_ALIVE;
 
@@ -876,7 +888,7 @@ ChannelState WebSocketChannel::HandleFrameByState(
       uint16_t code = kWebSocketNormalClosure;
       std::string reason;
       std::string message;
-      if (!ParseClose(data_buffer, size, &code, &reason, &message)) {
+      if (!ParseClose(std::move(data_buffer), size, &code, &reason, &message)) {
         return FailChannel(message, code, reason);
       }
       // TODO(ricea): Find a way to safely log the message from the close
@@ -896,7 +908,7 @@ ChannelState WebSocketChannel::HandleFrameByState(
 ChannelState WebSocketChannel::HandleDataFrame(
     WebSocketFrameHeader::OpCode opcode,
     bool final,
-    const scoped_refptr<IOBuffer>& data_buffer,
+    scoped_refptr<IOBuffer> data_buffer,
     uint64_t size) {
   if (state_ != CONNECTED) {
     DVLOG(3) << "Ignored data packet received in state " << state_;
@@ -963,14 +975,11 @@ ChannelState WebSocketChannel::HandleDataFrame(
     final = false;
   }
 
-  // TODO(ricea): Can this copy be eliminated?
-  const char* const data_begin = size ? data_buffer->data() : NULL;
-  const char* const data_end = data_begin + size;
-  const std::vector<char> data(data_begin, data_end);
   current_receive_quota_ -= size;
 
   // Sends the received frame to the renderer process.
-  return event_interface_->OnDataFrame(final, opcode_to_send, data);
+  return event_interface_->OnDataFrame(final, opcode_to_send,
+                                       std::move(data_buffer), size);
 }
 
 ChannelState WebSocketChannel::HandleCloseFrame(uint16_t code,
@@ -1033,10 +1042,10 @@ ChannelState WebSocketChannel::RespondToClosingHandshake() {
   return event_interface_->OnClosingHandshake();
 }
 
-ChannelState WebSocketChannel::SendFrameFromIOBuffer(
+ChannelState WebSocketChannel::SendFrameInternal(
     bool fin,
     WebSocketFrameHeader::OpCode op_code,
-    const scoped_refptr<IOBuffer>& buffer,
+    scoped_refptr<IOBuffer> buffer,
     uint64_t size) {
   DCHECK(state_ == CONNECTED || state_ == RECV_CLOSED);
   DCHECK(stream_);
@@ -1046,7 +1055,7 @@ ChannelState WebSocketChannel::SendFrameFromIOBuffer(
   header.final = fin;
   header.masked = true;
   header.payload_length = size;
-  frame->data = buffer;
+  frame->data = std::move(buffer);
 
   if (data_being_sent_) {
     // Either the link to the WebSocket server is saturated, or several messages
@@ -1108,14 +1117,13 @@ ChannelState WebSocketChannel::SendClose(uint16_t code,
     std::copy(
         reason.begin(), reason.end(), body->data() + kWebSocketCloseCodeLength);
   }
-  if (SendFrameFromIOBuffer(
-          true, WebSocketFrameHeader::kOpCodeClose, body, size) ==
-      CHANNEL_DELETED)
+  if (SendFrameInternal(true, WebSocketFrameHeader::kOpCodeClose,
+                        std::move(body), size) == CHANNEL_DELETED)
     return CHANNEL_DELETED;
   return CHANNEL_ALIVE;
 }
 
-bool WebSocketChannel::ParseClose(const scoped_refptr<IOBuffer>& buffer,
+bool WebSocketChannel::ParseClose(scoped_refptr<IOBuffer> buffer,
                                   uint64_t size,
                                   uint16_t* code,
                                   std::string* reason,

net/websockets/websocket_channel.h

@@ -82,16 +82,17 @@ class NET_EXPORT WebSocketChannel {
   // caller to ensure that they have sufficient send quota to send this data,
   // otherwise the connection will be closed without sending. |fin| indicates
   // the last frame in a message, equivalent to "FIN" as specified in section
-  // 5.2 of RFC6455. |data| is the "Payload Data". If |op_code| is kOpCodeText,
-  // or it is kOpCodeContinuation and the type the message is Text, then |data|
-  // must be a chunk of a valid UTF-8 message, however there is no requirement
-  // for |data| to be split on character boundaries. Calling SendFrame may
-  // result in synchronous calls to |event_interface_| which may result in this
-  // object being deleted. In that case, the return value will be
-  // CHANNEL_DELETED.
+  // 5.2 of RFC6455. |buffer->data()| is the "Payload Data". If |op_code| is
+  // kOpCodeText, or it is kOpCodeContinuation and the type the message is
+  // Text, then |buffer->data()| must be a chunk of a valid UTF-8 message,
+  // however there is no requirement for |buffer->data()| to be split on
+  // character boundaries. Calling SendFrame may result in synchronous calls to
+  // |event_interface_| which may result in this object being deleted. In that
+  // case, the return value will be CHANNEL_DELETED.
   ChannelState SendFrame(bool fin,
                          WebSocketFrameHeader::OpCode op_code,
-                         const std::vector<char>& data);
+                         scoped_refptr<IOBuffer> buffer,
+                         size_t buffer_size);
 
   // Sends |quota| units of flow control to the remote side. If the underlying
   // transport has a concept of |quota|, then it permits the remote server to
@@ -159,7 +160,7 @@ class NET_EXPORT WebSocketChannel {
    public:
     PendingReceivedFrame(bool final,
                          WebSocketFrameHeader::OpCode opcode,
-                         const scoped_refptr<IOBuffer>& data,
+                         scoped_refptr<IOBuffer> data,
                          uint64_t offset,
                          uint64_t size);
     PendingReceivedFrame(const PendingReceivedFrame& other);
@@ -286,7 +287,7 @@ class NET_EXPORT WebSocketChannel {
   // HandleFrame() method.
   ChannelState HandleFrameByState(const WebSocketFrameHeader::OpCode opcode,
                                   bool final,
-                                  const scoped_refptr<IOBuffer>& data_buffer,
+                                  scoped_refptr<IOBuffer> data_buffer,
                                   uint64_t size) WARN_UNUSED_RESULT;
 
   // Forwards a received data frame to the renderer, if connected. If
@@ -294,7 +295,7 @@ class NET_EXPORT WebSocketChannel {
   // will fail the channel. Also checks the UTF-8 validity of text frames.
   ChannelState HandleDataFrame(WebSocketFrameHeader::OpCode opcode,
                                bool final,
-                               const scoped_refptr<IOBuffer>& data_buffer,
+                               scoped_refptr<IOBuffer> data_buffer,
                                uint64_t size) WARN_UNUSED_RESULT;
 
   // Handles an incoming close frame with |code| and |reason|.
@@ -309,10 +310,10 @@ class NET_EXPORT WebSocketChannel {
   // when the current write finishes. |fin| and |op_code| are defined as for
   // SendFrame() above, except that |op_code| may also be a control frame
   // opcode.
-  ChannelState SendFrameFromIOBuffer(bool fin,
-                                     WebSocketFrameHeader::OpCode op_code,
-                                     const scoped_refptr<IOBuffer>& buffer,
-                                     uint64_t size) WARN_UNUSED_RESULT;
+  ChannelState SendFrameInternal(bool fin,
+                                 WebSocketFrameHeader::OpCode op_code,
+                                 scoped_refptr<IOBuffer> buffer,
+                                 uint64_t buffer_size) WARN_UNUSED_RESULT;
 
   // Performs the "Fail the WebSocket Connection" operation as defined in
   // RFC6455. A NotifyFailure message is sent to the renderer with |message|.
@@ -340,7 +341,7 @@ class NET_EXPORT WebSocketChannel {
   // is 1, or the supplied code is not permitted to be sent over the network,
   // then false is returned and |message| is set to an appropriate console
   // message.
-  bool ParseClose(const scoped_refptr<IOBuffer>& buffer,
+  bool ParseClose(scoped_refptr<IOBuffer> buffer,
                   uint64_t size,
                   uint16_t* code,
                   std::string* reason,

net/websockets/websocket_end_to_end_test.cc

@@ -71,7 +71,8 @@ class ConnectTestingEventInterface : public WebSocketEventInterface {
 
   ChannelState OnDataFrame(bool fin,
                            WebSocketMessageType type,
-                           const std::vector<char>& data) override;
+                           scoped_refptr<IOBuffer> data,
+                           size_t data_size) override;
 
   ChannelState OnFlowControl(int64_t quota) override;
 
@@ -142,7 +143,8 @@ ChannelState ConnectTestingEventInterface::OnAddChannelResponse(
 ChannelState ConnectTestingEventInterface::OnDataFrame(
     bool fin,
     WebSocketMessageType type,
-    const std::vector<char>& data) {
+    scoped_refptr<IOBuffer> data,
+    size_t data_size) {
   return CHANNEL_ALIVE;
 }
 

net/websockets/websocket_event_interface.h

@@ -13,12 +13,14 @@
 
 #include "base/compiler_specific.h"  // for WARN_UNUSED_RESULT
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "net/base/net_export.h"
 
 class GURL;
 
 namespace net {
 
+class IOBuffer;
 class SSLInfo;
 struct WebSocketHandshakeRequestInfo;
 struct WebSocketHandshakeResponseInfo;
@@ -47,10 +49,10 @@ class NET_EXPORT WebSocketEventInterface {
 
   // Called when a data frame has been received from the remote host and needs
   // to be forwarded to the renderer process.
-  virtual ChannelState OnDataFrame(
-      bool fin,
-      WebSocketMessageType type,
-      const std::vector<char>& data) WARN_UNUSED_RESULT = 0;
+  virtual ChannelState OnDataFrame(bool fin,
+                                   WebSocketMessageType type,
+                                   scoped_refptr<IOBuffer> buffer,
+                                   size_t buffer_size) WARN_UNUSED_RESULT = 0;
 
   // Called to provide more send quota for this channel to the renderer
   // process. Currently the quota units are always bytes of message body