WebRTC-DeprecateGlobalFieldTrialString/Enabled/ - part 11/inf
convert rtc_base/network and collateral.
This also remove last usage of system_wrappers/field_trials
in p2p/...Yay!
Bug: webrtc:10335
Change-Id: Ie8507b1f52bf7f3067e9b4bf8c81a825e4644fda
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256640
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36357}
diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn
index e09b62f..4015fc9 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -947,9 +947,12 @@
"../api:refcountedbase",
"../api:scoped_refptr",
"../api:sequence_checker",
+ "../api:webrtc_key_value_config",
"../api/numerics",
"../api/task_queue",
+ "../api/transport:field_trial_based_config",
"../system_wrappers:field_trial",
+ "memory:always_valid_pointer",
"network:sent_packet",
"synchronization:mutex",
"system:file_wrapper",
@@ -1230,6 +1233,8 @@
":threading",
"../api/units:time_delta",
"../api/units:timestamp",
+ "../test:scoped_key_value_config",
+ "memory:always_valid_pointer",
"memory:fifo_buffer",
"synchronization:mutex",
"task_utils:to_queued_task",
@@ -1546,6 +1551,7 @@
"../test:field_trial",
"../test:fileutils",
"../test:rtc_expect_death",
+ "../test:scoped_key_value_config",
"../test:test_main",
"../test:test_support",
"memory:fifo_buffer",
diff --git a/rtc_base/memory/always_valid_pointer.h b/rtc_base/memory/always_valid_pointer.h
index 570ad93..c6e0a70 100644
--- a/rtc_base/memory/always_valid_pointer.h
+++ b/rtc_base/memory/always_valid_pointer.h
@@ -41,6 +41,10 @@
Interface* operator->() { return pointer_; }
Interface& operator*() { return *pointer_; }
+ Interface* get() const { return pointer_; }
+ Interface* operator->() const { return pointer_; }
+ Interface& operator*() const { return *pointer_; }
+
private:
const std::unique_ptr<Interface> owned_instance_;
Interface* const pointer_;
diff --git a/rtc_base/nat_unittest.cc b/rtc_base/nat_unittest.cc
index af6256f..50feda9 100644
--- a/rtc_base/nat_unittest.cc
+++ b/rtc_base/nat_unittest.cc
@@ -37,6 +37,7 @@
#include "rtc_base/thread.h"
#include "rtc_base/virtual_socket_server.h"
#include "test/gtest.h"
+#include "test/scoped_key_value_config.h"
namespace rtc {
namespace {
@@ -219,8 +220,9 @@
}
void TestPhysicalInternal(const SocketAddress& int_addr) {
+ webrtc::test::ScopedKeyValueConfig field_trials;
PhysicalSocketServer socket_server;
- BasicNetworkManager network_manager(nullptr, &socket_server);
+ BasicNetworkManager network_manager(nullptr, &socket_server, &field_trials);
network_manager.StartUpdating();
// Process pending messages so the network list is updated.
Thread::Current()->ProcessMessages(0);
diff --git a/rtc_base/network.cc b/rtc_base/network.cc
index 5202630..b2bce37 100644
--- a/rtc_base/network.cc
+++ b/rtc_base/network.cc
@@ -29,8 +29,10 @@
#include "absl/algorithm/container.h"
#include "absl/strings/match.h"
#include "absl/strings/string_view.h"
+#include "api/transport/field_trial_based_config.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
+#include "rtc_base/memory/always_valid_pointer.h"
#include "rtc_base/network_monitor.h"
#include "rtc_base/socket.h" // includes something that makes windows happy
#include "rtc_base/string_encode.h"
@@ -38,7 +40,6 @@
#include "rtc_base/strings/string_builder.h"
#include "rtc_base/task_utils/to_queued_task.h"
#include "rtc_base/thread.h"
-#include "system_wrappers/include/field_trial.h"
namespace rtc {
namespace {
@@ -280,10 +281,13 @@
return nullptr;
}
-NetworkManagerBase::NetworkManagerBase()
+NetworkManagerBase::NetworkManagerBase(
+ const webrtc::WebRtcKeyValueConfig* field_trials)
: enumeration_permission_(NetworkManager::ENUMERATION_ALLOWED),
- signal_network_preference_change_(webrtc::field_trial::IsEnabled(
- "WebRTC-SignalNetworkPreferenceChange")) {}
+ signal_network_preference_change_(
+ field_trials
+ ? field_trials->IsEnabled("WebRTC-SignalNetworkPreferenceChange")
+ : false) {}
NetworkManagerBase::~NetworkManagerBase() {
for (const auto& kv : networks_map_) {
@@ -508,25 +512,17 @@
return false;
}
-BasicNetworkManager::BasicNetworkManager()
- : BasicNetworkManager(nullptr, nullptr) {}
-
-BasicNetworkManager::BasicNetworkManager(SocketFactory* socket_factory)
- : BasicNetworkManager(nullptr, socket_factory) {}
-
-BasicNetworkManager::BasicNetworkManager(
- NetworkMonitorFactory* network_monitor_factory)
- : BasicNetworkManager(network_monitor_factory, nullptr) {}
-
BasicNetworkManager::BasicNetworkManager(
NetworkMonitorFactory* network_monitor_factory,
- SocketFactory* socket_factory)
- : network_monitor_factory_(network_monitor_factory),
+ SocketFactory* socket_factory,
+ const webrtc::WebRtcKeyValueConfig* field_trials)
+ : field_trials_(field_trials),
+ network_monitor_factory_(network_monitor_factory),
socket_factory_(socket_factory),
allow_mac_based_ipv6_(
- webrtc::field_trial::IsEnabled("WebRTC-AllowMACBasedIPv6")),
+ field_trials_->IsEnabled("WebRTC-AllowMACBasedIPv6")),
bind_using_ifname_(
- !webrtc::field_trial::IsDisabled("WebRTC-BindUsingInterfaceName")) {}
+ !field_trials_->IsDisabled("WebRTC-BindUsingInterfaceName")) {}
BasicNetworkManager::~BasicNetworkManager() {
if (task_safety_flag_) {
@@ -936,7 +932,8 @@
return;
}
if (!network_monitor_) {
- network_monitor_.reset(network_monitor_factory_->CreateNetworkMonitor());
+ network_monitor_.reset(
+ network_monitor_factory_->CreateNetworkMonitor(*field_trials_));
if (!network_monitor_) {
return;
}
@@ -1060,24 +1057,6 @@
Network::Network(absl::string_view name,
absl::string_view desc,
const IPAddress& prefix,
- int prefix_length)
- : name_(name),
- description_(desc),
- prefix_(prefix),
- prefix_length_(prefix_length),
- key_(MakeNetworkKey(name, prefix, prefix_length)),
- scope_id_(0),
- ignored_(false),
- type_(ADAPTER_TYPE_UNKNOWN),
- preference_(0),
- use_differentiated_cellular_costs_(webrtc::field_trial::IsEnabled(
- "WebRTC-UseDifferentiatedCellularCosts")),
- add_network_cost_to_vpn_(
- webrtc::field_trial::IsEnabled("WebRTC-AddNetworkCostToVpn")) {}
-
-Network::Network(absl::string_view name,
- absl::string_view desc,
- const IPAddress& prefix,
int prefix_length,
AdapterType type)
: name_(name),
@@ -1088,11 +1067,7 @@
scope_id_(0),
ignored_(false),
type_(type),
- preference_(0),
- use_differentiated_cellular_costs_(webrtc::field_trial::IsEnabled(
- "WebRTC-UseDifferentiatedCellularCosts")),
- add_network_cost_to_vpn_(
- webrtc::field_trial::IsEnabled("WebRTC-AddNetworkCostToVpn")) {}
+ preference_(0) {}
Network::Network(const Network&) = default;
@@ -1162,11 +1137,23 @@
return mdns_responder_provider_->GetMdnsResponder();
}
-uint16_t Network::GetCost() const {
+uint16_t Network::GetCost(
+ const webrtc::WebRtcKeyValueConfig* field_trials) const {
+ return GetCost(
+ *webrtc::AlwaysValidPointer<const webrtc::WebRtcKeyValueConfig,
+ webrtc::FieldTrialBasedConfig>(field_trials));
+}
+
+uint16_t Network::GetCost(
+ const webrtc::WebRtcKeyValueConfig& field_trials) const {
AdapterType type = IsVpn() ? underlying_type_for_vpn_ : type_;
+ const bool use_differentiated_cellular_costs =
+ field_trials.IsEnabled("WebRTC-UseDifferentiatedCellularCosts");
+ const bool add_network_cost_to_vpn =
+ field_trials.IsEnabled("WebRTC-AddNetworkCostToVpn");
return ComputeNetworkCostByType(type, IsVpn(),
- use_differentiated_cellular_costs_,
- add_network_cost_to_vpn_);
+ use_differentiated_cellular_costs,
+ add_network_cost_to_vpn);
}
// This is the inverse of ComputeNetworkCostByType().
diff --git a/rtc_base/network.h b/rtc_base/network.h
index 063e255..d01a418 100644
--- a/rtc_base/network.h
+++ b/rtc_base/network.h
@@ -22,8 +22,11 @@
#include "absl/strings/string_view.h"
#include "api/array_view.h"
#include "api/sequence_checker.h"
+#include "api/transport/field_trial_based_config.h"
+#include "api/webrtc_key_value_config.h"
#include "rtc_base/ip_address.h"
#include "rtc_base/mdns_responder_interface.h"
+#include "rtc_base/memory/always_valid_pointer.h"
#include "rtc_base/network_monitor.h"
#include "rtc_base/network_monitor_factory.h"
#include "rtc_base/socket_factory.h"
@@ -205,7 +208,8 @@
// Base class for NetworkManager implementations.
class RTC_EXPORT NetworkManagerBase : public NetworkManager {
public:
- NetworkManagerBase();
+ NetworkManagerBase(
+ const webrtc::WebRtcKeyValueConfig* field_trials = nullptr);
~NetworkManagerBase() override;
void GetNetworks(NetworkList* networks) const override;
@@ -273,15 +277,28 @@
public NetworkBinderInterface,
public sigslot::has_slots<> {
public:
+ // This version is used by chromium.
ABSL_DEPRECATED(
"Use the version with socket_factory, see bugs.webrtc.org/13145")
- BasicNetworkManager();
- explicit BasicNetworkManager(SocketFactory* socket_factory);
- ABSL_DEPRECATED(
- "Use the version with socket_factory, see bugs.webrtc.org/13145")
- explicit BasicNetworkManager(NetworkMonitorFactory* network_monitor_factory);
- BasicNetworkManager(NetworkMonitorFactory* network_monitor_factory,
- SocketFactory* socket_factory);
+ explicit BasicNetworkManager(
+ const webrtc::WebRtcKeyValueConfig* field_trials = nullptr)
+ : BasicNetworkManager(
+ /* network_monitor_factory= */ nullptr,
+ /* socket_factory= */ nullptr,
+ field_trials) {}
+
+ // This is used by lots of downstream code.
+ BasicNetworkManager(
+ SocketFactory* socket_factory,
+ const webrtc::WebRtcKeyValueConfig* field_trials = nullptr)
+ : BasicNetworkManager(/* network_monitor_factory= */ nullptr,
+ socket_factory,
+ field_trials) {}
+
+ BasicNetworkManager(
+ NetworkMonitorFactory* network_monitor_factory,
+ SocketFactory* socket_factory,
+ const webrtc::WebRtcKeyValueConfig* field_trials = nullptr);
~BasicNetworkManager() override;
void StartUpdating() override;
@@ -354,6 +371,10 @@
Thread* thread_ = nullptr;
bool sent_first_update_ = true;
int start_count_ = 0;
+ // Chromium create BasicNetworkManager() w/o field trials.
+ webrtc::AlwaysValidPointer<const webrtc::WebRtcKeyValueConfig,
+ webrtc::FieldTrialBasedConfig>
+ field_trials_;
std::vector<std::string> network_ignore_list_;
NetworkMonitorFactory* const network_monitor_factory_;
SocketFactory* const socket_factory_;
@@ -372,13 +393,19 @@
Network(absl::string_view name,
absl::string_view description,
const IPAddress& prefix,
- int prefix_length);
+ int prefix_length)
+ : Network(name,
+ description,
+ prefix,
+ prefix_length,
+ rtc::ADAPTER_TYPE_UNKNOWN) {}
Network(absl::string_view name,
absl::string_view description,
const IPAddress& prefix,
int prefix_length,
AdapterType type);
+
Network(const Network&);
~Network();
@@ -515,7 +542,16 @@
}
}
- uint16_t GetCost() const;
+ // Note: This function is called "rarely".
+ // Twice per Network in BasicPortAllocator if
+ // PORTALLOCATOR_DISABLE_COSTLY_NETWORKS. Once in Port::Construct() (and when
+ // Port::OnNetworkTypeChanged is called).
+ ABSL_DEPRECATED(
+ "Use the version with field trials, see bugs.webrtc.org/webrtc:10335")
+ uint16_t GetCost(
+ const webrtc::WebRtcKeyValueConfig* field_trials = nullptr) const;
+ uint16_t GetCost(const webrtc::WebRtcKeyValueConfig& field_trials) const;
+
// A unique id assigned by the network manager, which may be signaled
// to the remote side in the candidate.
uint16_t id() const { return id_; }
@@ -567,8 +603,6 @@
int preference_;
bool active_ = true;
uint16_t id_ = 0;
- bool use_differentiated_cellular_costs_ = false;
- bool add_network_cost_to_vpn_ = false;
NetworkPreference network_preference_ = NetworkPreference::NEUTRAL;
friend class NetworkManager;
diff --git a/rtc_base/network_monitor_factory.h b/rtc_base/network_monitor_factory.h
index dadcd4a..b261ace 100644
--- a/rtc_base/network_monitor_factory.h
+++ b/rtc_base/network_monitor_factory.h
@@ -11,6 +11,10 @@
#ifndef RTC_BASE_NETWORK_MONITOR_FACTORY_H_
#define RTC_BASE_NETWORK_MONITOR_FACTORY_H_
+namespace webrtc {
+class WebRtcKeyValueConfig;
+} // namespace webrtc
+
namespace rtc {
// Forward declaring this so it's not part of the API surface; it's only
@@ -24,7 +28,8 @@
*/
class NetworkMonitorFactory {
public:
- virtual NetworkMonitorInterface* CreateNetworkMonitor() = 0;
+ virtual NetworkMonitorInterface* CreateNetworkMonitor(
+ const webrtc::WebRtcKeyValueConfig& field_trials) = 0;
virtual ~NetworkMonitorFactory();
diff --git a/rtc_base/network_unittest.cc b/rtc_base/network_unittest.cc
index 0b5f3e9..5830793 100644
--- a/rtc_base/network_unittest.cc
+++ b/rtc_base/network_unittest.cc
@@ -36,6 +36,7 @@
#include "rtc_base/logging.h" // For RTC_LOG_GLE
#endif
#include "test/field_trial.h"
+#include "test/scoped_key_value_config.h"
using ::testing::Contains;
using ::testing::Not;
@@ -121,7 +122,8 @@
class FakeNetworkMonitorFactory : public NetworkMonitorFactory {
public:
FakeNetworkMonitorFactory() {}
- NetworkMonitorInterface* CreateNetworkMonitor() override {
+ NetworkMonitorInterface* CreateNetworkMonitor(
+ const webrtc::WebRtcKeyValueConfig& field_trials) override {
return new FakeNetworkMonitor();
}
};
@@ -309,14 +311,18 @@
#endif // defined(WEBRTC_POSIX)
protected:
+ webrtc::test::ScopedKeyValueConfig field_trials_;
bool callback_called_;
};
class TestBasicNetworkManager : public BasicNetworkManager {
public:
TestBasicNetworkManager(NetworkMonitorFactory* network_monitor_factory,
- SocketFactory* socket_factory)
- : BasicNetworkManager(network_monitor_factory, socket_factory) {}
+ SocketFactory* socket_factory,
+ const webrtc::WebRtcKeyValueConfig& field_trials)
+ : BasicNetworkManager(network_monitor_factory,
+ socket_factory,
+ &field_trials) {}
using BasicNetworkManager::QueryDefaultLocalAddress;
using BasicNetworkManager::set_default_local_addresses;
};
@@ -404,7 +410,7 @@
// ALLOWED.
TEST_F(NetworkTest, TestUpdateNetworks) {
PhysicalSocketServer socket_server;
- BasicNetworkManager manager(nullptr, &socket_server);
+ BasicNetworkManager manager(nullptr, &socket_server, &field_trials_);
manager.SignalNetworksChanged.connect(static_cast<NetworkTest*>(this),
&NetworkTest::OnNetworksChanged);
EXPECT_EQ(NetworkManager::ENUMERATION_ALLOWED,
@@ -798,7 +804,7 @@
// to be preference-ordered by name. For example, "eth0" before "eth1".
TEST_F(NetworkTest, NetworksSortedByInterfaceName) {
PhysicalSocketServer socket_server;
- BasicNetworkManager manager(&socket_server);
+ BasicNetworkManager manager(&socket_server, &field_trials_);
Network* eth0 = new Network("test_eth0", "Test Network Adapter 1",
IPAddress(0x65432100U), 24);
eth0->AddIP(IPAddress(0x65432100U));
@@ -896,7 +902,8 @@
std::string ipv6_address = "1000:2000:3000:4000:0:0:0:1";
std::string ipv6_mask = "FFFF:FFFF:FFFF:FFFF::";
PhysicalSocketServer socket_server;
- BasicNetworkManager manager_without_monitor(nullptr, &socket_server);
+ BasicNetworkManager manager_without_monitor(nullptr, &socket_server,
+ &field_trials_);
manager_without_monitor.StartUpdating();
// A network created without a network monitor will get UNKNOWN type.
ifaddrs* addr_list = InstallIpv6Network(if_name, ipv6_address, ipv6_mask,
@@ -906,7 +913,8 @@
// With the fake network monitor the type should be correctly determined.
FakeNetworkMonitorFactory factory;
- BasicNetworkManager manager_with_monitor(&factory, &socket_server);
+ BasicNetworkManager manager_with_monitor(&factory, &socket_server,
+ &field_trials_);
manager_with_monitor.StartUpdating();
// Add the same ipv6 address as before but it has the right network type
// detected by the network monitor now.
@@ -1004,7 +1012,7 @@
// Sanity check that both interfaces are included by default.
FakeNetworkMonitorFactory factory;
PhysicalSocketServer socket_server;
- BasicNetworkManager manager(&factory, &socket_server);
+ BasicNetworkManager manager(&factory, &socket_server, &field_trials_);
manager.StartUpdating();
CallConvertIfAddrs(manager, list, /*include_ignored=*/false, &result);
EXPECT_EQ(2u, result.size());
@@ -1151,7 +1159,7 @@
TEST_F(NetworkTest, TestNetworkMonitoring) {
FakeNetworkMonitorFactory factory;
PhysicalSocketServer socket_server;
- BasicNetworkManager manager(&factory, &socket_server);
+ BasicNetworkManager manager(&factory, &socket_server, &field_trials_);
manager.SignalNetworksChanged.connect(static_cast<NetworkTest*>(this),
&NetworkTest::OnNetworksChanged);
manager.StartUpdating();
@@ -1182,7 +1190,7 @@
IPAddress ip;
FakeNetworkMonitorFactory factory;
PhysicalSocketServer socket_server;
- TestBasicNetworkManager manager(&factory, &socket_server);
+ TestBasicNetworkManager manager(&factory, &socket_server, field_trials_);
manager.SignalNetworksChanged.connect(static_cast<NetworkTest*>(this),
&NetworkTest::OnNetworksChanged);
manager.StartUpdating();
@@ -1358,7 +1366,7 @@
// Sanity check that both interfaces are included by default.
FakeNetworkMonitorFactory factory;
PhysicalSocketServer socket_server;
- BasicNetworkManager manager(&factory, &socket_server);
+ BasicNetworkManager manager(&factory, &socket_server, &field_trials_);
manager.StartUpdating();
CallConvertIfAddrs(manager, list, /*include_ignored=*/false, &result);
EXPECT_EQ(2u, result.size());
@@ -1390,6 +1398,7 @@
TEST_F(NetworkTest, NetworkCostVpn_Default) {
IPAddress ip1;
EXPECT_TRUE(IPFromString("2400:4030:1:2c00:be30:0:0:1", &ip1));
+ webrtc::test::ScopedKeyValueConfig field_trials;
Network* net1 = new Network("em1", "em1", TruncateIP(ip1, 64), 64);
net1->set_type(ADAPTER_TYPE_VPN);
@@ -1398,13 +1407,13 @@
Network* net2 = new Network("em1", "em1", TruncateIP(ip1, 64), 64);
net2->set_type(ADAPTER_TYPE_ETHERNET);
- EXPECT_EQ(net1->GetCost(), net2->GetCost());
+ EXPECT_EQ(net1->GetCost(field_trials), net2->GetCost(field_trials));
delete net1;
delete net2;
}
TEST_F(NetworkTest, NetworkCostVpn_VpnMoreExpensive) {
- webrtc::test::ScopedFieldTrials field_trials(
+ webrtc::test::ScopedKeyValueConfig field_trials(
"WebRTC-AddNetworkCostToVpn/Enabled/");
IPAddress ip1;
@@ -1417,13 +1426,13 @@
Network* net2 = new Network("em1", "em1", TruncateIP(ip1, 64), 64);
net2->set_type(ADAPTER_TYPE_ETHERNET);
- EXPECT_GT(net1->GetCost(), net2->GetCost());
+ EXPECT_GT(net1->GetCost(field_trials), net2->GetCost(field_trials));
delete net1;
delete net2;
}
TEST_F(NetworkTest, GuessAdapterFromNetworkCost) {
- webrtc::test::ScopedFieldTrials field_trials(
+ webrtc::test::ScopedKeyValueConfig field_trials(
"WebRTC-AddNetworkCostToVpn/Enabled/"
"WebRTC-UseDifferentiatedCellularCosts/Enabled/");
@@ -1435,7 +1444,8 @@
continue;
Network net1("em1", "em1", TruncateIP(ip1, 64), 64);
net1.set_type(type);
- auto [guess, vpn] = Network::GuessAdapterFromNetworkCost(net1.GetCost());
+ auto [guess, vpn] =
+ Network::GuessAdapterFromNetworkCost(net1.GetCost(field_trials));
EXPECT_FALSE(vpn);
if (type == rtc::ADAPTER_TYPE_LOOPBACK) {
EXPECT_EQ(guess, rtc::ADAPTER_TYPE_ETHERNET);
@@ -1451,7 +1461,8 @@
Network net1("em1", "em1", TruncateIP(ip1, 64), 64);
net1.set_type(rtc::ADAPTER_TYPE_VPN);
net1.set_underlying_type_for_vpn(type);
- auto [guess, vpn] = Network::GuessAdapterFromNetworkCost(net1.GetCost());
+ auto [guess, vpn] =
+ Network::GuessAdapterFromNetworkCost(net1.GetCost(field_trials));
EXPECT_TRUE(vpn);
if (type == rtc::ADAPTER_TYPE_LOOPBACK) {
EXPECT_EQ(guess, rtc::ADAPTER_TYPE_ETHERNET);