Allow socket API to send binary data.

chrome/browser/extensions/api/socket/socket_api.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_api.h"
 
 #include "base/bind.h"
-#include "base/values.h"
 #include "chrome/browser/extensions/api/api_resource_controller.h"
 #include "chrome/browser/extensions/api/socket/socket.h"
 #include "chrome/browser/extensions/api/socket/tcp_socket.h"
 #include "chrome/browser/extensions/api/socket/udp_socket.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "net/base/io_buffer.h"
 
 namespace extensions {
 
 const char kBytesWrittenKey[] = "bytesWritten";
-const char kMessageKey[] = "message";
+const char kDataKey[] = "data";
 const char kSocketIdKey[] = "socketId";
 const char kTCPOption[] = "tcp";
 const char kUDPOption[] = "udp";
 
 const char kSocketNotFoundError[] = "Socket not found";
 
 SocketCreateFunction::SocketCreateFunction()
     : src_id_(-1), event_notifier_(NULL) {
 }
 
@@ skipping 89 matching lines @@
 bool SocketDisconnectFunction::Respond() {
   return true;
 }
 
 bool SocketReadFunction::Prepare() {
   EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_));
   return true;
 }
 
 void SocketReadFunction::Work() {
-  std::string message;
+  // TODO(miket): this is an arbitrary number. Can we come up with one that
+  // makes sense?
+  const int buffer_len = 2048;

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

FWIW, some noodling around with getsockopt(SO_RCVBUF) on my machine tells me
that the kernel keeps a receive buffer of nearly 256kb (4 bytes shy).

+  scoped_refptr<net::IOBuffer> io_buffer(new net::IOBuffer(buffer_len));
   Socket* socket = controller()->GetSocket(socket_id_);
+  int bytes_read = -1;
   if (socket)
-    message = socket->Read();
+    bytes_read = socket->Read(io_buffer, buffer_len);
 
+  // TODO(miket): the buffer-to-array functionality appears twice, once here
+  // and once in socket.cc. When serial etc. is converted over, it'll appear
+  // there, too. What's a good single place for it to live? Keep in mind that
+  // this is short-term code, to be replaced with ArrayBuffer code.
   DictionaryValue* result = new DictionaryValue();
-  result->SetString(kMessageKey, message);
+  ListValue* data_value = new ListValue();
+  if (bytes_read > 0) {
+    size_t bytes_size = static_cast<size_t>(bytes_read);
+    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]));
+    }
+  }
+  result->Set(kDataKey, data_value);
   result_.reset(result);
 }
 
 bool SocketReadFunction::Respond() {
   return true;
 }
 
+SocketWriteFunction::SocketWriteFunction()
+    : socket_id_(0),
+      io_buffer_(NULL) {
+}
+
+SocketWriteFunction::~SocketWriteFunction() {
+}
+
 bool SocketWriteFunction::Prepare() {
   EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_));
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &message_));
+  base::ListValue *data_list_value;
+  EXTENSION_FUNCTION_VALIDATE(args_->GetList(1, &data_list_value));
+
+  size_t size = data_list_value->GetSize();
+  if (size != 0) {

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

https://memegen.googleplex.com/5604701

+    io_buffer_ = new net::IOBufferWithSize(size);
+    unsigned char* data_buffer =

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

Note reinterpret_cast. I think this is defensible; we're intentionally changing
a signed type to binary octets. Or if there's a Chromium coding convention that
I'm not aware of, perhaps there's a better way.

[asargent_no_longer_on_chrome, 2012/04/16 22:51:13]

Seems ok, but I think it might be better to use uint8 instead of unsigned char.

(As an aside, you will be frightened to hear that in some places we actually use
std::string to pass around binary data because, hey, here's a convenient wrapper
for a variable number of bytes with a length property).

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

Done.

+        reinterpret_cast<unsigned char*>(io_buffer_->data());
+    for (size_t i = 0; i < size; ++i) {
+      int int_value = -1;
+      data_list_value->GetInteger(i, &int_value);
+      DCHECK(int_value < 256);
+      DCHECK(int_value >= 0);
+      unsigned char truncated_int = static_cast<unsigned char>(int_value);
+      *data_buffer++ = truncated_int;
+    }
+  }
   return true;
 }
 
 void SocketWriteFunction::Work() {
   int bytes_written = -1;
   Socket* socket = controller()->GetSocket(socket_id_);
-  if (socket)
-    bytes_written = socket->Write(message_);
-  else
+  if (socket) {
+    bytes_written = socket->Write(io_buffer_, io_buffer_->size());
+  } else {

[asargent_no_longer_on_chrome, 2012/04/16 22:51:13]

nit: looks like elsewhere in this file you use the no-braces style for
single-line if/else body, so you should remain consistent here

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

Right! I was blindly listening to the lint checker. (how does one "blindly"
listen?)

     error_ = kSocketNotFoundError;
+  }
 
   DictionaryValue* result = new DictionaryValue();
   result->SetInteger(kBytesWrittenKey, bytes_written);
   result_.reset(result);
 }
 
 bool SocketWriteFunction::Respond() {
   return true;
 }
 
 }  // namespace extensions