Introduce rtc::PacketTransportInterface and let cricket::TransportChannel inherit.

webrtc/p2p/base/packettransportinterface.h

+/*
+ *  Copyright 2016 The WebRTC Project Authors. All rights reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef WEBRTC_P2P_BASE_PACKETTRANSPORTINTERFACE_H_
+#define WEBRTC_P2P_BASE_PACKETTRANSPORTINTERFACE_H_
+
+#include <string>
+#include <vector>
+
+#include "webrtc/base/sigslot.h"
+
+namespace rtc {
+struct PacketOptions;
+struct PacketTime;
+
+struct PacketTransportInterface : public sigslot::has_slots<> {

[Taylor Brandstetter, 2016/10/17 23:24:51]

Any reason to use struct? As a matter of style, we generally only use structs
for passive, data-holding objects.

[johan, 2016/10/18 09:17:09]

"struct" usually helps to let people consider twice before adding
private/protected members / implementation details to abstract interfaces.
Anyway, changing this to class with public-only members.

+  virtual ~PacketTransportInterface() {}
+  virtual const std::string debug_name() const = 0;

[Taylor Brandstetter, 2016/10/17 23:24:51]

nit: Could be just "name()", with a comment explaining that it's used to
identify the object for debugging purposes. Either way though, a comment would
be nice.

[johan, 2016/10/18 09:17:08]

Ack on adding a comment.
Please discuss with Peter whether you prefer name() or debug_name(). I'm fine
with any of them.

+  virtual bool writable() const = 0;

[Taylor Brandstetter, 2016/10/17 23:24:51]

Should have a comment explaining what "writable" means. Which I think would
generally be, "The underlying transport has been established." Maybe Peter has a
less vague suggestion, but anything would be better than nothing.

[johan, 2016/10/18 09:17:09]

Acknowledged.

+
+  // Attempts to send the given packet.  The return value is < 0 on failure.
+  // TODO(johan): Remove the default argument once channel code is updated.

[Taylor Brandstetter, 2016/10/17 23:24:50]

Could the comments document where the return codes are defined? Or if they're
defined by the subclasses, at least state that here.

[johan, 2016/10/18 09:17:09]

Documenting current behaviour. You won't like it :-) But fixing that should be
done in a dedicated CL.

+  virtual int SendPacket(const char* data,
+                         size_t len,
+                         const rtc::PacketOptions& options,
+                         int flags = 0) = 0;
+
+  // Sets a socket option. Note that not all options are
+  // supported by all transport types.
+  virtual int SetOption(rtc::Socket::Option opt, int value) = 0;
+
+  // TODO(pthatcher): Once Chrome's MockPacketTransportInterface implments
+  // this, remove the default implementation.
+  virtual bool GetOption(rtc::Socket::Option opt, int* value) { return false; }
+
+  // Returns the most recent error that occurred on this channel.
+  virtual int GetError() = 0;
+
+  sigslot::signal1<PacketTransportInterface*> SignalWritableState;

[Taylor Brandstetter, 2016/10/17 23:24:51]

Would be nice to have a comment here too ("Emitted when the writable state,
represented by |writable()|, changes.")

[johan, 2016/10/18 09:17:09]

Done.

+
+  // Emitted when the PacketTransportInterface's ability to send has changed.

[Taylor Brandstetter, 2016/10/17 23:24:51]

Again, not your fault that this comment was vague/wrong. But could you change it
to:

"Emitted when the PacketTransportInterface is ready to send packets. "Ready to
send" is more sensitive than the writable state; a transport may be writable,
but temporarily not able to send packets. For example, the underlying
transport's socket buffer may be full, as indicated by SendPacket's return code
and/or GetError."

[johan, 2016/10/18 09:17:08]

Done.

+  sigslot::signal1<PacketTransportInterface*> SignalReadyToSend;
+
+  // Signalled each time a packet is received on this channel.
+  sigslot::signal5<PacketTransportInterface*,
+                   const char*,
+                   size_t,
+                   const rtc::PacketTime&,
+                   int>
+      SignalReadPacket;
+
+  // Signalled each time a packet is sent on this channel.
+  sigslot::signal2<PacketTransportInterface*, const rtc::SentPacket&>
+      SignalSentPacket;
+};
+
+}  // namespace rtc
+
+#endif  // WEBRTC_P2P_BASE_PACKETTRANSPORTINTERFACE_H_