Reland "Don't create PacketSocketFactory inside BasicPortAllocatorSession"

This is a reland of commit 7d4634cef7

Original change's description:
> Don't create PacketSocketFactory inside BasicPortAllocatorSession
>
> This extends AlwaysValidPointer to avoid creating a unique_ptr inside it.
>
> Bug: webrtc:13145
> Change-Id: I73a4f18d0a7037b57f575b04b134e4f7eadceb79
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/263240
> Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
> Commit-Queue: Daniel.L (Byoungchan) Lee <daniel.l@hpcnt.com>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#37048}

Bug: webrtc:13145
Change-Id: Iec8091ada5862cb6aa48d45b2a426c05bda798f9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264826
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Owners-Override: Daniel.L (Byoungchan) Lee <daniel.l@hpcnt.com>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Christoffer Jansson <jansson@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37138}

p2p/base/p2p_transport_channel_unittest.cc

@@ -157,6 +157,7 @@ cricket::Candidate CreateUdpCandidate(const std::string& type,
 
 cricket::BasicPortAllocator* CreateBasicPortAllocator(
     rtc::NetworkManager* network_manager,
+    rtc::SocketServer* socket_server,
     const cricket::ServerAddresses& stun_servers,
     const rtc::SocketAddress& turn_server_udp,
     const rtc::SocketAddress& turn_server_tcp) {
@@ -172,11 +173,13 @@ cricket::BasicPortAllocator* CreateBasicPortAllocator(
   }
   std::vector<cricket::RelayServerConfig> turn_servers(1, turn_server);
 
-  cricket::BasicPortAllocator* allocator =
-      new cricket::BasicPortAllocator(network_manager);
+  std::unique_ptr<cricket::BasicPortAllocator> allocator =
+      std::make_unique<cricket::BasicPortAllocator>(
+          network_manager,
+          std::make_unique<rtc::BasicPacketSocketFactory>(socket_server));
   allocator->Initialize();
   allocator->SetConfiguration(stun_servers, turn_servers, 0, webrtc::NO_PRUNE);
-  return allocator;
+  return allocator.release();
 }
 
 class MockIceControllerFactory : public cricket::IceControllerFactoryInterface {
@@ -280,12 +283,12 @@ class P2PTransportChannelTestBase : public ::testing::Test,
 
     ServerAddresses stun_servers;
     stun_servers.insert(kStunAddr);
-    ep1_.allocator_.reset(
-        CreateBasicPortAllocator(&ep1_.network_manager_, stun_servers,
-                                 kTurnUdpIntAddr, rtc::SocketAddress()));
-    ep2_.allocator_.reset(
-        CreateBasicPortAllocator(&ep2_.network_manager_, stun_servers,
-                                 kTurnUdpIntAddr, rtc::SocketAddress()));
+    ep1_.allocator_.reset(CreateBasicPortAllocator(
+        &ep1_.network_manager_, ss_.get(), stun_servers, kTurnUdpIntAddr,
+        rtc::SocketAddress()));
+    ep2_.allocator_.reset(CreateBasicPortAllocator(
+        &ep2_.network_manager_, ss_.get(), stun_servers, kTurnUdpIntAddr,
+        rtc::SocketAddress()));
     webrtc::metrics::Reset();
   }
 
@@ -4911,7 +4914,7 @@ class P2PTransportChannelMostLikelyToWorkFirstTest
                      kTurnUdpExtAddr) {
     network_manager_.AddInterface(kPublicAddrs[0]);
     allocator_.reset(
-        CreateBasicPortAllocator(&network_manager_, ServerAddresses(),
+        CreateBasicPortAllocator(&network_manager_, ss(), ServerAddresses(),
                                  kTurnUdpIntAddr, rtc::SocketAddress()));
     allocator_->set_flags(allocator_->flags() | PORTALLOCATOR_DISABLE_STUN |
                           PORTALLOCATOR_DISABLE_TCP);

p2p/client/basic_port_allocator.cc

@@ -165,23 +165,35 @@ BasicPortAllocator::BasicPortAllocator(
   Init(relay_port_factory, nullptr);
   RTC_DCHECK(relay_port_factory_ != nullptr);
   RTC_DCHECK(network_manager_ != nullptr);
-  RTC_DCHECK(socket_factory_ != nullptr);
+  RTC_CHECK(socket_factory_ != nullptr);
   SetConfiguration(ServerAddresses(), std::vector<RelayServerConfig>(), 0,
                    webrtc::NO_PRUNE, customizer);
 }
 
-BasicPortAllocator::BasicPortAllocator(rtc::NetworkManager* network_manager)
-    : network_manager_(network_manager), socket_factory_(nullptr) {
+BasicPortAllocator::BasicPortAllocator(
+    rtc::NetworkManager* network_manager,
+    std::unique_ptr<rtc::PacketSocketFactory> owned_socket_factory)
+    : network_manager_(network_manager),
+      socket_factory_(std::move(owned_socket_factory)) {
   Init(nullptr, nullptr);
   RTC_DCHECK(relay_port_factory_ != nullptr);
   RTC_DCHECK(network_manager_ != nullptr);
+  RTC_CHECK(socket_factory_ != nullptr);
 }
 
-BasicPortAllocator::BasicPortAllocator(rtc::NetworkManager* network_manager,
-                                       const ServerAddresses& stun_servers)
-    : BasicPortAllocator(network_manager,
-                         /*socket_factory=*/nullptr,
-                         stun_servers) {}
+BasicPortAllocator::BasicPortAllocator(
+    rtc::NetworkManager* network_manager,
+    std::unique_ptr<rtc::PacketSocketFactory> owned_socket_factory,
+    const ServerAddresses& stun_servers)
+    : network_manager_(network_manager),
+      socket_factory_(std::move(owned_socket_factory)) {
+  Init(nullptr, nullptr);
+  RTC_DCHECK(relay_port_factory_ != nullptr);
+  RTC_DCHECK(network_manager_ != nullptr);
+  RTC_CHECK(socket_factory_ != nullptr);
+  SetConfiguration(stun_servers, std::vector<RelayServerConfig>(), 0,
+                   webrtc::NO_PRUNE, nullptr);
+}
 
 BasicPortAllocator::BasicPortAllocator(rtc::NetworkManager* network_manager,
                                        rtc::PacketSocketFactory* socket_factory,
@@ -190,6 +202,7 @@ BasicPortAllocator::BasicPortAllocator(rtc::NetworkManager* network_manager,
   Init(nullptr, nullptr);
   RTC_DCHECK(relay_port_factory_ != nullptr);
   RTC_DCHECK(network_manager_ != nullptr);
+  RTC_CHECK(socket_factory_ != nullptr);
   SetConfiguration(stun_servers, std::vector<RelayServerConfig>(), 0,
                    webrtc::NO_PRUNE, nullptr);
 }
@@ -394,11 +407,6 @@ void BasicPortAllocatorSession::SetCandidateFilter(uint32_t filter) {
 void BasicPortAllocatorSession::StartGettingPorts() {
   RTC_DCHECK_RUN_ON(network_thread_);
   state_ = SessionState::GATHERING;
-  if (!socket_factory_) {
-    owned_socket_factory_.reset(
-        new rtc::BasicPacketSocketFactory(network_thread_->socketserver()));
-    socket_factory_ = owned_socket_factory_.get();
-  }
 
   network_thread_->PostTask(webrtc::ToQueuedTask(
       network_safety_, [this] { GetPortConfigurations(); }));

p2p/client/basic_port_allocator.h

@@ -21,6 +21,7 @@
 #include "p2p/client/relay_port_factory_interface.h"
 #include "p2p/client/turn_port_factory.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/memory/always_valid_pointer.h"
 #include "rtc_base/network.h"
 #include "rtc_base/system/rtc_export.h"
 #include "rtc_base/task_utils/pending_task_safety_flag.h"
@@ -32,18 +33,19 @@ namespace cricket {
 class RTC_EXPORT BasicPortAllocator : public PortAllocator {
  public:
   // The NetworkManager is a mandatory argument. The other arguments are
-  // optional. All these objects are owned by caller and must have a life time
+  // optional. All pointers are owned by caller and must have a life time
   // that exceeds that of BasicPortAllocator.
-  // TODO(bugs.webrtc.org/13145): The SocketFactory should be mandatory, but
-  // currenly isn't. When not specified, one is created internally, based on the
-  // socket server associated with the thread calling CreateSession.
   BasicPortAllocator(rtc::NetworkManager* network_manager,
                      rtc::PacketSocketFactory* socket_factory,
                      webrtc::TurnCustomizer* customizer = nullptr,
                      RelayPortFactoryInterface* relay_port_factory = nullptr);
-  explicit BasicPortAllocator(rtc::NetworkManager* network_manager);
-  BasicPortAllocator(rtc::NetworkManager* network_manager,
-                     const ServerAddresses& stun_servers);
+  BasicPortAllocator(
+      rtc::NetworkManager* network_manager,
+      std::unique_ptr<rtc::PacketSocketFactory> owned_socket_factory);
+  BasicPortAllocator(
+      rtc::NetworkManager* network_manager,
+      std::unique_ptr<rtc::PacketSocketFactory> owned_socket_factory,
+      const ServerAddresses& stun_servers);
   BasicPortAllocator(rtc::NetworkManager* network_manager,
                      rtc::PacketSocketFactory* socket_factory,
                      const ServerAddresses& stun_servers);
@@ -62,7 +64,7 @@ class RTC_EXPORT BasicPortAllocator : public PortAllocator {
   // creates its own socket factory.
   rtc::PacketSocketFactory* socket_factory() {
     CheckRunOnValidThreadIfInitialized();
-    return socket_factory_;
+    return socket_factory_.get();
   }
 
   PortAllocatorSession* CreateSessionInternal(
@@ -97,7 +99,8 @@ class RTC_EXPORT BasicPortAllocator : public PortAllocator {
   const webrtc::FieldTrialsView* field_trials_;
   std::unique_ptr<webrtc::FieldTrialsView> owned_field_trials_;
   rtc::NetworkManager* network_manager_;
-  rtc::PacketSocketFactory* socket_factory_;
+  const webrtc::AlwaysValidPointerNoDefault<rtc::PacketSocketFactory>
+      socket_factory_;
   int network_ignore_mask_ = rtc::kDefaultNetworkIgnoreMask;
 
   // This is the factory being used.
@@ -273,7 +276,6 @@ class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession {
 
   BasicPortAllocator* allocator_;
   rtc::Thread* network_thread_;
-  std::unique_ptr<rtc::PacketSocketFactory> owned_socket_factory_;
   rtc::PacketSocketFactory* socket_factory_;
   bool allocation_started_;
   bool network_manager_started_;

p2p/client/basic_port_allocator_unittest.cc

@@ -161,8 +161,10 @@ class BasicPortAllocatorTestBase : public ::testing::Test,
     ServerAddresses stun_servers;
     stun_servers.insert(kStunAddr);
 
-    allocator_ =
-        std::make_unique<BasicPortAllocator>(&network_manager_, stun_servers);
+    allocator_ = std::make_unique<BasicPortAllocator>(
+        &network_manager_,
+        std::make_unique<rtc::BasicPacketSocketFactory>(fss_.get()),
+        stun_servers);
     allocator_->Initialize();
     allocator_->set_step_delay(kMinimumStepDelay);
     webrtc::metrics::Reset();
@@ -199,7 +201,9 @@ class BasicPortAllocatorTestBase : public ::testing::Test,
   }
   // Endpoint is on the public network. No STUN or TURN.
   void ResetWithNoServersOrNat() {
-    allocator_.reset(new BasicPortAllocator(&network_manager_));
+    allocator_.reset(new BasicPortAllocator(
+        &network_manager_,
+        std::make_unique<rtc::BasicPacketSocketFactory>(fss_.get())));
     allocator_->Initialize();
     allocator_->set_step_delay(kMinimumStepDelay);
   }
@@ -578,7 +582,9 @@ class BasicPortAllocatorTest : public FakeClockBase,
     // Add two IP addresses on the same interface.
     AddInterface(kClientAddr, "net1");
     AddInterface(kClientIPv6Addr, "net1");
-    allocator_.reset(new BasicPortAllocator(&network_manager_));
+    allocator_.reset(new BasicPortAllocator(
+        &network_manager_,
+        std::make_unique<rtc::BasicPacketSocketFactory>(fss_.get())));
     allocator_->Initialize();
     allocator_->SetConfiguration(allocator_->stun_servers(),
                                  allocator_->turn_servers(), 0,
@@ -619,7 +625,9 @@ class BasicPortAllocatorTest : public FakeClockBase,
       bool tcp_pruned) {
     turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
     AddInterface(kClientAddr);
-    allocator_.reset(new BasicPortAllocator(&network_manager_));
+    allocator_.reset(new BasicPortAllocator(
+        &network_manager_,
+        std::make_unique<rtc::BasicPacketSocketFactory>(fss_.get())));
     allocator_->Initialize();
     allocator_->SetConfiguration(allocator_->stun_servers(),
                                  allocator_->turn_servers(), 0, prune_policy);
@@ -671,7 +679,9 @@ class BasicPortAllocatorTest : public FakeClockBase,
     AddInterface(kClientIPv6Addr, "net1", rtc::ADAPTER_TYPE_WIFI);
     AddInterface(kClientAddr2, "net2", rtc::ADAPTER_TYPE_CELLULAR);
     AddInterface(kClientIPv6Addr2, "net2", rtc::ADAPTER_TYPE_CELLULAR);
-    allocator_.reset(new BasicPortAllocator(&network_manager_));
+    allocator_.reset(new BasicPortAllocator(
+        &network_manager_,
+        std::make_unique<rtc::BasicPacketSocketFactory>(fss_.get())));
     allocator_->Initialize();
     allocator_->SetConfiguration(allocator_->stun_servers(),
                                  allocator_->turn_servers(), 0,
@@ -1637,7 +1647,9 @@ TEST_F(BasicPortAllocatorTest, TestSharedSocketWithNat) {
 TEST_F(BasicPortAllocatorTest, TestSharedSocketWithoutNatUsingTurn) {
   turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
   AddInterface(kClientAddr);
-  allocator_.reset(new BasicPortAllocator(&network_manager_));
+  allocator_.reset(new BasicPortAllocator(
+      &network_manager_,
+      std::make_unique<rtc::BasicPacketSocketFactory>(fss_.get())));
   allocator_->Initialize();
 
   AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr);
@@ -1772,7 +1784,9 @@ TEST_F(BasicPortAllocatorTestWithRealClock,
   turn_server_.AddInternalSocket(rtc::SocketAddress("127.0.0.1", 3478),
                                  PROTO_UDP);
   AddInterface(kClientAddr);
-  allocator_.reset(new BasicPortAllocator(&network_manager_));
+  allocator_.reset(new BasicPortAllocator(
+      &network_manager_,
+      std::make_unique<rtc::BasicPacketSocketFactory>(fss_.get())));
   allocator_->Initialize();
   RelayServerConfig turn_server;
   RelayCredentials credentials(kTurnUsername, kTurnPassword);

pc/peer_connection_bundle_unittest.cc

@@ -224,8 +224,9 @@ class PeerConnectionBundleBaseTest : public ::testing::Test {
 
   WrapperPtr CreatePeerConnection(const RTCConfiguration& config) {
     auto* fake_network = NewFakeNetwork();
-    auto port_allocator =
-        std::make_unique<cricket::BasicPortAllocator>(fake_network);
+    auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
+        fake_network,
+        std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()));
     port_allocator->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP |
                               cricket::PORTALLOCATOR_DISABLE_RELAY);
     port_allocator->set_step_delay(cricket::kMinimumStepDelay);

pc/peer_connection_histogram_unittest.cc

@@ -270,7 +270,9 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
     fake_network->AddInterface(NextLocalAddress());
 
     std::unique_ptr<cricket::BasicPortAllocator> port_allocator(
-        new cricket::BasicPortAllocator(fake_network));
+        new cricket::BasicPortAllocator(
+            fake_network,
+            std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get())));
 
     deps.async_resolver_factory = std::move(resolver_factory);
     deps.allocator = std::move(port_allocator);
@@ -292,8 +294,9 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
     fake_network->AddInterface(NextLocalAddress());
     fake_network->AddInterface(kPrivateLocalAddress);
 
-    auto port_allocator =
-        std::make_unique<cricket::BasicPortAllocator>(fake_network);
+    auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
+        fake_network,
+        std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()));
     RTCConfiguration config;
     config.sdp_semantics = SdpSemantics::kUnifiedPlan;
     return CreatePeerConnection(config,
@@ -306,8 +309,9 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
     fake_network->AddInterface(NextLocalAddress());
     fake_network->AddInterface(kPrivateIpv6LocalAddress);
 
-    auto port_allocator =
-        std::make_unique<cricket::BasicPortAllocator>(fake_network);
+    auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
+        fake_network,
+        std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()));
 
     RTCConfiguration config;
     config.sdp_semantics = SdpSemantics::kUnifiedPlan;
@@ -339,8 +343,9 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
     if (!deps.allocator) {
       auto fake_network = NewFakeNetwork();
       fake_network->AddInterface(NextLocalAddress());
-      deps.allocator =
-          std::make_unique<cricket::BasicPortAllocator>(fake_network);
+      deps.allocator = std::make_unique<cricket::BasicPortAllocator>(
+          fake_network,
+          std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()));
     }
 
     auto observer = std::make_unique<ObserverForUsageHistogramTest>();

pc/peer_connection_ice_unittest.cc

@@ -46,6 +46,7 @@
 #include "pc/sdp_utils.h"
 #include "pc/session_description.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/internal/default_socket_server.h"
 #include "rtc_base/ip_address.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/net_helper.h"
@@ -158,8 +159,9 @@ class PeerConnectionIceBaseTest : public ::testing::Test {
 
   WrapperPtr CreatePeerConnection(const RTCConfiguration& config) {
     auto* fake_network = NewFakeNetwork();
-    auto port_allocator =
-        std::make_unique<cricket::BasicPortAllocator>(fake_network);
+    auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
+        fake_network,
+        std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()));
     port_allocator->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP |
                               cricket::PORTALLOCATOR_DISABLE_RELAY);
     port_allocator->set_step_delay(cricket::kMinimumStepDelay);

pc/peer_connection_rampup_tests.cc

@@ -184,7 +184,10 @@ class PeerConnectionRampUpTest : public ::testing::Test {
     auto observer = std::make_unique<MockPeerConnectionObserver>();
     webrtc::PeerConnectionDependencies dependencies(observer.get());
     cricket::BasicPortAllocator* port_allocator =
-        new cricket::BasicPortAllocator(fake_network_manager);
+        new cricket::BasicPortAllocator(
+            fake_network_manager,
+            std::make_unique<rtc::BasicPacketSocketFactory>(
+                firewall_socket_server_.get()));
     port_allocator->set_step_delay(cricket::kDefaultStepDelay);
     dependencies.allocator =
         std::unique_ptr<cricket::BasicPortAllocator>(port_allocator);

pc/test/integration_test_helpers.h

@@ -744,6 +744,7 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
   bool Init(const PeerConnectionFactory::Options* options,
             const PeerConnectionInterface::RTCConfiguration* config,
             webrtc::PeerConnectionDependencies dependencies,
+            rtc::SocketServer* socket_server,
             rtc::Thread* network_thread,
             rtc::Thread* worker_thread,
             std::unique_ptr<webrtc::FakeRtcEventLogFactory> event_log_factory,
@@ -758,7 +759,9 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
     fake_network_manager_->AddInterface(kDefaultLocalAddress);
 
     std::unique_ptr<cricket::PortAllocator> port_allocator(
-        new cricket::BasicPortAllocator(fake_network_manager_.get()));
+        new cricket::BasicPortAllocator(
+            fake_network_manager_.get(),
+            std::make_unique<rtc::BasicPacketSocketFactory>(socket_server)));
     port_allocator_ = port_allocator.get();
     fake_audio_capture_module_ = FakeAudioCaptureModule::Create();
     if (!fake_audio_capture_module_) {
@@ -1469,7 +1472,7 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test {
         new PeerConnectionIntegrationWrapper(debug_name));
 
     if (!client->Init(options, &modified_config, std::move(dependencies),
-                      network_thread_.get(), worker_thread_.get(),
+                      fss_.get(), network_thread_.get(), worker_thread_.get(),
                       std::move(event_log_factory), reset_encoder_factory,
                       reset_decoder_factory, create_media_engine)) {
       return nullptr;

rtc_base/memory/always_valid_pointer.h

@@ -105,6 +105,144 @@ class AlwaysValidPointer {
   Interface* const pointer_;
 };
 
+// This class is similar to AlwaysValidPointer, but it does not create
+// a default object and crashes if none of the input pointers are non-null.
+template <typename Interface>
+class AlwaysValidPointerNoDefault {
+ public:
+  explicit AlwaysValidPointerNoDefault(Interface* pointer) : pointer_(pointer) {
+    RTC_CHECK(pointer_);
+  }
+
+  // Create a pointer by
+  // a) taking over ownership of |instance|
+  // b) or fallback to |pointer|, without taking ownership.
+  // At least one of the arguments must be non-null.
+  explicit AlwaysValidPointerNoDefault(std::unique_ptr<Interface> instance,
+                                       Interface* pointer = nullptr)
+      : owned_instance_(std::move(instance)),
+        pointer_(owned_instance_ ? owned_instance_.get() : pointer) {
+    RTC_CHECK(pointer_);
+  }
+
+  Interface* get() { return pointer_; }
+  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_;
+};
+
+template <typename T, typename U, typename V, typename W>
+bool operator==(const AlwaysValidPointer<T, U>& a,
+                const AlwaysValidPointer<V, W>& b) {
+  return a.get() == b.get();
+}
+
+template <typename T, typename U, typename V, typename W>
+bool operator!=(const AlwaysValidPointer<T, U>& a,
+                const AlwaysValidPointer<V, W>& b) {
+  return !(a == b);
+}
+
+template <typename T, typename U>
+bool operator==(const AlwaysValidPointer<T, U>& a, std::nullptr_t) {
+  return a.get() == nullptr;
+}
+
+template <typename T, typename U>
+bool operator!=(const AlwaysValidPointer<T, U>& a, std::nullptr_t) {
+  return !(a == nullptr);
+}
+
+template <typename T, typename U>
+bool operator==(std::nullptr_t, const AlwaysValidPointer<T, U>& a) {
+  return a.get() == nullptr;
+}
+
+template <typename T, typename U>
+bool operator!=(std::nullptr_t, const AlwaysValidPointer<T, U>& a) {
+  return !(a == nullptr);
+}
+
+template <typename T, typename U>
+bool operator==(const AlwaysValidPointerNoDefault<T>& a,
+                const AlwaysValidPointerNoDefault<U>& b) {
+  return a.get() == b.get();
+}
+
+template <typename T, typename U>
+bool operator!=(const AlwaysValidPointerNoDefault<T>& a,
+                const AlwaysValidPointerNoDefault<U>& b) {
+  return !(a == b);
+}
+
+template <typename T>
+bool operator==(const AlwaysValidPointerNoDefault<T>& a, std::nullptr_t) {
+  return a.get() == nullptr;
+}
+
+template <typename T>
+bool operator!=(const AlwaysValidPointerNoDefault<T>& a, std::nullptr_t) {
+  return !(a == nullptr);
+}
+
+template <typename T>
+bool operator==(std::nullptr_t, const AlwaysValidPointerNoDefault<T>& a) {
+  return a.get() == nullptr;
+}
+
+template <typename T>
+bool operator!=(std::nullptr_t, const AlwaysValidPointerNoDefault<T>& a) {
+  return !(a == nullptr);
+}
+
+// Comparison with raw pointer.
+template <typename T, typename U, typename V>
+bool operator==(const AlwaysValidPointer<T, U>& a, const V* b) {
+  return a.get() == b;
+}
+
+template <typename T, typename U, typename V>
+bool operator!=(const AlwaysValidPointer<T, U>& a, const V* b) {
+  return !(a == b);
+}
+
+template <typename T, typename U, typename V>
+bool operator==(const T* a, const AlwaysValidPointer<U, V>& b) {
+  return a == b.get();
+}
+
+template <typename T, typename U, typename V>
+bool operator!=(const T* a, const AlwaysValidPointer<U, V>& b) {
+  return !(a == b);
+}
+
+template <typename T, typename U>
+bool operator==(const AlwaysValidPointerNoDefault<T>& a, const U* b) {
+  return a.get() == b;
+}
+
+template <typename T, typename U>
+bool operator!=(const AlwaysValidPointerNoDefault<T>& a, const U* b) {
+  return !(a == b);
+}
+
+template <typename T, typename U>
+bool operator==(const T* a, const AlwaysValidPointerNoDefault<U>& b) {
+  return a == b.get();
+}
+
+template <typename T, typename U>
+bool operator!=(const T* a, const AlwaysValidPointerNoDefault<U>& b) {
+  return !(a == b);
+}
+
 }  // namespace webrtc
 
 #endif  // RTC_BASE_MEMORY_ALWAYS_VALID_POINTER_H_

rtc_base/memory/always_valid_pointer_unittest.cc

@@ -90,4 +90,40 @@ TEST(AlwaysValidPointerTest, DefaultToLambda) {
   EXPECT_EQ(*ptr, "onkel skrue");
 }
 
+TEST(AlwaysValidPointerTest, NoDefaultObjectPassValidPointer) {
+  std::string str("foo");
+  AlwaysValidPointerNoDefault<std::string> ptr(&str);
+  EXPECT_EQ(*ptr, "foo");
+  EXPECT_EQ(ptr, &str);
+}
+
+TEST(AlwaysValidPointerTest, NoDefaultObjectWithTakeOverOwnership) {
+  std::unique_ptr<std::string> str = std::make_unique<std::string>("yum");
+  AlwaysValidPointerNoDefault<std::string> ptr(std::move(str));
+  EXPECT_EQ(*ptr, "yum");
+  std::unique_ptr<std::string> str2 = std::make_unique<std::string>("fun");
+  AlwaysValidPointerNoDefault<std::string> ptr2(std::move(str), str2.get());
+  EXPECT_EQ(*ptr2, "fun");
+  EXPECT_EQ(ptr2, str2.get());
+}
+
+#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
+
+TEST(AlwaysValidPointerTest, NoDefaultObjectPassNullPointer) {
+  auto pass_null = []() {
+    AlwaysValidPointerNoDefault<std::string> ptr(nullptr);
+  };
+  EXPECT_DEATH(pass_null(), "");
+}
+
+TEST(AlwaysValidPointerTest, NoDefaultObjectPassNullUniquePointer) {
+  auto pass_null = []() {
+    std::unique_ptr<std::string> str;
+    AlwaysValidPointerNoDefault<std::string> ptr(std::move(str));
+  };
+  EXPECT_DEATH(pass_null(), "");
+}
+
+#endif
+
 }  // namespace webrtc

test/network/network_emulation_pc_unittest.cc

@@ -21,6 +21,7 @@
 #include "media/engine/webrtc_media_engine.h"
 #include "media/engine/webrtc_media_engine_defaults.h"
 #include "modules/audio_device/include/test_audio_device.h"
+#include "p2p/base/basic_packet_socket_factory.h"
 #include "p2p/client/basic_port_allocator.h"
 #include "pc/peer_connection_wrapper.h"
 #include "pc/test/mock_peer_connection_observers.h"
@@ -78,11 +79,12 @@ rtc::scoped_refptr<PeerConnectionFactoryInterface> CreatePeerConnectionFactory(
 rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
     const rtc::scoped_refptr<PeerConnectionFactoryInterface>& pcf,
     PeerConnectionObserver* observer,
+    rtc::PacketSocketFactory* packet_socket_factory,
     rtc::NetworkManager* network_manager,
     EmulatedTURNServerInterface* turn_server = nullptr) {
   PeerConnectionDependencies pc_deps(observer);
-  auto port_allocator =
-      std::make_unique<cricket::BasicPortAllocator>(network_manager);
+  auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
+      network_manager, packet_socket_factory);
 
   // This test does not support TCP
   int flags = cricket::PORTALLOCATOR_DISABLE_TCP;
@@ -148,11 +150,13 @@ TEST(NetworkEmulationManagerPCTest, Run) {
     alice_pcf = CreatePeerConnectionFactory(signaling_thread.get(),
                                             alice_network->network_thread());
     alice_pc = CreatePeerConnection(alice_pcf, alice_observer.get(),
+                                    alice_network->packet_socket_factory(),
                                     alice_network->network_manager());
 
     bob_pcf = CreatePeerConnectionFactory(signaling_thread.get(),
                                           bob_network->network_thread());
     bob_pc = CreatePeerConnection(bob_pcf, bob_observer.get(),
+                                  bob_network->packet_socket_factory(),
                                   bob_network->network_manager());
   });
 
@@ -255,13 +259,14 @@ TEST(NetworkEmulationManagerPCTest, RunTURN) {
   signaling_thread->Invoke<void>(RTC_FROM_HERE, [&]() {
     alice_pcf = CreatePeerConnectionFactory(signaling_thread.get(),
                                             alice_network->network_thread());
-    alice_pc =
-        CreatePeerConnection(alice_pcf, alice_observer.get(),
-                             alice_network->network_manager(), alice_turn);
+    alice_pc = CreatePeerConnection(
+        alice_pcf, alice_observer.get(), alice_network->packet_socket_factory(),
+        alice_network->network_manager(), alice_turn);
 
     bob_pcf = CreatePeerConnectionFactory(signaling_thread.get(),
                                           bob_network->network_thread());
     bob_pc = CreatePeerConnection(bob_pcf, bob_observer.get(),
+                                  bob_network->packet_socket_factory(),
                                   bob_network->network_manager(), bob_turn);
   });
 

test/peer_scenario/peer_scenario_client.cc

@@ -281,8 +281,8 @@ PeerScenarioClient::PeerScenarioClient(
   pc_factory_->SetOptions(pc_options);
 
   PeerConnectionDependencies pc_deps(observer_.get());
-  pc_deps.allocator =
-      std::make_unique<cricket::BasicPortAllocator>(manager->network_manager());
+  pc_deps.allocator = std::make_unique<cricket::BasicPortAllocator>(
+      manager->network_manager(), manager->packet_socket_factory());
   pc_deps.allocator->set_flags(pc_deps.allocator->flags() |
                                cricket::PORTALLOCATOR_DISABLE_TCP);
   peer_connection_ =

test/peer_scenario/scenario_connection.cc

@@ -96,7 +96,8 @@ ScenarioIceConnectionImpl::ScenarioIceConnectionImpl(
           rtc::SSLFingerprint::CreateFromCertificate(*certificate_.get())
               .get()),
       port_allocator_(
-          new cricket::BasicPortAllocator(manager_->network_manager())),
+          new cricket::BasicPortAllocator(manager_->network_manager(),
+                                          manager_->packet_socket_factory())),
       jsep_controller_(
           new JsepTransportController(network_thread_,
                                       port_allocator_.get(),