Commit 9067b0f8 authored by Ken Rockot's avatar Ken Rockot Committed by Commit Bot

Allow Device Service access from any thread

This allows multiple device.mojom.DeviceService connections to the
Device Service and changes content::GetDeviceService() to safely
maintain independent sequence-local connections.

The new behavior corrects an issue where an inadvertent background-
thread client could trample the global Device Service instance early
in startup, leaving all main-thread clients with a permanently broken
connection.

Bug: 1041804
Change-Id: I3efcb661c9508e1e0007d00f2f9f963c26220de4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2168829Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#763452}
parent 4ffa6a29
...@@ -94,9 +94,10 @@ void BindDeviceServiceReceiver( ...@@ -94,9 +94,10 @@ void BindDeviceServiceReceiver(
service_slot; service_slot;
auto& service = service_slot->GetOrCreateValue(); auto& service = service_slot->GetOrCreateValue();
// This function should only be called once during the lifetime of the if (service) {
// service's bound sequence. service->AddReceiver(std::move(receiver));
DCHECK(!service); return;
}
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
JNIEnv* env = base::android::AttachCurrentThread(); JNIEnv* env = base::android::AttachCurrentThread();
......
...@@ -99,8 +99,7 @@ DeviceService::DeviceService( ...@@ -99,8 +99,7 @@ DeviceService::DeviceService(
const WakeLockContextCallback& wake_lock_context_callback, const WakeLockContextCallback& wake_lock_context_callback,
const base::android::JavaRef<jobject>& java_nfc_delegate, const base::android::JavaRef<jobject>& java_nfc_delegate,
mojo::PendingReceiver<mojom::DeviceService> receiver) mojo::PendingReceiver<mojom::DeviceService> receiver)
: receiver_(this, std::move(receiver)), : file_task_runner_(std::move(file_task_runner)),
file_task_runner_(std::move(file_task_runner)),
io_task_runner_(std::move(io_task_runner)), io_task_runner_(std::move(io_task_runner)),
url_loader_factory_(std::move(url_loader_factory)), url_loader_factory_(std::move(url_loader_factory)),
network_connection_tracker_(network_connection_tracker), network_connection_tracker_(network_connection_tracker),
...@@ -108,6 +107,7 @@ DeviceService::DeviceService( ...@@ -108,6 +107,7 @@ DeviceService::DeviceService(
wake_lock_context_callback_(wake_lock_context_callback), wake_lock_context_callback_(wake_lock_context_callback),
wake_lock_provider_(file_task_runner_, wake_lock_context_callback_), wake_lock_provider_(file_task_runner_, wake_lock_context_callback_),
java_interface_provider_initialized_(false) { java_interface_provider_initialized_(false) {
receivers_.Add(this, std::move(receiver));
java_nfc_delegate_.Reset(java_nfc_delegate); java_nfc_delegate_.Reset(java_nfc_delegate);
} }
#else #else
...@@ -118,13 +118,13 @@ DeviceService::DeviceService( ...@@ -118,13 +118,13 @@ DeviceService::DeviceService(
network::NetworkConnectionTracker* network_connection_tracker, network::NetworkConnectionTracker* network_connection_tracker,
const std::string& geolocation_api_key, const std::string& geolocation_api_key,
mojo::PendingReceiver<mojom::DeviceService> receiver) mojo::PendingReceiver<mojom::DeviceService> receiver)
: receiver_(this, std::move(receiver)), : file_task_runner_(std::move(file_task_runner)),
file_task_runner_(std::move(file_task_runner)),
io_task_runner_(std::move(io_task_runner)), io_task_runner_(std::move(io_task_runner)),
url_loader_factory_(std::move(url_loader_factory)), url_loader_factory_(std::move(url_loader_factory)),
network_connection_tracker_(network_connection_tracker), network_connection_tracker_(network_connection_tracker),
geolocation_api_key_(geolocation_api_key), geolocation_api_key_(geolocation_api_key),
wake_lock_provider_(file_task_runner_, wake_lock_context_callback_) { wake_lock_provider_(file_task_runner_, wake_lock_context_callback_) {
receivers_.Add(this, std::move(receiver));
#if (defined(OS_LINUX) && defined(USE_UDEV)) || defined(OS_WIN) || \ #if (defined(OS_LINUX) && defined(USE_UDEV)) || defined(OS_WIN) || \
defined(OS_MACOSX) defined(OS_MACOSX)
serial_port_manager_ = std::make_unique<SerialPortManagerImpl>( serial_port_manager_ = std::make_unique<SerialPortManagerImpl>(
...@@ -162,6 +162,11 @@ DeviceService::~DeviceService() { ...@@ -162,6 +162,11 @@ DeviceService::~DeviceService() {
#endif #endif
} }
void DeviceService::AddReceiver(
mojo::PendingReceiver<mojom::DeviceService> receiver) {
receivers_.Add(this, std::move(receiver));
}
void DeviceService::SetPlatformSensorProviderForTesting( void DeviceService::SetPlatformSensorProviderForTesting(
std::unique_ptr<PlatformSensorProvider> provider) { std::unique_ptr<PlatformSensorProvider> provider) {
DCHECK(!sensor_provider_); DCHECK(!sensor_provider_);
......
...@@ -12,7 +12,7 @@ ...@@ -12,7 +12,7 @@
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/receiver_set.h"
#include "services/device/geolocation/geolocation_provider.h" #include "services/device/geolocation/geolocation_provider.h"
#include "services/device/geolocation/geolocation_provider_impl.h" #include "services/device/geolocation/geolocation_provider_impl.h"
#include "services/device/geolocation/public_ip_address_geolocation_provider.h" #include "services/device/geolocation/public_ip_address_geolocation_provider.h"
...@@ -126,6 +126,8 @@ class DeviceService : public mojom::DeviceService { ...@@ -126,6 +126,8 @@ class DeviceService : public mojom::DeviceService {
#endif #endif
~DeviceService() override; ~DeviceService() override;
void AddReceiver(mojo::PendingReceiver<mojom::DeviceService> receiver);
void SetPlatformSensorProviderForTesting( void SetPlatformSensorProviderForTesting(
std::unique_ptr<PlatformSensorProvider> provider); std::unique_ptr<PlatformSensorProvider> provider);
...@@ -199,7 +201,7 @@ class DeviceService : public mojom::DeviceService { ...@@ -199,7 +201,7 @@ class DeviceService : public mojom::DeviceService {
void BindUsbDeviceManagerTest( void BindUsbDeviceManagerTest(
mojo::PendingReceiver<mojom::UsbDeviceManagerTest> receiver) override; mojo::PendingReceiver<mojom::UsbDeviceManagerTest> receiver) override;
mojo::Receiver<mojom::DeviceService> receiver_; mojo::ReceiverSet<mojom::DeviceService> receivers_;
std::unique_ptr<PowerMonitorMessageBroadcaster> std::unique_ptr<PowerMonitorMessageBroadcaster>
power_monitor_message_broadcaster_; power_monitor_message_broadcaster_;
std::unique_ptr<PublicIpAddressGeolocationProvider> std::unique_ptr<PublicIpAddressGeolocationProvider>
......
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