Define Stable API for WebRTC/Quartc

net/quic/quartc/quartc_factory_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_FACTORY_INTERFACE_H_
+#define NET_QUIC_QUARTC_QUARTC_FACTORY_INTERFACE_H_
+
+#include "net/quic/quartc/quartc_session_interface.h"
+
+namespace net {
+
+// Used to create instances for QuartcSession.

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

Either we should call it QuartcSessionFactory or the comment should say "Creates
Quartc objects, such as QuartcSession".  In other words, either we  expect to
someday create more than just a QuartcSession, in which case the comment should
reflect that.  Or we don't, in which case the name should reflect that.

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

Done.

+class QuartcFactoryInterface {
+ public:
+  virtual ~QuartcFactoryInterface() {}
+
+  // This struct contains the required parameters to create a QuartcSession.

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

You can probably just remove this comment.

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

Done.

+  struct QuartcConfig {

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

Shouldn't this be named QuartcSessionConfig?

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

Done.

+    // Used to determine the perspective of the session.

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

This could be more clear by saying something like "In a QuartcSession, there are
two endpoints.  One endpoint must be the server and one endpoint must be the
client."

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

I think this should be something like "The QuartcSession must act as a Server on
one endpoint and act as a Client on the other one."

+    bool is_server = false;
+    // Used by the client to create a unique server ID for the
+    // QuicCryptoClientStream when doing crypto handshake.
+    std::string remote_fingerprint_value;

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

Why not something like "unique_remote_server_id"  Then the comment could just be
something like "This is only needed when is_server = false.  It must be unique
for each endpoint the local endpoint may communicate with.  For example, a
WebRTC client could use the remote endpoint's crypto fingerprint."

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

Done.

+    // Used for creating a QuartcPacketWriter for QuicConnection, which is
+    // required when creating a QuartcSession. The QuartcPacketWriter will not
+    // take ownership of it.

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

A better comment would be something like "The way the QuicConnection will send
and receive packets, like a virtual UDP socket.  For WebRTC, this will typically
be an IceTransport."

+    QuartcSessionInterface::Transport* transport = nullptr;

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

Can we call this a QuartcSessionInterface::PacketTransport?  That would make it
more clear it's about the low-level thing moving packets.

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

Done.

+  };
+
+  virtual std::unique_ptr<QuartcSessionInterface> CreateQuartcSession(
+      const QuartcConfig& quartc_config) = 0;
+};
+
+// Creates a new instance of QuartcFactoryInterface.
+std::unique_ptr<QuartcFactoryInterface> CreateQuartcFactory();
+
+}  // namepace net
+
+#endif  // NET_QUIC_QUARTC_QUARTC_FACTORY_INTERFACE_H_