Commit 402c8ae1 authored by reillyg's avatar reillyg Committed by Commit bot

Use the task scheduler in the USB service on macOS and Windows

This updates the libusb-based USB service to use the task scheduler
which means it will be enabled on macOS and Windows. The new Windows
USB service will be updated next.

By executing these tasks on a sequence with the CONTINUE_ON_SHUTDOWN
trait a device or driver bug that causes a request to lock up should
not prevent Chrome from shutting down.

BUG=652416,703585

Review-Url: https://codereview.chromium.org/2857473002
Cr-Commit-Position: refs/heads/master@{#468967}
parent 0b33b320
...@@ -13,10 +13,10 @@ ...@@ -13,10 +13,10 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/location.h" #include "base/location.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/sequence_checker.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/synchronization/lock.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "components/device_event_log/device_event_log.h" #include "components/device_event_log/device_event_log.h"
#include "device/usb/usb_context.h" #include "device/usb/usb_context.h"
...@@ -168,7 +168,7 @@ class UsbDeviceHandleImpl::InterfaceClaimer ...@@ -168,7 +168,7 @@ class UsbDeviceHandleImpl::InterfaceClaimer
int alternate_setting_; int alternate_setting_;
const scoped_refptr<base::SingleThreadTaskRunner> task_runner_; const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
ResultCallback release_callback_; ResultCallback release_callback_;
base::ThreadChecker thread_checker_; base::SequenceChecker sequence_checker_;
DISALLOW_COPY_AND_ASSIGN(InterfaceClaimer); DISALLOW_COPY_AND_ASSIGN(InterfaceClaimer);
}; };
...@@ -183,7 +183,7 @@ UsbDeviceHandleImpl::InterfaceClaimer::InterfaceClaimer( ...@@ -183,7 +183,7 @@ UsbDeviceHandleImpl::InterfaceClaimer::InterfaceClaimer(
task_runner_(task_runner) {} task_runner_(task_runner) {}
UsbDeviceHandleImpl::InterfaceClaimer::~InterfaceClaimer() { UsbDeviceHandleImpl::InterfaceClaimer::~InterfaceClaimer() {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(sequence_checker_.CalledOnValidSequence());
int rc = libusb_release_interface(handle_->handle(), interface_number_); int rc = libusb_release_interface(handle_->handle(), interface_number_);
if (rc != LIBUSB_SUCCESS) { if (rc != LIBUSB_SUCCESS) {
USB_LOG(DEBUG) << "Failed to release interface: " USB_LOG(DEBUG) << "Failed to release interface: "
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/threading/thread_restrictions.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "components/device_event_log/device_event_log.h" #include "components/device_event_log/device_event_log.h"
...@@ -22,15 +23,14 @@ ...@@ -22,15 +23,14 @@
#include "device/usb/usb_descriptors.h" #include "device/usb/usb_descriptors.h"
#include "device/usb/usb_device_handle_impl.h" #include "device/usb/usb_device_handle_impl.h"
#include "device/usb/usb_error.h" #include "device/usb/usb_error.h"
#include "device/usb/usb_service.h"
#include "third_party/libusb/src/libusb/libusb.h" #include "third_party/libusb/src/libusb/libusb.h"
namespace device { namespace device {
UsbDeviceImpl::UsbDeviceImpl( UsbDeviceImpl::UsbDeviceImpl(scoped_refptr<UsbContext> context,
scoped_refptr<UsbContext> context, PlatformUsbDevice platform_device,
PlatformUsbDevice platform_device, const libusb_device_descriptor& descriptor)
const libusb_device_descriptor& descriptor,
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner)
: UsbDevice(descriptor.bcdUSB, : UsbDevice(descriptor.bcdUSB,
descriptor.bDeviceClass, descriptor.bDeviceClass,
descriptor.bDeviceSubClass, descriptor.bDeviceSubClass,
...@@ -42,9 +42,7 @@ UsbDeviceImpl::UsbDeviceImpl( ...@@ -42,9 +42,7 @@ UsbDeviceImpl::UsbDeviceImpl(
base::string16(), base::string16(),
base::string16()), base::string16()),
platform_device_(platform_device), platform_device_(platform_device),
context_(context), context_(context) {
task_runner_(base::ThreadTaskRunnerHandle::Get()),
blocking_task_runner_(blocking_task_runner) {
CHECK(platform_device) << "platform_device cannot be NULL"; CHECK(platform_device) << "platform_device cannot be NULL";
libusb_ref_device(platform_device); libusb_ref_device(platform_device);
ReadAllConfigurations(); ReadAllConfigurations();
...@@ -59,9 +57,12 @@ UsbDeviceImpl::~UsbDeviceImpl() { ...@@ -59,9 +57,12 @@ UsbDeviceImpl::~UsbDeviceImpl() {
void UsbDeviceImpl::Open(const OpenCallback& callback) { void UsbDeviceImpl::Open(const OpenCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
blocking_task_runner_->PostTask( scoped_refptr<base::SequencedTaskRunner> blocking_task_runner =
UsbService::CreateBlockingTaskRunner();
blocking_task_runner->PostTask(
FROM_HERE, FROM_HERE,
base::Bind(&UsbDeviceImpl::OpenOnBlockingThread, this, callback)); base::Bind(&UsbDeviceImpl::OpenOnBlockingThread, this, callback,
base::ThreadTaskRunnerHandle::Get(), blocking_task_runner));
} }
void UsbDeviceImpl::ReadAllConfigurations() { void UsbDeviceImpl::ReadAllConfigurations() {
...@@ -99,24 +100,31 @@ void UsbDeviceImpl::RefreshActiveConfiguration() { ...@@ -99,24 +100,31 @@ void UsbDeviceImpl::RefreshActiveConfiguration() {
ActiveConfigurationChanged(config_value); ActiveConfigurationChanged(config_value);
} }
void UsbDeviceImpl::OpenOnBlockingThread(const OpenCallback& callback) { void UsbDeviceImpl::OpenOnBlockingThread(
const OpenCallback& callback,
scoped_refptr<base::TaskRunner> task_runner,
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner) {
base::ThreadRestrictions::AssertIOAllowed();
PlatformUsbDeviceHandle handle; PlatformUsbDeviceHandle handle;
const int rv = libusb_open(platform_device_, &handle); const int rv = libusb_open(platform_device_, &handle);
if (LIBUSB_SUCCESS == rv) { if (LIBUSB_SUCCESS == rv) {
task_runner_->PostTask( task_runner->PostTask(
FROM_HERE, base::Bind(&UsbDeviceImpl::Opened, this, handle, callback)); FROM_HERE, base::Bind(&UsbDeviceImpl::Opened, this, handle, callback,
blocking_task_runner));
} else { } else {
USB_LOG(EVENT) << "Failed to open device: " USB_LOG(EVENT) << "Failed to open device: "
<< ConvertPlatformUsbErrorToString(rv); << ConvertPlatformUsbErrorToString(rv);
task_runner_->PostTask(FROM_HERE, base::Bind(callback, nullptr)); task_runner->PostTask(FROM_HERE, base::Bind(callback, nullptr));
} }
} }
void UsbDeviceImpl::Opened(PlatformUsbDeviceHandle platform_handle, void UsbDeviceImpl::Opened(
const OpenCallback& callback) { PlatformUsbDeviceHandle platform_handle,
const OpenCallback& callback,
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
scoped_refptr<UsbDeviceHandle> device_handle = new UsbDeviceHandleImpl( scoped_refptr<UsbDeviceHandle> device_handle = new UsbDeviceHandleImpl(
context_, this, platform_handle, blocking_task_runner_); context_, this, platform_handle, blocking_task_runner);
handles().push_back(device_handle.get()); handles().push_back(device_handle.get());
callback.Run(device_handle); callback.Run(device_handle);
} }
......
...@@ -70,8 +70,7 @@ class UsbDeviceImpl : public UsbDevice { ...@@ -70,8 +70,7 @@ class UsbDeviceImpl : public UsbDevice {
// Called by UsbServiceImpl only; // Called by UsbServiceImpl only;
UsbDeviceImpl(scoped_refptr<UsbContext> context, UsbDeviceImpl(scoped_refptr<UsbContext> context,
PlatformUsbDevice platform_device, PlatformUsbDevice platform_device,
const libusb_device_descriptor& descriptor, const libusb_device_descriptor& descriptor);
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
~UsbDeviceImpl() override; ~UsbDeviceImpl() override;
...@@ -84,9 +83,13 @@ class UsbDeviceImpl : public UsbDevice { ...@@ -84,9 +83,13 @@ class UsbDeviceImpl : public UsbDevice {
private: private:
void GetAllConfigurations(); void GetAllConfigurations();
void OpenOnBlockingThread(const OpenCallback& callback); void OpenOnBlockingThread(
const OpenCallback& callback,
scoped_refptr<base::TaskRunner> task_runner,
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
void Opened(PlatformUsbDeviceHandle platform_handle, void Opened(PlatformUsbDeviceHandle platform_handle,
const OpenCallback& callback); const OpenCallback& callback,
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
base::ThreadChecker thread_checker_; base::ThreadChecker thread_checker_;
PlatformUsbDevice platform_device_; PlatformUsbDevice platform_device_;
...@@ -95,9 +98,6 @@ class UsbDeviceImpl : public UsbDevice { ...@@ -95,9 +98,6 @@ class UsbDeviceImpl : public UsbDevice {
// Retain the context so that it will not be released before UsbDevice. // Retain the context so that it will not be released before UsbDevice.
scoped_refptr<UsbContext> context_; scoped_refptr<UsbContext> context_;
scoped_refptr<base::SequencedTaskRunner> task_runner_;
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
DISALLOW_COPY_AND_ASSIGN(UsbDeviceImpl); DISALLOW_COPY_AND_ASSIGN(UsbDeviceImpl);
}; };
......
...@@ -54,9 +54,9 @@ std::unique_ptr<UsbService> UsbService::Create( ...@@ -54,9 +54,9 @@ std::unique_ptr<UsbService> UsbService::Create(
if (base::FeatureList::IsEnabled(kNewUsbBackend)) if (base::FeatureList::IsEnabled(kNewUsbBackend))
return base::WrapUnique(new UsbServiceWin(blocking_task_runner)); return base::WrapUnique(new UsbServiceWin(blocking_task_runner));
else else
return base::WrapUnique(new UsbServiceImpl(blocking_task_runner)); return base::WrapUnique(new UsbServiceImpl());
#elif defined(OS_MACOSX) #elif defined(OS_MACOSX)
return base::WrapUnique(new UsbServiceImpl(blocking_task_runner)); return base::WrapUnique(new UsbServiceImpl());
#else #else
return nullptr; return nullptr;
#endif #endif
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/task_scheduler/post_task.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "components/device_event_log/device_event_log.h" #include "components/device_event_log/device_event_log.h"
#include "device/usb/usb_device_handle.h" #include "device/usb/usb_device_handle.h"
...@@ -216,15 +217,19 @@ void OnDeviceOpenedReadDescriptors( ...@@ -216,15 +217,19 @@ void OnDeviceOpenedReadDescriptors(
} // namespace } // namespace
UsbServiceImpl::UsbServiceImpl( UsbServiceImpl::UsbServiceImpl()
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_in) : UsbService(nullptr),
: UsbService(std::move(blocking_task_runner_in)),
#if defined(OS_WIN) #if defined(OS_WIN)
device_observer_(this), device_observer_(this),
#endif #endif
weak_factory_(this) { weak_factory_(this) {
blocking_task_runner()->PostTask( base::PostTaskWithTraits(
FROM_HERE, FROM_HERE,
base::TaskTraits()
.MayBlock()
.WithPriority(base::TaskPriority::USER_VISIBLE)
.WithShutdownBehavior(
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN),
base::Bind(&InitializeUsbContextOnBlockingThread, task_runner(), base::Bind(&InitializeUsbContextOnBlockingThread, task_runner(),
base::Bind(&UsbServiceImpl::OnUsbContext, base::Bind(&UsbServiceImpl::OnUsbContext,
weak_factory_.GetWeakPtr()))); weak_factory_.GetWeakPtr())));
...@@ -324,11 +329,17 @@ void UsbServiceImpl::RefreshDevices() { ...@@ -324,11 +329,17 @@ void UsbServiceImpl::RefreshDevices() {
pending_path_enumerations_.pop(); pending_path_enumerations_.pop();
} }
blocking_task_runner()->PostTask( base::PostTaskWithTraits(
FROM_HERE, FROM_HERE,
base::TaskTraits()
.MayBlock()
.WithPriority(base::TaskPriority::USER_VISIBLE)
.WithShutdownBehavior(
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN),
base::Bind(&GetDeviceListOnBlockingThread, device_path, context_, base::Bind(&GetDeviceListOnBlockingThread, device_path, context_,
task_runner(), base::Bind(&UsbServiceImpl::OnDeviceList, task_runner(),
weak_factory_.GetWeakPtr()))); base::Bind(&UsbServiceImpl::OnDeviceList,
weak_factory_.GetWeakPtr())));
} }
void UsbServiceImpl::OnDeviceList(libusb_device** platform_devices, void UsbServiceImpl::OnDeviceList(libusb_device** platform_devices,
...@@ -437,8 +448,8 @@ void UsbServiceImpl::EnumerateDevice(PlatformUsbDevice platform_device, ...@@ -437,8 +448,8 @@ void UsbServiceImpl::EnumerateDevice(PlatformUsbDevice platform_device,
return; return;
} }
scoped_refptr<UsbDeviceImpl> device(new UsbDeviceImpl( scoped_refptr<UsbDeviceImpl> device(
context_, platform_device, descriptor, blocking_task_runner())); new UsbDeviceImpl(context_, platform_device, descriptor));
base::Closure add_device = base::Closure add_device =
base::Bind(&UsbServiceImpl::AddDevice, weak_factory_.GetWeakPtr(), base::Bind(&UsbServiceImpl::AddDevice, weak_factory_.GetWeakPtr(),
refresh_complete, device); refresh_complete, device);
......
...@@ -25,10 +25,6 @@ ...@@ -25,10 +25,6 @@
struct libusb_device; struct libusb_device;
struct libusb_context; struct libusb_context;
namespace base {
class SequencedTaskRunner;
}
namespace device { namespace device {
typedef struct libusb_device* PlatformUsbDevice; typedef struct libusb_device* PlatformUsbDevice;
...@@ -42,8 +38,7 @@ class UsbServiceImpl : ...@@ -42,8 +38,7 @@ class UsbServiceImpl :
#endif // OS_WIN #endif // OS_WIN
public UsbService { public UsbService {
public: public:
explicit UsbServiceImpl( UsbServiceImpl();
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
~UsbServiceImpl() override; ~UsbServiceImpl() override;
private: private:
......
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