diff --git a/p2p/base/dtls_transport_unittest.cc b/p2p/base/dtls_transport_unittest.cc index ddf18746d8..93653c116f 100644 --- a/p2p/base/dtls_transport_unittest.cc +++ b/p2p/base/dtls_transport_unittest.cc @@ -108,8 +108,11 @@ class DtlsTestClient : public sigslot::has_slots<> { dtls_transport_->SetLocalCertificate(certificate_); dtls_transport_->SignalWritableState.connect( this, &DtlsTestClient::OnTransportWritableState); - dtls_transport_->SignalReadPacket.connect( - this, &DtlsTestClient::OnTransportReadPacket); + dtls_transport_->RegisterReceivedPacketCallback( + this, [&](rtc::PacketTransportInternal* transport, + const rtc::ReceivedPacket& packet) { + OnTransportReadPacket(transport, packet); + }); dtls_transport_->SignalSentPacket.connect( this, &DtlsTestClient::OnTransportSentPacket); } @@ -209,9 +212,11 @@ class DtlsTestClient : public sigslot::has_slots<> { size_t NumPacketsReceived() { return received_.size(); } // Inverse of SendPackets. - bool VerifyPacket(const uint8_t* data, size_t size, uint32_t* out_num) { - if (size != packet_size_ || - (data[0] != 0 && static_cast(data[0]) != 0x80)) { + bool VerifyPacket(rtc::ArrayView payload, uint32_t* out_num) { + const uint8_t* data = payload.data(); + size_t size = payload.size(); + + if (size != packet_size_ || (data[0] != 0 && (data[0]) != 0x80)) { return false; } uint32_t packet_num = rtc::GetBE32(data + kPacketNumOffset); @@ -248,18 +253,21 @@ class DtlsTestClient : public sigslot::has_slots<> { } void OnTransportReadPacket(rtc::PacketTransportInternal* transport, - const char* data, - size_t size, - const int64_t& /* packet_time_us */, - int flags) { + const rtc::ReceivedPacket& packet) { uint32_t packet_num = 0; - ASSERT_TRUE(VerifyPacket(reinterpret_cast(data), size, - &packet_num)); + ASSERT_TRUE(VerifyPacket(packet.payload(), &packet_num)); received_.insert(packet_num); - // Only DTLS-SRTP packets should have the bypass flag set. - int expected_flags = - (certificate_ && IsRtpLeadByte(data[0])) ? PF_SRTP_BYPASS : 0; - ASSERT_EQ(expected_flags, flags); + switch (packet.decryption_info()) { + case rtc::ReceivedPacket::kSrtpEncrypted: + ASSERT_TRUE(certificate_ && IsRtpLeadByte(packet.payload()[0])); + break; + case rtc::ReceivedPacket::kDtlsDecrypted: + ASSERT_TRUE(certificate_ && !IsRtpLeadByte(packet.payload()[0])); + break; + case rtc::ReceivedPacket::kNotDecrypted: + ASSERT_FALSE(certificate_); + break; + } } void OnTransportSentPacket(rtc::PacketTransportInternal* transport, @@ -291,7 +299,7 @@ class DtlsTestClient : public sigslot::has_slots<> { if (data[0] == 23) { ASSERT_TRUE(VerifyEncryptedPacket(data, size)); } else if (IsRtpLeadByte(data[0])) { - ASSERT_TRUE(VerifyPacket(data, size, NULL)); + ASSERT_TRUE(VerifyPacket(packet.payload(), NULL)); } } } diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 3109088285..69ea4287e8 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -478,8 +478,11 @@ class P2PTransportChannelTestBase : public ::testing::Test, [this](IceTransportInternal* transport, const Candidates& candidates) { OnCandidatesRemoved(transport, candidates); }); - channel->SignalReadPacket.connect( - this, &P2PTransportChannelTestBase::OnReadPacket); + channel->RegisterReceivedPacketCallback( + this, [&](rtc::PacketTransportInternal* transport, + const rtc::ReceivedPacket& packet) { + OnReadPacket(transport, packet); + }); channel->SignalRoleConflict.connect( this, &P2PTransportChannelTestBase::OnRoleConflict); channel->SignalNetworkRouteChanged.connect( @@ -917,12 +920,11 @@ class P2PTransportChannelTestBase : public ::testing::Test, } void OnReadPacket(rtc::PacketTransportInternal* transport, - const char* data, - size_t len, - const int64_t& /* packet_time_us */, - int flags) { + const rtc::ReceivedPacket& packet) { std::list& packets = GetPacketList(transport); - packets.push_front(std::string(data, len)); + packets.push_front( + std::string(reinterpret_cast(packet.payload().data()), + packet.payload().size())); } void OnRoleConflict(IceTransportInternal* channel) { diff --git a/p2p/base/packet_transport_internal.cc b/p2p/base/packet_transport_internal.cc index cbe8c55eb3..4d82501b71 100644 --- a/p2p/base/packet_transport_internal.cc +++ b/p2p/base/packet_transport_internal.cc @@ -43,20 +43,7 @@ void PacketTransportInternal::DeregisterReceivedPacketCallback(void* id) { void PacketTransportInternal::NotifyPacketReceived( const rtc::ReceivedPacket& packet) { RTC_DCHECK_RUN_ON(&network_checker_); - if (!SignalReadPacket.is_empty()) { - // TODO(bugs.webrtc.org:15368): Replace with - // received_packet_callbacklist_. - int flags = 0; - if (packet.decryption_info() == rtc::ReceivedPacket::kSrtpEncrypted) { - flags = 1; - } - SignalReadPacket( - this, reinterpret_cast(packet.payload().data()), - packet.payload().size(), - packet.arrival_time() ? packet.arrival_time()->us() : -1, flags); - } else { - received_packet_callback_list_.Send(this, packet); - } + received_packet_callback_list_.Send(this, packet); } } // namespace rtc diff --git a/p2p/base/packet_transport_internal.h b/p2p/base/packet_transport_internal.h index b7581729f4..98c37ab385 100644 --- a/p2p/base/packet_transport_internal.h +++ b/p2p/base/packet_transport_internal.h @@ -90,18 +90,6 @@ class RTC_EXPORT PacketTransportInternal : public sigslot::has_slots<> { void DeregisterReceivedPacketCallback(void* id); - // Signalled each time a packet is received on this channel. - // TODO(bugs.webrtc.org:15368): Deprecate and remove. Replace with - // RegisterReceivedPacketCallback. - sigslot::signal5 - SignalReadPacket; - // Signalled each time a packet is sent on this channel. sigslot::signal2 SignalSentPacket; diff --git a/p2p/base/packet_transport_internal_unittest.cc b/p2p/base/packet_transport_internal_unittest.cc index f17e43f62e..1c9df852d1 100644 --- a/p2p/base/packet_transport_internal_unittest.cc +++ b/p2p/base/packet_transport_internal_unittest.cc @@ -20,65 +20,8 @@ namespace { using ::testing::MockFunction; -class SigslotPacketReceiver : public sigslot::has_slots<> { - public: - bool packet_received() const { return packet_received_; } - - void OnPacketReceived(rtc::PacketTransportInternal*, - const char*, - size_t, - const int64_t&, - int flags) { - packet_received_ = true; - flags_ = flags; - } - - bool packet_received_ = false; - int flags_ = -1; -}; - TEST(PacketTransportInternal, - PacketFlagsCorrectWithUnDecryptedPacketUsingSigslot) { - rtc::FakePacketTransport packet_transport("test"); - SigslotPacketReceiver receiver; - packet_transport.SignalReadPacket.connect( - &receiver, &SigslotPacketReceiver::OnPacketReceived); - - packet_transport.NotifyPacketReceived( - rtc::ReceivedPacket({}, rtc::SocketAddress(), absl::nullopt, - rtc::ReceivedPacket::kNotDecrypted)); - ASSERT_TRUE(receiver.packet_received_); - EXPECT_EQ(receiver.flags_, 0); -} - -TEST(PacketTransportInternal, PacketFlagsCorrectWithSrtpPacketUsingSigslot) { - rtc::FakePacketTransport packet_transport("test"); - SigslotPacketReceiver receiver; - packet_transport.SignalReadPacket.connect( - &receiver, &SigslotPacketReceiver::OnPacketReceived); - - packet_transport.NotifyPacketReceived( - rtc::ReceivedPacket({}, rtc::SocketAddress(), absl::nullopt, - rtc::ReceivedPacket::kSrtpEncrypted)); - ASSERT_TRUE(receiver.packet_received_); - EXPECT_EQ(receiver.flags_, 1); -} - -TEST(PacketTransportInternal, PacketFlagsCorrectWithDtlsDecryptedUsingSigslot) { - rtc::FakePacketTransport packet_transport("test"); - SigslotPacketReceiver receiver; - packet_transport.SignalReadPacket.connect( - &receiver, &SigslotPacketReceiver::OnPacketReceived); - - packet_transport.NotifyPacketReceived( - rtc::ReceivedPacket({}, rtc::SocketAddress(), absl::nullopt, - rtc::ReceivedPacket::kDtlsDecrypted)); - ASSERT_TRUE(receiver.packet_received_); - EXPECT_EQ(receiver.flags_, 0); -} - -TEST(PacketTransportInternal, - NotifyPacketReceivedPassThrougPacketToRegisterListener) { + NotifyPacketReceivedPassthrougPacketToRegisteredListener) { rtc::FakePacketTransport packet_transport("test"); MockFunction receiver;