From f53776849710d511a97ea39ee17e4437aaaef85d Mon Sep 17 00:00:00 2001 From: mikescarlett Date: Tue, 29 Mar 2016 12:14:55 -0700 Subject: [PATCH] Update QuicTransportChannel to latest version of libquic This CL integrates recent Chromium changes that were merged into https://github.com/devsisters/libquic/tree/master. It also performs minor cleanup, such as removing accidental usage of Chromium logging symbols (e.g. LOG(INFO), LOG(ERROR) instead of LOG(LS_INFO), LOG(LS_ERROR)) and using Chromium's scoped_ptr instead of rtc::scoped_ptr. BUG= Review URL: https://codereview.webrtc.org/1834233002 Cr-Commit-Position: refs/heads/master@{#12146} --- webrtc/p2p/quic/quicconnectionhelper.cc | 22 ++++- webrtc/p2p/quic/quicconnectionhelper.h | 10 +- .../p2p/quic/quicconnectionhelper_unittest.cc | 8 +- webrtc/p2p/quic/quicsession.cc | 12 ++- webrtc/p2p/quic/quicsession.h | 10 +- webrtc/p2p/quic/quicsession_unittest.cc | 92 ++++++++--------- webrtc/p2p/quic/quictransportchannel.cc | 99 ++++++++++--------- webrtc/p2p/quic/quictransportchannel.h | 9 +- .../p2p/quic/quictransportchannel_unittest.cc | 10 +- webrtc/p2p/quic/reliablequicstream.h | 1 - .../p2p/quic/reliablequicstream_unittest.cc | 36 +++---- 11 files changed, 164 insertions(+), 145 deletions(-) diff --git a/webrtc/p2p/quic/quicconnectionhelper.cc b/webrtc/p2p/quic/quicconnectionhelper.cc index af5b4b213b..f9b4697d50 100644 --- a/webrtc/p2p/quic/quicconnectionhelper.cc +++ b/webrtc/p2p/quic/quicconnectionhelper.cc @@ -14,13 +14,21 @@ namespace cricket { QuicAlarm* QuicConnectionHelper::CreateAlarm( net::QuicAlarm::Delegate* delegate) { - return new QuicAlarm(GetClock(), thread_, delegate); + return new QuicAlarm(GetClock(), thread_, + net::QuicArenaScopedPtr(delegate)); +} + +net::QuicArenaScopedPtr QuicConnectionHelper::CreateAlarm( + net::QuicArenaScopedPtr delegate, + net::QuicConnectionArena* arena) { + return net::QuicArenaScopedPtr( + new QuicAlarm(GetClock(), thread_, std::move(delegate))); } QuicAlarm::QuicAlarm(const net::QuicClock* clock, rtc::Thread* thread, - QuicAlarm::Delegate* delegate) - : net::QuicAlarm(delegate), clock_(clock), thread_(thread) {} + net::QuicArenaScopedPtr delegate) + : net::QuicAlarm(std::move(delegate)), clock_(clock), thread_(thread) {} QuicAlarm::~QuicAlarm() {} @@ -39,7 +47,7 @@ void QuicAlarm::OnMessage(rtc::Message* msg) { Fire(); } -int64 QuicAlarm::GetDelay() const { +int64_t QuicAlarm::GetDelay() const { return deadline().Subtract(clock_->Now()).ToMilliseconds(); } @@ -47,7 +55,7 @@ void QuicAlarm::SetImpl() { DCHECK(deadline().IsInitialized()); CancelImpl(); // Unregister if already posted. - int64 delay_ms = GetDelay(); + int64_t delay_ms = GetDelay(); if (delay_ms < 0) { delay_ms = 0; } @@ -71,4 +79,8 @@ net::QuicRandom* QuicConnectionHelper::GetRandomGenerator() { return net::QuicRandom::GetInstance(); } +net::QuicBufferAllocator* QuicConnectionHelper::GetBufferAllocator() { + return &buffer_allocator_; +} + } // namespace cricket diff --git a/webrtc/p2p/quic/quicconnectionhelper.h b/webrtc/p2p/quic/quicconnectionhelper.h index 02691727e5..3588ad4226 100644 --- a/webrtc/p2p/quic/quicconnectionhelper.h +++ b/webrtc/p2p/quic/quicconnectionhelper.h @@ -15,6 +15,7 @@ #include "net/quic/quic_alarm.h" #include "net/quic/quic_clock.h" #include "net/quic/quic_connection.h" +#include "net/quic/quic_simple_buffer_allocator.h" #include "webrtc/base/thread.h" namespace cricket { @@ -25,7 +26,7 @@ class QuicAlarm : public net::QuicAlarm, public rtc::MessageHandler { public: QuicAlarm(const net::QuicClock* clock, rtc::Thread* thread, - QuicAlarm::Delegate* delegate); + net::QuicArenaScopedPtr delegate); ~QuicAlarm() override; @@ -33,7 +34,7 @@ class QuicAlarm : public net::QuicAlarm, public rtc::MessageHandler { void OnMessage(rtc::Message* msg) override; // Helper method to get the delay in ms for posting task. - int64 GetDelay() const; + int64_t GetDelay() const; protected: // net::QuicAlarm overrides. @@ -55,9 +56,14 @@ class QuicConnectionHelper : public net::QuicConnectionHelperInterface { const net::QuicClock* GetClock() const override; net::QuicRandom* GetRandomGenerator() override; QuicAlarm* CreateAlarm(net::QuicAlarm::Delegate* delegate) override; + net::QuicArenaScopedPtr CreateAlarm( + net::QuicArenaScopedPtr delegate, + net::QuicConnectionArena* arena) override; + net::QuicBufferAllocator* GetBufferAllocator() override; private: net::QuicClock clock_; + net::SimpleBufferAllocator buffer_allocator_; rtc::Thread* thread_; }; diff --git a/webrtc/p2p/quic/quicconnectionhelper_unittest.cc b/webrtc/p2p/quic/quicconnectionhelper_unittest.cc index 3d2dee5b2f..1a7313c245 100644 --- a/webrtc/p2p/quic/quicconnectionhelper_unittest.cc +++ b/webrtc/p2p/quic/quicconnectionhelper_unittest.cc @@ -12,6 +12,7 @@ #include "net/quic/quic_time.h" #include "webrtc/base/gunit.h" +#include "webrtc/base/scoped_ptr.h" using cricket::QuicAlarm; using cricket::QuicConnectionHelper; @@ -67,7 +68,10 @@ class QuicAlarmTest : public ::testing::Test { public: QuicAlarmTest() : delegate_(new MockAlarmDelegate()), - alarm_(new QuicAlarm(&clock_, rtc::Thread::Current(), delegate_)) {} + alarm_(new QuicAlarm( + &clock_, + rtc::Thread::Current(), + net::QuicArenaScopedPtr(delegate_))) {} // Make the alarm fire after the given microseconds (us). Negative values // imply the alarm should fire immediately. @@ -85,7 +89,7 @@ class QuicAlarmTest : public ::testing::Test { // Used for setting clock time relative to alarm. MockClock clock_; - scoped_ptr alarm_; + rtc::scoped_ptr alarm_; }; // Test that the alarm is fired. diff --git a/webrtc/p2p/quic/quicsession.cc b/webrtc/p2p/quic/quicsession.cc index 4197528703..281af5e3a7 100644 --- a/webrtc/p2p/quic/quicsession.cc +++ b/webrtc/p2p/quic/quicsession.cc @@ -20,7 +20,7 @@ namespace cricket { -QuicSession::QuicSession(scoped_ptr connection, +QuicSession::QuicSession(rtc::scoped_ptr connection, const net::QuicConfig& config) : net::QuicSession(connection.release(), config) {} @@ -54,7 +54,7 @@ bool QuicSession::ExportKeyingMaterial(base::StringPiece label, void QuicSession::OnCryptoHandshakeEvent(CryptoHandshakeEvent event) { net::QuicSession::OnCryptoHandshakeEvent(event); if (event == HANDSHAKE_CONFIRMED) { - LOG(INFO) << "QuicSession handshake complete"; + LOG(LS_INFO) << "QuicSession handshake complete"; RTC_DCHECK(IsEncryptionEstablished()); RTC_DCHECK(IsCryptoHandshakeConfirmed()); @@ -88,9 +88,11 @@ ReliableQuicStream* QuicSession::CreateDataStream(net::QuicStreamId id) { return new ReliableQuicStream(id, this); } -void QuicSession::OnConnectionClosed(net::QuicErrorCode error, bool from_peer) { - net::QuicSession::OnConnectionClosed(error, from_peer); - SignalConnectionClosed(error, from_peer); +void QuicSession::OnConnectionClosed(net::QuicErrorCode error, + net::ConnectionCloseSource source) { + net::QuicSession::OnConnectionClosed(error, source); + SignalConnectionClosed(error, + source == net::ConnectionCloseSource::FROM_PEER); } bool QuicSession::OnReadPacket(const char* data, size_t data_len) { diff --git a/webrtc/p2p/quic/quicsession.h b/webrtc/p2p/quic/quicsession.h index 689d43709e..e0ea296b59 100644 --- a/webrtc/p2p/quic/quicsession.h +++ b/webrtc/p2p/quic/quicsession.h @@ -29,7 +29,7 @@ namespace cricket { // reading/writing of data using QUIC packets. class QuicSession : public net::QuicSession, public sigslot::has_slots<> { public: - QuicSession(scoped_ptr connection, + QuicSession(rtc::scoped_ptr connection, const net::QuicConfig& config); ~QuicSession() override; @@ -43,9 +43,6 @@ class QuicSession : public net::QuicSession, public sigslot::has_slots<> { net::QuicCryptoStream* GetCryptoStream() override { return crypto_stream_.get(); } - // TODO(mikescarlett): Verify whether outgoing streams should be owned by - // caller. It appears these are deleted in the net::QuicSession destructor, - // but Chromium's documentation says this should not happen. ReliableQuicStream* CreateOutgoingDynamicStream( net::SpdyPriority priority) override; @@ -53,7 +50,8 @@ class QuicSession : public net::QuicSession, public sigslot::has_slots<> { void OnCryptoHandshakeEvent(CryptoHandshakeEvent event) override; // QuicConnectionVisitorInterface overrides. - void OnConnectionClosed(net::QuicErrorCode error, bool from_peer) override; + void OnConnectionClosed(net::QuicErrorCode error, + net::ConnectionCloseSource source) override; // Exports keying material for SRTP. bool ExportKeyingMaterial(base::StringPiece label, @@ -84,7 +82,7 @@ class QuicSession : public net::QuicSession, public sigslot::has_slots<> { virtual ReliableQuicStream* CreateDataStream(net::QuicStreamId id); private: - scoped_ptr crypto_stream_; + rtc::scoped_ptr crypto_stream_; RTC_DISALLOW_COPY_AND_ASSIGN(QuicSession); }; diff --git a/webrtc/p2p/quic/quicsession_unittest.cc b/webrtc/p2p/quic/quicsession_unittest.cc index 765a47f0c2..2e2d6ae0a6 100644 --- a/webrtc/p2p/quic/quicsession_unittest.cc +++ b/webrtc/p2p/quic/quicsession_unittest.cc @@ -28,8 +28,9 @@ #include "webrtc/p2p/quic/quicconnectionhelper.h" #include "webrtc/p2p/quic/reliablequicstream.h" -using net::IPAddressNumber; +using net::IPAddress; using net::IPEndPoint; +using net::PerPacketOptions; using net::Perspective; using net::ProofVerifyContext; using net::ProofVerifyDetails; @@ -60,15 +61,15 @@ using cricket::TransportChannel; using rtc::Thread; // Timeout for running asynchronous operations within unit tests. -const int kTimeoutMs = 1000; +static const int kTimeoutMs = 1000; // Testing SpdyPriority value for creating outgoing ReliableQuicStream. -const uint8 kDefaultPriority = 3; +static const uint8_t kDefaultPriority = 3; // TExport keying material function -const char kExporterLabel[] = "label"; -const char kExporterContext[] = "context"; -const size_t kExporterContextLen = sizeof(kExporterContext); +static const char kExporterLabel[] = "label"; +static const char kExporterContext[] = "context"; +static const size_t kExporterContextLen = sizeof(kExporterContext); // Identifies QUIC server session -const QuicServerId kServerId("www.google.com", 443); +static const QuicServerId kServerId("www.google.com", 443); // Used by QuicCryptoServerConfig to provide server credentials, returning a // canned response equal to |success|. @@ -77,20 +78,21 @@ class FakeProofSource : public net::ProofSource { explicit FakeProofSource(bool success) : success_(success) {} // ProofSource override. - bool GetProof(const net::IPAddressNumber& server_ip, + bool GetProof(const IPAddress& server_ip, const std::string& hostname, const std::string& server_config, + net::QuicVersion quic_version, + base::StringPiece chlo_hash, bool ecdsa_ok, - const std::vector** out_certs, + scoped_refptr* out_certs, std::string* out_signature, std::string* out_leaf_cert_sct) override { if (success_) { - std::vector* certs = new std::vector(); - certs->push_back("Required to establish handshake"); - std::string signature("Signature"); - - *out_certs = certs; - *out_signature = signature; + std::vector certs; + certs.push_back("Required to establish handshake"); + *out_certs = new ProofSource::Chain(certs); + *out_signature = "Signature"; + *out_leaf_cert_sct = "Time"; } return success_; } @@ -134,8 +136,9 @@ class FakeQuicPacketWriter : public QuicPacketWriter { // Sends packets across the network. WriteResult WritePacket(const char* buffer, size_t buf_len, - const IPAddressNumber& self_address, - const IPEndPoint& peer_address) override { + const IPAddress& self_address, + const IPEndPoint& peer_address, + PerPacketOptions* options) override { rtc::PacketOptions packet_options; int rv = fake_channel_->SendPacket(buffer, buf_len, packet_options, 0); net::WriteStatus status; @@ -173,26 +176,12 @@ class FakeQuicPacketWriter : public QuicPacketWriter { FakeTransportChannel* fake_channel_; }; -// Creates a FakePacketWriter for a given QuicConnection instance. -class FakePacketWriterFactory : public QuicConnection::PacketWriterFactory { - public: - explicit FakePacketWriterFactory(FakeTransportChannel* channel) - : channel_(channel) {} - - QuicPacketWriter* Create(QuicConnection* connection) const override { - return new FakeQuicPacketWriter(channel_); - } - - private: - FakeTransportChannel* channel_; -}; - // Wrapper for QuicSession and transport channel that stores incoming data. class QuicSessionForTest : public QuicSession { public: - QuicSessionForTest(scoped_ptr connection, + QuicSessionForTest(rtc::scoped_ptr connection, const net::QuicConfig& config, - scoped_ptr channel) + rtc::scoped_ptr channel) : QuicSession(std::move(connection), config), channel_(std::move(channel)) { channel_->SignalReadPacket.connect( @@ -230,7 +219,7 @@ class QuicSessionForTest : public QuicSession { private: // Transports QUIC packets to/from peer. - scoped_ptr channel_; + rtc::scoped_ptr channel_; // Stores data received by peer once it is sent from the other peer. std::string last_received_data_; // Handles incoming streams from sender. @@ -246,8 +235,8 @@ class QuicSessionTest : public ::testing::Test, // Instantiates |client_peer_| and |server_peer_|. void CreateClientAndServerSessions(); - scoped_ptr CreateSession( - scoped_ptr channel, + rtc::scoped_ptr CreateSession( + rtc::scoped_ptr channel, Perspective perspective); QuicCryptoClientStream* CreateCryptoClientStream(QuicSessionForTest* session, @@ -255,8 +244,9 @@ class QuicSessionTest : public ::testing::Test, QuicCryptoServerStream* CreateCryptoServerStream(QuicSessionForTest* session, bool handshake_success); - scoped_ptr CreateConnection(FakeTransportChannel* channel, - Perspective perspective); + rtc::scoped_ptr CreateConnection( + FakeTransportChannel* channel, + Perspective perspective); void StartHandshake(bool client_handshake_success, bool server_handshake_success); @@ -278,16 +268,16 @@ class QuicSessionTest : public ::testing::Test, QuicConfig config_; QuicClock clock_; - scoped_ptr client_peer_; - scoped_ptr server_peer_; + rtc::scoped_ptr client_peer_; + rtc::scoped_ptr server_peer_; }; // Initializes "client peer" who begins crypto handshake and "server peer" who // establishes encryption with client. void QuicSessionTest::CreateClientAndServerSessions() { - scoped_ptr channel1( + rtc::scoped_ptr channel1( new FakeTransportChannel("channel1", 0)); - scoped_ptr channel2( + rtc::scoped_ptr channel2( new FakeTransportChannel("channel2", 0)); // Prevent channel1->OnReadPacket and channel2->OnReadPacket from calling @@ -304,12 +294,12 @@ void QuicSessionTest::CreateClientAndServerSessions() { server_peer_ = CreateSession(std::move(channel2), Perspective::IS_SERVER); } -scoped_ptr QuicSessionTest::CreateSession( - scoped_ptr channel, +rtc::scoped_ptr QuicSessionTest::CreateSession( + rtc::scoped_ptr channel, Perspective perspective) { - scoped_ptr quic_connection = + rtc::scoped_ptr quic_connection = CreateConnection(channel.get(), perspective); - return scoped_ptr(new QuicSessionForTest( + return rtc::scoped_ptr(new QuicSessionForTest( std::move(quic_connection), config_, std::move(channel))); } @@ -336,16 +326,16 @@ QuicCryptoServerStream* QuicSessionTest::CreateCryptoServerStream( return new QuicCryptoServerStream(server_config, session); } -scoped_ptr QuicSessionTest::CreateConnection( +rtc::scoped_ptr QuicSessionTest::CreateConnection( FakeTransportChannel* channel, Perspective perspective) { - FakePacketWriterFactory writer_factory(channel); + FakeQuicPacketWriter* writer = new FakeQuicPacketWriter(channel); - IPAddressNumber ip(net::kIPv4AddressSize, 0); + IPAddress ip(0, 0, 0, 0); bool owns_writer = true; - return scoped_ptr(new QuicConnection( - 0, net::IPEndPoint(ip, 0), &quic_helper_, writer_factory, owns_writer, + return rtc::scoped_ptr(new QuicConnection( + 0, net::IPEndPoint(ip, 0), &quic_helper_, writer, owns_writer, perspective, net::QuicSupportedVersions())); } diff --git a/webrtc/p2p/quic/quictransportchannel.cc b/webrtc/p2p/quic/quictransportchannel.cc index eae936ba53..8c46fb0bf1 100644 --- a/webrtc/p2p/quic/quictransportchannel.cc +++ b/webrtc/p2p/quic/quictransportchannel.cc @@ -19,6 +19,7 @@ #include "net/quic/quic_connection.h" #include "net/quic/quic_crypto_client_stream.h" #include "net/quic/quic_crypto_server_stream.h" +#include "net/quic/quic_packet_writer.h" #include "net/quic/quic_protocol.h" #include "webrtc/base/checks.h" #include "webrtc/base/helpers.h" @@ -33,7 +34,7 @@ namespace { // given that |channel_| only receives packets specific to this channel, // in which case we already know the QUIC packets have the correct destination. const net::QuicConnectionId kConnectionId = 0; -const net::IPAddressNumber kConnectionIpAddress(net::kIPv4AddressSize, 0); +const net::IPAddress kConnectionIpAddress(0, 0, 0, 0); const net::IPEndPoint kConnectionIpEndpoint(kConnectionIpAddress, 0); // Arbitrary server port number for net::QuicCryptoClientConfig. @@ -73,21 +74,21 @@ class DummyProofSource : public net::ProofSource { ~DummyProofSource() override {} // ProofSource override. - bool GetProof(const net::IPAddressNumber& server_ip, + bool GetProof(const net::IPAddress& server_ip, const std::string& hostname, const std::string& server_config, + net::QuicVersion quic_version, + base::StringPiece chlo_hash, bool ecdsa_ok, - const std::vector** out_certs, + scoped_refptr* out_chain, std::string* out_signature, std::string* out_leaf_cert_sct) override { - LOG(INFO) << "GetProof() providing dummy credentials for insecure QUIC"; - std::vector* certs = new std::vector(); - certs->push_back("Dummy cert"); - std::string signature("Dummy signature"); - - *out_certs = certs; - *out_signature = signature; - + LOG(LS_INFO) << "GetProof() providing dummy credentials for insecure QUIC"; + std::vector certs; + certs.push_back("Dummy cert"); + *out_chain = new ProofSource::Chain(certs); + *out_signature = "Dummy signature"; + *out_leaf_cert_sct = "Dummy timestamp"; return true; } }; @@ -111,7 +112,7 @@ class InsecureProofVerifier : public net::ProofVerifier { std::string* error_details, scoped_ptr* verify_details, net::ProofVerifierCallback* callback) override { - LOG(INFO) << "VerifyProof() ignoring credentials and returning success"; + LOG(LS_INFO) << "VerifyProof() ignoring credentials and returning success"; return net::QUIC_SUCCESS; } }; @@ -157,7 +158,8 @@ QuicTransportChannel::~QuicTransportChannel() {} bool QuicTransportChannel::SetLocalCertificate( const rtc::scoped_refptr& certificate) { if (!certificate) { - LOG_J(ERROR, this) << "No local certificate was supplied. Not doing QUIC."; + LOG_J(LS_ERROR, this) + << "No local certificate was supplied. Not doing QUIC."; return false; } if (!local_certificate_) { @@ -166,11 +168,12 @@ bool QuicTransportChannel::SetLocalCertificate( } if (certificate == local_certificate_) { // This may happen during renegotiation. - LOG_J(INFO, this) << "Ignoring identical certificate"; + LOG_J(LS_INFO, this) << "Ignoring identical certificate"; return true; } - LOG_J(ERROR, this) << "Local certificate of the QUIC connection already set. " - "Can't change the local certificate once it's active."; + LOG_J(LS_ERROR, this) + << "Local certificate of the QUIC connection already set. " + "Can't change the local certificate once it's active."; return false; } @@ -181,14 +184,14 @@ QuicTransportChannel::GetLocalCertificate() const { bool QuicTransportChannel::SetSslRole(rtc::SSLRole role) { if (ssl_role_ && *ssl_role_ == role) { - LOG_J(WARNING, this) << "Ignoring SSL Role identical to current role."; + LOG_J(LS_WARNING, this) << "Ignoring SSL Role identical to current role."; return true; } if (quic_state_ != QUIC_TRANSPORT_CONNECTED) { ssl_role_ = rtc::Optional(role); return true; } - LOG_J(ERROR, this) + LOG_J(LS_ERROR, this) << "SSL Role can't be reversed after the session is setup."; return false; } @@ -206,7 +209,7 @@ bool QuicTransportChannel::SetRemoteFingerprint(const std::string& digest_alg, size_t digest_len) { if (digest_alg.empty()) { RTC_DCHECK(!digest_len); - LOG_J(ERROR, this) << "Remote peer doesn't support digest algorithm."; + LOG_J(LS_ERROR, this) << "Remote peer doesn't support digest algorithm."; return false; } std::string remote_fingerprint_value(reinterpret_cast(digest), @@ -216,7 +219,8 @@ bool QuicTransportChannel::SetRemoteFingerprint(const std::string& digest_alg, if (remote_fingerprint_ && remote_fingerprint_->value == remote_fingerprint_value && remote_fingerprint_->algorithm == digest_alg) { - LOG_J(INFO, this) << "Ignoring identical remote fingerprint and algorithm"; + LOG_J(LS_INFO, this) + << "Ignoring identical remote fingerprint and algorithm"; return true; } remote_fingerprint_ = rtc::Optional(RemoteFingerprint()); @@ -254,7 +258,7 @@ int QuicTransportChannel::SendPacket(const char* data, if ((flags & PF_SRTP_BYPASS) && IsRtpPacket(data, size)) { return channel_->SendPacket(data, size, options); } - LOG(ERROR) << "Failed to send an invalid SRTP bypass packet using QUIC."; + LOG(LS_ERROR) << "Failed to send an invalid SRTP bypass packet using QUIC."; return -1; } @@ -266,7 +270,7 @@ int QuicTransportChannel::SendPacket(const char* data, void QuicTransportChannel::OnWritableState(TransportChannel* channel) { ASSERT(rtc::Thread::Current() == worker_thread_); ASSERT(channel == channel_); - LOG_J(VERBOSE, this) + LOG_J(LS_VERBOSE, this) << "QuicTransportChannel: channel writable state changed to " << channel_->writable(); switch (quic_state_) { @@ -300,7 +304,7 @@ void QuicTransportChannel::OnWritableState(TransportChannel* channel) { void QuicTransportChannel::OnReceivingState(TransportChannel* channel) { ASSERT(rtc::Thread::Current() == worker_thread_); ASSERT(channel == channel_); - LOG_J(VERBOSE, this) + LOG_J(LS_VERBOSE, this) << "QuicTransportChannel: channel receiving state changed to " << channel_->receiving(); if (quic_state_ == QUIC_TRANSPORT_CONNECTED) { @@ -322,7 +326,7 @@ void QuicTransportChannel::OnReadPacket(TransportChannel* channel, case QUIC_TRANSPORT_NEW: // This would occur if other peer is ready to start QUIC but this peer // hasn't started QUIC. - LOG_J(INFO, this) << "Dropping packet received before QUIC started."; + LOG_J(LS_INFO, this) << "Dropping packet received before QUIC started."; break; case QUIC_TRANSPORT_CONNECTING: case QUIC_TRANSPORT_CONNECTED: @@ -330,13 +334,14 @@ void QuicTransportChannel::OnReadPacket(TransportChannel* channel, // Is this potentially a QUIC packet? if (IsQuicPacket(data, size)) { if (!HandleQuicPacket(data, size)) { - LOG_J(ERROR, this) << "Failed to handle QUIC packet."; + LOG_J(LS_ERROR, this) << "Failed to handle QUIC packet."; return; } } else { // If this is an RTP packet, signal upwards as a bypass packet. if (!IsRtpPacket(data, size)) { - LOG_J(ERROR, this) << "Received unexpected non-QUIC, non-RTP packet."; + LOG_J(LS_ERROR, this) + << "Received unexpected non-QUIC, non-RTP packet."; return; } SignalReadPacket(this, data, size, packet_time, PF_SRTP_BYPASS); @@ -389,19 +394,21 @@ void QuicTransportChannel::OnConnectionRemoved(TransportChannelImpl* channel) { bool QuicTransportChannel::MaybeStartQuic() { if (!channel_->writable()) { - LOG_J(ERROR, this) << "Couldn't start QUIC handshake."; + LOG_J(LS_ERROR, this) << "Couldn't start QUIC handshake."; return false; } if (!CreateQuicSession() || !StartQuicHandshake()) { - LOG_J(WARNING, this) << "Underlying channel is writable but cannot start " - "the QUIC handshake."; + LOG_J(LS_WARNING, this) + << "Underlying channel is writable but cannot start " + "the QUIC handshake."; return false; } // Verify connection is not closed due to QUIC bug or network failure. // A closed connection should not happen since |channel_| is writable. if (!quic_->connection()->connected()) { - LOG_J(ERROR, this) << "QUIC connection should not be closed if underlying " - "channel is writable."; + LOG_J(LS_ERROR, this) + << "QUIC connection should not be closed if underlying " + "channel is writable."; return false; } // Indicate that |quic_| is ready to receive QUIC packets. @@ -417,7 +424,7 @@ bool QuicTransportChannel::CreateQuicSession() { ? net::Perspective::IS_CLIENT : net::Perspective::IS_SERVER; bool owns_writer = false; - scoped_ptr connection(new net::QuicConnection( + rtc::scoped_ptr connection(new net::QuicConnection( kConnectionId, kConnectionIpEndpoint, &helper_, this, owns_writer, perspective, net::QuicSupportedVersions())); quic_.reset(new QuicSession(std::move(connection), config_)); @@ -442,7 +449,7 @@ bool QuicTransportChannel::StartQuicHandshake() { new net::ProofVerifyContext(), quic_crypto_client_config_.get(), this); quic_->StartClientHandshake(crypto_stream); - LOG_J(INFO, this) << "QuicTransportChannel: Started client handshake."; + LOG_J(LS_INFO, this) << "QuicTransportChannel: Started client handshake."; } else { RTC_DCHECK_EQ(*ssl_role_, rtc::SSL_SERVER); // Provide credentials to remote peer; owned by QuicCryptoServerConfig. @@ -454,7 +461,8 @@ bool QuicTransportChannel::StartQuicHandshake() { std::string source_address_token_secret; if (!rtc::CreateRandomString(kInputKeyingMaterialLength, &source_address_token_secret)) { - LOG_J(ERROR, this) << "Error generating input keying material for HKDF."; + LOG_J(LS_ERROR, this) + << "Error generating input keying material for HKDF."; return false; } quic_crypto_server_config_.reset(new net::QuicCryptoServerConfig( @@ -468,7 +476,7 @@ bool QuicTransportChannel::StartQuicHandshake() { new net::QuicCryptoServerStream(quic_crypto_server_config_.get(), quic_.get()); quic_->StartServerHandshake(crypto_stream); - LOG_J(INFO, this) << "QuicTransportChannel: Started server handshake."; + LOG_J(LS_INFO, this) << "QuicTransportChannel: Started server handshake."; } return true; } @@ -481,8 +489,9 @@ bool QuicTransportChannel::HandleQuicPacket(const char* data, size_t size) { net::WriteResult QuicTransportChannel::WritePacket( const char* buffer, size_t buf_len, - const net::IPAddressNumber& self_address, - const net::IPEndPoint& peer_address) { + const net::IPAddress& self_address, + const net::IPEndPoint& peer_address, + net::PerPacketOptions* options) { // QUIC should never call this if IsWriteBlocked, but just in case... if (IsWriteBlocked()) { return net::WriteResult(net::WRITE_STATUS_BLOCKED, EWOULDBLOCK); @@ -514,9 +523,9 @@ void QuicTransportChannel::OnHandshakeComplete() { void QuicTransportChannel::OnConnectionClosed(net::QuicErrorCode error, bool from_peer) { - LOG_J(INFO, this) << "Connection closed by " << (from_peer ? "other" : "this") - << " peer " - << "with QUIC error " << error; + LOG_J(LS_INFO, this) << "Connection closed by " + << (from_peer ? "other" : "this") << " peer " + << "with QUIC error " << error; // TODO(mikescarlett): Allow the QUIC session to be reset when the connection // does not close due to failure. set_quic_state(QUIC_TRANSPORT_CLOSED); @@ -525,13 +534,13 @@ void QuicTransportChannel::OnConnectionClosed(net::QuicErrorCode error, void QuicTransportChannel::OnProofValid( const net::QuicCryptoClientConfig::CachedState& cached) { - LOG_J(INFO, this) << "Cached proof marked valid"; + LOG_J(LS_INFO, this) << "Cached proof marked valid"; } void QuicTransportChannel::OnProofVerifyDetailsAvailable( const net::ProofVerifyDetails& verify_details) { - LOG_J(INFO, this) << "Proof verify details available from" - << " QuicCryptoClientStream"; + LOG_J(LS_INFO, this) << "Proof verify details available from" + << " QuicCryptoClientStream"; } bool QuicTransportChannel::HasDataToWrite() const { @@ -544,8 +553,8 @@ void QuicTransportChannel::OnCanWrite() { } void QuicTransportChannel::set_quic_state(QuicTransportState state) { - LOG_J(VERBOSE, this) << "set_quic_state from:" << quic_state_ << " to " - << state; + LOG_J(LS_VERBOSE, this) << "set_quic_state from:" << quic_state_ << " to " + << state; quic_state_ = state; } diff --git a/webrtc/p2p/quic/quictransportchannel.h b/webrtc/p2p/quic/quictransportchannel.h index f9001088e2..dff57957df 100644 --- a/webrtc/p2p/quic/quictransportchannel.h +++ b/webrtc/p2p/quic/quictransportchannel.h @@ -14,6 +14,7 @@ #include #include +#include "net/quic/quic_crypto_client_stream.h" #include "net/quic/quic_packet_writer.h" #include "webrtc/base/optional.h" #include "webrtc/base/scoped_ptr.h" @@ -154,6 +155,9 @@ class QuicTransportChannel : public TransportChannelImpl, void AddRemoteCandidate(const Candidate& candidate) override { channel_->AddRemoteCandidate(candidate); } + void RemoveRemoteCandidate(const Candidate& candidate) override { + channel_->RemoveRemoteCandidate(candidate); + } void SetIceConfig(const IceConfig& config) override { channel_->SetIceConfig(config); } @@ -165,8 +169,9 @@ class QuicTransportChannel : public TransportChannelImpl, // Called from net::QuicConnection when |quic_| has packets to write. net::WriteResult WritePacket(const char* buffer, size_t buf_len, - const net::IPAddressNumber& self_address, - const net::IPEndPoint& peer_address) override; + const net::IPAddress& self_address, + const net::IPEndPoint& peer_address, + net::PerPacketOptions* options) override; // Whether QuicTransportChannel buffers data when unable to write. If this is // set to false, then net::QuicConnection buffers unsent packets. bool IsWriteBlockedDataBuffered() const override { return false; } diff --git a/webrtc/p2p/quic/quictransportchannel_unittest.cc b/webrtc/p2p/quic/quictransportchannel_unittest.cc index c5c4d2012e..7b5b51848a 100644 --- a/webrtc/p2p/quic/quictransportchannel_unittest.cc +++ b/webrtc/p2p/quic/quictransportchannel_unittest.cc @@ -46,7 +46,7 @@ static const char kIceUfrag[] = "TESTICEUFRAG0001"; static const char kIcePwd[] = "TESTICEPWD00000000000001"; // QUIC packet parameters. -static const net::IPAddressNumber kIpAddress(net::kIPv4AddressSize, 0); +static const net::IPAddress kIpAddress(0, 0, 0, 0); static const net::IPEndPoint kIpEndpoint(kIpAddress, 0); // Detects incoming RTP packets. @@ -144,7 +144,7 @@ class QuicTestPeer : public sigslot::has_slots<> { if (!get_digest_algorithm || digest_algorithm.empty()) { return nullptr; } - scoped_ptr fingerprint( + rtc::scoped_ptr fingerprint( rtc::SSLFingerprint::Create(digest_algorithm, cert->identity())); if (digest_algorithm != rtc::DIGEST_SHA_256) { return nullptr; @@ -409,7 +409,7 @@ TEST_F(QuicTransportChannelTest, QuicWritePacket) { peer1_.ice_channel()->SetWritable(false); EXPECT_TRUE(peer1_.quic_channel()->IsWriteBlocked()); net::WriteResult write_blocked_result = peer1_.quic_channel()->WritePacket( - packet.data(), packet.size(), kIpAddress, kIpEndpoint); + packet.data(), packet.size(), kIpAddress, kIpEndpoint, nullptr); EXPECT_EQ(net::WRITE_STATUS_BLOCKED, write_blocked_result.status); EXPECT_EQ(EWOULDBLOCK, write_blocked_result.error_code); @@ -418,13 +418,13 @@ TEST_F(QuicTransportChannelTest, QuicWritePacket) { EXPECT_FALSE(peer1_.quic_channel()->IsWriteBlocked()); peer1_.SetWriteError(EWOULDBLOCK); net::WriteResult ignore_error_result = peer1_.quic_channel()->WritePacket( - packet.data(), packet.size(), kIpAddress, kIpEndpoint); + packet.data(), packet.size(), kIpAddress, kIpEndpoint, nullptr); EXPECT_EQ(net::WRITE_STATUS_OK, ignore_error_result.status); EXPECT_EQ(0, ignore_error_result.bytes_written); peer1_.SetWriteError(kNoWriteError); net::WriteResult no_error_result = peer1_.quic_channel()->WritePacket( - packet.data(), packet.size(), kIpAddress, kIpEndpoint); + packet.data(), packet.size(), kIpAddress, kIpEndpoint, nullptr); EXPECT_EQ(net::WRITE_STATUS_OK, no_error_result.status); EXPECT_EQ(static_cast(packet.size()), no_error_result.bytes_written); } diff --git a/webrtc/p2p/quic/reliablequicstream.h b/webrtc/p2p/quic/reliablequicstream.h index 33ed665b46..61d060f850 100644 --- a/webrtc/p2p/quic/reliablequicstream.h +++ b/webrtc/p2p/quic/reliablequicstream.h @@ -29,7 +29,6 @@ class ReliableQuicStream : public net::ReliableQuicStream, // ReliableQuicStream overrides. void OnDataAvailable() override; void OnClose() override; - net::SpdyPriority Priority() const override { return 0; } // Process decrypted data into encrypted QUIC packets, which get sent to the // QuicPacketWriter. rtc::SR_BLOCK is returned if the operation blocks instead diff --git a/webrtc/p2p/quic/reliablequicstream_unittest.cc b/webrtc/p2p/quic/reliablequicstream_unittest.cc index f9fb5cd7cd..aeb3e1af87 100644 --- a/webrtc/p2p/quic/reliablequicstream_unittest.cc +++ b/webrtc/p2p/quic/reliablequicstream_unittest.cc @@ -26,8 +26,9 @@ using cricket::QuicConnectionHelper; using cricket::ReliableQuicStream; using net::FecProtection; -using net::IPAddressNumber; +using net::IPAddress; using net::IPEndPoint; +using net::PerPacketOptions; using net::Perspective; using net::QuicAckListenerInterface; using net::QuicConfig; @@ -46,6 +47,9 @@ using net::SpdyPriority; using rtc::SR_SUCCESS; using rtc::SR_BLOCK; +// Arbitrary number for a stream's write blocked priority. +static const SpdyPriority kDefaultPriority = 3; + // QuicSession that does not create streams and writes data from // ReliableQuicStream to a string. class MockQuicSession : public QuicSession { @@ -115,8 +119,9 @@ class DummyPacketWriter : public QuicPacketWriter { // QuicPacketWriter overrides. virtual net::WriteResult WritePacket(const char* buffer, size_t buf_len, - const IPAddressNumber& self_address, - const IPEndPoint& peer_address) { + const IPAddress& self_address, + const IPEndPoint& peer_address, + PerPacketOptions* options) { return net::WriteResult(net::WRITE_STATUS_ERROR, 0); } @@ -132,17 +137,6 @@ class DummyPacketWriter : public QuicPacketWriter { } }; -// QuicPacketWriter is not necessary, so this creates a packet writer that -// doesn't do anything. -class DummyPacketWriterFactory : public QuicConnection::PacketWriterFactory { - public: - DummyPacketWriterFactory() {} - - QuicPacketWriter* Create(QuicConnection* connection) const override { - return new DummyPacketWriter(); - } -}; - class ReliableQuicStreamTest : public ::testing::Test, public sigslot::has_slots<> { public: @@ -155,13 +149,13 @@ class ReliableQuicStreamTest : public ::testing::Test, QuicConnectionHelper* quic_helper = new QuicConnectionHelper(rtc::Thread::Current()); Perspective perspective = Perspective::IS_SERVER; - net::IPAddressNumber ip(net::kIPv4AddressSize, 0); + net::IPAddress ip(0, 0, 0, 0); - bool owns_writer = false; + bool owns_writer = true; QuicConnection* connection = new QuicConnection( - 0, IPEndPoint(ip, 0), quic_helper, DummyPacketWriterFactory(), - owns_writer, perspective, net::QuicSupportedVersions()); + 0, IPEndPoint(ip, 0), quic_helper, new DummyPacketWriter(), owns_writer, + perspective, net::QuicSupportedVersions()); session_.reset( new MockQuicSession(connection, QuicConfig(), &write_buffer_)); @@ -170,7 +164,7 @@ class ReliableQuicStreamTest : public ::testing::Test, this, &ReliableQuicStreamTest::OnDataReceived); stream_->SignalClosed.connect(this, &ReliableQuicStreamTest::OnClosed); - session_->register_write_blocked_stream(stream_->id(), stream_->Priority()); + session_->register_write_blocked_stream(stream_->id(), kDefaultPriority); } void OnDataReceived(QuicStreamId id, const char* data, size_t length) { @@ -181,8 +175,8 @@ class ReliableQuicStreamTest : public ::testing::Test, void OnClosed(QuicStreamId id, QuicErrorCode err) { closed_ = true; } protected: - scoped_ptr stream_; - scoped_ptr session_; + rtc::scoped_ptr stream_; + rtc::scoped_ptr session_; // Data written by the ReliableQuicStream. std::string write_buffer_;