diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index 332d951a18..d83726eab5 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -279,7 +279,6 @@ void P2PTransportChannel::AddAllocatorSession( // We now only want to apply new candidates that we receive to the ports // created by this new session because these are replacing those of the // previous sessions. - removed_ports_.insert(removed_ports_.end(), ports_.begin(), ports_.end()); ports_.clear(); allocator_sessions_.push_back(std::move(session)); @@ -306,20 +305,16 @@ void P2PTransportChannel::SetIceRole(IceRole ice_role) { ASSERT(worker_thread_ == rtc::Thread::Current()); if (ice_role_ != ice_role) { ice_role_ = ice_role; - for (PortInterface* port : ports_) { - port->SetIceRole(ice_role); - } - // Update role on removed ports as well, because they may still have - // connections alive that should be using the correct role. - for (PortInterface* port : removed_ports_) { - port->SetIceRole(ice_role); + for (std::vector::iterator it = ports_.begin(); + it != ports_.end(); ++it) { + (*it)->SetIceRole(ice_role); } } } void P2PTransportChannel::SetIceTiebreaker(uint64_t tiebreaker) { ASSERT(worker_thread_ == rtc::Thread::Current()); - if (!ports_.empty() || !removed_ports_.empty()) { + if (!ports_.empty()) { LOG(LS_ERROR) << "Attempt to change tiebreaker after Port has been allocated."; return; @@ -972,13 +967,13 @@ int P2PTransportChannel::SetOption(rtc::Socket::Option opt, int value) { it->second = value; } - for (PortInterface* port : ports_) { - int val = port->SetOption(opt, value); + for (size_t i = 0; i < ports_.size(); ++i) { + int val = ports_[i]->SetOption(opt, value); if (val < 0) { // Because this also occurs deferred, probably no point in reporting an // error - LOG(WARNING) << "SetOption(" << opt << ", " << value - << ") failed: " << port->GetError(); + LOG(WARNING) << "SetOption(" << opt << ", " << value << ") failed: " + << ports_[i]->GetError(); } } return 0; @@ -1518,9 +1513,10 @@ void P2PTransportChannel::OnPortDestroyed(PortInterface* port) { ASSERT(worker_thread_ == rtc::Thread::Current()); // Remove this port from the list (if we didn't drop it already). - ports_.erase(std::remove(ports_.begin(), ports_.end(), port)); - removed_ports_.erase( - std::remove(removed_ports_.begin(), removed_ports_.end(), port)); + std::vector::iterator iter = + std::find(ports_.begin(), ports_.end(), port); + if (iter != ports_.end()) + ports_.erase(iter); LOG(INFO) << "Removed port from p2p socket: " << static_cast(ports_.size()) << " remaining"; @@ -1537,7 +1533,6 @@ void P2PTransportChannel::OnPortNetworkInactive(PortInterface* port) { if (it == ports_.end()) { return; } - removed_ports_.push_back(*it); ports_.erase(it); LOG(INFO) << "Removed port due to inactive networks: " << ports_.size() << " remaining"; diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 83f809b48a..ada3ecfb8b 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -297,14 +297,7 @@ class P2PTransportChannel : public TransportChannelImpl, bool incoming_only_; int error_; std::vector> allocator_sessions_; - // |ports_| contains ports that are used to form new connections when - // new remote candidates are added. - std::vector ports_; - // |removed_ports_| contains ports that have been removed from |ports_| and - // are not being used to form new connections, but that aren't yet destroyed. - // They may have existing connections, and they still fire signals such as - // SignalUnknownAddress. - std::vector removed_ports_; + std::vector ports_; // |connections_| is a sorted list with the first one always be the // |best_connection_| when it's not nullptr. The combination of diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index fa04b864bf..587f971f1e 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -2693,58 +2693,6 @@ TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) { EXPECT_TRUE(!ch.allocator_session()->IsGettingPorts()); } -// Test that the ICE role is updated even on ports with inactive networks when -// doing continual gathering. These ports may still have connections that need -// a correct role, in case the network becomes active before the connection is -// destroyed. -TEST_F(P2PTransportChannelPingTest, - TestIceRoleUpdatedOnPortAfterSignalNetworkInactive) { - cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr); - cricket::P2PTransportChannel ch( - "test channel", cricket::ICE_CANDIDATE_COMPONENT_DEFAULT, &pa); - // Starts with ICEROLE_CONTROLLING. - PrepareChannel(&ch); - cricket::IceConfig config = CreateIceConfig(1000, true); - ch.SetIceConfig(config); - ch.Connect(); - ch.MaybeStartGathering(); - ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 1)); - - cricket::Connection* conn = WaitForConnectionTo(&ch, "1.1.1.1", 1); - ASSERT_TRUE(conn != nullptr); - - // Make the fake port signal that its network is inactive, then change the - // ICE role and expect it to be updated. - conn->port()->SignalNetworkInactive(conn->port()); - ch.SetIceRole(cricket::ICEROLE_CONTROLLED); - EXPECT_EQ(cricket::ICEROLE_CONTROLLED, conn->port()->GetIceRole()); -} - -// Test that the ICE role is updated even on ports with inactive networks. -// These ports may still have connections that need a correct role, for the -// pings sent by those connections until they're replaced by newer-generation -// connections. -TEST_F(P2PTransportChannelPingTest, TestIceRoleUpdatedOnPortAfterIceRestart) { - cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr); - cricket::P2PTransportChannel ch( - "test channel", cricket::ICE_CANDIDATE_COMPONENT_DEFAULT, &pa); - // Starts with ICEROLE_CONTROLLING. - PrepareChannel(&ch); - ch.Connect(); - ch.MaybeStartGathering(); - ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 1)); - - cricket::Connection* conn = WaitForConnectionTo(&ch, "1.1.1.1", 1); - ASSERT_TRUE(conn != nullptr); - - // Do an ICE restart, change the role, and expect the old port to have its - // role updated. - ch.SetIceCredentials(kIceUfrag[1], kIcePwd[1]); - ch.MaybeStartGathering(); - ch.SetIceRole(cricket::ICEROLE_CONTROLLED); - EXPECT_EQ(cricket::ICEROLE_CONTROLLED, conn->port()->GetIceRole()); -} - class P2PTransportChannelMostLikelyToWorkFirstTest : public P2PTransportChannelPingTest { public: