Commit 1010feef authored by Ryan Hansberry's avatar Ryan Hansberry Committed by Commit Bot

[Nearby] Synchronously construct service controller.

The service controller passed to ServiceControllerProxy was being
initialized asynchronously, and could occasionally be passed into
ServiceControllerProxy before being correctly initialized, causing
a segfault once accessed. This CL changes that behavior to simply
immediately initialize the service controller.

Fixed: 1147293
Change-Id: Ie4e396706b5477befca47805c8984c697a814bcd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527633Reviewed-by: default avatarJames Vecore <vecore@google.com>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825958}
parent 43258689
...@@ -185,19 +185,6 @@ NearbyConnections::NearbyConnections( ...@@ -185,19 +185,6 @@ NearbyConnections::NearbyConnections(
on_disconnect_(std::move(on_disconnect)), on_disconnect_(std::move(on_disconnect)),
service_controller_(std::move(service_controller)), service_controller_(std::move(service_controller)),
thread_task_runner_(base::ThreadTaskRunnerHandle::Get()) { thread_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
// Note: Some tests pass a value for |service_controller_|, but this value is
// expected to be null during normal operation.
if (!service_controller_) {
// Post a task which initializes |service_controller_|. This must be posted
// as a task instead of being completed synchrnously here because
// OfflineServiceController invokes NearbyConnections::GetInstance(), which
// requires that the instance be initialized by completing this constructor.
thread_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&NearbyConnections::InitializeOfflineServiceController,
weak_ptr_factory_.GetWeakPtr()));
}
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()));
...@@ -244,6 +231,15 @@ NearbyConnections::NearbyConnections( ...@@ -244,6 +231,15 @@ NearbyConnections::NearbyConnections(
// There should only be one instance of NearbyConnections in a process. // There should only be one instance of NearbyConnections in a process.
DCHECK(!g_instance); DCHECK(!g_instance);
g_instance = this; g_instance = this;
// Note: Some tests pass a value for |service_controller_|, but this value is
// expected to be null during normal operation.
if (!service_controller_) {
// OfflineServiceController indirectly invokes
// NearbyConnections::GetInstance(), so it must be initialized after
// |g_instance| is set.
service_controller_ = std::make_unique<OfflineServiceController>();
}
} }
NearbyConnections::~NearbyConnections() { NearbyConnections::~NearbyConnections() {
...@@ -253,11 +249,6 @@ NearbyConnections::~NearbyConnections() { ...@@ -253,11 +249,6 @@ NearbyConnections::~NearbyConnections() {
g_instance = nullptr; g_instance = nullptr;
} }
void NearbyConnections::InitializeOfflineServiceController() {
DCHECK(!service_controller_);
service_controller_ = std::make_unique<OfflineServiceController>();
}
void NearbyConnections::OnDisconnect() { void NearbyConnections::OnDisconnect() {
if (on_disconnect_) if (on_disconnect_)
std::move(on_disconnect_).Run(); std::move(on_disconnect_).Run();
......
...@@ -153,7 +153,6 @@ class NearbyConnections : public mojom::NearbyConnections { ...@@ -153,7 +153,6 @@ class NearbyConnections : public mojom::NearbyConnections {
private: private:
Core* GetCore(const std::string& service_id); Core* GetCore(const std::string& service_id);
void InitializeOfflineServiceController();
void OnDisconnect(); void OnDisconnect();
mojo::Receiver<mojom::NearbyConnections> nearby_connections_; mojo::Receiver<mojom::NearbyConnections> nearby_connections_;
......
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