Fixing webrtc::IceTransportState.
Currently the IceTransportState goes to connected before the connection
is writable and also goes to checking before any candidate pairs are
being checked. This change fixes these cases.
Bug: webrtc:10352, chromium:933802
Change-Id: I64a67c7f76a94a6be9da413740ddc8762fe966ce
Reviewed-on: https://webrtc-review.googlesource.com/c/124102
Commit-Queue: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26842}
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 392a841..b62c0b4 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -414,12 +414,17 @@
return webrtc::IceTransportState::kDisconnected;
}
- if (gathering_state_ == kIceGatheringNew)
+ if (!had_connection_ && !has_connection) {
return webrtc::IceTransportState::kNew;
- else if (has_connection)
- return webrtc::IceTransportState::kConnected;
- else
+ }
+
+ if (has_connection && !writable()) {
+ // A candidate pair has been formed by adding a remote candidate
+ // and gathering a local candidate.
return webrtc::IceTransportState::kChecking;
+ }
+
+ return webrtc::IceTransportState::kConnected;
}
void P2PTransportChannel::SetIceParameters(const IceParameters& ice_params) {
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index acd66c1..775545b 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -4136,21 +4136,29 @@
EXPECT_EQ(webrtc::IceTransportState::kNew, ch.GetIceTransportState());
PrepareChannel(&ch);
ch.MaybeStartGathering();
+ // After gathering we are still in the kNew state because we aren't checking
+ // any connections yet.
+ EXPECT_EQ(webrtc::IceTransportState::kNew, ch.GetIceTransportState());
EXPECT_EQ(IceTransportState::STATE_INIT, ch.GetState());
ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 1));
+ // Checking candidates that have been added with gathered candidates.
+ ASSERT_GT(ch.connections().size(), 0u);
+ EXPECT_EQ(webrtc::IceTransportState::kChecking, ch.GetIceTransportState());
Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1, &clock);
Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2, &clock);
- // Gathering complete with candidates.
- EXPECT_EQ(webrtc::IceTransportState::kConnected, ch.GetIceTransportState());
ASSERT_TRUE(conn1 != nullptr);
ASSERT_TRUE(conn2 != nullptr);
// Now there are two connections, so the transport channel is connecting.
EXPECT_EQ(IceTransportState::STATE_CONNECTING, ch.GetState());
+ // No connections are writable yet, so we should still be in the kChecking
+ // state.
+ EXPECT_EQ(webrtc::IceTransportState::kChecking, ch.GetIceTransportState());
// |conn1| becomes writable and receiving; it then should prune |conn2|.
conn1->ReceivedPingResponse(LOW_RTT, "id");
EXPECT_TRUE_SIMULATED_WAIT(conn2->pruned(), kShortTimeout, clock);
EXPECT_EQ(IceTransportState::STATE_COMPLETED, ch.GetState());
+ EXPECT_EQ(webrtc::IceTransportState::kConnected, ch.GetIceTransportState());
conn1->Prune(); // All connections are pruned.
// Need to wait until the channel state is updated.
EXPECT_EQ_SIMULATED_WAIT(IceTransportState::STATE_FAILED, ch.GetState(),