Clean up IPv6 fixes field trial artifacts.

sam-vi · WebRTC LUCI CQ · commit df7df199abd6 · 2023-04-25T14:59:55.000Z
The fixes have been default enabled, so clean up dead code. Bug: webrtc:14334 Change-Id: I4967d5ad451ac333c54294fc14bea6c7ba1445e3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/301180 Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> Reviewed-by: Jonas Oreland <jonaso@webrtc.org> Commit-Queue: Sameer Vijaykar <samvi@google.com> Cr-Commit-Position: refs/heads/main@{#39949}
diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc
@@ -517,12 +517,10 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> {
     return &networks_.back();
   }
 
-  rtc::Network* MakeNetworkMultipleAddrs(
-      const SocketAddress& global_addr,
-      const SocketAddress& link_local_addr,
-      const webrtc::FieldTrialsView* field_trials) {
+  rtc::Network* MakeNetworkMultipleAddrs(const SocketAddress& global_addr,
+                                         const SocketAddress& link_local_addr) {
     networks_.emplace_back("unittest", "unittest", global_addr.ipaddr(), 32,
-                           rtc::ADAPTER_TYPE_UNKNOWN, field_trials);
+                           rtc::ADAPTER_TYPE_UNKNOWN);
     networks_.back().AddIP(link_local_addr.ipaddr());
     networks_.back().AddIP(global_addr.ipaddr());
     networks_.back().AddIP(link_local_addr.ipaddr());
@@ -548,8 +546,8 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> {
       PacketSocketFactory* socket_factory) {
     auto port = UDPPort::Create(
         &main_, socket_factory,
-        MakeNetworkMultipleAddrs(global_addr, link_local_addr, &field_trials_),
-        0, 0, username_, password_, true, absl::nullopt, &field_trials_);
+        MakeNetworkMultipleAddrs(global_addr, link_local_addr), 0, 0, username_,
+        password_, true, absl::nullopt, &field_trials_);
     port->SetIceTiebreaker(kTiebreakerDefault);
     return port;
   }
diff --git a/p2p/base/stun_port.cc b/p2p/base/stun_port.cc
@@ -29,15 +29,6 @@
 
 namespace cricket {
 
-namespace {
-
-bool ResolveStunHostnameForFamily(const webrtc::FieldTrialsView& field_trials) {
-  // TODO(bugs.webrtc.org/14334) cleanup
-  return true;
-}
-
-}  // namespace
-
 // TODO(?): Move these to a common place (used in relayport too)
 const int RETRY_TIMEOUT = 50 * 1000;  // 50 seconds
 
@@ -152,11 +143,7 @@ void UDPPort::AddressResolver::Resolve(
       done_(it->first, it->second->result().GetError());
     }
   };
-  if (ResolveStunHostnameForFamily(field_trials)) {
-    resolver_ptr->Start(address, family, std::move(callback));
-  } else {
-    resolver_ptr->Start(address, std::move(callback));
-  }
+  resolver_ptr->Start(address, family, std::move(callback));
 }
 
 bool UDPPort::AddressResolver::GetResolvedAddress(
diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc
@@ -34,15 +34,6 @@
 
 namespace cricket {
 
-namespace {
-
-bool ResolveTurnHostnameForFamily(const webrtc::FieldTrialsView& field_trials) {
-  // TODO(bugs.webrtc.org/14334) cleanup
-  return true;
-}
-
-}  // namespace
-
 using ::webrtc::SafeTask;
 using ::webrtc::TaskQueueBase;
 using ::webrtc::TimeDelta;
@@ -850,12 +841,7 @@ void TurnPort::ResolveTurnAddress(const rtc::SocketAddress& address) {
     server_address_.address = resolved_address;
     PrepareAddress();
   };
-  // TODO(bugs.webrtc.org/14733): remove duplicate resolution with STUN port.
-  if (ResolveTurnHostnameForFamily(field_trials())) {
-    resolver_->Start(address, Network()->family(), std::move(callback));
-  } else {
-    resolver_->Start(address, std::move(callback));
-  }
+  resolver_->Start(address, Network()->family(), std::move(callback));
 }
 
 void TurnPort::OnSendStunPacket(const void* data,
diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc
@@ -154,12 +154,6 @@ std::string NetworksToString(const std::vector<const rtc::Network*>& networks) {
   return ost.Release();
 }
 
-bool IsDiversifyIpv6InterfacesEnabled(
-    const webrtc::FieldTrialsView* field_trials) {
-  // TODO(bugs.webrtc.org/14334) cleanup
-  return true;
-}
-
 }  // namespace
 
 const uint32_t DISABLE_ALL_PHASES =
@@ -804,41 +798,20 @@ std::vector<const rtc::Network*> BasicPortAllocatorSession::GetNetworks() {
   }
 
   // Lastly, if we have a limit for the number of IPv6 network interfaces (by
-  // default, it's 5), remove networks to ensure that limit is satisfied.
-  //
-  // TODO(deadbeef): Instead of just taking the first N arbitrary IPv6
-  // networks, we could try to choose a set that's "most likely to work". It's
-  // hard to define what that means though; it's not just "lowest cost".
-  // Alternatively, we could just focus on making our ICE pinging logic smarter
-  // such that this filtering isn't necessary in the first place.
-  const webrtc::FieldTrialsView* field_trials = allocator_->field_trials();
-  if (IsDiversifyIpv6InterfacesEnabled(field_trials)) {
-    std::vector<const rtc::Network*> ipv6_networks;
-    for (auto it = networks.begin(); it != networks.end();) {
-      if ((*it)->prefix().family() == AF_INET6) {
-        ipv6_networks.push_back(*it);
-        it = networks.erase(it);
-        continue;
-      }
-      ++it;
-    }
-    ipv6_networks =
-        SelectIPv6Networks(ipv6_networks, allocator_->max_ipv6_networks());
-    networks.insert(networks.end(), ipv6_networks.begin(), ipv6_networks.end());
-  } else {
-    int ipv6_networks = 0;
-    for (auto it = networks.begin(); it != networks.end();) {
-      if ((*it)->prefix().family() == AF_INET6) {
-        if (ipv6_networks >= allocator_->max_ipv6_networks()) {
-          it = networks.erase(it);
-          continue;
-        } else {
-          ++ipv6_networks;
-        }
-      }
-      ++it;
+  // default, it's 5), pick IPv6 networks from different interfaces in a
+  // priority order and stick to the limit.
+  std::vector<const rtc::Network*> ipv6_networks;
+  for (auto it = networks.begin(); it != networks.end();) {
+    if ((*it)->prefix().family() == AF_INET6) {
+      ipv6_networks.push_back(*it);
+      it = networks.erase(it);
+      continue;
     }
+    ++it;
   }
+  ipv6_networks =
+      SelectIPv6Networks(ipv6_networks, allocator_->max_ipv6_networks());
+  networks.insert(networks.end(), ipv6_networks.begin(), ipv6_networks.end());
   return networks;
 }
 
diff --git a/p2p/client/basic_port_allocator_unittest.cc b/p2p/client/basic_port_allocator_unittest.cc
@@ -2649,8 +2649,7 @@ TEST_F(BasicPortAllocatorTest, IPv6EtherAndWifiHaveHigherPriorityThanOthers) {
   EXPECT_TRUE(HasNetwork(networks, ethe1));
 }
 
-TEST_F(BasicPortAllocatorTest,
-       Select2DifferentIntefacesIfDiversifyIpv6InterfacesEnabled) {
+TEST_F(BasicPortAllocatorTest, Select2DifferentIntefaces) {
   allocator().set_max_ipv6_networks(2);
   AddInterface(kClientIPv6Addr, "ethe1", rtc::ADAPTER_TYPE_ETHERNET);
   AddInterface(kClientIPv6Addr2, "ethe2", rtc::ADAPTER_TYPE_ETHERNET);
@@ -2675,8 +2674,7 @@ TEST_F(BasicPortAllocatorTest,
   EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientIPv6Addr3));
 }
 
-TEST_F(BasicPortAllocatorTest,
-       Select3DifferentIntefacesIfDiversifyIpv6InterfacesEnabled) {
+TEST_F(BasicPortAllocatorTest, Select3DifferentIntefaces) {
   allocator().set_max_ipv6_networks(3);
   AddInterface(kClientIPv6Addr, "ethe1", rtc::ADAPTER_TYPE_ETHERNET);
   AddInterface(kClientIPv6Addr2, "ethe2", rtc::ADAPTER_TYPE_ETHERNET);
@@ -2702,8 +2700,7 @@ TEST_F(BasicPortAllocatorTest,
   EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientIPv6Addr5));
 }
 
-TEST_F(BasicPortAllocatorTest,
-       Select4DifferentIntefacesIfDiversifyIpv6InterfacesEnabled) {
+TEST_F(BasicPortAllocatorTest, Select4DifferentIntefaces) {
   allocator().set_max_ipv6_networks(4);
   AddInterface(kClientIPv6Addr, "ethe1", rtc::ADAPTER_TYPE_ETHERNET);
   AddInterface(kClientIPv6Addr2, "ethe2", rtc::ADAPTER_TYPE_ETHERNET);
diff --git a/rtc_base/fake_network.h b/rtc_base/fake_network.h
@@ -110,7 +110,7 @@ class FakeNetworkManager : public NetworkManagerBase {
       IPAddress prefix = TruncateIP(it->socket_address.ipaddr(), prefix_length);
       auto net = std::make_unique<Network>(
           it->socket_address.hostname(), it->socket_address.hostname(), prefix,
-          prefix_length, it->adapter_type, /*field_trials=*/nullptr);
+          prefix_length, it->adapter_type);
       if (it->underlying_vpn_adapter_type.has_value()) {
         net->set_underlying_type_for_vpn(*it->underlying_vpn_adapter_type);
       }
diff --git a/rtc_base/network.cc b/rtc_base/network.cc
@@ -186,11 +186,6 @@ bool ShouldAdapterChangeTriggerNetworkChange(rtc::AdapterType old_type,
   return true;
 }
 
-bool PreferGlobalIPv6Address(const webrtc::FieldTrialsView* field_trials) {
-  // TODO(bugs.webrtc.org/14334) cleanup
-  return true;
-}
-
 #if defined(WEBRTC_WIN)
 bool IpAddressAttributesEnabled(const webrtc::FieldTrialsView* field_trials) {
   // Field trial key reserved in bugs.webrtc.org/14334
@@ -334,7 +329,7 @@ std::unique_ptr<Network> NetworkManagerBase::CreateNetwork(
     int prefix_length,
     AdapterType type) const {
   return std::make_unique<Network>(name, description, prefix, prefix_length,
-                                   type, field_trials_.get());
+                                   type);
 }
 
 std::vector<const Network*> NetworkManagerBase::GetAnyAddressNetworks() {
@@ -1119,10 +1114,8 @@ Network::Network(absl::string_view name,
                  absl::string_view desc,
                  const IPAddress& prefix,
                  int prefix_length,
-                 AdapterType type,
-                 const webrtc::FieldTrialsView* field_trials)
-    : field_trials_(field_trials),
-      name_(name),
+                 AdapterType type)
+    : name_(name),
       description_(desc),
       prefix_(prefix),
       prefix_length_(prefix_length),
@@ -1166,15 +1159,13 @@ IPAddress Network::GetBestIP() const {
   }
 
   InterfaceAddress selected_ip, link_local_ip, ula_ip;
-  const bool prefer_global_ipv6_to_link_local =
-      PreferGlobalIPv6Address(field_trials_);
 
   for (const InterfaceAddress& ip : ips_) {
     // Ignore any address which has been deprecated already.
     if (ip.ipv6_flags() & IPV6_ADDRESS_FLAG_DEPRECATED)
       continue;
 
-    if (prefer_global_ipv6_to_link_local && IPIsLinkLocal(ip)) {
+    if (IPIsLinkLocal(ip)) {
       link_local_ip = ip;
       continue;
     }
@@ -1193,7 +1184,7 @@ IPAddress Network::GetBestIP() const {
   }
 
   if (IPIsUnspec(selected_ip)) {
-    if (prefer_global_ipv6_to_link_local && !IPIsUnspec(link_local_ip)) {
+    if (!IPIsUnspec(link_local_ip)) {
       // No proper global IPv6 address found, use link local address instead.
       selected_ip = link_local_ip;
     } else if (!IPIsUnspec(ula_ip)) {
diff --git a/rtc_base/network.h b/rtc_base/network.h
@@ -382,21 +382,18 @@ class RTC_EXPORT Network {
   Network(absl::string_view name,
           absl::string_view description,
           const IPAddress& prefix,
-          int prefix_length,
-          const webrtc::FieldTrialsView* field_trials = nullptr)
+          int prefix_length)
       : Network(name,
                 description,
                 prefix,
                 prefix_length,
-                rtc::ADAPTER_TYPE_UNKNOWN,
-                field_trials) {}
+                rtc::ADAPTER_TYPE_UNKNOWN) {}
 
   Network(absl::string_view name,
           absl::string_view description,
           const IPAddress& prefix,
           int prefix_length,
-          AdapterType type,
-          const webrtc::FieldTrialsView* field_trials = nullptr);
+          AdapterType type);
 
   Network(const Network&);
   ~Network();
@@ -579,7 +576,6 @@ class RTC_EXPORT Network {
   std::string ToString() const;
 
  private:
-  const webrtc::FieldTrialsView* field_trials_ = nullptr;
   const DefaultLocalAddressProvider* default_local_address_provider_ = nullptr;
   const MdnsResponderProvider* mdns_responder_provider_ = nullptr;
   std::string name_;
diff --git a/rtc_base/network_unittest.cc b/rtc_base/network_unittest.cc
@@ -340,11 +340,9 @@ TEST_F(NetworkTest, TestNetworkConstruct) {
 
 TEST_F(NetworkTest, TestIsIgnoredNetworkIgnoresIPsStartingWith0) {
   Network ipv4_network1("test_eth0", "Test Network Adapter 1",
-                        IPAddress(0x12345600U), 24, ADAPTER_TYPE_ETHERNET,
-                        &field_trials_);
+                        IPAddress(0x12345600U), 24, ADAPTER_TYPE_ETHERNET);
   Network ipv4_network2("test_eth1", "Test Network Adapter 2",
-                        IPAddress(0x010000U), 24, ADAPTER_TYPE_ETHERNET,
-                        &field_trials_);
+                        IPAddress(0x010000U), 24, ADAPTER_TYPE_ETHERNET);
   PhysicalSocketServer socket_server;
   BasicNetworkManager network_manager(&socket_server);
   network_manager.StartUpdating();
@@ -824,19 +822,19 @@ TEST_F(NetworkTest, NetworksSortedByInterfaceName) {
 
 TEST_F(NetworkTest, TestNetworkAdapterTypes) {
   Network wifi("wlan0", "Wireless Adapter", IPAddress(0x12345600U), 24,
-               ADAPTER_TYPE_WIFI, &field_trials_);
+               ADAPTER_TYPE_WIFI);
   EXPECT_EQ(ADAPTER_TYPE_WIFI, wifi.type());
   Network ethernet("eth0", "Ethernet", IPAddress(0x12345600U), 24,
-                   ADAPTER_TYPE_ETHERNET, &field_trials_);
+                   ADAPTER_TYPE_ETHERNET);
   EXPECT_EQ(ADAPTER_TYPE_ETHERNET, ethernet.type());
   Network cellular("test_cell", "Cellular Adapter", IPAddress(0x12345600U), 24,
-                   ADAPTER_TYPE_CELLULAR, &field_trials_);
+                   ADAPTER_TYPE_CELLULAR);
   EXPECT_EQ(ADAPTER_TYPE_CELLULAR, cellular.type());
   Network vpn("bridge_test", "VPN Adapter", IPAddress(0x12345600U), 24,
-              ADAPTER_TYPE_VPN, &field_trials_);
+              ADAPTER_TYPE_VPN);
   EXPECT_EQ(ADAPTER_TYPE_VPN, vpn.type());
   Network unknown("test", "Test Adapter", IPAddress(0x12345600U), 24,
-                  ADAPTER_TYPE_UNKNOWN, &field_trials_);
+                  ADAPTER_TYPE_UNKNOWN);
   EXPECT_EQ(ADAPTER_TYPE_UNKNOWN, unknown.type());
 }
 
@@ -1167,7 +1165,7 @@ TEST_F(NetworkTest, TestGetBestIPWithPreferGlobalIPv6ToLinkLocalEnabled) {
 
   // Create a network with this prefix.
   Network ipv6_network("test_eth0", "Test NetworkAdapter", TruncateIP(ip, 64),
-                       64, ADAPTER_TYPE_UNKNOWN, &field_trials_);
+                       64, ADAPTER_TYPE_UNKNOWN);
 
   // When there is no address added, it should return an unspecified
   // address.

Original file line number	Diff line number	Diff line change
`@@ -110,7 +110,7 @@ class FakeNetworkManager : public NetworkManagerBase {`
`110`	`110`	`IPAddress prefix = TruncateIP(it->socket_address.ipaddr(), prefix_length);`
`111`	`111`	`auto net = std::make_unique<Network>(`
`112`	`112`	`it->socket_address.hostname(), it->socket_address.hostname(), prefix,`
`113`		`- prefix_length, it->adapter_type, /field_trials=/nullptr);`
	`113`	`+ prefix_length, it->adapter_type);`
`114`	`114`	`if (it->underlying_vpn_adapter_type.has_value()) {`
`115`	`115`	`net->set_underlying_type_for_vpn(*it->underlying_vpn_adapter_type);`
`116`	`116`	`}`