diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 090d5a4e22..6350df549d 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -696,6 +696,9 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { // Make sure that nomination reaching ICE controlled asap. "send_ping_on_switch_ice_controlling", &field_trials_.send_ping_on_switch_ice_controlling, + // Make sure that nomination reaching ICE controlled asap. + "send_ping_on_selected_ice_controlling", + &field_trials_.send_ping_on_selected_ice_controlling, // Reply to nomination ASAP. "send_ping_on_nomination_ice_controlled", &field_trials_.send_ping_on_nomination_ice_controlled, @@ -1768,9 +1771,10 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn, RTC_LOG(LS_INFO) << ToString() << ": No selected connection"; } - if (field_trials_.send_ping_on_switch_ice_controlling && - ice_role_ == ICEROLE_CONTROLLING && old_selected_connection != nullptr && - conn != nullptr) { + if (conn != nullptr && ice_role_ == ICEROLE_CONTROLLING && + ((field_trials_.send_ping_on_switch_ice_controlling && + old_selected_connection != nullptr) || + field_trials_.send_ping_on_selected_ice_controlling)) { PingConnection(conn); MarkConnectionPinged(conn); } diff --git a/p2p/base/p2p_transport_channel_ice_field_trials.h b/p2p/base/p2p_transport_channel_ice_field_trials.h index f30366fd1f..00e1151ba3 100644 --- a/p2p/base/p2p_transport_channel_ice_field_trials.h +++ b/p2p/base/p2p_transport_channel_ice_field_trials.h @@ -44,8 +44,14 @@ struct IceFieldTrials { int rtt_estimate_halftime_ms = 500; // Sending a PING directly after a switch on ICE_CONTROLLING-side. + // TODO(jonaso) : Deprecate this in favor of + // |send_ping_on_selected_ice_controlling|. bool send_ping_on_switch_ice_controlling = false; + // Sending a PING directly after selecting a connection + // (i.e either a switch or the inital selection). + bool send_ping_on_selected_ice_controlling = false; + // Sending a PING directly after a nomination on ICE_CONTROLLED-side. bool send_ping_on_nomination_ice_controlled = false; diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index ad62920c5a..5acb75b740 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -3826,6 +3826,33 @@ TEST_F(P2PTransportChannelPingTest, TestPingOnSwitch) { EXPECT_EQ(conn2->num_pings_sent(), before + 1); } +// Test the field trial send_ping_on_switch_ice_controlling +// that sends a ping directly when selecteing a new connection +// on the ICE_CONTROLLING-side (i.e also initial selection). +TEST_F(P2PTransportChannelPingTest, TestPingOnSelected) { + webrtc::test::ScopedFieldTrials field_trials( + "WebRTC-IceFieldTrials/send_ping_on_selected_ice_controlling:true/"); + FakePortAllocator pa(rtc::Thread::Current(), nullptr); + P2PTransportChannel ch("receiving state change", 1, &pa); + PrepareChannel(&ch); + ch.SetIceConfig(ch.config()); + ch.SetIceRole(ICEROLE_CONTROLLING); + ch.MaybeStartGathering(); + ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1)); + Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1); + ASSERT_TRUE(conn1 != nullptr); + + const int before = conn1->num_pings_sent(); + + // A connection needs to be writable before it is selected for transmission. + conn1->ReceivedPingResponse(LOW_RTT, "id"); + EXPECT_EQ_WAIT(conn1, ch.selected_connection(), kDefaultTimeout); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1)); + + // And the additional ping should have been sent directly. + EXPECT_EQ(conn1->num_pings_sent(), before + 1); +} + // The controlled side will select a connection as the "selected connection" // based on requests from an unknown address before the controlling side // nominates a connection, and will nominate a connection from an unknown