Define Stable API for WebRTC/Quartc

net/quic/quartc/quartc_packet_writer.h

+// Copyright (c) 2016 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.
+
+#ifndef NET_QUIC_QUARTC_QUARTC_PACKET_WRITER_H_
+#define NET_QUIC_QUARTC_QUARTC_PACKET_WRITER_H_
+
+#include "net/quic/core/quic_packet_writer.h"
+#include "net/quic/quartc/quartc_session_interface.h"
+
+namespace net {
+
+// This class writes the packets to network layer with a |transport_| which is
+// implemented on WebRTC side.

[pthatcher2, 2016/10/05 22:12:06]

This might be more clear as "Implements a QuicPacketWriter using a
QuartcSessionInterface::PacketTransport, which allows a QuicConnection to use
(for example), a WebRTC IceTransport."

[zhihuang1, 2016/10/13 06:22:40]

Done.

+class NET_EXPORT_PRIVATE QuartcPacketWriter : public QuicPacketWriter {
+ public:
+  QuartcPacketWriter(QuartcSessionInterface::Transport* transport);
+  ~QuartcPacketWriter() override {}
+
+  // Called from QuicConnection when the session has packets to write.

[pthatcher2, 2016/10/05 22:12:06]

This would be more clear simply as "The QuicConnection calls WritePacket and the
QuicPacketWriter writes them to the QuartcSessionInterface::PacketTransport."

[pthatcher1, 2016/10/14 18:58:58]

Did you look at this comment?

[zhihuang1, 2016/10/16 00:45:27]

Ah yes, I did and I also made the changes. Sorry for not replying for this.

+  WriteResult WritePacket(const char* buffer,
+                          size_t buf_len,
+                          const IPAddress& self_address,
+                          const IPEndPoint& peer_address,
+                          PerPacketOptions* options) override;
+
+  // If this is set to false, then QuicConnection buffers unsent packets.

[pthatcher2, 2016/10/05 22:12:06]

Please put the implementation here so it's clear it's always false.  Then the
comment can say "Returns false so the QuicConnection will buffer unset packets."

[zhihuang1, 2016/10/13 06:22:40]

This is not allowed by the Chromium style checker.

+  bool IsWriteBlockedDataBuffered() const override;
+
+  // Whether the underneath |transport_| is blocked. If this returns true,
+  // outgoing QUIC packets are queued by QuicConnection until
+  // Transport::Observer::OnCanWrite() is called.
+  bool IsWriteBlocked() const override;
+
+  // Maximum size of the QUIC packet which can be written.
+  QuicByteCount GetMaxPacketSize(const IPEndPoint& peer_address) const override;

[pthatcher2, 2016/10/05 22:12:06]

Same here: can you put the implementation here to make it more readable?

[zhihuang1, 2016/10/13 06:22:40]

Same here. This is not allowed because of Chromium style.

+
+  // This method is not used because the network layer in WebRTC will determine
+  // the writing states.
+  void SetWritable() override;

[pthatcher2, 2016/10/05 22:12:06]

And here too

+
+ private:
+  // QuartcPacketWriter will not own the transport.

[pthatcher2, 2016/10/05 22:12:06]

What does own the transport?  

[zhihuang1, 2016/10/13 06:22:40]

The cricket::QuicTransportChannel will implement the PacketTransport and pass
itself in.

+  QuartcSessionInterface::Transport* transport_ = nullptr;
+};
+
+}  // namespace net
+
+#endif  // NET_QUIC_QUARTC_QUARTC_PACKET_WRITER_H_