Allow socket API to send binary data.

chrome/browser/extensions/api/socket/socket.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 "chrome/browser/extensions/api/socket/socket.h"
 
 #include "base/bind.h"
 #include "chrome/browser/extensions/api/api_resource_event_notifier.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_errors.h"
 #include "net/socket/socket.h"
 
 namespace extensions {
 
 Socket::Socket(const std::string& address, int port,
                APIResourceEventNotifier* event_notifier)
     : APIResource(APIResource::SocketResource, event_notifier),
       address_(address),
       port_(port),
-      is_connected_(false),
-      read_buffer_(new net::IOBufferWithSize(kMaxRead)) {
+      is_connected_(false) {
 }
 
 Socket::~Socket() {
   // Derived destructors should make sure the socket has been closed.
   DCHECK(!is_connected_);
 }
 
-void Socket::OnDataRead(int result) {
-  std::string message;
-  if (result >= 0)
-    message = std::string(read_buffer_->data(), result);
-  event_notifier()->OnDataRead(result, message);
+void Socket::OnDataRead(scoped_refptr<net::IOBuffer> io_buffer, int result) {
+  // OnDataRead will take ownership of data_value.
+  ListValue* data_value = new ListValue();
+  if (result >= 0) {
+    size_t bytes_size = static_cast<size_t>(result);
+    const char* io_buffer_start = io_buffer->data();
+    for (size_t i = 0; i < bytes_size; ++i) {
+      data_value->Set(i, Value::CreateIntegerValue(io_buffer_start[i]));
+    }
+  }
+  event_notifier()->OnDataRead(result, data_value);
 }
 
 void Socket::OnWriteComplete(int result) {
   event_notifier()->OnWriteComplete(result);
 }
 
-std::string Socket::Read() {
-  int result = socket()->Read(
-      read_buffer_, kMaxRead,
-      base::Bind(&Socket::OnDataRead, base::Unretained(this)));
-  if (result == net::ERR_IO_PENDING)
-    return "";
-  if (result < 0)
-    return "";
-  return std::string(read_buffer_->data(), result);
+int Socket::Read(scoped_refptr<net::IOBuffer> io_buffer, int io_buffer_len) {
+  return socket()->Read(
+      io_buffer.get(),
+      io_buffer_len,
+      base::Bind(&Socket::OnDataRead, base::Unretained(this), io_buffer));

[jeremya, 2012/04/16 22:28:27]

Is it possible that this read will call the callback after this Socket object
has been destructed (e.g. the page is closed, or the socket is closed, etc.)?

[miket_OOO, 2012/04/16 23:15:53]

I can't point you to the code that proves it, but sockets are not threadsafe and
aggressively assert to that effect. So if there were an ordering problem, it
would be a bug in the socket code (not extensions::socket but net::socket). I am
not aware of buggy behavior like this.

 }
 
-int Socket::Write(const std::string& message) {
-  int length = message.length();
-  scoped_refptr<net::StringIOBuffer> io_buffer(
-      new net::StringIOBuffer(message));
-  scoped_refptr<net::DrainableIOBuffer> buffer(
-      new net::DrainableIOBuffer(io_buffer, length));
+int Socket::Write(scoped_refptr<net::IOBuffer> io_buffer, int byte_count) {
+  scoped_refptr<net::DrainableIOBuffer> drainable_buffer(
+      new net::DrainableIOBuffer(io_buffer.get(), byte_count));
 
   int bytes_sent = 0;
-  while (buffer->BytesRemaining()) {
+  while (drainable_buffer->BytesRemaining()) {
     int result = socket()->Write(
-        buffer, buffer->BytesRemaining(),
+        drainable_buffer, drainable_buffer->BytesRemaining(),
         base::Bind(&Socket::OnWriteComplete, base::Unretained(this)));
     if (result <= 0)
       // We pass all errors, including ERROR_IO_PENDING, back to the caller.
       return bytes_sent > 0 ? bytes_sent : result;

[jeremya, 2012/04/16 22:28:27]

(Side note, since you don't change this behaviour in this patch, but: what
happens if we get a number of positive results from Write(), followed by an
error that is not ERR_IO_PENDING? How does the client detect the error?)

[miket_OOO, 2012/04/16 23:15:53]

We're OK, but I think the code needs to be refactored. The client learns of the
error by getting a negative number, but the number of bytes successfully written
so far is lost. I should stick to our goal of mirroring the underlying socket
API and get rid of the whole DrainableIOBuffer. Instead we should make a single
Write call and return that result. I'll do that.

     bytes_sent += result;
-    buffer->DidConsume(result);
+    drainable_buffer->DidConsume(result);
   }
   return bytes_sent;
 }
 
 }  // namespace extensions