Skip to content

Commit

Permalink
Reland "Add an active ICE controller that wraps a legacy controller (w…
Browse files Browse the repository at this point in the history
…ebrtc-sdk#7/n)"

This is a reland of commit 6326c9c

Original change's description:
> Add an active ICE controller that wraps a legacy controller (webrtc-sdk#7/n)
>
> The wrapping ICE controller will allow existing ICE controller implementations to migrate to the active interface, and eventually deprecate the legacy interface.
>
> Follow-up CL has unit tests for P2PTransportChannel using the new wrapping controller.
>
> Bug: webrtc:14367, webrtc:14131
> Change-Id: I6c517449ff1e503e8268a7ef91afda793723fdeb
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275302
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
> Commit-Queue: Sameer Vijaykar <samvi@google.com>
> Cr-Commit-Position: refs/heads/main@{#38130}

Bug: webrtc:14367, webrtc:14131
Change-Id: I5662595db1df8c06b3acac9b39749f236906fa7e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276044
Auto-Submit: Sameer Vijaykar <samvi@google.com>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38149}
  • Loading branch information
sam-vi authored and WebRTC LUCI CQ committed Sep 21, 2022
1 parent 2f650fa commit 52dd1a5
Show file tree
Hide file tree
Showing 7 changed files with 731 additions and 6 deletions.
6 changes: 4 additions & 2 deletions api/ice_transport_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,10 @@ struct IceTransportInit final {
// constructed and used.
//
// 2. If the field trial is enabled
// - then an active ICE controller factory must be supplied and is used.
// - the legacy ICE controller factory is not used in this case.
// a. If an active ICE controller factory is supplied, it is used and
// the legacy ICE controller factory is not used.
// b. If not, a default active ICE controller is used, wrapping over the
// supplied or the default legacy ICE controller.
void set_active_ice_controller_factory(
cricket::ActiveIceControllerFactoryInterface*
active_ice_controller_factory) {
Expand Down
4 changes: 4 additions & 0 deletions p2p/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ rtc_library("rtc_p2p") {
"base/turn_port.cc",
"base/turn_port.h",
"base/udp_port.h",
"base/wrapping_active_ice_controller.cc",
"base/wrapping_active_ice_controller.h",
"client/basic_port_allocator.cc",
"client/basic_port_allocator.h",
"client/relay_port_factory_interface.h",
Expand Down Expand Up @@ -201,6 +203,7 @@ if (rtc_include_tests) {
"base/fake_packet_transport.h",
"base/mock_active_ice_controller.h",
"base/mock_async_resolver.h",
"base/mock_ice_agent.h",
"base/mock_ice_controller.h",
"base/mock_ice_transport.h",
"base/test_stun_server.cc",
Expand Down Expand Up @@ -260,6 +263,7 @@ if (rtc_include_tests) {
"base/transport_description_unittest.cc",
"base/turn_port_unittest.cc",
"base/turn_server_unittest.cc",
"base/wrapping_active_ice_controller_unittest.cc",
"client/basic_port_allocator_unittest.cc",
]
deps = [
Expand Down
50 changes: 50 additions & 0 deletions p2p/base/mock_ice_agent.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright 2018 The WebRTC Project Authors. All rights reserved.
*
* Use of this source code is governed by a BSD-style license
* that can be found in the LICENSE file in the root of the source
* tree. An additional intellectual property rights grant can be found
* in the file PATENTS. All contributing project authors may
* be found in the AUTHORS file in the root of the source tree.
*/

#ifndef P2P_BASE_MOCK_ICE_AGENT_H_
#define P2P_BASE_MOCK_ICE_AGENT_H_

#include <vector>

#include "p2p/base/connection.h"
#include "p2p/base/ice_agent_interface.h"
#include "p2p/base/ice_switch_reason.h"
#include "p2p/base/transport_description.h"
#include "test/gmock.h"

namespace cricket {

class MockIceAgent : public IceAgentInterface {
public:
~MockIceAgent() override = default;

MOCK_METHOD(int64_t, GetLastPingSentMs, (), (override, const));
MOCK_METHOD(IceRole, GetIceRole, (), (override, const));
MOCK_METHOD(void, OnStartedPinging, (), (override));
MOCK_METHOD(void, UpdateConnectionStates, (), (override));
MOCK_METHOD(void, UpdateState, (), (override));
MOCK_METHOD(void,
ForgetLearnedStateForConnections,
(std::vector<const Connection*>),
(override));
MOCK_METHOD(void, SendPingRequest, (const Connection*), (override));
MOCK_METHOD(void,
SwitchSelectedConnection,
(const Connection*, IceSwitchReason),
(override));
MOCK_METHOD(bool,
PruneConnections,
(std::vector<const Connection*>),
(override));
};

} // namespace cricket

#endif // P2P_BASE_MOCK_ICE_AGENT_H_
14 changes: 10 additions & 4 deletions p2p/base/p2p_transport_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "p2p/base/connection.h"
#include "p2p/base/connection_info.h"
#include "p2p/base/port.h"
#include "p2p/base/wrapping_active_ice_controller.h"
#include "rtc_base/checks.h"
#include "rtc_base/crc32.h"
#include "rtc_base/experiments/struct_parameters_parser.h"
Expand Down Expand Up @@ -2472,10 +2473,15 @@ P2PTransportChannel::IceControllerAdapter::IceControllerAdapter(
P2PTransportChannel* transport)
: transport_(transport) {
if (UseActiveIceControllerFieldTrialEnabled(field_trials)) {
RTC_DCHECK(active_ice_controller_factory);
ActiveIceControllerFactoryArgs active_args{args,
/* ice_agent= */ transport};
active_ice_controller_ = active_ice_controller_factory->Create(active_args);
if (active_ice_controller_factory) {
ActiveIceControllerFactoryArgs active_args{args,
/* ice_agent= */ transport};
active_ice_controller_ =
active_ice_controller_factory->Create(active_args);
} else {
active_ice_controller_ = std::make_unique<WrappingActiveIceController>(
/* ice_agent= */ transport, ice_controller_factory, args);
}
} else {
if (ice_controller_factory != nullptr) {
legacy_ice_controller_ = ice_controller_factory->Create(args);
Expand Down
253 changes: 253 additions & 0 deletions p2p/base/wrapping_active_ice_controller.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,253 @@
/*
* Copyright 2022 The WebRTC Project Authors. All rights reserved.
*
* Use of this source code is governed by a BSD-style license
* that can be found in the LICENSE file in the root of the source
* tree. An additional intellectual property rights grant can be found
* in the file PATENTS. All contributing project authors may
* be found in the AUTHORS file in the root of the source tree.
*/

#include "p2p/base/wrapping_active_ice_controller.h"

#include <memory>
#include <utility>
#include <vector>

#include "api/sequence_checker.h"
#include "api/task_queue/pending_task_safety_flag.h"
#include "api/units/time_delta.h"
#include "p2p/base/basic_ice_controller.h"
#include "p2p/base/connection.h"
#include "p2p/base/ice_agent_interface.h"
#include "p2p/base/ice_controller_interface.h"
#include "p2p/base/ice_switch_reason.h"
#include "p2p/base/ice_transport_internal.h"
#include "p2p/base/transport_description.h"
#include "rtc_base/logging.h"
#include "rtc_base/thread.h"
#include "rtc_base/time_utils.h"

namespace {
using ::webrtc::SafeTask;
using ::webrtc::TimeDelta;
} // unnamed namespace

namespace cricket {

WrappingActiveIceController::WrappingActiveIceController(
IceAgentInterface* ice_agent,
std::unique_ptr<IceControllerInterface> wrapped)
: network_thread_(rtc::Thread::Current()),
wrapped_(std::move(wrapped)),
agent_(*ice_agent) {
RTC_DCHECK(ice_agent != nullptr);
}

WrappingActiveIceController::WrappingActiveIceController(
IceAgentInterface* ice_agent,
IceControllerFactoryInterface* wrapped_factory,
const IceControllerFactoryArgs& wrapped_factory_args)
: network_thread_(rtc::Thread::Current()), agent_(*ice_agent) {
RTC_DCHECK(ice_agent != nullptr);
if (wrapped_factory) {
wrapped_ = wrapped_factory->Create(wrapped_factory_args);
} else {
wrapped_ = std::make_unique<BasicIceController>(wrapped_factory_args);
}
}

WrappingActiveIceController::~WrappingActiveIceController() {}

void WrappingActiveIceController::SetIceConfig(const IceConfig& config) {
RTC_DCHECK_RUN_ON(network_thread_);
wrapped_->SetIceConfig(config);
}

bool WrappingActiveIceController::GetUseCandidateAttribute(
const Connection* connection,
NominationMode mode,
IceMode remote_ice_mode) const {
RTC_DCHECK_RUN_ON(network_thread_);
return wrapped_->GetUseCandidateAttr(connection, mode, remote_ice_mode);
}

void WrappingActiveIceController::OnConnectionAdded(
const Connection* connection) {
RTC_DCHECK_RUN_ON(network_thread_);
wrapped_->AddConnection(connection);
}

void WrappingActiveIceController::OnConnectionPinged(
const Connection* connection) {
RTC_DCHECK_RUN_ON(network_thread_);
wrapped_->MarkConnectionPinged(connection);
}

void WrappingActiveIceController::OnConnectionUpdated(
const Connection* connection) {
RTC_LOG(LS_VERBOSE) << "Connection report for " << connection->ToString();
// Do nothing. Native ICE controllers have direct access to Connection, so no
// need to update connection state separately.
}

void WrappingActiveIceController::OnConnectionSwitched(
const Connection* connection) {
RTC_DCHECK_RUN_ON(network_thread_);
selected_connection_ = connection;
wrapped_->SetSelectedConnection(connection);
}

void WrappingActiveIceController::OnConnectionDestroyed(
const Connection* connection) {
RTC_DCHECK_RUN_ON(network_thread_);
wrapped_->OnConnectionDestroyed(connection);
}

void WrappingActiveIceController::MaybeStartPinging() {
RTC_DCHECK_RUN_ON(network_thread_);
if (started_pinging_) {
return;
}

if (wrapped_->HasPingableConnection()) {
network_thread_->PostTask(
SafeTask(task_safety_.flag(), [this]() { SelectAndPingConnection(); }));
agent_.OnStartedPinging();
started_pinging_ = true;
}
}

void WrappingActiveIceController::SelectAndPingConnection() {
RTC_DCHECK_RUN_ON(network_thread_);
agent_.UpdateConnectionStates();

IceControllerInterface::PingResult result =
wrapped_->SelectConnectionToPing(agent_.GetLastPingSentMs());
HandlePingResult(result);
}

void WrappingActiveIceController::HandlePingResult(
IceControllerInterface::PingResult result) {
RTC_DCHECK_RUN_ON(network_thread_);

if (result.connection.has_value()) {
agent_.SendPingRequest(result.connection.value());
}

network_thread_->PostDelayedTask(
SafeTask(task_safety_.flag(), [this]() { SelectAndPingConnection(); }),
TimeDelta::Millis(result.recheck_delay_ms));
}

void WrappingActiveIceController::OnSortAndSwitchRequest(
IceSwitchReason reason) {
RTC_DCHECK_RUN_ON(network_thread_);
if (!sort_pending_) {
network_thread_->PostTask(SafeTask(task_safety_.flag(), [this, reason]() {
SortAndSwitchToBestConnection(reason);
}));
sort_pending_ = true;
}
}

void WrappingActiveIceController::OnImmediateSortAndSwitchRequest(
IceSwitchReason reason) {
RTC_DCHECK_RUN_ON(network_thread_);
SortAndSwitchToBestConnection(reason);
}

void WrappingActiveIceController::SortAndSwitchToBestConnection(
IceSwitchReason reason) {
RTC_DCHECK_RUN_ON(network_thread_);

// Make sure the connection states are up-to-date since this affects how they
// will be sorted.
agent_.UpdateConnectionStates();

// Any changes after this point will require a re-sort.
sort_pending_ = false;

IceControllerInterface::SwitchResult result =
wrapped_->SortAndSwitchConnection(reason);
HandleSwitchResult(reason, result);
UpdateStateOnConnectionsResorted();
}

bool WrappingActiveIceController::OnImmediateSwitchRequest(
IceSwitchReason reason,
const Connection* selected) {
RTC_DCHECK_RUN_ON(network_thread_);
IceControllerInterface::SwitchResult result =
wrapped_->ShouldSwitchConnection(reason, selected);
HandleSwitchResult(reason, result);
return result.connection.has_value();
}

void WrappingActiveIceController::HandleSwitchResult(
IceSwitchReason reason_for_switch,
IceControllerInterface::SwitchResult result) {
RTC_DCHECK_RUN_ON(network_thread_);
if (result.connection.has_value()) {
RTC_LOG(LS_INFO) << "Switching selected connection due to: "
<< IceSwitchReasonToString(reason_for_switch);
agent_.SwitchSelectedConnection(result.connection.value(),
reason_for_switch);
}

if (result.recheck_event.has_value()) {
// If we do not switch to the connection because it missed the receiving
// threshold, the new connection is in a better receiving state than the
// currently selected connection. So we need to re-check whether it needs
// to be switched at a later time.
network_thread_->PostDelayedTask(
SafeTask(task_safety_.flag(),
[this, recheck_reason = result.recheck_event->reason]() {
SortAndSwitchToBestConnection(recheck_reason);
}),
TimeDelta::Millis(result.recheck_event->recheck_delay_ms));
}

agent_.ForgetLearnedStateForConnections(
result.connections_to_forget_state_on);
}

void WrappingActiveIceController::UpdateStateOnConnectionsResorted() {
RTC_DCHECK_RUN_ON(network_thread_);
PruneConnections();

// Update the internal state of the ICE agentl.
agent_.UpdateState();

// Also possibly start pinging.
// We could start pinging if:
// * The first connection was created.
// * ICE credentials were provided.
// * A TCP connection became connected.
MaybeStartPinging();
}

void WrappingActiveIceController::PruneConnections() {
RTC_DCHECK_RUN_ON(network_thread_);

// The controlled side can prune only if the selected connection has been
// nominated because otherwise it may prune the connection that will be
// selected by the controlling side.
// TODO(honghaiz): This is not enough to prevent a connection from being
// pruned too early because with aggressive nomination, the controlling side
// will nominate every connection until it becomes writable.
if (agent_.GetIceRole() == ICEROLE_CONTROLLING ||
(selected_connection_ && selected_connection_->nominated())) {
std::vector<const Connection*> connections_to_prune =
wrapped_->PruneConnections();
agent_.PruneConnections(connections_to_prune);
}
}

// Only for unit tests
const Connection* WrappingActiveIceController::FindNextPingableConnection() {
RTC_DCHECK_RUN_ON(network_thread_);
return wrapped_->FindNextPingableConnection();
}

} // namespace cricket
Loading

0 comments on commit 52dd1a5

Please sign in to comment.