Modify PeerConnection for end-to-end QuicDataChannel usage

webrtc/p2p/quic/quictransportchannel.h

Index: webrtc/p2p/quic/quictransportchannel.h
diff --git a/webrtc/p2p/quic/quictransportchannel.h b/webrtc/p2p/quic/quictransportchannel.h
index a3c0106d0a096e088721339d93c5628604041049..18af7f4c7cb0007ceffd66c40314c4cfbb5c3ab6 100644
--- a/webrtc/p2p/quic/quictransportchannel.h
+++ b/webrtc/p2p/quic/quictransportchannel.h
@@ -18,6 +18,7 @@
 #include "net/quic/quic_packet_writer.h"
 #include "webrtc/base/optional.h"
 #include "webrtc/base/scoped_ptr.h"
+#include "webrtc/p2p/base/transport.h"
 #include "webrtc/p2p/base/transportchannelimpl.h"
 #include "webrtc/p2p/quic/quicconnectionhelper.h"
 #include "webrtc/p2p/quic/quicsession.h"
@@ -48,7 +49,7 @@ enum QuicTransportState {
 //       TransportChannelImpl* channel_;
 //   }
 //
-//   - Data written to SendPacket() is passed directly to |channel_| if  it is
+//   - Data written to SendPacket() is passed directly to |channel_| if it is
 //     an SRTP packet with the PF_SRTP_BYPASS flag.
 //
 //   - |quic_| passes outgoing packets to WritePacket(), which transfers them
@@ -61,9 +62,15 @@ enum QuicTransportState {
 //   - When the QUIC handshake is completed, quic_state() returns
 //     QUIC_TRANSPORT_CONNECTED and SRTP keying material can be exported.
 //
-// TODO(mikescarlett): Implement secure QUIC handshake, 0-RTT handshakes, and
-// QUIC data streams.
+//   - CreateQuicStream() creates a locally-initiated QUIC stream with an
+//     internally assigned ID. SignalIncomingStream is emitted when the remote
+//     peer initiates a QUIC stream with a new ID.
+//
+// TODO(mikescarlett): Implement secure QUIC handshake and 0-RTT handshakes.
+// Eliminate inheritance of Transport when TransportController no longer
+// requires it internally.
 class QuicTransportChannel : public TransportChannelImpl,
+                             public Transport,

[Taylor Brandstetter, 2016/04/01 23:23:42]

As discussed, I think it will be confusing for a QuicTransportChannel to also be
a Transport. Can you make a separate "QuicTransport" class, which is essentially
just a QuicTransportChannel factory? Look at P2PTransport for example.

[mikescarlett, 2016/04/05 19:58:53]

Done and split into a new CL.

                              public net::QuicPacketWriter,
                              public net::QuicCryptoClientStream::ProofHandler {
  public:
@@ -75,7 +82,7 @@ class QuicTransportChannel : public TransportChannelImpl,
   // TODO(mikescarlett): Implement certificate authentication.
   bool SetLocalCertificate(
       const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) override;
-  rtc::scoped_refptr<rtc::RTCCertificate> GetLocalCertificate() const override;
+  rtc::scoped_refptr<rtc::RTCCertificate> GetLocalCertificate() const;
   // TODO(mikescarlett): Implement fingerprint authentication.
   bool SetRemoteFingerprint(const std::string& digest_alg,
                             const uint8_t* digest,
@@ -199,6 +206,12 @@ class QuicTransportChannel : public TransportChannelImpl,
   void OnProofVerifyDetailsAvailable(
       const net::ProofVerifyDetails& verify_details) override;
 
+  // Transport override.
+  bool GetLocalCertificate(
+      rtc::scoped_refptr<rtc::RTCCertificate>* certificate) override {
+    return false;
+  }
+
   // Returns true if |quic_| has queued data which wasn't written due
   // to |channel_| being write blocked.
   bool HasDataToWrite() const;
@@ -206,6 +219,35 @@ class QuicTransportChannel : public TransportChannelImpl,
   void OnCanWrite();
   // Connectivity state of QuicTransportChannel.
   QuicTransportState quic_state() const { return quic_state_; }
+  // Creates a new QUIC stream that can send or receive data.

[pthatcher1, 2016/03/30 20:34:50]

Isn't this just for sending data (because each message gets its own stream)?

[mikescarlett, 2016/04/05 19:58:53]

Changed comment.

+  ReliableQuicStream* CreateQuicStream();
+
+  // Emitted when |quic_| creates a QUIC stream to receive incoming data from
+  // the remote peer, when the stream did not exist previously.
+  sigslot::signal1<ReliableQuicStream*> SignalIncomingStream;
+  // Emitted when the QuicTransportChannel state becomes QUIC_TRANSPORT_CLOSED.
+  // It might occur before this channel is writable if the handshake fails, or
+  // after a successful handshake when a peer explicitly closes the connection.
+  sigslot::signal0<> SignalClosed;
+
+  // Testing methods.
+  const TransportDescription* local_description_for_test() const {

[Taylor Brandstetter, 2016/04/01 23:23:42]

I don't like having "for test" methods that expose the internals of a class. Is
there a way to do without these?

[mikescarlett, 2016/04/05 19:58:53]

This class doesn't need these anyway since it no longer subclasses Transport.

+    return local_description();
+  }
+  const TransportDescription* remote_description_for_test() const {
+    return remote_description();
+  }
+
+ protected:
+  // Transport overrides.
+  bool NegotiateTransportDescription(ContentAction local_role,
+                                     std::string* error_desc) override;
+  // TODO(mikescarlett): Remove these methods once Transport does not require
+  // them. The QuicTransportChannel should be created/deleted directly.
+  TransportChannelImpl* CreateTransportChannel(int component) override {
+    return this;
+  }
+  void DestroyTransportChannel(TransportChannelImpl* channel) override {}
 
  private:
   // Fingerprint of remote peer.
@@ -240,6 +282,8 @@ class QuicTransportChannel : public TransportChannelImpl,
   void OnHandshakeComplete();
   // Called when |quic_| has closed the connection.
   void OnConnectionClosed(net::QuicErrorCode error, bool from_peer);
+  // Called when |quic_| has created a new QUIC stream for incoming data.
+  void OnIncomingStream(ReliableQuicStream* stream);
 
   // Called by OnReadPacket() when a QUIC packet is received.
   bool HandleQuicPacket(const char* data, size_t size);