Improve socket.write() to make sure all data will be sent out.

chrome/browser/extensions/api/socket/socket.cc

Index: chrome/browser/extensions/api/socket/socket.cc
diff --git a/chrome/browser/extensions/api/socket/socket.cc b/chrome/browser/extensions/api/socket/socket.cc
index 5dc62b2db8e56fd01af3df8dde0ba649cd550da0..9e318132ed987261e1b77274f05115534604d8e2 100644
--- a/chrome/browser/extensions/api/socket/socket.cc
+++ b/chrome/browser/extensions/api/socket/socket.cc
@@ -25,6 +25,55 @@ Socket::~Socket() {
   DCHECK(!is_connected_);
 }
 
+void Socket::Write(scoped_refptr<net::IOBuffer> io_buffer,
+                   int byte_count,
+                   const CompletionCallback& callback) {
+  DCHECK(!callback.is_null());
+  write_queue_.push(WriteRequest(io_buffer, byte_count, callback));
+  WriteData();
+}
+
+void Socket::WriteData() {
+  // IO is pending.
+  if (io_buffer_write_.get())
+    return;
+
+  WriteRequest& request = write_queue_.front();
+
+  DCHECK(request.byte_count > request.byte_written);
+  io_buffer_write_ = new net::WrappedIOBuffer(

[jeremya, 2012/05/17 01:09:06]

Why does io_buffer_write_ need to be stored in a member variable? It seems like
you could just have a bool is_write_pending_ instead, which is true when waiting
for an OnWriteComplete callback.

Another possible architecture would be to have a WriteRequest
pending_write_request_ member variable, which represents the WriteRequest
currently being processed.

[Peng, 2012/05/17 02:42:22]

Because WriteImpl() & TCPClientSocket::Write() take real IOBuffer pointer as
argument. I through we should not delete the IOBuffer until OnComplete be
called. 

+      request.io_buffer->data() + request.byte_written);
+  int result = WriteImpl(
+      io_buffer_write_.get(),
+      request.byte_count - request.byte_written,
+      base::Bind(&Socket::OnWriteComplete, base::Unretained(this)));
+
+  if (result != net::ERR_IO_PENDING)
+    OnWriteComplete(result);
+}
+
+void Socket::OnWriteComplete(int result) {
+  io_buffer_write_ = NULL;

[jeremya, 2012/05/17 01:09:06]

Is this leaking?

[Peng, 2012/05/17 02:42:22]

It is a scoped_refptr<net::IOBuffer>. So it is not a leaking.

+
+  WriteRequest& request = write_queue_.front();
+
+  if (result >= 0) {

[jeremya, 2012/05/17 01:09:06]

What does result == 0 mean?

[Peng, 2012/05/17 02:42:22]

I remember Linux allows call write() with a zero size buffer to test if the
connection is alive.

+    request.byte_written += result;
+    if (request.byte_written < request.byte_count) {
+      WriteData();
+      return;
+    }
+    DCHECK(request.byte_written == request.byte_count);
+    result = request.byte_written;
+  }
+
+  request.callback.Run(result);
+  write_queue_.pop();
+
+  if (!write_queue_.empty())
+    WriteData();
+}
+
 // static
 bool Socket::StringAndPortToIPEndPoint(const std::string& ip_address_str,
                                        int port,