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

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/address_list.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(APIResourceEventNotifier* event_notifier)
     : APIResource(APIResource::SocketResource, event_notifier),
       port_(0),
       is_connected_(false) {
 }
 
 Socket::~Socket() {
   // Derived destructors should make sure the socket has been closed.
   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,
                                        net::IPEndPoint* ip_end_point) {
   DCHECK(ip_end_point);
   net::IPAddressNumber ip_number;
   if (!net::ParseIPLiteralToNumber(ip_address_str, &ip_number))
     return false;
 
   *ip_end_point = net::IPEndPoint(ip_number, port);
@@ skipping 19 matching lines @@
   DCHECK(port);
   *ip_address_str = address.ToStringWithoutPort();
   if (ip_address_str->empty()) {
     *port = 0;
   } else {
     *port = address.port();
   }
 }
 
 }  // namespace extensions