From 8c9ff203c5f3c87891b46067ef6ec21b37d2dde4 Mon Sep 17 00:00:00 2001 From: "guoweis@webrtc.org" Date: Thu, 4 Dec 2014 07:56:02 +0000 Subject: [PATCH] Redo the change of https://webrtc-codereview.appspot.com/30949004/ The previous change causes a build issue as there is subclass of TransportChannel in chromium. To break the circular dependency, a stub of implementation for GetState() is provided and will be removed once the jingle_glue::MockTransportChannel has the function defined. TBR=pthatcher@webrtc.org BUG=411086 Review URL: https://webrtc-codereview.appspot.com/34369004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@7806 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 | 9 +++++++ webrtc/p2p/base/transportchannelimpl.h | 1 - webrtc/p2p/base/transportchannelproxy.cc | 8 ++++++ webrtc/p2p/base/transportchannelproxy.h | 2 ++ 12 files changed, 94 insertions(+), 15 deletions(-) diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index d12f724dd..e629bb53c 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 30bc98162..375d36d47 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 d007903f5..6ecdf569e 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 8e3d50de3..10e19f03f 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 1cda7c10c..59344547d 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 002643790..b78c2c554 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 3041cad2c..bc8431616 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 05c455e4e..07b204cf7 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 f91c4a8e1..d50f025ea 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,12 @@ class TransportChannel : public sigslot::has_slots<> { readable_(false), writable_(false) {} virtual ~TransportChannel() {} + // TODO(guoweis) - Make this pure virtual once all subclasses of + // TransportChannel have this defined. + virtual TransportChannelState GetState() const { + return TransportChannelState::STATE_CONNECTING; + } + // 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 060df7fdb..6c2eac8ce 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 b5e09571a..cb5f6ce33 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 cfd07f858..188039ef5 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);