diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index ceec13ab0c..a23b25dbc9 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -1172,14 +1172,6 @@ class PeerConnectionObserver { // A new ICE candidate has been gathered. virtual void OnIceCandidate(const IceCandidateInterface* candidate) = 0; - // Gathering of an ICE candidate failed. - // See https://w3c.github.io/webrtc-pc/#event-icecandidateerror - // |host_candidate| is a stringified socket address. - virtual void OnIceCandidateError(const std::string& host_candidate, - const std::string& url, - int error_code, - const std::string& error_text) {} - // Ice candidates have been removed. // TODO(honghaiz): Make this a pure virtual method when all its subclasses // implement it. diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h index e3d98dbdd1..89b2107c5a 100644 --- a/p2p/base/ice_transport_internal.h +++ b/p2p/base/ice_transport_internal.h @@ -270,9 +270,6 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal { sigslot::signal2 SignalCandidateGathered; - sigslot::signal2 - SignalCandidateError; - sigslot::signal2 SignalCandidatesRemoved; diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index d510820d75..d469ff7dd6 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -180,8 +180,6 @@ void P2PTransportChannel::AddAllocatorSession( session->SignalPortsPruned.connect(this, &P2PTransportChannel::OnPortsPruned); session->SignalCandidatesReady.connect( this, &P2PTransportChannel::OnCandidatesReady); - session->SignalCandidateError.connect(this, - &P2PTransportChannel::OnCandidateError); session->SignalCandidatesRemoved.connect( this, &P2PTransportChannel::OnCandidatesRemoved); session->SignalCandidatesAllocationDone.connect( @@ -880,13 +878,6 @@ void P2PTransportChannel::OnCandidatesReady( } } -void P2PTransportChannel::OnCandidateError( - PortAllocatorSession* session, - const IceCandidateErrorEvent& event) { - RTC_DCHECK(network_thread_ == rtc::Thread::Current()); - SignalCandidateError(this, event); -} - void P2PTransportChannel::OnCandidatesAllocationDone( PortAllocatorSession* session) { RTC_DCHECK_RUN_ON(network_thread_); diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index 0bcbe10958..5e537ce4af 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -307,8 +307,6 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { const std::vector& ports); void OnCandidatesReady(PortAllocatorSession* session, const std::vector& candidates); - void OnCandidateError(PortAllocatorSession* session, - const IceCandidateErrorEvent& event); void OnCandidatesRemoved(PortAllocatorSession* session, const std::vector& candidates); void OnCandidatesAllocationDone(PortAllocatorSession* session); diff --git a/p2p/base/port.h b/p2p/base/port.h index 8e6281f689..a0e2606a22 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -15,7 +15,6 @@ #include #include #include -#include #include #include "absl/types/optional.h" @@ -131,23 +130,6 @@ struct ProtocolAddress { bool operator!=(const ProtocolAddress& o) const { return !(*this == o); } }; -struct IceCandidateErrorEvent { - IceCandidateErrorEvent() = default; - IceCandidateErrorEvent(std::string host_candidate, - std::string url, - int error_code, - std::string error_text) - : host_candidate(std::move(host_candidate)), - url(std::move(url)), - error_code(error_code), - error_text(std::move(error_text)) {} - - std::string host_candidate; - std::string url; - int error_code = 0; - std::string error_text; -}; - typedef std::set ServerAddresses; // Represents a local communication mechanism that can be used to create @@ -245,10 +227,9 @@ class Port : public PortInterface, // Fired when candidates are discovered by the port. When all candidates // are discovered that belong to port SignalAddressReady is fired. sigslot::signal2 SignalCandidateReady; + // Provides all of the above information in one handy object. const std::vector& Candidates() const override; - // Fired when candidate discovery failed using certain server. - sigslot::signal2 SignalCandidateError; // SignalPortComplete is sent when port completes the task of candidates // allocation. diff --git a/p2p/base/port_allocator.h b/p2p/base/port_allocator.h index d78b6cbc65..d0605b6bb6 100644 --- a/p2p/base/port_allocator.h +++ b/p2p/base/port_allocator.h @@ -271,8 +271,6 @@ class RTC_EXPORT PortAllocatorSession : public sigslot::has_slots<> { SignalPortsPruned; sigslot::signal2&> SignalCandidatesReady; - sigslot::signal2 - SignalCandidateError; // Candidates should be signaled to be removed when the port that generated // the candidates is removed. sigslot::signal2&> diff --git a/p2p/base/stun.cc b/p2p/base/stun.cc index 3a44909a60..d2897747bb 100644 --- a/p2p/base/stun.cc +++ b/p2p/base/stun.cc @@ -61,7 +61,6 @@ const char STUN_ERROR_REASON_SERVER_ERROR[] = "Server Error"; const char TURN_MAGIC_COOKIE_VALUE[] = {'\x72', '\xC6', '\x4B', '\xC6'}; const char EMPTY_TRANSACTION_ID[] = "0000000000000000"; const uint32_t STUN_FINGERPRINT_XOR_VALUE = 0x5354554E; -const int SERVER_NOT_REACHABLE_ERROR = 701; // StunMessage diff --git a/p2p/base/stun.h b/p2p/base/stun.h index 33410b5bae..caaa4745b4 100644 --- a/p2p/base/stun.h +++ b/p2p/base/stun.h @@ -581,9 +581,6 @@ enum TurnErrorType { STUN_ERROR_WRONG_CREDENTIALS = 441, STUN_ERROR_UNSUPPORTED_PROTOCOL = 442 }; - -extern const int SERVER_NOT_REACHABLE_ERROR; - extern const char STUN_ERROR_REASON_FORBIDDEN[]; extern const char STUN_ERROR_REASON_ALLOCATION_MISMATCH[]; extern const char STUN_ERROR_REASON_WRONG_CREDENTIALS[]; diff --git a/p2p/base/stun_port.cc b/p2p/base/stun_port.cc index 4662b0d17a..65112f2fec 100644 --- a/p2p/base/stun_port.cc +++ b/p2p/base/stun_port.cc @@ -79,10 +79,7 @@ class StunBindingRequest : public StunRequest { << " reason=" << attr->reason(); } - port_->OnStunBindingOrResolveRequestFailed( - server_addr_, attr ? attr->number() : STUN_ERROR_GLOBAL_FAILURE, - attr ? attr->reason() - : "STUN binding response with no error code attribute."); + port_->OnStunBindingOrResolveRequestFailed(server_addr_); int64_t now = rtc::TimeMillis(); if (WithinLifetime(now) && @@ -96,9 +93,8 @@ class StunBindingRequest : public StunRequest { RTC_LOG(LS_ERROR) << "Binding request timed out from " << port_->GetLocalAddress().ToSensitiveString() << " (" << port_->Network()->name() << ")"; - port_->OnStunBindingOrResolveRequestFailed( - server_addr_, SERVER_NOT_REACHABLE_ERROR, - "STUN allocate request timed out."); + + port_->OnStunBindingOrResolveRequestFailed(server_addr_); } private: @@ -108,7 +104,6 @@ class StunBindingRequest : public StunRequest { int lifetime = port_->stun_keepalive_lifetime(); return lifetime < 0 || rtc::TimeDiff(now, start_time_) <= lifetime; } - UDPPort* port_; const rtc::SocketAddress server_addr_; @@ -450,8 +445,7 @@ void UDPPort::OnResolveResult(const rtc::SocketAddress& input, int error) { RTC_LOG(LS_WARNING) << ToString() << ": StunPort: stun host lookup received error " << error; - OnStunBindingOrResolveRequestFailed(input, SERVER_NOT_REACHABLE_ERROR, - "STUN host lookup received error."); + OnStunBindingOrResolveRequestFailed(input); return; } @@ -475,10 +469,8 @@ void UDPPort::SendStunBindingRequest(const rtc::SocketAddress& stun_addr) { } else { // Since we can't send stun messages to the server, we should mark this // port ready. - const char* reason = "STUN server address is incompatible."; - RTC_LOG(LS_WARNING) << reason; - OnStunBindingOrResolveRequestFailed(stun_addr, SERVER_NOT_REACHABLE_ERROR, - reason); + RTC_LOG(LS_WARNING) << "STUN server address is incompatible."; + OnStunBindingOrResolveRequestFailed(stun_addr); } } } @@ -538,14 +530,7 @@ void UDPPort::OnStunBindingRequestSucceeded( } void UDPPort::OnStunBindingOrResolveRequestFailed( - const rtc::SocketAddress& stun_server_addr, - int error_code, - const std::string& reason) { - rtc::StringBuilder url; - url << "stun:" << stun_server_addr.ToString(); - SignalCandidateError( - this, IceCandidateErrorEvent(GetLocalAddress().ToSensitiveString(), - url.str(), error_code, reason)); + const rtc::SocketAddress& stun_server_addr) { if (bind_request_failed_servers_.find(stun_server_addr) != bind_request_failed_servers_.end()) { return; diff --git a/p2p/base/stun_port.h b/p2p/base/stun_port.h index 3c4234949a..1ee07279c9 100644 --- a/p2p/base/stun_port.h +++ b/p2p/base/stun_port.h @@ -222,9 +222,7 @@ class UDPPort : public Port { const rtc::SocketAddress& stun_server_addr, const rtc::SocketAddress& stun_reflected_addr); void OnStunBindingOrResolveRequestFailed( - const rtc::SocketAddress& stun_server_addr, - int error_code, - const std::string& reason); + const rtc::SocketAddress& stun_server_addr); // Sends STUN requests to the server. void OnSendPacket(const void* data, size_t size, StunRequest* req); diff --git a/p2p/base/stun_port_unittest.cc b/p2p/base/stun_port_unittest.cc index 98e41f9aa3..a23c063a8f 100644 --- a/p2p/base/stun_port_unittest.cc +++ b/p2p/base/stun_port_unittest.cc @@ -88,8 +88,6 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> { stun_port_->SignalPortComplete.connect(this, &StunPortTestBase::OnPortComplete); stun_port_->SignalPortError.connect(this, &StunPortTestBase::OnPortError); - stun_port_->SignalCandidateError.connect( - this, &StunPortTestBase::OnCandidateError); } void CreateSharedUdpPort(const rtc::SocketAddress& server_addr, @@ -147,10 +145,6 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> { done_ = true; error_ = true; } - void OnCandidateError(cricket::Port* port, - const cricket::IceCandidateErrorEvent& event) { - error_event_ = event; - } void SetKeepaliveDelay(int delay) { stun_keepalive_delay_ = delay; } void SetKeepaliveLifetime(int lifetime) { @@ -173,9 +167,6 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> { bool error_; int stun_keepalive_delay_; int stun_keepalive_lifetime_; - - protected: - cricket::IceCandidateErrorEvent error_event_; }; class StunPortTestWithRealClock : public StunPortTestBase {}; @@ -221,15 +212,6 @@ TEST_F(StunPortTest, TestPrepareAddressFail) { EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock); EXPECT_TRUE(error()); EXPECT_EQ(0U, port()->Candidates().size()); - EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, - cricket::SERVER_NOT_REACHABLE_ERROR, kTimeoutMs, - fake_clock); - ASSERT_NE(error_event_.error_text.find("."), std::string::npos); - ASSERT_NE( - error_event_.host_candidate.find(kLocalAddr.HostAsSensitiveURIString()), - std::string::npos); - std::string server_url = "stun:" + kBadAddr.ToString(); - ASSERT_EQ(error_event_.url, server_url); } // Test that we can get an address from a STUN server specified by a hostname. @@ -255,8 +237,6 @@ TEST_F(StunPortTestWithRealClock, TestPrepareAddressHostnameFail) { EXPECT_TRUE_WAIT(done(), kTimeoutMs); EXPECT_TRUE(error()); EXPECT_EQ(0U, port()->Candidates().size()); - EXPECT_EQ_WAIT(error_event_.error_code, cricket::SERVER_NOT_REACHABLE_ERROR, - kTimeoutMs); } // This test verifies keepalive response messages don't result in @@ -323,9 +303,6 @@ TEST_F(StunPortTest, TestMultipleStunServersWithBadServer) { PrepareAddress(); EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock); EXPECT_EQ(1U, port()->Candidates().size()); - std::string server_url = "stun:" + kBadAddr.ToString(); - ASSERT_EQ_SIMULATED_WAIT(error_event_.url, server_url, kTimeoutMs, - fake_clock); } // Test that two candidates are allocated if the two STUN servers return diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index f9104978b3..bcf574e332 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -326,8 +326,7 @@ void TurnPort::PrepareAddress() { if (credentials_.username.empty() || credentials_.password.empty()) { RTC_LOG(LS_ERROR) << "Allocation can't be started without setting the" " TURN server credentials for the user."; - OnAllocateError(STUN_ERROR_UNAUTHORIZED, - "Missing TURN server credentials."); + OnAllocateError(); return; } @@ -344,8 +343,7 @@ void TurnPort::PrepareAddress() { RTC_LOG(LS_ERROR) << "IP address family does not match. server: " << server_address_.address.family() << " local: " << Network()->GetBestIP().family(); - OnAllocateError(STUN_ERROR_GLOBAL_FAILURE, - "IP address family does not match."); + OnAllocateError(); return; } @@ -357,8 +355,7 @@ void TurnPort::PrepareAddress() { << server_address_.address.ToSensitiveString(); if (!CreateTurnClientSocket()) { RTC_LOG(LS_ERROR) << "Failed to create TURN client socket"; - OnAllocateError(STUN_ERROR_GLOBAL_FAILURE, - "Failed to create TURN client socket."); + OnAllocateError(); return; } if (server_address_.proto == PROTO_UDP) { @@ -476,9 +473,7 @@ void TurnPort::OnSocketConnect(rtc::AsyncPacketSocket* socket) { << socket_address.ipaddr().ToString() << ", rather than an address associated with network:" << Network()->ToString() << ". Discarding TURN port."; - OnAllocateError( - STUN_ERROR_GLOBAL_FAILURE, - "Address not associated with the desired network interface."); + OnAllocateError(); return; } } @@ -506,8 +501,7 @@ void TurnPort::OnAllocateMismatch() { RTC_LOG(LS_WARNING) << ToString() << ": Giving up on the port after " << allocate_mismatch_retries_ << " retries for STUN_ERROR_ALLOCATION_MISMATCH"; - OnAllocateError(STUN_ERROR_ALLOCATION_MISMATCH, - "Maximum retries reached for allocation mismatch."); + OnAllocateError(); return; } @@ -792,8 +786,7 @@ void TurnPort::OnResolveResult(rtc::AsyncResolverInterface* resolver) { if (resolver_->GetError() != 0 && (server_address_.proto == PROTO_TCP || server_address_.proto == PROTO_TLS)) { if (!CreateTurnClientSocket()) { - OnAllocateError(SERVER_NOT_REACHABLE_ERROR, - "TURN host lookup received error."); + OnAllocateError(); } return; } @@ -807,8 +800,7 @@ void TurnPort::OnResolveResult(rtc::AsyncResolverInterface* resolver) { RTC_LOG(LS_WARNING) << ToString() << ": TURN host lookup received error " << resolver_->GetError(); error_ = resolver_->GetError(); - OnAllocateError(SERVER_NOT_REACHABLE_ERROR, - "TURN host lookup received error."); + OnAllocateError(); return; } // Signal needs both resolved and unresolved address. After signal is sent @@ -855,20 +847,14 @@ void TurnPort::OnAllocateSuccess(const rtc::SocketAddress& address, ProtoToString(server_address_.proto), // The first hop protocol. "", // TCP canddiate type, empty for turn candidates. RELAY_PORT_TYPE, GetRelayPreference(server_address_.proto), - server_priority_, ReconstructedServerUrl(false /* use_hostname */), - true); + server_priority_, ReconstructedServerUrl(), true); } -void TurnPort::OnAllocateError(int error_code, const std::string& reason) { +void TurnPort::OnAllocateError() { // We will send SignalPortError asynchronously as this can be sent during // port initialization. This way it will not be blocking other port // creation. thread()->Post(RTC_FROM_HERE, this, MSG_ALLOCATE_ERROR); - SignalCandidateError( - this, - IceCandidateErrorEvent(GetLocalAddress().ToSensitiveString(), - ReconstructedServerUrl(true /* use_hostname */), - error_code, reason)); } void TurnPort::OnRefreshError() { @@ -901,7 +887,7 @@ void TurnPort::Release() { void TurnPort::Close() { if (!ready()) { - OnAllocateError(SERVER_NOT_REACHABLE_ERROR, ""); + OnAllocateError(); } request_manager_.Clear(); // Stop the port from creating new connections. @@ -955,8 +941,7 @@ void TurnPort::OnMessage(rtc::Message* message) { } void TurnPort::OnAllocateRequestTimeout() { - OnAllocateError(SERVER_NOT_REACHABLE_ERROR, - "TURN allocate request timed out."); + OnAllocateError(); } void TurnPort::HandleDataIndication(const char* data, @@ -1264,7 +1249,7 @@ bool TurnPort::SetEntryChannelId(const rtc::SocketAddress& address, return true; } -std::string TurnPort::ReconstructedServerUrl(bool use_hostname) { +std::string TurnPort::ReconstructedServerUrl() { // draft-petithuguenin-behave-turn-uris-01 // turnURI = scheme ":" turn-host [ ":" turn-port ] // [ "?transport=" transport ] @@ -1287,10 +1272,8 @@ std::string TurnPort::ReconstructedServerUrl(bool use_hostname) { break; } rtc::StringBuilder url; - url << scheme << ":" - << (use_hostname ? server_address_.address.hostname() - : server_address_.address.ipaddr().ToString()) - << ":" << server_address_.address.port() << "?transport=" << transport; + url << scheme << ":" << server_address_.address.ipaddr().ToString() << ":" + << server_address_.address.port() << "?transport=" << transport; return url.Release(); } @@ -1405,8 +1388,7 @@ void TurnAllocateRequest::OnErrorResponse(StunMessage* response) { << ": Received TURN allocate error response, id=" << rtc::hex_encode(id()) << ", code=" << error_code << ", rtt=" << Elapsed(); - const StunErrorCodeAttribute* attr = response->GetErrorCode(); - port_->OnAllocateError(error_code, attr ? attr->reason() : ""); + port_->OnAllocateError(); } } @@ -1422,8 +1404,7 @@ void TurnAllocateRequest::OnAuthChallenge(StunMessage* response, int code) { RTC_LOG(LS_WARNING) << port_->ToString() << ": Failed to authenticate with the server " "after challenge."; - const StunErrorCodeAttribute* attr = response->GetErrorCode(); - port_->OnAllocateError(STUN_ERROR_UNAUTHORIZED, attr ? attr->reason() : ""); + port_->OnAllocateError(); return; } @@ -1456,7 +1437,7 @@ void TurnAllocateRequest::OnTryAlternate(StunMessage* response, int code) { // According to RFC 5389 section 11, there are use cases where // authentication of response is not possible, we're not validating // message integrity. - const StunErrorCodeAttribute* error_code_attr = response->GetErrorCode(); + // Get the alternate server address attribute value. const StunAddressAttribute* alternate_server_attr = response->GetAddress(STUN_ATTR_ALTERNATE_SERVER); @@ -1464,13 +1445,11 @@ void TurnAllocateRequest::OnTryAlternate(StunMessage* response, int code) { RTC_LOG(LS_WARNING) << port_->ToString() << ": Missing STUN_ATTR_ALTERNATE_SERVER " "attribute in try alternate error response"; - port_->OnAllocateError(STUN_ERROR_TRY_ALTERNATE, - error_code_attr ? error_code_attr->reason() : ""); + port_->OnAllocateError(); return; } if (!port_->SetAlternateServer(alternate_server_attr->GetAddress())) { - port_->OnAllocateError(STUN_ERROR_TRY_ALTERNATE, - error_code_attr ? error_code_attr->reason() : ""); + port_->OnAllocateError(); return; } diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index 5edbb1cc58..e929d3e7cc 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -310,7 +310,7 @@ class TurnPort : public Port { void OnStunAddress(const rtc::SocketAddress& address); void OnAllocateSuccess(const rtc::SocketAddress& address, const rtc::SocketAddress& stun_address); - void OnAllocateError(int error_code, const std::string& reason); + void OnAllocateError(); void OnAllocateRequestTimeout(); void HandleDataIndication(const char* data, @@ -349,7 +349,7 @@ class TurnPort : public Port { bool FailAndPruneConnection(const rtc::SocketAddress& address); // Reconstruct the URL of the server which the candidate is gathered from. - std::string ReconstructedServerUrl(bool use_hostname); + std::string ReconstructedServerUrl(); void TurnCustomizerMaybeModifyOutgoingStunMessage(StunMessage* message); bool TurnCustomizerAllowChannelData(const void* data, diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index b51a1266b1..e713b2a901 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -66,7 +66,6 @@ static const SocketAddress kTurnIPv6IntAddr( static const SocketAddress kTurnUdpIPv6IntAddr( "2400:4030:1:2c00:be30:abcd:efab:cdef", cricket::TURN_SERVER_PORT); -static const SocketAddress kTurnInvalidAddr("www.google.invalid", 3478); static const char kCandidateFoundation[] = "foundation"; static const char kIceUfrag1[] = "TESTICEUFRAG0001"; @@ -177,10 +176,6 @@ class TurnPortTest : public ::testing::Test, void OnTurnPortComplete(Port* port) { turn_ready_ = true; } void OnTurnPortError(Port* port) { turn_error_ = true; } - void OnCandidateError(Port* port, - const cricket::IceCandidateErrorEvent& event) { - error_event_ = event; - } void OnTurnUnknownAddress(PortInterface* port, const SocketAddress& addr, ProtocolType proto, @@ -321,8 +316,6 @@ class TurnPortTest : public ::testing::Test, turn_port_->SignalPortComplete.connect(this, &TurnPortTest::OnTurnPortComplete); turn_port_->SignalPortError.connect(this, &TurnPortTest::OnTurnPortError); - turn_port_->SignalCandidateError.connect(this, - &TurnPortTest::OnCandidateError); turn_port_->SignalUnknownAddress.connect( this, &TurnPortTest::OnTurnUnknownAddress); turn_port_->SignalCreatePermissionResult.connect( @@ -762,7 +755,6 @@ class TurnPortTest : public ::testing::Test, std::vector udp_packets_; rtc::PacketOptions options; std::unique_ptr turn_customizer_; - cricket::IceCandidateErrorEvent error_event_; }; TEST_F(TurnPortTest, TestTurnPortType) { @@ -810,17 +802,6 @@ TEST_F(TurnPortTest, TestTurnAllocate) { EXPECT_NE(0, turn_port_->Candidates()[0].address().port()); } -// Test bad credentials. -TEST_F(TurnPortTest, TestTurnBadCredentials) { - CreateTurnPort(kTurnUsername, "bad", kTurnUdpProtoAddr); - turn_port_->PrepareAddress(); - EXPECT_TRUE_SIMULATED_WAIT(turn_error_, kSimulatedRtt * 3, fake_clock_); - ASSERT_EQ(0U, turn_port_->Candidates().size()); - EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, STUN_ERROR_UNAUTHORIZED, - kSimulatedRtt * 3, fake_clock_); - EXPECT_EQ(error_event_.error_text, "Unauthorized"); -} - // Testing a normal UDP allocation using TCP connection. TEST_F(TurnPortTest, TestTurnTcpAllocate) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); @@ -880,15 +861,6 @@ TEST_F(TurnPortTest, // Shouldn't take more than 1 RTT to realize the bound address isn't the one // expected. EXPECT_TRUE_SIMULATED_WAIT(turn_error_, kSimulatedRtt, fake_clock_); - EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, STUN_ERROR_GLOBAL_FAILURE, - kSimulatedRtt, fake_clock_); - ASSERT_NE(error_event_.error_text.find("."), std::string::npos); - ASSERT_NE( - error_event_.host_candidate.find(kLocalAddr2.HostAsSensitiveURIString()), - std::string::npos); - std::string server_url = - "turn:" + kTurnTcpIntAddr.ToString() + "?transport=tcp"; - ASSERT_EQ(error_event_.url, server_url); } // A caveat for the above logic: if the socket ends up bound to one of the IPs @@ -949,18 +921,14 @@ TEST_F(TurnPortTest, TCPPortNotDiscardedIfBoundToTemporaryIP) { TEST_F(TurnPortTest, TestTurnTcpOnAddressResolveFailure) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); CreateTurnPort(kTurnUsername, kTurnPassword, - ProtocolAddress(kTurnInvalidAddr, PROTO_TCP)); + ProtocolAddress(rtc::SocketAddress("www.google.invalid", 3478), + PROTO_TCP)); turn_port_->PrepareAddress(); EXPECT_TRUE_WAIT(turn_error_, kResolverTimeout); // As VSS doesn't provide a DNS resolution, name resolve will fail. TurnPort // will proceed in creating a TCP socket which will fail as there is no // server on the above domain and error will be set to SOCKET_ERROR. EXPECT_EQ(SOCKET_ERROR, turn_port_->error()); - EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, SERVER_NOT_REACHABLE_ERROR, - kSimulatedRtt, fake_clock_); - std::string server_url = - "turn:" + kTurnInvalidAddr.ToString() + "?transport=tcp"; - ASSERT_EQ(error_event_.url, server_url); } // Testing turn port will attempt to create TLS socket on address resolution @@ -968,7 +936,8 @@ TEST_F(TurnPortTest, TestTurnTcpOnAddressResolveFailure) { TEST_F(TurnPortTest, TestTurnTlsOnAddressResolveFailure) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TLS); CreateTurnPort(kTurnUsername, kTurnPassword, - ProtocolAddress(kTurnInvalidAddr, PROTO_TLS)); + ProtocolAddress(rtc::SocketAddress("www.google.invalid", 3478), + PROTO_TLS)); turn_port_->PrepareAddress(); EXPECT_TRUE_WAIT(turn_error_, kResolverTimeout); EXPECT_EQ(SOCKET_ERROR, turn_port_->error()); @@ -978,7 +947,8 @@ TEST_F(TurnPortTest, TestTurnTlsOnAddressResolveFailure) { // and return allocate failure. TEST_F(TurnPortTest, TestTurnUdpOnAddressResolveFailure) { CreateTurnPort(kTurnUsername, kTurnPassword, - ProtocolAddress(kTurnInvalidAddr, PROTO_UDP)); + ProtocolAddress(rtc::SocketAddress("www.google.invalid", 3478), + PROTO_UDP)); turn_port_->PrepareAddress(); EXPECT_TRUE_WAIT(turn_error_, kResolverTimeout); // Error from turn port will not be socket error. diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc index b1f147dcdc..4f418ee7f6 100644 --- a/p2p/client/basic_port_allocator.cc +++ b/p2p/client/basic_port_allocator.cc @@ -953,8 +953,6 @@ void BasicPortAllocatorSession::AddAllocatedPort(Port* port, port->SignalCandidateReady.connect( this, &BasicPortAllocatorSession::OnCandidateReady); - port->SignalCandidateError.connect( - this, &BasicPortAllocatorSession::OnCandidateError); port->SignalPortComplete.connect(this, &BasicPortAllocatorSession::OnPortComplete); port->SignalDestroyed.connect(this, @@ -1026,15 +1024,6 @@ void BasicPortAllocatorSession::OnCandidateReady(Port* port, } } -void BasicPortAllocatorSession::OnCandidateError( - Port* port, - const IceCandidateErrorEvent& event) { - RTC_DCHECK_RUN_ON(network_thread_); - RTC_DCHECK(FindPort(port)); - - SignalCandidateError(this, event); -} - Port* BasicPortAllocatorSession::GetBestTurnPortForNetwork( const std::string& network_name) const { RTC_DCHECK_RUN_ON(network_thread_); diff --git a/p2p/client/basic_port_allocator.h b/p2p/client/basic_port_allocator.h index 13611e724f..26eea1ef52 100644 --- a/p2p/client/basic_port_allocator.h +++ b/p2p/client/basic_port_allocator.h @@ -231,7 +231,6 @@ class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession, AllocationSequence* seq, bool prepare_address); void OnCandidateReady(Port* port, const Candidate& c); - void OnCandidateError(Port* port, const IceCandidateErrorEvent& event); void OnPortComplete(Port* port); void OnPortError(Port* port); void OnProtocolEnabled(AllocationSequence* seq, ProtocolType proto); diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 56d9a47ed7..93949c6657 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -523,8 +523,6 @@ JsepTransportController::CreateDtlsTransport( this, &JsepTransportController::OnTransportGatheringState_n); dtls->ice_transport()->SignalCandidateGathered.connect( this, &JsepTransportController::OnTransportCandidateGathered_n); - dtls->ice_transport()->SignalCandidateError.connect( - this, &JsepTransportController::OnTransportCandidateError_n); dtls->ice_transport()->SignalCandidatesRemoved.connect( this, &JsepTransportController::OnTransportCandidatesRemoved_n); dtls->ice_transport()->SignalRoleConflict.connect( @@ -1379,14 +1377,6 @@ void JsepTransportController::OnTransportCandidateGathered_n( }); } -void JsepTransportController::OnTransportCandidateError_n( - cricket::IceTransportInternal* transport, - const cricket::IceCandidateErrorEvent& event) { - RTC_DCHECK(network_thread_->IsCurrent()); - - invoker_.AsyncInvoke(RTC_FROM_HERE, signaling_thread_, - [this, event] { SignalIceCandidateError(event); }); -} void JsepTransportController::OnTransportCandidatesRemoved_n( cricket::IceTransportInternal* transport, const cricket::Candidates& candidates) { diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index fcae15373f..995c703ce1 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -235,9 +235,6 @@ class JsepTransportController : public sigslot::has_slots<> { sigslot::signal2&> SignalIceCandidatesGathered; - sigslot::signal1 - SignalIceCandidateError; - sigslot::signal1&> SignalIceCandidatesRemoved; @@ -378,9 +375,6 @@ class JsepTransportController : public sigslot::has_slots<> { void OnTransportGatheringState_n(cricket::IceTransportInternal* transport); void OnTransportCandidateGathered_n(cricket::IceTransportInternal* transport, const cricket::Candidate& candidate); - void OnTransportCandidateError_n( - cricket::IceTransportInternal* transport, - const cricket::IceCandidateErrorEvent& event); void OnTransportCandidatesRemoved_n(cricket::IceTransportInternal* transport, const cricket::Candidates& candidates); void OnTransportRoleConflict_n(cricket::IceTransportInternal* transport); diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index b94b883989..fde0433278 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1075,8 +1075,6 @@ bool PeerConnection::Initialize( this, &PeerConnection::OnTransportControllerGatheringState); transport_controller_->SignalIceCandidatesGathered.connect( this, &PeerConnection::OnTransportControllerCandidatesGathered); - transport_controller_->SignalIceCandidateError.connect( - this, &PeerConnection::OnTransportControllerCandidateError); transport_controller_->SignalIceCandidatesRemoved.connect( this, &PeerConnection::OnTransportControllerCandidatesRemoved); transport_controller_->SignalDtlsHandshakeError.connect( @@ -4171,16 +4169,6 @@ void PeerConnection::OnIceCandidate( Observer()->OnIceCandidate(candidate.get()); } -void PeerConnection::OnIceCandidateError(const std::string& host_candidate, - const std::string& url, - int error_code, - const std::string& error_text) { - if (IsClosed()) { - return; - } - Observer()->OnIceCandidateError(host_candidate, url, error_code, error_text); -} - void PeerConnection::OnIceCandidatesRemoved( const std::vector& candidates) { if (IsClosed()) { @@ -6119,12 +6107,6 @@ void PeerConnection::OnTransportControllerCandidatesGathered( } } -void PeerConnection::OnTransportControllerCandidateError( - const cricket::IceCandidateErrorEvent& event) { - OnIceCandidateError(event.host_candidate, event.url, event.error_code, - event.error_text); -} - void PeerConnection::OnTransportControllerCandidatesRemoved( const std::vector& candidates) { // Sanity check. diff --git a/pc/peer_connection.h b/pc/peer_connection.h index e7362d9f39..d2fb768a33 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -413,18 +413,12 @@ class PeerConnection : public PeerConnectionInternal, PeerConnectionInterface::PeerConnectionState new_state) RTC_RUN_ON(signaling_thread()); - // Called any time the IceGatheringState changes. + // Called any time the IceGatheringState changes void OnIceGatheringChange(IceGatheringState new_state) RTC_RUN_ON(signaling_thread()); // New ICE candidate has been gathered. void OnIceCandidate(std::unique_ptr candidate) RTC_RUN_ON(signaling_thread()); - // Gathering of an ICE candidate failed. - void OnIceCandidateError(const std::string& host_candidate, - const std::string& url, - int error_code, - const std::string& error_text) - RTC_RUN_ON(signaling_thread()); // Some local ICE candidates have been removed. void OnIceCandidatesRemoved(const std::vector& candidates) RTC_RUN_ON(signaling_thread()); @@ -1006,9 +1000,6 @@ class PeerConnection : public PeerConnectionInternal, const std::string& transport_name, const std::vector& candidates) RTC_RUN_ON(signaling_thread()); - void OnTransportControllerCandidateError( - const cricket::IceCandidateErrorEvent& event) - RTC_RUN_ON(signaling_thread()); void OnTransportControllerCandidatesRemoved( const std::vector& candidates) RTC_RUN_ON(signaling_thread()); diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index 3eeeeb8647..395c43768d 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -565,9 +565,6 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, const cricket::Candidate& last_candidate_gathered() const { return last_candidate_gathered_; } - const cricket::IceCandidateErrorEvent& error_event() const { - return error_event_; - } // Sets the mDNS responder for the owned fake network manager and keeps a // reference to the responder. @@ -961,13 +958,6 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, SendIceMessage(candidate->sdp_mid(), candidate->sdp_mline_index(), ice_sdp); last_candidate_gathered_ = candidate->candidate(); } - void OnIceCandidateError(const std::string& host_candidate, - const std::string& url, - int error_code, - const std::string& error_text) override { - error_event_ = cricket::IceCandidateErrorEvent(host_candidate, url, - error_code, error_text); - } void OnDataChannel( rtc::scoped_refptr data_channel) override { RTC_LOG(LS_INFO) << debug_name_ << ": OnDataChannel"; @@ -1000,7 +990,6 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, int signaling_delay_ms_ = 0; bool signal_ice_candidates_ = true; cricket::Candidate last_candidate_gathered_; - cricket::IceCandidateErrorEvent error_event_; // Store references to the video sources we've created, so that we can stop // them, if required. @@ -5165,46 +5154,6 @@ TEST_P(PeerConnectionIntegrationTest, RegatherAfterChangingIceTransportType) { ClosePeerConnections(); } -TEST_P(PeerConnectionIntegrationTest, OnIceCandidateError) { - webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-GatherOnCandidateFilterChanged/Enabled/"); - static const rtc::SocketAddress turn_server_internal_address{"88.88.88.0", - 3478}; - static const rtc::SocketAddress turn_server_external_address{"88.88.88.1", 0}; - - CreateTurnServer(turn_server_internal_address, turn_server_external_address); - - webrtc::PeerConnectionInterface::IceServer ice_server; - ice_server.urls.push_back("turn:88.88.88.0:3478"); - ice_server.username = "test"; - ice_server.password = "123"; - - PeerConnectionInterface::RTCConfiguration caller_config; - caller_config.servers.push_back(ice_server); - caller_config.type = webrtc::PeerConnectionInterface::kRelay; - caller_config.continual_gathering_policy = PeerConnection::GATHER_CONTINUALLY; - - PeerConnectionInterface::RTCConfiguration callee_config; - callee_config.servers.push_back(ice_server); - callee_config.type = webrtc::PeerConnectionInterface::kRelay; - callee_config.continual_gathering_policy = PeerConnection::GATHER_CONTINUALLY; - - ASSERT_TRUE( - CreatePeerConnectionWrappersWithConfig(caller_config, callee_config)); - - // Do normal offer/answer and wait for ICE to complete. - ConnectFakeSignaling(); - caller()->AddAudioVideoTracks(); - callee()->AddAudioVideoTracks(); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - EXPECT_EQ_WAIT(401, caller()->error_event().error_code, kDefaultTimeout); - EXPECT_EQ("Unauthorized", caller()->error_event().error_text); - EXPECT_EQ("turn:88.88.88.0:3478?transport=udp", caller()->error_event().url); - EXPECT_NE(std::string::npos, - caller()->error_event().host_candidate.find(":")); -} - INSTANTIATE_TEST_SUITE_P(PeerConnectionIntegrationTest, PeerConnectionIntegrationTest, Values(SdpSemantics::kPlanB,