Commit 1a41863f authored by Himanshu Jaju's avatar Himanshu Jaju Committed by Commit Bot

Fix WebRTC threading issues.

- Adds a SingleThreadTaskRunner for WebRTC medium to allow jingle calls
- Replaces SequencedTaskRunner with SingleThreadTaskRunner for platform SingleThreadExecutor impl

Bug: 1122532
Change-Id: I611ef85ec96fbcad3eeb160fd74b9e688edfc5f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379727Reviewed-by: default avatarAlex Chau <alexchau@chromium.org>
Commit-Queue: Alex Chau <alexchau@chromium.org>
Auto-Submit: Himanshu Jaju <himanshujaju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802757}
parent e85f66cf
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/synchronization/waitable_event.h" #include "base/synchronization/waitable_event.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/nearby_sharing/logging/logging.h" #include "chrome/browser/nearby_sharing/logging/logging.h"
#include "chrome/services/sharing/nearby/nearby_connections_conversions.h" #include "chrome/services/sharing/nearby/nearby_connections_conversions.h"
#include "chrome/services/sharing/nearby/platform_v2/input_file.h" #include "chrome/services/sharing/nearby/platform_v2/input_file.h"
...@@ -91,7 +92,8 @@ NearbyConnections::NearbyConnections( ...@@ -91,7 +92,8 @@ NearbyConnections::NearbyConnections(
std::unique_ptr<Core> core) std::unique_ptr<Core> core)
: nearby_connections_(this, std::move(nearby_connections)), : nearby_connections_(this, std::move(nearby_connections)),
on_disconnect_(std::move(on_disconnect)), on_disconnect_(std::move(on_disconnect)),
core_(std::move(core)) { core_(std::move(core)),
thread_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
nearby_connections_.set_disconnect_handler(base::BindOnce( nearby_connections_.set_disconnect_handler(base::BindOnce(
&NearbyConnections::OnDisconnect, weak_ptr_factory_.GetWeakPtr())); &NearbyConnections::OnDisconnect, weak_ptr_factory_.GetWeakPtr()));
...@@ -445,6 +447,11 @@ base::File NearbyConnections::ExtractOutputFile(int64_t payload_id) { ...@@ -445,6 +447,11 @@ base::File NearbyConnections::ExtractOutputFile(int64_t payload_id) {
return file; return file;
} }
scoped_refptr<base::SingleThreadTaskRunner>
NearbyConnections::GetThreadTaskRunner() {
return thread_task_runner_;
}
} // namespace connections } // namespace connections
} // namespace nearby } // namespace nearby
} // namespace location } // namespace location
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/files/file.h" #include "base/files/file.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/single_thread_task_runner.h"
#include "base/synchronization/lock.h" #include "base/synchronization/lock.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/thread_annotations.h" #include "base/thread_annotations.h"
...@@ -111,6 +112,9 @@ class NearbyConnections : public mojom::NearbyConnections { ...@@ -111,6 +112,9 @@ class NearbyConnections : public mojom::NearbyConnections {
// Returns the file associated with |payload_id| for OutputFile. // Returns the file associated with |payload_id| for OutputFile.
base::File ExtractOutputFile(int64_t payload_id); base::File ExtractOutputFile(int64_t payload_id);
// Returns the task runner for the thread that created |this|.
scoped_refptr<base::SingleThreadTaskRunner> GetThreadTaskRunner();
private: private:
void OnDisconnect(); void OnDisconnect();
...@@ -142,6 +146,8 @@ class NearbyConnections : public mojom::NearbyConnections { ...@@ -142,6 +146,8 @@ class NearbyConnections : public mojom::NearbyConnections {
base::flat_map<int64_t, base::File> output_file_map_ base::flat_map<int64_t, base::File> output_file_map_
GUARDED_BY(output_file_lock_); GUARDED_BY(output_file_lock_);
scoped_refptr<base::SingleThreadTaskRunner> thread_task_runner_;
base::WeakPtrFactory<NearbyConnections> weak_ptr_factory_{this}; base::WeakPtrFactory<NearbyConnections> weak_ptr_factory_{this};
}; };
......
...@@ -53,7 +53,9 @@ int GetCurrentTid() { ...@@ -53,7 +53,9 @@ int GetCurrentTid() {
std::unique_ptr<SubmittableExecutor> std::unique_ptr<SubmittableExecutor>
ImplementationPlatform::CreateSingleThreadExecutor() { ImplementationPlatform::CreateSingleThreadExecutor() {
return std::make_unique<chrome::SubmittableExecutor>( return std::make_unique<chrome::SubmittableExecutor>(
base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()})); base::ThreadPool::CreateSingleThreadTaskRunner(
{base::MayBlock()},
base::SingleThreadTaskRunnerThreadMode::DEDICATED));
} }
std::unique_ptr<SubmittableExecutor> std::unique_ptr<SubmittableExecutor>
...@@ -175,8 +177,9 @@ std::unique_ptr<WebRtcMedium> ImplementationPlatform::CreateWebRtcMedium() { ...@@ -175,8 +177,9 @@ std::unique_ptr<WebRtcMedium> ImplementationPlatform::CreateWebRtcMedium() {
if (!socket_manager || !mdns_responder || !ice_config_fetcher || !messenger) if (!socket_manager || !mdns_responder || !ice_config_fetcher || !messenger)
return nullptr; return nullptr;
return std::make_unique<chrome::WebRtcMedium>(socket_manager, mdns_responder, return std::make_unique<chrome::WebRtcMedium>(
ice_config_fetcher, messenger); socket_manager, mdns_responder, ice_config_fetcher, messenger,
connections.GetThreadTaskRunner());
} }
std::unique_ptr<Mutex> ImplementationPlatform::CreateMutex(Mutex::Mode mode) { std::unique_ptr<Mutex> ImplementationPlatform::CreateMutex(Mutex::Mode mode) {
......
...@@ -159,16 +159,26 @@ WebRtcMedium::WebRtcMedium( ...@@ -159,16 +159,26 @@ WebRtcMedium::WebRtcMedium(
network::mojom::P2PSocketManager* socket_manager, network::mojom::P2PSocketManager* socket_manager,
network::mojom::MdnsResponder* mdns_responder, network::mojom::MdnsResponder* mdns_responder,
sharing::mojom::IceConfigFetcher* ice_config_fetcher, sharing::mojom::IceConfigFetcher* ice_config_fetcher,
sharing::mojom::WebRtcSignalingMessenger* webrtc_signaling_messenger) sharing::mojom::WebRtcSignalingMessenger* webrtc_signaling_messenger,
scoped_refptr<base::SingleThreadTaskRunner> task_runner)
: p2p_socket_manager_(socket_manager), : p2p_socket_manager_(socket_manager),
mdns_responder_(mdns_responder), mdns_responder_(mdns_responder),
ice_config_fetcher_(ice_config_fetcher), ice_config_fetcher_(ice_config_fetcher),
webrtc_signaling_messenger_(webrtc_signaling_messenger) {} webrtc_signaling_messenger_(webrtc_signaling_messenger),
task_runner_(std::move(task_runner)) {}
WebRtcMedium::~WebRtcMedium() = default; WebRtcMedium::~WebRtcMedium() = default;
void WebRtcMedium::CreatePeerConnection( void WebRtcMedium::CreatePeerConnection(
webrtc::PeerConnectionObserver* observer, webrtc::PeerConnectionObserver* observer,
PeerConnectionCallback callback) { PeerConnectionCallback callback) {
task_runner_->PostTask(
FROM_HERE, base::BindOnce(&WebRtcMedium::FetchIceServers,
weak_ptr_factory_.GetWeakPtr(), observer,
std::move(callback)));
}
void WebRtcMedium::FetchIceServers(webrtc::PeerConnectionObserver* observer,
PeerConnectionCallback callback) {
ice_config_fetcher_->GetIceServers(base::BindOnce( ice_config_fetcher_->GetIceServers(base::BindOnce(
&WebRtcMedium::OnIceServersFetched, weak_ptr_factory_.GetWeakPtr(), &WebRtcMedium::OnIceServersFetched, weak_ptr_factory_.GetWeakPtr(),
observer, std::move(callback))); observer, std::move(callback)));
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <memory> #include <memory>
#include "base/single_thread_task_runner.h"
#include "chrome/services/sharing/public/mojom/webrtc.mojom.h" #include "chrome/services/sharing/public/mojom/webrtc.mojom.h"
#include "chrome/services/sharing/public/mojom/webrtc_signaling_messenger.mojom.h" #include "chrome/services/sharing/public/mojom/webrtc_signaling_messenger.mojom.h"
#include "services/network/public/mojom/mdns_responder.mojom.h" #include "services/network/public/mojom/mdns_responder.mojom.h"
...@@ -32,7 +33,8 @@ class WebRtcMedium : public api::WebRtcMedium { ...@@ -32,7 +33,8 @@ class WebRtcMedium : public api::WebRtcMedium {
network::mojom::P2PSocketManager* socket_manager, network::mojom::P2PSocketManager* socket_manager,
network::mojom::MdnsResponder* mdns_responder, network::mojom::MdnsResponder* mdns_responder,
sharing::mojom::IceConfigFetcher* ice_config_fetcher, sharing::mojom::IceConfigFetcher* ice_config_fetcher,
sharing::mojom::WebRtcSignalingMessenger* webrtc_signaling_messenger); sharing::mojom::WebRtcSignalingMessenger* webrtc_signaling_messenger,
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
~WebRtcMedium() override; ~WebRtcMedium() override;
// api::WebRtcMedium: // api::WebRtcMedium:
...@@ -42,6 +44,8 @@ class WebRtcMedium : public api::WebRtcMedium { ...@@ -42,6 +44,8 @@ class WebRtcMedium : public api::WebRtcMedium {
absl::string_view self_id) override; absl::string_view self_id) override;
private: private:
void FetchIceServers(webrtc::PeerConnectionObserver* observer,
PeerConnectionCallback callback);
void OnIceServersFetched( void OnIceServersFetched(
webrtc::PeerConnectionObserver* observer, webrtc::PeerConnectionObserver* observer,
PeerConnectionCallback callback, PeerConnectionCallback callback,
...@@ -54,6 +58,8 @@ class WebRtcMedium : public api::WebRtcMedium { ...@@ -54,6 +58,8 @@ class WebRtcMedium : public api::WebRtcMedium {
std::unique_ptr<sharing::IpcPacketSocketFactory> socket_factory_; std::unique_ptr<sharing::IpcPacketSocketFactory> socket_factory_;
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
base::WeakPtrFactory<WebRtcMedium> weak_ptr_factory_{this}; base::WeakPtrFactory<WebRtcMedium> weak_ptr_factory_{this};
}; };
......
...@@ -50,7 +50,8 @@ class WebRtcMediumTest : public ::testing::Test { ...@@ -50,7 +50,8 @@ class WebRtcMediumTest : public ::testing::Test {
webrtc_medium_(socket_manager_.get(), webrtc_medium_(socket_manager_.get(),
mdns_responder_.get(), mdns_responder_.get(),
ice_config_fetcher_.get(), ice_config_fetcher_.get(),
messenger_.get()) {} messenger_.get(),
base::ThreadTaskRunnerHandle::Get()) {}
~WebRtcMediumTest() override { ~WebRtcMediumTest() override {
// Let libjingle threads finish. // Let libjingle threads finish.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment