diff --git a/webrtc/api/quicdatachannel.cc b/webrtc/api/quicdatachannel.cc index 5493382e1a..f4f573276c 100644 --- a/webrtc/api/quicdatachannel.cc +++ b/webrtc/api/quicdatachannel.cc @@ -61,12 +61,10 @@ bool ParseQuicDataMessageHeader(const char* data, QuicDataChannel::QuicDataChannel(rtc::Thread* signaling_thread, rtc::Thread* worker_thread, - rtc::Thread* network_thread, const std::string& label, const DataChannelInit& config) : signaling_thread_(signaling_thread), worker_thread_(worker_thread), - network_thread_(network_thread), id_(config.id), state_(kConnecting), buffered_amount_(0), @@ -93,12 +91,12 @@ bool QuicDataChannel::Send(const DataBuffer& buffer) { << " is not open so cannot send."; return false; } - return network_thread_->Invoke( - RTC_FROM_HERE, rtc::Bind(&QuicDataChannel::Send_n, this, buffer)); + return worker_thread_->Invoke( + RTC_FROM_HERE, rtc::Bind(&QuicDataChannel::Send_w, this, buffer)); } -bool QuicDataChannel::Send_n(const DataBuffer& buffer) { - RTC_DCHECK(network_thread_->IsCurrent()); +bool QuicDataChannel::Send_w(const DataBuffer& buffer) { + RTC_DCHECK(worker_thread_->IsCurrent()); // Encode and send the header containing the data channel ID and message ID. rtc::CopyOnWriteBuffer header; @@ -258,7 +256,7 @@ DataChannelInterface::DataState QuicDataChannel::SetTransportChannel_w() { } void QuicDataChannel::OnIncomingMessage(Message&& message) { - RTC_DCHECK(network_thread_->IsCurrent()); + RTC_DCHECK(worker_thread_->IsCurrent()); RTC_DCHECK(message.stream); if (!observer_) { LOG(LS_WARNING) << "QUIC data channel " << id_ @@ -297,7 +295,7 @@ void QuicDataChannel::OnIncomingMessage(Message&& message) { void QuicDataChannel::OnDataReceived(net::QuicStreamId stream_id, const char* data, size_t len) { - RTC_DCHECK(network_thread_->IsCurrent()); + RTC_DCHECK(worker_thread_->IsCurrent()); RTC_DCHECK(data); const auto& kv = incoming_quic_messages_.find(stream_id); if (kv == incoming_quic_messages_.end()) { @@ -327,7 +325,7 @@ void QuicDataChannel::OnDataReceived(net::QuicStreamId stream_id, } void QuicDataChannel::OnReadyToSend(cricket::TransportChannel* channel) { - RTC_DCHECK(network_thread_->IsCurrent()); + RTC_DCHECK(worker_thread_->IsCurrent()); RTC_DCHECK(channel == quic_transport_channel_); LOG(LS_INFO) << "QuicTransportChannel is ready to send"; invoker_.AsyncInvoke( @@ -344,7 +342,7 @@ void QuicDataChannel::OnWriteBlockedStreamClosed(net::QuicStreamId stream_id, void QuicDataChannel::OnIncomingQueuedStreamClosed(net::QuicStreamId stream_id, int error) { - RTC_DCHECK(network_thread_->IsCurrent()); + RTC_DCHECK(worker_thread_->IsCurrent()); LOG(LS_VERBOSE) << "Incoming queued stream " << stream_id << " is closed."; incoming_quic_messages_.erase(stream_id); } diff --git a/webrtc/api/quicdatachannel.h b/webrtc/api/quicdatachannel.h index 18a10acbdf..a6b987b144 100644 --- a/webrtc/api/quicdatachannel.h +++ b/webrtc/api/quicdatachannel.h @@ -88,7 +88,6 @@ class QuicDataChannel : public rtc::RefCountedObject, QuicDataChannel(rtc::Thread* signaling_thread, rtc::Thread* worker_thread, - rtc::Thread* network_thread, const std::string& label, const DataChannelInit& config); ~QuicDataChannel() override; @@ -156,13 +155,11 @@ class QuicDataChannel : public rtc::RefCountedObject, void OnReadyToSend(cricket::TransportChannel* channel); void OnConnectionClosed(); - // Network thread methods. + // Worker thread methods. // Sends the data buffer to the remote peer using an outgoing QUIC stream. // Returns true if the data buffer can be successfully sent, or if it is // queued to be sent later. - bool Send_n(const DataBuffer& buffer); - - // Worker thread methods. + bool Send_w(const DataBuffer& buffer); // Connects the |quic_transport_channel_| signals to this QuicDataChannel, // then returns the new QuicDataChannel state. DataState SetTransportChannel_w(); @@ -188,10 +185,8 @@ class QuicDataChannel : public rtc::RefCountedObject, cricket::QuicTransportChannel* quic_transport_channel_ = nullptr; // Signaling thread for DataChannelInterface methods. rtc::Thread* const signaling_thread_; - // Worker thread for |quic_transport_channel_| callbacks. + // Worker thread for sending data and |quic_transport_channel_| callbacks. rtc::Thread* const worker_thread_; - // Network thread for sending data and |quic_transport_channel_| callbacks. - rtc::Thread* const network_thread_; rtc::AsyncInvoker invoker_; // Map of QUIC stream ID => ReliableQuicStream* for write blocked QUIC // streams. diff --git a/webrtc/api/quicdatachannel_unittest.cc b/webrtc/api/quicdatachannel_unittest.cc index 7245ccfa21..e701c29b4f 100644 --- a/webrtc/api/quicdatachannel_unittest.cc +++ b/webrtc/api/quicdatachannel_unittest.cc @@ -120,9 +120,8 @@ class FakeQuicDataTransport : public sigslot::has_slots<> { DataChannelInit config; config.id = id; config.protocol = protocol; - rtc::scoped_refptr data_channel( - new QuicDataChannel(rtc::Thread::Current(), rtc::Thread::Current(), - rtc::Thread::Current(), label, config)); + rtc::scoped_refptr data_channel(new QuicDataChannel( + rtc::Thread::Current(), rtc::Thread::Current(), label, config)); data_channel_by_id_[id] = data_channel; return data_channel; } @@ -202,6 +201,8 @@ class QuicDataChannelPeer { // Connects |ice_transport_channel_| to that of the other peer. void Connect(QuicDataChannelPeer* other_peer) { + ice_transport_channel_->Connect(); + other_peer->ice_transport_channel_->Connect(); ice_transport_channel_->SetDestination(other_peer->ice_transport_channel_); } diff --git a/webrtc/api/quicdatatransport.cc b/webrtc/api/quicdatatransport.cc index c1caf54067..70ad03dbfd 100644 --- a/webrtc/api/quicdatatransport.cc +++ b/webrtc/api/quicdatatransport.cc @@ -17,14 +17,10 @@ namespace webrtc { QuicDataTransport::QuicDataTransport(rtc::Thread* signaling_thread, - rtc::Thread* worker_thread, - rtc::Thread* network_thread) - : signaling_thread_(signaling_thread), - worker_thread_(worker_thread), - network_thread_(network_thread) { + rtc::Thread* worker_thread) + : signaling_thread_(signaling_thread), worker_thread_(worker_thread) { RTC_DCHECK(signaling_thread_); RTC_DCHECK(worker_thread_); - RTC_DCHECK(network_thread_); } QuicDataTransport::~QuicDataTransport() {} @@ -72,8 +68,8 @@ rtc::scoped_refptr QuicDataTransport::CreateDataChannel( LOG(LS_ERROR) << "QUIC data channel already exists with id " << config->id; return nullptr; } - rtc::scoped_refptr data_channel(new QuicDataChannel( - signaling_thread_, worker_thread_, network_thread_, label, *config)); + rtc::scoped_refptr data_channel( + new QuicDataChannel(signaling_thread_, worker_thread_, label, *config)); if (quic_transport_channel_) { if (!data_channel->SetTransportChannel(quic_transport_channel_)) { LOG(LS_ERROR) diff --git a/webrtc/api/quicdatatransport.h b/webrtc/api/quicdatatransport.h index 96fe2a0ad7..f0c427d1b5 100644 --- a/webrtc/api/quicdatatransport.h +++ b/webrtc/api/quicdatatransport.h @@ -36,9 +36,7 @@ namespace webrtc { // exists, it sends the QUIC stream to the QuicDataChannel. class QuicDataTransport : public sigslot::has_slots<> { public: - QuicDataTransport(rtc::Thread* signaling_thread, - rtc::Thread* worker_thread, - rtc::Thread* network_thread); + QuicDataTransport(rtc::Thread* signaling_thread, rtc::Thread* worker_thread); ~QuicDataTransport() override; // Sets the QUIC transport channel for the QuicDataChannels and the @@ -82,10 +80,9 @@ class QuicDataTransport : public sigslot::has_slots<> { quic_stream_by_id_; // QuicTransportChannel for sending/receiving data. cricket::QuicTransportChannel* quic_transport_channel_ = nullptr; - // Threads for the QUIC data channel. + // Signaling and worker threads for the QUIC data channel. rtc::Thread* const signaling_thread_; rtc::Thread* const worker_thread_; - rtc::Thread* const network_thread_; }; } // namespace webrtc diff --git a/webrtc/api/quicdatatransport_unittest.cc b/webrtc/api/quicdatatransport_unittest.cc index 975898ef1f..d668c55b0b 100644 --- a/webrtc/api/quicdatatransport_unittest.cc +++ b/webrtc/api/quicdatatransport_unittest.cc @@ -64,9 +64,7 @@ class FakeObserver : public DataChannelObserver { class QuicDataTransportPeer { public: QuicDataTransportPeer() - : quic_data_transport_(rtc::Thread::Current(), - rtc::Thread::Current(), - rtc::Thread::Current()), + : quic_data_transport_(rtc::Thread::Current(), rtc::Thread::Current()), ice_transport_channel_(new FakeTransportChannel("data", 0)), quic_transport_channel_(ice_transport_channel_) { ice_transport_channel_->SetAsync(true); @@ -82,6 +80,8 @@ class QuicDataTransportPeer { // Connects |ice_transport_channel_| to that of the other peer. void Connect(QuicDataTransportPeer* other_peer) { + ice_transport_channel_->Connect(); + other_peer->ice_transport_channel_->Connect(); ice_transport_channel_->SetDestination(other_peer->ice_transport_channel_); } diff --git a/webrtc/p2p/quic/quicsession_unittest.cc b/webrtc/p2p/quic/quicsession_unittest.cc index 65996e5c1a..2f3aaae332 100644 --- a/webrtc/p2p/quic/quicsession_unittest.cc +++ b/webrtc/p2p/quic/quicsession_unittest.cc @@ -295,6 +295,8 @@ void QuicSessionTest::CreateClientAndServerSessions() { channel2->SetAsync(true); // Configure peers to send packets to each other. + channel1->Connect(); + channel2->Connect(); channel1->SetDestination(channel2.get()); client_peer_ = CreateSession(std::move(channel1), Perspective::IS_CLIENT); diff --git a/webrtc/p2p/quic/quictransportchannel.cc b/webrtc/p2p/quic/quictransportchannel.cc index 29819c6269..e86fe6a0b2 100644 --- a/webrtc/p2p/quic/quictransportchannel.cc +++ b/webrtc/p2p/quic/quictransportchannel.cc @@ -395,8 +395,7 @@ void QuicTransportChannel::OnRouteChange(TransportChannel* channel, void QuicTransportChannel::OnSelectedCandidatePairChanged( TransportChannel* channel, CandidatePairInterface* selected_candidate_pair, - int last_sent_packet_id, - bool ready_to_send) { + int last_sent_packet_id bool ready_to_send) { ASSERT(channel == channel_.get()); SignalSelectedCandidatePairChanged(this, selected_candidate_pair, last_sent_packet_id, ready_to_send); diff --git a/webrtc/p2p/quic/quictransportchannel.h b/webrtc/p2p/quic/quictransportchannel.h index 1ab13fa0b2..22a33eaf0f 100644 --- a/webrtc/p2p/quic/quictransportchannel.h +++ b/webrtc/p2p/quic/quictransportchannel.h @@ -166,6 +166,9 @@ class QuicTransportChannel : public TransportChannelImpl, void SetIceConfig(const IceConfig& config) override { channel_->SetIceConfig(config); } + void Connect() override { + channel_->Connect(); + } // QuicPacketWriter overrides. // Called from net::QuicConnection when |quic_| has packets to write. diff --git a/webrtc/p2p/quic/quictransportchannel_unittest.cc b/webrtc/p2p/quic/quictransportchannel_unittest.cc index 49ca29cd14..0e16390a89 100644 --- a/webrtc/p2p/quic/quictransportchannel_unittest.cc +++ b/webrtc/p2p/quic/quictransportchannel_unittest.cc @@ -112,6 +112,8 @@ class QuicTestPeer : public sigslot::has_slots<> { // Connects |ice_channel_| to that of the other peer. void Connect(QuicTestPeer* other_peer) { + ice_channel_->Connect(); + other_peer->ice_channel_->Connect(); ice_channel_->SetDestination(other_peer->ice_channel_); } @@ -417,6 +419,8 @@ TEST_F(QuicTransportChannelTest, TransferInvalidSrtp) { // Test that QuicTransportChannel::WritePacket blocks when the ICE // channel is not writable, and otherwise succeeds. TEST_F(QuicTransportChannelTest, QuicWritePacket) { + peer1_.ice_channel()->Connect(); + peer2_.ice_channel()->Connect(); peer1_.ice_channel()->SetDestination(peer2_.ice_channel()); std::string packet = "FAKEQUICPACKET"; diff --git a/webrtc/p2p/quic/reliablequicstream_unittest.cc b/webrtc/p2p/quic/reliablequicstream_unittest.cc index ff517afb58..cf9f5e92dd 100644 --- a/webrtc/p2p/quic/reliablequicstream_unittest.cc +++ b/webrtc/p2p/quic/reliablequicstream_unittest.cc @@ -49,7 +49,6 @@ using rtc::SR_BLOCK; // Arbitrary number for a stream's write blocked priority. static const SpdyPriority kDefaultPriority = 3; -static const net::QuicStreamId kStreamId = 5; // QuicSession that does not create streams and writes data from // ReliableQuicStream to a string. @@ -79,7 +78,7 @@ class MockQuicSession : public QuicSession { net::ReliableQuicStream* CreateIncomingDynamicStream( QuicStreamId id) override { - return new ReliableQuicStream(kStreamId, this); + return nullptr; } net::ReliableQuicStream* CreateOutgoingDynamicStream( @@ -143,6 +142,7 @@ class ReliableQuicStreamTest : public ::testing::Test, ReliableQuicStreamTest() {} void CreateReliableQuicStream() { + const net::QuicStreamId kStreamId = 5; // Arbitrary values for QuicConnection. QuicConnectionHelper* quic_helper = @@ -232,7 +232,7 @@ TEST_F(ReliableQuicStreamTest, BufferData) { // Read an entire string. TEST_F(ReliableQuicStreamTest, ReadDataWhole) { CreateReliableQuicStream(); - net::QuicStreamFrame frame(kStreamId, false, 0, "Hello, World!"); + net::QuicStreamFrame frame(-1, false, 0, "Hello, World!"); stream_->OnStreamFrame(frame); EXPECT_EQ("Hello, World!", read_buffer_); @@ -241,7 +241,7 @@ TEST_F(ReliableQuicStreamTest, ReadDataWhole) { // Read part of a string. TEST_F(ReliableQuicStreamTest, ReadDataPartial) { CreateReliableQuicStream(); - net::QuicStreamFrame frame(kStreamId, false, 0, "Hello, World!"); + net::QuicStreamFrame frame(-1, false, 0, "Hello, World!"); frame.frame_length = 5; stream_->OnStreamFrame(frame);