Make PacketReader/PacketWriter interfaces async-only.

blimp/net/stream_packet_writer.cc

 // Copyright 2015 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 "blimp/net/stream_packet_writer.h"
 
 #include <iostream>
 
 #include "base/callback_helpers.h"
 #include "base/logging.h"
@@ skipping 29 matching lines @@
       socket_(socket),
       header_buffer_(
           new net::DrainableIOBuffer(new net::IOBuffer(kPacketHeaderSizeBytes),
                                      kPacketHeaderSizeBytes)),
       weak_factory_(this) {
   DCHECK(socket_);
 }
 
 StreamPacketWriter::~StreamPacketWriter() {}
 
-int StreamPacketWriter::WritePacket(scoped_refptr<net::DrainableIOBuffer> data,
-                                    const net::CompletionCallback& callback) {
+void StreamPacketWriter::WritePacket(scoped_refptr<net::DrainableIOBuffer> data,
+                                     const net::CompletionCallback& callback) {
   DCHECK_EQ(WriteState::IDLE, write_state_);
   DCHECK(data);
   if (data->BytesRemaining() == 0) {
     // The packet is empty; your argument is invalid.
     DLOG(ERROR) << "Attempted to write zero-length packet.";
-    return net::ERR_INVALID_ARGUMENT;
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE, base::Bind(callback, net::ERR_INVALID_ARGUMENT));
   }
 
   write_state_ = WriteState::HEADER;
   header_buffer_->SetOffset(0);
   *reinterpret_cast<uint32*>(header_buffer_->data()) =
       base::HostToNet32(data->BytesRemaining());
   payload_buffer_ = data;
 
   int result = DoWriteLoop(false);

[Wez, 2015/11/20 23:42:13]

Why are we passing DoWriteLoop a bool here, when it takes an int?

[Kevin M, 2015/11/24 21:34:36]

Done.

   if (result == net::ERR_IO_PENDING) {
-    // Store the completion callback to invoke when DoWriteLoop completes
-    // asynchronously.
-    callback_ = callback;
-  } else {
     // Release the payload buffer, since the write operation has completed
     // synchronously.

[Wez, 2015/11/20 23:42:13]

Not according to the result test above, it hasn't.

Does this actually pass the unit-tests - if so then we need a new one to trap
this error!

[Kevin M, 2015/11/24 21:34:36]

Done. I must've been interrupted mid-CL, because this portion wasn't done fully.

     payload_buffer_ = nullptr;
+
+    // Adapt synchronous completion to an asynchronous style.
+    base::MessageLoop::current()->PostTask(FROM_HERE,
+                                           base::Bind(callback, result));
+  } else {
+    callback_ = callback;
   }
-
-  return result;
 }
 
 int StreamPacketWriter::DoWriteLoop(int result) {
   DCHECK_NE(net::ERR_IO_PENDING, result);
   DCHECK_GE(result, 0);
   DCHECK_NE(WriteState::IDLE, write_state_);
 
   while (result >= 0 && write_state_ != WriteState::IDLE) {
     VLOG(2) << "DoWriteLoop (state=" << write_state_ << ", result=" << result
             << ")";
@@ skipping 57 matching lines @@
 
   // If the write finished, either successfully or by error, inform the
   // caller.
   if (result != net::ERR_IO_PENDING) {
     payload_buffer_ = nullptr;
     base::ResetAndReturn(&callback_).Run(result);
   }
 }
 
 }  // namespace blimp