From af2adda252deeacb29f703e32c38de1dccdd1b87 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Tue, 4 Dec 2018 11:16:19 +0100 Subject: [PATCH] Explicit comparisons on NetworkRoute. Since not all fields are compared on NetworkRoute structs, the == operator overload doesn't really make the code easier to read. In fact the feature that it only compares a subset of the fields is only used once, at the other places, all fields are compared. Removing the overload makes it more clear what is compared at each call site. Bug: webrtc:9883 Change-Id: I74f7eb32b602aa33fd282a815b71a172ae3f6a8b Reviewed-on: https://webrtc-review.googlesource.com/c/113001 Reviewed-by: Karl Wiberg Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#25891} --- call/rtp_transport_controller_send.cc | 4 +++- pc/channel_unittest.cc | 11 +++++------ pc/rtptransport_unittest.cc | 8 ++++++-- rtc_base/networkroute.h | 10 ---------- 4 files changed, 14 insertions(+), 19 deletions(-) diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index a86e18bf23..ac0f8de807 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -236,7 +236,9 @@ void RtpTransportControllerSend::OnNetworkRouteChanged( // No need to reset BWE if this is the first time the network connects. return; } - if (kv->second != network_route) { + if (kv->second.connected != network_route.connected || + kv->second.local_network_id != network_route.local_network_id || + kv->second.remote_network_id != network_route.remote_network_id) { kv->second = network_route; BitrateConstraints bitrate_config = bitrate_configurator_.GetConfig(); RTC_LOG(LS_INFO) << "Network route changed on transport " << transport_name diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index a6e783f92c..07e9e188ae 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -925,12 +925,11 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { }); WaitForThreads(); EXPECT_EQ(1, media_channel1->num_network_route_changes()); - rtc::NetworkRoute expected_network_route; - expected_network_route.connected = true; - expected_network_route.local_network_id = kLocalNetId; - expected_network_route.remote_network_id = kRemoteNetId; - expected_network_route.last_sent_packet_id = kLastPacketId; - EXPECT_EQ(expected_network_route, media_channel1->last_network_route()); + EXPECT_TRUE(media_channel1->last_network_route().connected); + EXPECT_EQ(kLocalNetId, + media_channel1->last_network_route().local_network_id); + EXPECT_EQ(kRemoteNetId, + media_channel1->last_network_route().remote_network_id); EXPECT_EQ(kLastPacketId, media_channel1->last_network_route().last_sent_packet_id); EXPECT_EQ(kTransportOverheadPerPacket + kSrtpOverheadPerPacket, diff --git a/pc/rtptransport_unittest.cc b/pc/rtptransport_unittest.cc index 97bc408c20..3b28c67d33 100644 --- a/pc/rtptransport_unittest.cc +++ b/pc/rtptransport_unittest.cc @@ -193,7 +193,9 @@ TEST(RtpTransportTest, SetRtpTransportWithNetworkRouteChanged) { fake_rtp.SetNetworkRoute(absl::optional(network_route)); transport.SetRtpPacketTransport(&fake_rtp); ASSERT_TRUE(observer.network_route()); - EXPECT_EQ(network_route, *(observer.network_route())); + EXPECT_TRUE(observer.network_route()->connected); + EXPECT_EQ(kLocalNetId, observer.network_route()->local_network_id); + EXPECT_EQ(kRemoteNetId, observer.network_route()->remote_network_id); EXPECT_EQ(kTransportOverheadPerPacket, observer.network_route()->packet_overhead); EXPECT_EQ(kLastPacketId, observer.network_route()->last_sent_packet_id); @@ -220,7 +222,9 @@ TEST(RtpTransportTest, SetRtcpTransportWithNetworkRouteChanged) { fake_rtcp.SetNetworkRoute(absl::optional(network_route)); transport.SetRtcpPacketTransport(&fake_rtcp); ASSERT_TRUE(observer.network_route()); - EXPECT_EQ(network_route, *(observer.network_route())); + EXPECT_TRUE(observer.network_route()->connected); + EXPECT_EQ(kLocalNetId, observer.network_route()->local_network_id); + EXPECT_EQ(kRemoteNetId, observer.network_route()->remote_network_id); EXPECT_EQ(kTransportOverheadPerPacket, observer.network_route()->packet_overhead); EXPECT_EQ(kLastPacketId, observer.network_route()->last_sent_packet_id); diff --git a/rtc_base/networkroute.h b/rtc_base/networkroute.h index 31c083135c..be02731186 100644 --- a/rtc_base/networkroute.h +++ b/rtc_base/networkroute.h @@ -27,16 +27,6 @@ struct NetworkRoute { int last_sent_packet_id = -1; // The overhead in bytes from IP layer and above. int packet_overhead = 0; - - // |last_sent_packet_id| and |packet_overhead| do not affect the NetworkRoute - // comparison. - bool operator==(const NetworkRoute& nr) const { - return connected == nr.connected && - local_network_id == nr.local_network_id && - remote_network_id == nr.remote_network_id; - } - - bool operator!=(const NetworkRoute& nr) const { return !(*this == nr); } }; } // namespace rtc