From 06495bcbb7477b8df1e9f2e34eae0efd11e0b9fe Mon Sep 17 00:00:00 2001 From: hbos Date: Mon, 2 Jan 2017 08:08:18 -0800 Subject: [PATCH] RTCIceCandidatePairStats.[state/priority] added, ConnectionInfo updated. State and priority added to ConnectionInfo. The Connection::State enum is replaced by IceCandidatePairState enum class. At P2PTransportChannel::GetStats, Connection::stats is called, producing ConnectionInfo for the connection that is then filled in with additional values from the Connection. This is refactored so that all values are set by Connection::stats. RTCStatsCollector is updated to surface the ConnectionInfo stats. BUG=webrtc:6755, chromium:633550, chromium:627816 Review-Url: https://codereview.webrtc.org/2597423003 Cr-Commit-Position: refs/heads/master@{#15870} --- webrtc/api/rtcstats_integrationtest.cc | 4 +-- webrtc/api/rtcstatscollector.cc | 20 +++++++++++++ webrtc/api/rtcstatscollector_unittest.cc | 4 +++ webrtc/api/stats/rtcstats_objects.h | 3 -- webrtc/p2p/base/jseptransport.cc | 22 +++++++++++++++ webrtc/p2p/base/jseptransport.h | 25 ++++------------- webrtc/p2p/base/p2ptransportchannel.cc | 13 ++------- .../p2p/base/p2ptransportchannel_unittest.cc | 4 +-- webrtc/p2p/base/port.cc | 28 +++++++++++++------ webrtc/p2p/base/port.h | 26 +++++++++-------- webrtc/p2p/base/turnport_unittest.cc | 3 +- webrtc/stats/rtcstats_objects.cc | 3 +- 12 files changed, 95 insertions(+), 60 deletions(-) diff --git a/webrtc/api/rtcstats_integrationtest.cc b/webrtc/api/rtcstats_integrationtest.cc index 577cc064f0..556907ce51 100644 --- a/webrtc/api/rtcstats_integrationtest.cc +++ b/webrtc/api/rtcstats_integrationtest.cc @@ -359,8 +359,8 @@ class RTCStatsReportVerifier { candidate_pair.local_candidate_id, RTCLocalIceCandidateStats::kType); verifier.TestMemberIsIDReference( candidate_pair.remote_candidate_id, RTCRemoteIceCandidateStats::kType); - verifier.TestMemberIsUndefined(candidate_pair.state); - verifier.TestMemberIsUndefined(candidate_pair.priority); + verifier.TestMemberIsDefined(candidate_pair.state); + verifier.TestMemberIsNonNegative(candidate_pair.priority); verifier.TestMemberIsUndefined(candidate_pair.nominated); verifier.TestMemberIsDefined(candidate_pair.writable); verifier.TestMemberIsUndefined(candidate_pair.readable); diff --git a/webrtc/api/rtcstatscollector.cc b/webrtc/api/rtcstatscollector.cc index 85113e69a2..4e5e8d9739 100644 --- a/webrtc/api/rtcstatscollector.cc +++ b/webrtc/api/rtcstatscollector.cc @@ -114,6 +114,23 @@ const char* DataStateToRTCDataChannelState( } } +const char* IceCandidatePairStateToRTCStatsIceCandidatePairState( + cricket::IceCandidatePairState state) { + switch (state) { + case cricket::IceCandidatePairState::WAITING: + return RTCStatsIceCandidatePairState::kWaiting; + case cricket::IceCandidatePairState::IN_PROGRESS: + return RTCStatsIceCandidatePairState::kInProgress; + case cricket::IceCandidatePairState::SUCCEEDED: + return RTCStatsIceCandidatePairState::kSucceeded; + case cricket::IceCandidatePairState::FAILED: + return RTCStatsIceCandidatePairState::kFailed; + default: + RTC_NOTREACHED(); + return nullptr; + } +} + std::unique_ptr CodecStatsFromRtpCodecParameters( uint64_t timestamp_us, bool inbound, bool audio, const RtpCodecParameters& codec_params) { @@ -646,6 +663,9 @@ void RTCStatsCollector::ProduceIceCandidateAndPairStats_n( timestamp_us, info.local_candidate, true, report); candidate_pair_stats->remote_candidate_id = ProduceIceCandidateStats( timestamp_us, info.remote_candidate, false, report); + candidate_pair_stats->state = + IceCandidatePairStateToRTCStatsIceCandidatePairState(info.state); + candidate_pair_stats->priority = info.priority; // TODO(hbos): This writable is different than the spec. It goes to // false after a certain amount of time without a response passes. // crbug.com/633550 diff --git a/webrtc/api/rtcstatscollector_unittest.cc b/webrtc/api/rtcstatscollector_unittest.cc index ee090543d9..6e8485312f 100644 --- a/webrtc/api/rtcstatscollector_unittest.cc +++ b/webrtc/api/rtcstatscollector_unittest.cc @@ -1066,6 +1066,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { connection_info.sent_ping_requests_before_first_response = 2000; connection_info.recv_ping_responses = 4321; connection_info.sent_ping_responses = 1000; + connection_info.state = cricket::IceCandidatePairState::IN_PROGRESS; + connection_info.priority = 5555; cricket::TransportChannelStats transport_channel_stats; transport_channel_stats.component = cricket::ICE_CANDIDATE_COMPONENT_RTP; @@ -1092,6 +1094,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { expected_pair.local_candidate_id = "RTCIceCandidate_" + local_candidate->id(); expected_pair.remote_candidate_id = "RTCIceCandidate_" + remote_candidate->id(); + expected_pair.state = RTCStatsIceCandidatePairState::kInProgress; + expected_pair.priority = 5555; expected_pair.writable = true; expected_pair.bytes_sent = 42; expected_pair.bytes_received = 1234; diff --git a/webrtc/api/stats/rtcstats_objects.h b/webrtc/api/stats/rtcstats_objects.h index d18b248ad7..e85119a2f2 100644 --- a/webrtc/api/stats/rtcstats_objects.h +++ b/webrtc/api/stats/rtcstats_objects.h @@ -32,7 +32,6 @@ struct RTCStatsIceCandidatePairState { static const char* kInProgress; static const char* kFailed; static const char* kSucceeded; - static const char* kCancelled; }; // https://w3c.github.io/webrtc-pc/#rtcicecandidatetype-enum @@ -120,9 +119,7 @@ class RTCIceCandidatePairStats final : public RTCStats { RTCStatsMember remote_candidate_id; // TODO(hbos): Support enum types? // "RTCStatsMember"? - // TODO(hbos): Not collected by |RTCStatsCollector|. crbug.com/633550 RTCStatsMember state; - // TODO(hbos): Not collected by |RTCStatsCollector|. crbug.com/633550 RTCStatsMember priority; // TODO(hbos): Not collected by |RTCStatsCollector|. crbug.com/633550 RTCStatsMember nominated; diff --git a/webrtc/p2p/base/jseptransport.cc b/webrtc/p2p/base/jseptransport.cc index bf4d86c3af..46cf4c4ec5 100644 --- a/webrtc/p2p/base/jseptransport.cc +++ b/webrtc/p2p/base/jseptransport.cc @@ -41,6 +41,28 @@ static bool VerifyIceParams(const TransportDescription& desc) { return true; } +ConnectionInfo::ConnectionInfo() + : best_connection(false), + writable(false), + receiving(false), + timeout(false), + new_connection(false), + rtt(0), + sent_total_bytes(0), + sent_bytes_second(0), + sent_discarded_packets(0), + sent_total_packets(0), + sent_ping_requests_total(0), + sent_ping_requests_before_first_response(0), + sent_ping_responses(0), + recv_total_bytes(0), + recv_bytes_second(0), + recv_ping_requests(0), + recv_ping_responses(0), + key(nullptr), + state(IceCandidatePairState::WAITING), + priority(0) {} + bool BadTransportDescription(const std::string& desc, std::string* err_desc) { if (err_desc) { *err_desc = desc; diff --git a/webrtc/p2p/base/jseptransport.h b/webrtc/p2p/base/jseptransport.h index cc05ec097f..ff0edebca6 100644 --- a/webrtc/p2p/base/jseptransport.h +++ b/webrtc/p2p/base/jseptransport.h @@ -31,6 +31,7 @@ namespace cricket { class TransportChannelImpl; class TransportChannelImpl; +enum class IceCandidatePairState; typedef std::vector Candidates; @@ -83,25 +84,7 @@ enum ContinualGatheringPolicy { // Stats that we can return about the connections for a transport channel. // TODO(hta): Rename to ConnectionStats struct ConnectionInfo { - ConnectionInfo() - : best_connection(false), - writable(false), - receiving(false), - timeout(false), - new_connection(false), - rtt(0), - sent_total_bytes(0), - sent_bytes_second(0), - sent_discarded_packets(0), - sent_total_packets(0), - sent_ping_requests_total(0), - sent_ping_requests_before_first_response(0), - sent_ping_responses(0), - recv_total_bytes(0), - recv_bytes_second(0), - recv_ping_requests(0), - recv_ping_responses(0), - key(NULL) {} + ConnectionInfo(); bool best_connection; // Is this the best connection we have? bool writable; // Has this connection received a STUN response? @@ -127,6 +110,10 @@ struct ConnectionInfo { Candidate local_candidate; // The local candidate for this connection. Candidate remote_candidate; // The remote candidate for this connection. void* key; // A static value that identifies this conn. + // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-state + IceCandidatePairState state; + // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-priority + uint64_t priority; }; // Information about all the connections of a channel. diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index 5f858c4858..6071e079b3 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -1012,17 +1012,8 @@ bool P2PTransportChannel::GetStats(ConnectionInfos *infos) { for (Connection* connection : connections_) { ConnectionInfo info = connection->stats(); info.best_connection = (selected_connection_ == connection); - info.receiving = connection->receiving(); - info.writable = (connection->write_state() == Connection::STATE_WRITABLE); - info.timeout = - (connection->write_state() == Connection::STATE_WRITE_TIMEOUT); - info.new_connection = !connection->reported(); + infos->push_back(std::move(info)); connection->set_reported(true); - info.rtt = connection->rtt(); - info.local_candidate = connection->local_candidate(); - info.remote_candidate = connection->remote_candidate(); - info.key = connection; - infos->push_back(info); } return true; @@ -1559,7 +1550,7 @@ bool P2PTransportChannel::IsPingable(const Connection* conn, } // A failed connection will not be pinged. - if (conn->state() == Connection::STATE_FAILED) { + if (conn->state() == IceCandidatePairState::FAILED) { return false; } diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index 3a9ec189b0..ae47fa6a28 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -4017,14 +4017,14 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) { ASSERT_TRUE(conn2 != nullptr); EXPECT_TRUE_SIMULATED_WAIT(!conn2->active(), kDefaultTimeout, clock); // |conn2| should not send a ping yet. - EXPECT_EQ(Connection::STATE_WAITING, conn2->state()); + EXPECT_EQ(IceCandidatePairState::WAITING, conn2->state()); EXPECT_EQ(TransportChannelState::STATE_COMPLETED, ch.GetState()); // Wait for |conn1| becoming not receiving. EXPECT_TRUE_SIMULATED_WAIT(!conn1->receiving(), kMediumTimeout, clock); // Make sure conn2 is not deleted. conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2, &clock); ASSERT_TRUE(conn2 != nullptr); - EXPECT_EQ_SIMULATED_WAIT(Connection::STATE_INPROGRESS, conn2->state(), + EXPECT_EQ_SIMULATED_WAIT(IceCandidatePairState::IN_PROGRESS, conn2->state(), kDefaultTimeout, clock); conn2->ReceivedPingResponse(LOW_RTT, "id"); EXPECT_EQ_SIMULATED_WAIT(conn2, ch.selected_connection(), kDefaultTimeout, diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index eb63436dc8..dc99a338d5 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -868,7 +868,7 @@ Connection::Connection(Port* port, last_data_received_(0), last_ping_response_received_(0), reported_(false), - state_(STATE_WAITING), + state_(IceCandidatePairState::WAITING), receiving_timeout_(WEAK_CONNECTION_RECEIVE_TIMEOUT), time_created_ms_(rtc::TimeMillis()) { // All of our connections start in WAITING state. @@ -937,8 +937,8 @@ void Connection::UpdateReceiving(int64_t now) { SignalStateChange(this); } -void Connection::set_state(State state) { - State old_state = state_; +void Connection::set_state(IceCandidatePairState state) { + IceCandidatePairState old_state = state_; state_ = state; if (state != old_state) { LOG_J(LS_VERBOSE, this) << "set_state"; @@ -1126,12 +1126,12 @@ void Connection::Destroy() { } void Connection::FailAndDestroy() { - set_state(Connection::STATE_FAILED); + set_state(IceCandidatePairState::FAILED); Destroy(); } void Connection::FailAndPrune() { - set_state(Connection::STATE_FAILED); + set_state(IceCandidatePairState::FAILED); Prune(); } @@ -1223,7 +1223,7 @@ void Connection::Ping(int64_t now) { << ", id=" << rtc::hex_encode(req->id()) << ", nomination=" << nomination_; requests_.Send(req); - state_ = STATE_INPROGRESS; + state_ = IceCandidatePairState::IN_PROGRESS; num_pings_sent_++; } @@ -1250,7 +1250,7 @@ void Connection::ReceivedPingResponse(int rtt, const std::string& request_id) { last_ping_response_received_ = rtc::TimeMillis(); UpdateReceiving(last_ping_response_received_); set_write_state(STATE_WRITABLE); - set_state(STATE_SUCCEEDED); + set_state(IceCandidatePairState::SUCCEEDED); rtt_samples_++; rtt_ = (RTT_RATIO * rtt_ + rtt) / (RTT_RATIO + 1); } @@ -1331,8 +1331,8 @@ std::string Connection::ToString() const { << ":" << remote.protocol() << ":" << remote.address().ToSensitiveString() << "|" << CONNECT_STATE_ABBREV[connected()] << RECEIVE_STATE_ABBREV[receiving()] << WRITE_STATE_ABBREV[write_state()] - << ICESTATE[state()] << "|" << remote_nomination() << "|" << nomination() - << "|" << priority() << "|"; + << ICESTATE[static_cast(state())] << "|" << remote_nomination() << "|" + << nomination() << "|" << priority() << "|"; if (rtt_ < DEFAULT_RTT) { ss << rtt_ << "]"; } else { @@ -1471,6 +1471,16 @@ ConnectionInfo Connection::stats() { stats_.recv_total_bytes = recv_rate_tracker_.TotalSampleCount(); stats_.sent_bytes_second = round(send_rate_tracker_.ComputeRate()); stats_.sent_total_bytes = send_rate_tracker_.TotalSampleCount(); + stats_.receiving = receiving_; + stats_.writable = write_state_ == STATE_WRITABLE; + stats_.timeout = write_state_ == STATE_WRITE_TIMEOUT; + stats_.new_connection = !reported_; + stats_.rtt = rtt_; + stats_.local_candidate = local_candidate(); + stats_.remote_candidate = remote_candidate(); + stats_.key = this; + stats_.state = state_; + stats_.priority = priority(); return stats_; } diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h index 6b1a551963..f3c4ed167c 100644 --- a/webrtc/p2p/base/port.h +++ b/webrtc/p2p/base/port.h @@ -92,6 +92,16 @@ enum IcePriorityValue { ICE_TYPE_PREFERENCE_HOST = 126 }; +// States are from RFC 5245. http://tools.ietf.org/html/rfc5245#section-5.7.4 +enum class IceCandidatePairState { + WAITING = 0, // Check has not been performed, Waiting pair on CL. + IN_PROGRESS, // Check has been sent, transaction is in progress. + SUCCEEDED, // Check already done, produced a successful result. + FAILED, // Check for this connection failed. + // According to spec there should also be a frozen state, but nothing is ever + // frozen because we have not implemented ICE freezing logic. +}; + const char* ProtoToString(ProtocolType proto); bool StringToProto(const char* value, ProtocolType* proto); @@ -419,14 +429,6 @@ class Connection : public CandidatePairInterface, uint32_t nomination; }; - // States are from RFC 5245. http://tools.ietf.org/html/rfc5245#section-5.7.4 - enum State { - STATE_WAITING = 0, // Check has not been performed, Waiting pair on CL. - STATE_INPROGRESS, // Check has been sent, transaction is in progress. - STATE_SUCCEEDED, // Check already done, produced a successful result. - STATE_FAILED // Check for this connection failed. - }; - virtual ~Connection(); // The local port where this connection sends and receives packets. @@ -467,6 +469,8 @@ class Connection : public CandidatePairInterface, // Estimate of the round-trip time over this connection. int rtt() const { return rtt_; } + // Gets the |ConnectionInfo| stats, where |best_connection| has not been + // populated (default value false). ConnectionInfo stats(); sigslot::signal1 SignalStateChange; @@ -575,7 +579,7 @@ class Connection : public CandidatePairInterface, // Invoked when Connection receives STUN error response with 487 code. void HandleRoleConflictFromPeer(); - State state() const { return state_; } + IceCandidatePairState state() const { return state_; } int num_pings_sent() const { return num_pings_sent_; } @@ -630,7 +634,7 @@ class Connection : public CandidatePairInterface, // Changes the state and signals if necessary. void set_write_state(WriteState value); void UpdateReceiving(int64_t now); - void set_state(State state); + void set_state(IceCandidatePairState state); void set_connected(bool value); uint32_t nomination() const { return nomination_; } @@ -686,7 +690,7 @@ class Connection : public CandidatePairInterface, std::vector pings_since_last_response_; bool reported_; - State state_; + IceCandidatePairState state_; // Time duration to switch from receiving to not receiving. int receiving_timeout_; int64_t time_created_ms_; diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc index e0b1420106..edad8122da 100644 --- a/webrtc/p2p/base/turnport_unittest.cc +++ b/webrtc/p2p/base/turnport_unittest.cc @@ -334,7 +334,8 @@ class TurnPortTest : public testing::Test, } bool CheckConnectionFailedAndPruned(Connection* conn) { - return conn && !conn->active() && conn->state() == Connection::STATE_FAILED; + return conn && !conn->active() && + conn->state() == IceCandidatePairState::FAILED; } // Checks that |turn_port_| has a nonempty set of connections and they are all diff --git a/webrtc/stats/rtcstats_objects.cc b/webrtc/stats/rtcstats_objects.cc index d4fea49519..487df930d6 100644 --- a/webrtc/stats/rtcstats_objects.cc +++ b/webrtc/stats/rtcstats_objects.cc @@ -19,10 +19,9 @@ const char* RTCDataChannelState::kClosed = "closed"; const char* RTCStatsIceCandidatePairState::kFrozen = "frozen"; const char* RTCStatsIceCandidatePairState::kWaiting = "waiting"; -const char* RTCStatsIceCandidatePairState::kInProgress = "inprogress"; +const char* RTCStatsIceCandidatePairState::kInProgress = "in-progress"; const char* RTCStatsIceCandidatePairState::kFailed = "failed"; const char* RTCStatsIceCandidatePairState::kSucceeded = "succeeded"; -const char* RTCStatsIceCandidatePairState::kCancelled = "cancelled"; // Strings defined in https://tools.ietf.org/html/rfc5245. const char* RTCIceCandidateType::kHost = "host";