From ff72f9e692d0918b32646dadaf382aa4355d8437 Mon Sep 17 00:00:00 2001 From: "guoweis@webrtc.org" Date: Thu, 4 Dec 2014 00:14:49 +0000 Subject: [PATCH] Implement GetState() for channel's connectivity check state. Previously, IceState is considered completed when there is only one connection (and the rest was trimmed). However, since the trimming logic is only done within the scope of network, when IPv6 and IPv4 both exist, the completion event is never fired. This change adds the GetState() to each channel and it could decide what Completion means. The transport object then aggregates all channels before determining it's completed. Each channel's IceState will be aggregrated at Transport level for overall Ice state BUG=411086 R=pthatcher@webrtc.org Review URL: https://webrtc-codereview.appspot.com/30949004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@7804 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/p2p/base/dtlstransportchannel.h | 7 +++--- webrtc/p2p/base/fakesession.h | 13 +++++++++- webrtc/p2p/base/p2ptransportchannel.cc | 32 +++++++++++++++++++++++- webrtc/p2p/base/p2ptransportchannel.h | 4 +-- webrtc/p2p/base/port.cc | 18 ++++++++++--- webrtc/p2p/base/port.h | 1 + webrtc/p2p/base/rawtransportchannel.h | 3 +++ webrtc/p2p/base/transport.cc | 11 +++++--- webrtc/p2p/base/transportchannel.h | 5 ++++ webrtc/p2p/base/transportchannelimpl.h | 1 - webrtc/p2p/base/transportchannelproxy.cc | 8 ++++++ webrtc/p2p/base/transportchannelproxy.h | 2 ++ 12 files changed, 90 insertions(+), 15 deletions(-) diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index d12f724dde..e629bb53c0 100644 --- a/webrtc/p2p/base/dtlstransportchannel.h +++ b/webrtc/p2p/base/dtlstransportchannel.h @@ -109,9 +109,6 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { virtual IceRole GetIceRole() const { return channel_->GetIceRole(); } - virtual size_t GetConnectionCount() const { - return channel_->GetConnectionCount(); - } virtual bool SetLocalIdentity(rtc::SSLIdentity *identity); virtual bool GetLocalIdentity(rtc::SSLIdentity** identity) const; @@ -175,6 +172,10 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { virtual Transport* GetTransport() { return transport_; } + + virtual TransportChannelState GetState() const { + return channel_->GetState(); + } virtual void SetIceTiebreaker(uint64 tiebreaker) { channel_->SetIceTiebreaker(tiebreaker); } diff --git a/webrtc/p2p/base/fakesession.h b/webrtc/p2p/base/fakesession.h index 30bc981626..375d36d473 100644 --- a/webrtc/p2p/base/fakesession.h +++ b/webrtc/p2p/base/fakesession.h @@ -82,9 +82,20 @@ class FakeTransportChannel : public TransportChannelImpl, return transport_; } + virtual TransportChannelState GetState() const { + if (connection_count_ == 0) { + return TransportChannelState::STATE_FAILED; + } + + if (connection_count_ == 1) { + return TransportChannelState::STATE_COMPLETED; + } + + return TransportChannelState::STATE_FAILED; + } + virtual void SetIceRole(IceRole role) { role_ = role; } virtual IceRole GetIceRole() const { return role_; } - virtual size_t GetConnectionCount() const { return connection_count_; } virtual void SetIceTiebreaker(uint64 tiebreaker) { tiebreaker_ = tiebreaker; } virtual bool GetIceProtocolType(IceProtocolType* type) const { *type = ice_proto_; diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index d007903f50..6ecdf569e0 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -226,6 +226,36 @@ void P2PTransportChannel::SetIceTiebreaker(uint64 tiebreaker) { tiebreaker_ = tiebreaker; } +// Currently a channel is considered ICE completed once there is no +// more than one connection per Network. This works for a single NIC +// with both IPv4 and IPv6 enabled. However, this condition won't +// happen when there are multiple NICs and all of them have +// connectivity. +// TODO(guoweis): Change Completion to be driven by a channel level +// timer. +TransportChannelState P2PTransportChannel::GetState() const { + std::set networks; + + if (connections_.size() == 0) { + return TransportChannelState::STATE_FAILED; + } + + for (uint32 i = 0; i < connections_.size(); ++i) { + rtc::Network* network = connections_[i]->port()->Network(); + if (networks.find(network) == networks.end()) { + networks.insert(network); + } else { + LOG_J(LS_VERBOSE, this) << "Ice not completed yet for this channel as " + << network->ToString() + << " has more than 1 connection."; + return TransportChannelState::STATE_CONNECTING; + } + } + LOG_J(LS_VERBOSE, this) << "Ice is completed for this channel."; + + return TransportChannelState::STATE_COMPLETED; +} + bool P2PTransportChannel::GetIceProtocolType(IceProtocolType* type) const { *type = protocol_type_; return true; @@ -1065,7 +1095,7 @@ void P2PTransportChannel::HandleAllTimedOut() { // If we have a best connection, return it, otherwise return top one in the // list (later we will mark it best). Connection* P2PTransportChannel::GetBestConnectionOnNetwork( - rtc::Network* network) { + rtc::Network* network) const { // If the best connection is on this network, then it wins. if (best_connection_ && (best_connection_->port()->Network() == network)) return best_connection_; diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 8e3d50de32..10e19f03f9 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -59,10 +59,10 @@ class P2PTransportChannel : public TransportChannelImpl, // From TransportChannelImpl: virtual Transport* GetTransport() { return transport_; } + virtual TransportChannelState GetState() const; virtual void SetIceRole(IceRole role); virtual IceRole GetIceRole() const { return ice_role_; } virtual void SetIceTiebreaker(uint64 tiebreaker); - virtual size_t GetConnectionCount() const { return connections_.size(); } virtual bool GetIceProtocolType(IceProtocolType* type) const; virtual void SetIceProtocolType(IceProtocolType type); virtual void SetIceCredentials(const std::string& ice_ufrag, @@ -164,7 +164,7 @@ class P2PTransportChannel : public TransportChannelImpl, void HandleNotWritable(); void HandleAllTimedOut(); - Connection* GetBestConnectionOnNetwork(rtc::Network* network); + Connection* GetBestConnectionOnNetwork(rtc::Network* network) const; bool CreateConnections(const Candidate &remote_candidate, PortInterface* origin_port, bool readable); bool CreateConnection(PortInterface* port, const Candidate& remote_candidate, diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index 1cda7c10cc..59344547d6 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -927,7 +927,8 @@ void Connection::set_write_state(WriteState value) { WriteState old_value = write_state_; write_state_ = value; if (value != old_value) { - LOG_J(LS_VERBOSE, this) << "set_write_state"; + LOG_J(LS_VERBOSE, this) << "set_write_state from: " << old_value << " to " + << value; SignalStateChange(this); CheckTimeout(); } @@ -1102,8 +1103,10 @@ void Connection::UpdateState(uint32 now) { pings_since_last_response_[i]); pings.append(buf).append(" "); } - LOG_J(LS_VERBOSE, this) << "UpdateState(): pings_since_last_response_=" << - pings << ", rtt=" << rtt << ", now=" << now; + LOG_J(LS_VERBOSE, this) << "UpdateState(): pings_since_last_response_=" + << pings << ", rtt=" << rtt << ", now=" << now + << ", last ping received: " << last_ping_received_ + << ", last data_received: " << last_data_received_; // Check the readable state. // @@ -1187,6 +1190,12 @@ void Connection::ReceivedPing() { set_read_state(STATE_READABLE); } +std::string Connection::ToDebugId() const { + std::stringstream ss; + ss << std::hex << this; + return ss.str(); +} + std::string Connection::ToString() const { const char CONNECT_STATE_ABBREV[2] = { '-', // not connected (false) @@ -1212,7 +1221,8 @@ std::string Connection::ToString() const { const Candidate& local = local_candidate(); const Candidate& remote = remote_candidate(); std::stringstream ss; - ss << "Conn[" << port_->content_name() + ss << "Conn[" << ToDebugId() + << ":" << port_->content_name() << ":" << local.id() << ":" << local.component() << ":" << local.generation() << ":" << local.type() << ":" << local.protocol() diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h index 002643790c..b78c2c5544 100644 --- a/webrtc/p2p/base/port.h +++ b/webrtc/p2p/base/port.h @@ -517,6 +517,7 @@ class Connection : public rtc::MessageHandler, void ReceivedPing(); // Debugging description of this connection + std::string ToDebugId() const; std::string ToString() const; std::string ToSensitiveString() const; diff --git a/webrtc/p2p/base/rawtransportchannel.h b/webrtc/p2p/base/rawtransportchannel.h index 3041cad2c2..bc8431616b 100644 --- a/webrtc/p2p/base/rawtransportchannel.h +++ b/webrtc/p2p/base/rawtransportchannel.h @@ -54,6 +54,9 @@ class RawTransportChannel : public TransportChannelImpl, // Implements TransportChannelImpl. virtual Transport* GetTransport() { return raw_transport_; } + virtual TransportChannelState GetState() const { + return TransportChannelState::STATE_COMPLETED; + } virtual void SetIceCredentials(const std::string& ice_ufrag, const std::string& ice_pwd) {} virtual void SetRemoteIceCredentials(const std::string& ice_ufrag, diff --git a/webrtc/p2p/base/transport.cc b/webrtc/p2p/base/transport.cc index 05c455e4e2..07b204cf7d 100644 --- a/webrtc/p2p/base/transport.cc +++ b/webrtc/p2p/base/transport.cc @@ -669,8 +669,7 @@ void Transport::OnChannelConnectionRemoved(TransportChannelImpl* channel) { return; } - size_t connections = channel->GetConnectionCount(); - if (connections == 0) { + if (channel->GetState() == TransportChannelState::STATE_FAILED) { // A Transport has failed if any of its channels have no remaining // connections. signaling_thread_->Post(this, MSG_FAILED); @@ -680,6 +679,12 @@ void Transport::OnChannelConnectionRemoved(TransportChannelImpl* channel) { void Transport::MaybeCompleted_w() { ASSERT(worker_thread()->IsCurrent()); + // When there is no channel created yet, calling this function could fire an + // IceConnectionCompleted event prematurely. + if (channels_.size() == 0) { + return; + } + // A Transport's ICE process is completed if all of its channels are writable, // have finished allocating candidates, and have pruned all but one of their // connections. @@ -687,7 +692,7 @@ void Transport::MaybeCompleted_w() { for (iter = channels_.begin(); iter != channels_.end(); ++iter) { const TransportChannelImpl* channel = iter->second.get(); if (!(channel->writable() && - channel->GetConnectionCount() == 1 && + channel->GetState() == TransportChannelState::STATE_COMPLETED && channel->GetIceRole() == ICEROLE_CONTROLLING && iter->second.candidates_allocated())) { return; diff --git a/webrtc/p2p/base/transportchannel.h b/webrtc/p2p/base/transportchannel.h index f91c4a8e14..348ba9523d 100644 --- a/webrtc/p2p/base/transportchannel.h +++ b/webrtc/p2p/base/transportchannel.h @@ -36,6 +36,9 @@ enum PacketFlags { // crypto provided by the transport (e.g. DTLS) }; +// Used to indicate channel's connection state. +enum TransportChannelState { STATE_CONNECTING, STATE_COMPLETED, STATE_FAILED }; + // A TransportChannel represents one logical stream of packets that are sent // between the two sides of a session. class TransportChannel : public sigslot::has_slots<> { @@ -46,6 +49,8 @@ class TransportChannel : public sigslot::has_slots<> { readable_(false), writable_(false) {} virtual ~TransportChannel() {} + virtual TransportChannelState GetState() const = 0; + // TODO(mallinath) - Remove this API, as it's no longer useful. // Returns the session id of this channel. virtual const std::string SessionId() const { return std::string(); } diff --git a/webrtc/p2p/base/transportchannelimpl.h b/webrtc/p2p/base/transportchannelimpl.h index 060df7fdb3..6c2eac8ce2 100644 --- a/webrtc/p2p/base/transportchannelimpl.h +++ b/webrtc/p2p/base/transportchannelimpl.h @@ -36,7 +36,6 @@ class TransportChannelImpl : public TransportChannel { virtual IceRole GetIceRole() const = 0; virtual void SetIceRole(IceRole role) = 0; virtual void SetIceTiebreaker(uint64 tiebreaker) = 0; - virtual size_t GetConnectionCount() const = 0; // To toggle G-ICE/ICE. virtual bool GetIceProtocolType(IceProtocolType* type) const = 0; virtual void SetIceProtocolType(IceProtocolType type) = 0; diff --git a/webrtc/p2p/base/transportchannelproxy.cc b/webrtc/p2p/base/transportchannelproxy.cc index b5e09571a3..cb5f6ce33f 100644 --- a/webrtc/p2p/base/transportchannelproxy.cc +++ b/webrtc/p2p/base/transportchannelproxy.cc @@ -111,6 +111,14 @@ int TransportChannelProxy::GetError() { return impl_->GetError(); } +TransportChannelState TransportChannelProxy::GetState() const { + ASSERT(rtc::Thread::Current() == worker_thread_); + if (!impl_) { + return TransportChannelState::STATE_CONNECTING; + } + return impl_->GetState(); +} + bool TransportChannelProxy::GetStats(ConnectionInfos* infos) { ASSERT(rtc::Thread::Current() == worker_thread_); if (!impl_) { diff --git a/webrtc/p2p/base/transportchannelproxy.h b/webrtc/p2p/base/transportchannelproxy.h index cfd07f8585..188039ef5a 100644 --- a/webrtc/p2p/base/transportchannelproxy.h +++ b/webrtc/p2p/base/transportchannelproxy.h @@ -41,6 +41,8 @@ class TransportChannelProxy : public TransportChannel, const std::string& name() const { return name_; } TransportChannelImpl* impl() { return impl_; } + virtual TransportChannelState GetState() const; + // Sets the implementation to which we will proxy. void SetImplementation(TransportChannelImpl* impl);