Define Stable API for WebRTC/Quartc

net/quic/quartc/quartc_reliable_stream_interface.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_RELIABLE_STREAM_INTERFACE_H_
+#define NET_QUIC_QUARTC_QUARTC_RELIABLE_STREAM_INTERFACE_H_
+
+namespace net {

[Ryan Hamilton, 2016/09/23 19:56:44]

nit: newline after

[zhihuang1, 2016/09/24 06:54:05]

Done.

+class QuartcReliableStreamInterface {

[Ryan Hamilton, 2016/09/23 19:56:44]

nit: Just to make this shorter, I'd drop "Reliable" from the name since streams
are always reliable. (I should remove Reliable from the underlying QUIC class
too)

[Ryan Hamilton, 2016/09/23 19:56:44]

Please add a class comment.

[zhihuang1, 2016/09/24 06:54:05]

Sounds good. I like shorter name :)

+ public:
+  virtual uint32_t stream_id() = 0;

[Ryan Hamilton, 2016/09/23 19:56:43]

nit: it's common in net to have a newline after each of these methods.  Also,
this presumably returns the ID of the underlying QUIC stream. Can you comment to
this effect?

+  // The size of how much data has been buffered by the QuicConnection so far.

[Ryan Hamilton, 2016/09/23 19:56:44]

What does it mean for data to be buffered by the connection? Is this data being
sent or received?

[zhihuang1, 2016/09/24 06:54:05]

This means when the underlying WebRTC network is not writable, then
QuicPacketWriter::IsWriteBlocked() will return true and the QuicConnection is
expected to buffered the unsent data.

+  virtual uint64_t buffered_amount() = 0;
+
+  // For forward-compatibility
+  struct WriteParameters {
+    bool fin = false;
+  };
+  // Write the data to the stream.
+  virtual void Write(const char* data,
+                     size_t size,
+                     const WriteParameters& param) = 0;
+  virtual void Close() = 0;

[Ryan Hamilton, 2016/09/23 19:56:44]

I assume this does not delete the stream?

[zhihuang1, 2016/09/24 06:54:05]

I think it does.

This will unregister the stream and call QuicSession::CloseStream(id) which will
put the current stream to the closed_stream_ set. This set will be cleared when
calling PostProcessAfterData()

+
+  // Implemented on the WebRTC side.
+  class Observer {

[Ryan Hamilton, 2016/09/23 19:56:44]

nit: From a recent net discussion I think Delegate would be a better name for
this class. Observer is more commonly an optional possibly many to one object
for which AddObserver and RemoveObserver would be implemented. Whereas delegate
more strongly implies a 1:1 relationship.

+   public:
+    // Called when the data are ready.
+    virtual void OnReceived(QuartcReliableStreamInterface* stream,
+                            const char* data,
+                            size_t size) = 0;
+
+    // TODO(zhihuang) Creates a map from the integer error_code to WebRTC native
+    // error code.
+    virtual void OnClose(QuartcReliableStreamInterface* stream,
+                         int error_code) = 0;
+
+    // Called when the buffered data has been sent.
+    virtual void OnBufferedAmountChanged(
+        QuartcReliableStreamInterface* stream) = 0;
+  };
+  virtual void SetObserver(Observer*) = 0;

[Ryan Hamilton, 2016/09/23 19:56:44]

Can you comment on lifetime/ownership here? I assume the delegate/observer is
expected to outlive the stream and is still owned by the caller and that this
method is not expected to be called multiple times?

[zhihuang1, 2016/09/24 06:54:05]

You are right. The delegate will not be owned and this method is expected to be
called once.

+};
+}  // namespace net
+
+#endif  // NET_QUIC_QUARTC_QUARTC_RELIABLE_STREAM_INTERFACE_H_