Revert "Move injection of PacketSocketFactory from PC to PCF"
This reverts commit 905c3a6c73d293882ef11942066ccda52a9e14d1.
Reason for revert: New test fails internal tests, with a similar problem as the failed android test: No networks are detected on the test bot.
Original change's description:
> Move injection of PacketSocketFactory from PC to PCF
>
> Injection via PeerConnectionDependecies was broken, in not accepting
> ownership of the injected object.
>
> Bug: webrtc:7447, webrtc:14204
> Change-Id: Ic53f05d51928b006fc1e46d502633d88471eb518
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266140
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Niels Moller <nisse@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#37270}
Bug: webrtc:7447, webrtc:14204
Change-Id: Ib412d09699a48d8f5db27e2960e365b536ab3aa8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266146
Owners-Override: Niels Moller <nisse@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37273}
diff --git a/api/BUILD.gn b/api/BUILD.gn
index cd1c104..5f6bc49 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -1100,17 +1100,6 @@
]
}
- rtc_source_set("mock_packet_socket_factory") {
- visibility = [ "*" ]
- testonly = true
- sources = [ "test/mock_packet_socket_factory.h" ]
-
- deps = [
- ":packet_socket_factory",
- "../test:test_support",
- ]
- }
-
rtc_source_set("mock_peerconnectioninterface") {
visibility = [ "*" ]
testonly = true
@@ -1333,7 +1322,6 @@
":mock_frame_decryptor",
":mock_frame_encryptor",
":mock_media_stream_interface",
- ":mock_packet_socket_factory",
":mock_peer_connection_factory_interface",
":mock_peerconnectioninterface",
":mock_rtp",
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index bd2f46f..e5bbd9c 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -1379,9 +1379,10 @@
PeerConnectionObserver* observer = nullptr;
// Optional dependencies
// TODO(bugs.webrtc.org/7447): remove port allocator once downstream is
- // updated. The recommended way to inject networking components is to pass a
- // PacketSocketFactory when creating the PeerConnectionFactory.
+ // updated. For now, you can only set one of allocator and
+ // packet_socket_factory, not both.
std::unique_ptr<cricket::PortAllocator> allocator;
+ std::unique_ptr<rtc::PacketSocketFactory> packet_socket_factory;
// Factory for creating resolvers that look up hostnames in DNS
std::unique_ptr<webrtc::AsyncDnsResolverFactoryInterface>
async_dns_resolver_factory;
@@ -1421,7 +1422,6 @@
rtc::Thread* worker_thread = nullptr;
rtc::Thread* signaling_thread = nullptr;
rtc::SocketFactory* socket_factory = nullptr;
- std::unique_ptr<rtc::PacketSocketFactory> packet_socket_factory;
std::unique_ptr<TaskQueueFactory> task_queue_factory;
std::unique_ptr<cricket::MediaEngineInterface> media_engine;
std::unique_ptr<CallFactoryInterface> call_factory;
diff --git a/api/test/mock_packet_socket_factory.h b/api/test/mock_packet_socket_factory.h
deleted file mode 100644
index 7e59556..0000000
--- a/api/test/mock_packet_socket_factory.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * Copyright (c) 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.
- */
-
-#ifndef API_TEST_MOCK_PACKET_SOCKET_FACTORY_H_
-#define API_TEST_MOCK_PACKET_SOCKET_FACTORY_H_
-
-#include <memory>
-#include <string>
-
-#include "api/packet_socket_factory.h"
-#include "test/gmock.h"
-
-namespace rtc {
-class MockPacketSocketFactory : public PacketSocketFactory {
- public:
- MOCK_METHOD(AsyncPacketSocket*,
- CreateUdpSocket,
- (const SocketAddress&, uint16_t, uint16_t),
- (override));
- MOCK_METHOD(AsyncListenSocket*,
- CreateServerTcpSocket,
- (const SocketAddress&, uint16_t, uint16_t, int opts),
- (override));
- MOCK_METHOD(AsyncPacketSocket*,
- CreateClientTcpSocket,
- (const SocketAddress& local_address,
- const SocketAddress&,
- const ProxyInfo&,
- const std::string&,
- const PacketSocketTcpOptions&),
- (override));
- MOCK_METHOD(std::unique_ptr<webrtc::AsyncDnsResolverInterface>,
- CreateAsyncDnsResolver,
- (),
- (override));
-};
-
-static_assert(!std::is_abstract_v<MockPacketSocketFactory>, "");
-
-} // namespace rtc
-
-#endif // API_TEST_MOCK_PACKET_SOCKET_FACTORY_H_
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index b5f22ac..8e6523a 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -2327,7 +2327,6 @@
"../api:make_ref_counted",
"../api:media_stream_interface",
"../api:mock_encoder_selector",
- "../api:mock_packet_socket_factory",
"../api:mock_video_track",
"../api:packet_socket_factory",
"../api:priority",
diff --git a/pc/connection_context.cc b/pc/connection_context.cc
index 469c5f9..27bc822 100644
--- a/pc/connection_context.cc
+++ b/pc/connection_context.cc
@@ -103,7 +103,6 @@
network_monitor_factory_(
std::move(dependencies->network_monitor_factory)),
call_factory_(std::move(dependencies->call_factory)),
- default_socket_factory_(std::move(dependencies->packet_socket_factory)),
sctp_factory_(
MaybeCreateSctpFactory(std::move(dependencies->sctp_factory),
network_thread(),
@@ -147,10 +146,9 @@
default_network_manager_ = std::make_unique<rtc::BasicNetworkManager>(
network_monitor_factory_.get(), socket_factory, &field_trials());
- if (!default_socket_factory_) {
- default_socket_factory_ =
- std::make_unique<rtc::BasicPacketSocketFactory>(socket_factory);
- }
+ default_socket_factory_ =
+ std::make_unique<rtc::BasicPacketSocketFactory>(socket_factory);
+
// Set warning levels on the threads, to give warnings when response
// may be slower than is expected of the thread.
// Since some of the threads may be the same, start with the least
diff --git a/pc/connection_context.h b/pc/connection_context.h
index fa79480..3869164 100644
--- a/pc/connection_context.h
+++ b/pc/connection_context.h
@@ -92,7 +92,7 @@
RTC_DCHECK_RUN_ON(signaling_thread_);
return default_network_manager_.get();
}
- rtc::PacketSocketFactory* default_socket_factory() {
+ rtc::BasicPacketSocketFactory* default_socket_factory() {
RTC_DCHECK_RUN_ON(signaling_thread_);
return default_socket_factory_.get();
}
@@ -141,7 +141,7 @@
std::unique_ptr<webrtc::CallFactoryInterface> const call_factory_
RTC_GUARDED_BY(worker_thread());
- std::unique_ptr<rtc::PacketSocketFactory> default_socket_factory_
+ std::unique_ptr<rtc::BasicPacketSocketFactory> default_socket_factory_
RTC_GUARDED_BY(signaling_thread_);
std::unique_ptr<SctpTransportFactoryInterface> const sctp_factory_;
};
diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc
index e99ddce..276d7ef 100644
--- a/pc/peer_connection_factory.cc
+++ b/pc/peer_connection_factory.cc
@@ -204,6 +204,9 @@
const PeerConnectionInterface::RTCConfiguration& configuration,
PeerConnectionDependencies dependencies) {
RTC_DCHECK_RUN_ON(signaling_thread());
+ RTC_DCHECK(!(dependencies.allocator && dependencies.packet_socket_factory))
+ << "You can't set both allocator and packet_socket_factory; "
+ "the former is going away (see bugs.webrtc.org/7447";
// Set internal defaults if optional dependencies are not set.
if (!dependencies.cert_generator) {
@@ -212,8 +215,14 @@
network_thread());
}
if (!dependencies.allocator) {
+ rtc::PacketSocketFactory* packet_socket_factory;
+ if (dependencies.packet_socket_factory)
+ packet_socket_factory = dependencies.packet_socket_factory.get();
+ else
+ packet_socket_factory = context_->default_socket_factory();
+
dependencies.allocator = std::make_unique<cricket::BasicPortAllocator>(
- context_->default_network_manager(), context_->default_socket_factory(),
+ context_->default_network_manager(), packet_socket_factory,
configuration.turn_customizer);
dependencies.allocator->SetPortRange(
configuration.port_allocator_config.min_port,
diff --git a/pc/peer_connection_factory_unittest.cc b/pc/peer_connection_factory_unittest.cc
index 2edec0c..128f72c 100644
--- a/pc/peer_connection_factory_unittest.cc
+++ b/pc/peer_connection_factory_unittest.cc
@@ -20,7 +20,6 @@
#include "api/data_channel_interface.h"
#include "api/jsep.h"
#include "api/media_stream_interface.h"
-#include "api/test/mock_packet_socket_factory.h"
#include "api/video_codecs/builtin_video_decoder_factory.h"
#include "api/video_codecs/builtin_video_encoder_factory.h"
#include "media/base/fake_frame_source.h"
@@ -32,8 +31,6 @@
#include "p2p/base/port_interface.h"
#include "pc/test/fake_audio_capture_module.h"
#include "pc/test/fake_video_track_source.h"
-#include "pc/test/mock_peer_connection_observers.h"
-#include "rtc_base/gunit.h"
#include "rtc_base/rtc_certificate_generator.h"
#include "rtc_base/socket_address.h"
#include "rtc_base/time_utils.h"
@@ -54,10 +51,6 @@
using webrtc::VideoTrackInterface;
using webrtc::VideoTrackSourceInterface;
-using ::testing::_;
-using ::testing::InvokeWithoutArgs;
-using ::testing::Return;
-
namespace {
static const char kStunIceServer[] = "stun:stun.l.google.com:19302";
@@ -499,38 +492,3 @@
EXPECT_EQ(3, local_renderer.num_rendered_frames());
EXPECT_FALSE(local_renderer.black_frame());
}
-
-#ifdef WEBRTC_ANDROID
-// TODO(bugs.webrtc.org/14204): Enable on android. Fails on Nexus 5X android
-// test bot, logging "Machine has no networks; no ports will be allocated"
-#define MAYBE_UsesPacketSocketFactory DISABLED_UsesPacketSocketFactory
-#else
-#define MAYBE_UsesPacketSocketFactory UsesPacketSocketFactory
-#endif
-TEST(PeerConnectionFactoryDependenciesTest, MAYBE_UsesPacketSocketFactory) {
- constexpr int64_t kWaitTimeoutMs = 10000;
- auto mock_socket_factory = std::make_unique<rtc::MockPacketSocketFactory>();
-
- rtc::Event called;
- EXPECT_CALL(*mock_socket_factory, CreateUdpSocket(_, _, _))
- .WillOnce(InvokeWithoutArgs([&] {
- called.Set();
- return nullptr;
- }))
- .WillRepeatedly(Return(nullptr));
-
- webrtc::PeerConnectionFactoryDependencies pcf_dependencies;
- pcf_dependencies.packet_socket_factory = std::move(mock_socket_factory);
-
- rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> pcf =
- CreateModularPeerConnectionFactory(std::move(pcf_dependencies));
-
- PeerConnectionInterface::RTCConfiguration config;
- config.ice_candidate_pool_size = 2;
- NullPeerConnectionObserver observer;
- auto pc = pcf->CreatePeerConnectionOrError(
- config, webrtc::PeerConnectionDependencies(&observer));
- ASSERT_TRUE(pc.ok());
-
- called.Wait(kWaitTimeoutMs);
-}
