Port QuicServer to Chrome network stack

net/quic/quic_server_packet_writer.cc

Index: net/quic/quic_server_packet_writer.cc
diff --git a/net/quic/quic_server_packet_writer.cc b/net/quic/quic_server_packet_writer.cc
index c2635b699eb3efd06713015d8ba6fcd1a2d93a4e..e7e939364d3e0f5117354e12efac3a380855fd32 100644
--- a/net/quic/quic_server_packet_writer.cc
+++ b/net/quic/quic_server_packet_writer.cc
@@ -6,18 +6,20 @@
 
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/message_loop/message_loop.h"

[wtc, 2014/06/20 21:40:50]

Delete this header.

[dmziegler, 2014/06/23 18:31:24]

Done.

 #include "base/metrics/sparse_histogram.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
+#include "net/quic/crypto/quic_random.h"

[wtc, 2014/06/20 21:40:50]

I believe we can delete this header.

[dmziegler, 2014/06/23 18:31:24]

Done.

 
 namespace net {
 
-QuicServerPacketWriter::QuicServerPacketWriter() : weak_factory_(this) {
-}
-
-QuicServerPacketWriter::QuicServerPacketWriter(UDPServerSocket* socket)
+QuicServerPacketWriter::QuicServerPacketWriter(
+    QuicBlockedWriterInterface* blocked_writer,
+    UDPServerSocket* socket)
     : weak_factory_(this),
       socket_(socket),
+      blocked_writer_(blocked_writer),
       write_blocked_(false) {
 }
 
@@ -26,8 +28,8 @@ QuicServerPacketWriter::~QuicServerPacketWriter() {
 
 WriteResult QuicServerPacketWriter::WritePacket(
     const char* buffer, size_t buf_len,
-    const net::IPAddressNumber& self_address,
-    const net::IPEndPoint& peer_address) {
+    const IPAddressNumber& self_address,
+    const IPEndPoint& peer_address) {
   scoped_refptr<StringIOBuffer> buf(
       new StringIOBuffer(std::string(buffer, buf_len)));
   DCHECK(!IsWriteBlocked());
@@ -46,10 +48,19 @@ WriteResult QuicServerPacketWriter::WritePacket(
       write_blocked_ = true;
     }
   }
-
   return WriteResult(status, rv);
 }
 
+WriteResult QuicServerPacketWriter::WritePacketWithCallback(
+    const char* buffer,
+    size_t buf_len,
+    const IPAddressNumber& self_address,
+    const IPEndPoint& peer_address,
+    WriteCallback callback) {
+  callback_ = callback;
+  return WritePacket(buffer, buf_len, self_address, peer_address);

[wtc, 2014/06/20 21:40:50]

IMPORTANT: I think we should clear callback_ if WritePacket returns a status
other than WRITE_STATUS_BLOCKED. We can also just test the |write_blocked_|
member.

[dmziegler, 2014/06/23 18:31:24]

Done.

+}
+
 bool QuicServerPacketWriter::IsWriteBlockedDataBuffered() const {
   // UDPServerSocket::SendTo buffers the data until the Write is permitted.
   return true;
@@ -67,8 +78,8 @@ void QuicServerPacketWriter::OnWriteComplete(int rv) {
   DCHECK_NE(rv, ERR_IO_PENDING);
   write_blocked_ = false;
   WriteResult result(rv < 0 ? WRITE_STATUS_ERROR : WRITE_STATUS_OK, rv);
-  connection_->OnPacketSent(result);
-  connection_->OnCanWrite();
+  callback_.Run(result);

[wtc, 2014/06/20 21:40:50]

BUG: I think we need to clear |callback_|. Our convention is to clear callback_
before running it. You can use the ResetAndRun method to do that.

[dmziegler, 2014/06/23 18:31:24]

Done.

+  blocked_writer_->OnCanWrite();

[wtc, 2014/06/20 21:40:50]

Can you explain this change? It seems that callback_.Run(result) is equivalent
to the original code (connection_->OnPacketSent(result) +
connection_->OnCanWrite()), so the net effect is the new
blocked_writer_->OnCanWrite() call. What does this accomplish?

[dmziegler, 2014/06/23 18:31:24]

As explained in person, both lines are actually different. The first calls back
to the specific connection which performed the write (rather than a single
connection associated with the writer), and the second calls
QuicDispatcher::OnCanWrite, which then takes care of calling
QuicConnection::OnCanWrite on the relevant connections.

 }
 
 }  // namespace net