Skip to content

Commit

Permalink
Renaming BlockingTaskHelper to BlockingTaskRunnerHelper
Browse files Browse the repository at this point in the history 
Helper is not associated with a single task, therefore
it was renamed to avoid confusion.

Bug: 946590
Change-Id: Ib60a9f70c0f60e8c922c46ac75a4a73737b69f1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1541250
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#645706}
  • Loading branch information
@arskama
arskama authored and Commit Bot committed Mar 29, 2019
1 parent 01b3aae commit 28c1179
Show file tree
Hide file tree
Showing 13 changed files with 125 additions and 108 deletions.
71 changes: 39 additions & 32 deletions device/usb/usb_device_handle_usbfs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,13 @@ UsbTransferStatus ConvertTransferResult(int rc) {

} // namespace

class UsbDeviceHandleUsbfs::BlockingTaskHelper {
class UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper {
public:
BlockingTaskHelper(base::ScopedFD fd,
scoped_refptr<UsbDeviceHandleUsbfs> device_handle,
scoped_refptr<base::SequencedTaskRunner> task_runner);
~BlockingTaskHelper();
BlockingTaskRunnerHelper(
base::ScopedFD fd,
scoped_refptr<UsbDeviceHandleUsbfs> device_handle,
scoped_refptr<base::SequencedTaskRunner> task_runner);
~BlockingTaskRunnerHelper();

void Start();
void ReleaseFileDescriptor();
Expand All @@ -156,7 +157,7 @@ class UsbDeviceHandleUsbfs::BlockingTaskHelper {
std::unique_ptr<base::FileDescriptorWatcher::Controller> watch_controller_;
base::SequenceChecker sequence_checker_;

DISALLOW_COPY_AND_ASSIGN(BlockingTaskHelper);
DISALLOW_COPY_AND_ASSIGN(BlockingTaskRunnerHelper);
};

struct UsbDeviceHandleUsbfs::Transfer {
Expand Down Expand Up @@ -195,38 +196,38 @@ struct UsbDeviceHandleUsbfs::Transfer {
usbdevfs_urb urb;
};

UsbDeviceHandleUsbfs::BlockingTaskHelper::BlockingTaskHelper(
UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::BlockingTaskRunnerHelper(
base::ScopedFD fd,
scoped_refptr<UsbDeviceHandleUsbfs> device_handle,
scoped_refptr<base::SequencedTaskRunner> task_runner)
: fd_(std::move(fd)),
device_handle_(std::move(device_handle)),
task_runner_(std::move(task_runner)) {}

UsbDeviceHandleUsbfs::BlockingTaskHelper::~BlockingTaskHelper() {
UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::~BlockingTaskRunnerHelper() {
DCHECK(sequence_checker_.CalledOnValidSequence());
}

void UsbDeviceHandleUsbfs::BlockingTaskHelper::Start() {
void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::Start() {
sequence_checker_.DetachFromSequence();
DCHECK(sequence_checker_.CalledOnValidSequence());

// Linux indicates that URBs are available to reap by marking the file
// descriptor writable.
watch_controller_ = base::FileDescriptorWatcher::WatchWritable(
fd_.get(),
base::BindRepeating(&BlockingTaskHelper::OnFileCanWriteWithoutBlocking,
base::Unretained(this)));
fd_.get(), base::BindRepeating(
&BlockingTaskRunnerHelper::OnFileCanWriteWithoutBlocking,
base::Unretained(this)));
}

void UsbDeviceHandleUsbfs::BlockingTaskHelper::ReleaseFileDescriptor() {
void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::ReleaseFileDescriptor() {
// This method intentionally leaks the file descriptor.
DCHECK(sequence_checker_.CalledOnValidSequence());
watch_controller_.reset();
ignore_result(fd_.release());
}

void UsbDeviceHandleUsbfs::BlockingTaskHelper::SetConfiguration(
void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::SetConfiguration(
int configuration_value,
ResultCallback callback) {
DCHECK(sequence_checker_.CalledOnValidSequence());
Expand All @@ -243,7 +244,7 @@ void UsbDeviceHandleUsbfs::BlockingTaskHelper::SetConfiguration(
std::move(callback)));
}

void UsbDeviceHandleUsbfs::BlockingTaskHelper::ReleaseInterface(
void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::ReleaseInterface(
int interface_number,
ResultCallback callback) {
DCHECK(sequence_checker_.CalledOnValidSequence());
Expand All @@ -264,7 +265,7 @@ void UsbDeviceHandleUsbfs::BlockingTaskHelper::ReleaseInterface(
}
}

void UsbDeviceHandleUsbfs::BlockingTaskHelper::SetInterface(
void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::SetInterface(
int interface_number,
int alternate_setting,
ResultCallback callback) {
Expand All @@ -285,7 +286,7 @@ void UsbDeviceHandleUsbfs::BlockingTaskHelper::SetInterface(
base::BindOnce(std::move(callback), rc == 0));
}

void UsbDeviceHandleUsbfs::BlockingTaskHelper::ResetDevice(
void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::ResetDevice(
ResultCallback callback) {
DCHECK(sequence_checker_.CalledOnValidSequence());

Expand All @@ -301,7 +302,7 @@ void UsbDeviceHandleUsbfs::BlockingTaskHelper::ResetDevice(
base::BindOnce(std::move(callback), rc == 0));
}

void UsbDeviceHandleUsbfs::BlockingTaskHelper::ClearHalt(
void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::ClearHalt(
uint8_t endpoint_address,
ResultCallback callback) {
DCHECK(sequence_checker_.CalledOnValidSequence());
Expand All @@ -318,7 +319,8 @@ void UsbDeviceHandleUsbfs::BlockingTaskHelper::ClearHalt(
base::BindOnce(std::move(callback), rc == 0));
}

void UsbDeviceHandleUsbfs::BlockingTaskHelper::DiscardUrb(Transfer* transfer) {
void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::DiscardUrb(
Transfer* transfer) {
DCHECK(sequence_checker_.CalledOnValidSequence());

base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
Expand All @@ -330,7 +332,8 @@ void UsbDeviceHandleUsbfs::BlockingTaskHelper::DiscardUrb(Transfer* transfer) {
device_handle_, transfer));
}

void UsbDeviceHandleUsbfs::BlockingTaskHelper::OnFileCanWriteWithoutBlocking() {
void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::
OnFileCanWriteWithoutBlocking() {
DCHECK(sequence_checker_.CalledOnValidSequence());

const size_t MAX_URBS_PER_EVENT = 10;
Expand Down Expand Up @@ -418,9 +421,10 @@ UsbDeviceHandleUsbfs::UsbDeviceHandleUsbfs(
DCHECK(fd.is_valid());
DCHECK(blocking_task_runner_);

helper_.reset(new BlockingTaskHelper(std::move(fd), this, task_runner_));
helper_.reset(
new BlockingTaskRunnerHelper(std::move(fd), this, task_runner_));
blocking_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&BlockingTaskHelper::Start,
FROM_HERE, base::BindOnce(&BlockingTaskRunnerHelper::Start,
base::Unretained(helper_.get())));
}

Expand Down Expand Up @@ -466,7 +470,7 @@ void UsbDeviceHandleUsbfs::SetConfiguration(int configuration_value,
blocking_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(
&UsbDeviceHandleUsbfs::BlockingTaskHelper::SetConfiguration,
&UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::SetConfiguration,
base::Unretained(helper_.get()), configuration_value,
std::move(callback)));
}
Expand Down Expand Up @@ -514,7 +518,7 @@ void UsbDeviceHandleUsbfs::ReleaseInterface(int interface_number,
blocking_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(
&UsbDeviceHandleUsbfs::BlockingTaskHelper::ReleaseInterface,
&UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::ReleaseInterface,
base::Unretained(helper_.get()), interface_number,
std::move(callback)));
}
Expand All @@ -535,9 +539,10 @@ void UsbDeviceHandleUsbfs::SetInterfaceAlternateSetting(
// to block.
blocking_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&UsbDeviceHandleUsbfs::BlockingTaskHelper::SetInterface,
base::Unretained(helper_.get()), interface_number,
alternate_setting, std::move(callback)));
base::BindOnce(
&UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::SetInterface,
base::Unretained(helper_.get()), interface_number, alternate_setting,
std::move(callback)));
}

void UsbDeviceHandleUsbfs::ResetDevice(ResultCallback callback) {
Expand All @@ -553,8 +558,9 @@ void UsbDeviceHandleUsbfs::ResetDevice(ResultCallback callback) {
// is okay to block.
blocking_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&UsbDeviceHandleUsbfs::BlockingTaskHelper::ResetDevice,
base::Unretained(helper_.get()), std::move(callback)));
base::BindOnce(
&UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::ResetDevice,
base::Unretained(helper_.get()), std::move(callback)));
}

void UsbDeviceHandleUsbfs::ClearHalt(uint8_t endpoint_address,
Expand All @@ -571,7 +577,7 @@ void UsbDeviceHandleUsbfs::ClearHalt(uint8_t endpoint_address,
// to block.
blocking_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&UsbDeviceHandleUsbfs::BlockingTaskHelper::ClearHalt,
base::BindOnce(&UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::ClearHalt,
base::Unretained(helper_.get()), endpoint_address,
std::move(callback)));
}
Expand Down Expand Up @@ -931,8 +937,9 @@ void UsbDeviceHandleUsbfs::CancelTransfer(Transfer* transfer,

blocking_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&UsbDeviceHandleUsbfs::BlockingTaskHelper::DiscardUrb,
base::Unretained(helper_.get()), transfer));
base::BindOnce(
&UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::DiscardUrb,
base::Unretained(helper_.get()), transfer));

// Cancelling |timeout_closure| and running completion callbacks may free
// |this| so these operations must be performed at the end of this function.
Expand Down
4 changes: 2 additions & 2 deletions device/usb/usb_device_handle_usbfs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class UsbDeviceHandleUsbfs : public UsbDeviceHandle {
virtual void CloseBlocking();

private:
class BlockingTaskHelper;
class BlockingTaskRunnerHelper;
struct Transfer;
struct InterfaceInfo {
uint8_t alternate_setting;
Expand Down Expand Up @@ -138,7 +138,7 @@ class UsbDeviceHandleUsbfs : public UsbDeviceHandle {

// Helper object exists on the blocking task thread and all calls to it and
// its destruction must be posted there.
std::unique_ptr<BlockingTaskHelper> helper_;
std::unique_ptr<BlockingTaskRunnerHelper> helper_;

std::list<std::unique_ptr<Transfer>> transfers_;
base::SequenceChecker sequence_checker_;
Expand Down
23 changes: 12 additions & 11 deletions device/usb/usb_service_linux.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ void OnDeviceOpenedToReadDescriptors(

} // namespace

class UsbServiceLinux::BlockingTaskHelper : public UdevWatcher::Observer {
class UsbServiceLinux::BlockingTaskRunnerHelper : public UdevWatcher::Observer {
public:
BlockingTaskHelper(base::WeakPtr<UsbServiceLinux> service);
~BlockingTaskHelper() override;
BlockingTaskRunnerHelper(base::WeakPtr<UsbServiceLinux> service);
~BlockingTaskRunnerHelper() override;

void Start();

Expand All @@ -84,22 +84,22 @@ class UsbServiceLinux::BlockingTaskHelper : public UdevWatcher::Observer {

base::SequenceChecker sequence_checker_;

DISALLOW_COPY_AND_ASSIGN(BlockingTaskHelper);
DISALLOW_COPY_AND_ASSIGN(BlockingTaskRunnerHelper);
};

UsbServiceLinux::BlockingTaskHelper::BlockingTaskHelper(
UsbServiceLinux::BlockingTaskRunnerHelper::BlockingTaskRunnerHelper(
base::WeakPtr<UsbServiceLinux> service)
: service_(service), task_runner_(base::SequencedTaskRunnerHandle::Get()) {
// Detaches from the sequence on which this object was created. It will be
// bound to its owning sequence when Start() is called.
sequence_checker_.DetachFromSequence();
}

UsbServiceLinux::BlockingTaskHelper::~BlockingTaskHelper() {
UsbServiceLinux::BlockingTaskRunnerHelper::~BlockingTaskRunnerHelper() {
DCHECK(sequence_checker_.CalledOnValidSequence());
}

void UsbServiceLinux::BlockingTaskHelper::Start() {
void UsbServiceLinux::BlockingTaskRunnerHelper::Start() {
DCHECK(sequence_checker_.CalledOnValidSequence());
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
base::BlockingType::MAY_BLOCK);
Expand All @@ -114,7 +114,7 @@ void UsbServiceLinux::BlockingTaskHelper::Start() {
FROM_HERE, base::BindOnce(&UsbServiceLinux::HelperStarted, service_));
}

void UsbServiceLinux::BlockingTaskHelper::OnDeviceAdded(
void UsbServiceLinux::BlockingTaskRunnerHelper::OnDeviceAdded(
ScopedUdevDevicePtr device) {
DCHECK(sequence_checker_.CalledOnValidSequence());

Expand Down Expand Up @@ -187,7 +187,7 @@ void UsbServiceLinux::BlockingTaskHelper::OnDeviceAdded(
active_configuration, bus_number, port_number));
}

void UsbServiceLinux::BlockingTaskHelper::OnDeviceRemoved(
void UsbServiceLinux::BlockingTaskRunnerHelper::OnDeviceRemoved(
ScopedUdevDevicePtr device) {
DCHECK(sequence_checker_.CalledOnValidSequence());
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
Expand All @@ -205,9 +205,10 @@ UsbServiceLinux::UsbServiceLinux()
: UsbService(),
blocking_task_runner_(CreateBlockingTaskRunner()),
weak_factory_(this) {
helper_ = std::make_unique<BlockingTaskHelper>(weak_factory_.GetWeakPtr());
helper_ =
std::make_unique<BlockingTaskRunnerHelper>(weak_factory_.GetWeakPtr());
blocking_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&BlockingTaskHelper::Start,
FROM_HERE, base::BindOnce(&BlockingTaskRunnerHelper::Start,
base::Unretained(helper_.get())));
}

Expand Down
4 changes: 2 additions & 2 deletions device/usb/usb_service_linux.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class UsbServiceLinux : public UsbService {
using DeviceMap =
std::unordered_map<std::string, scoped_refptr<UsbDeviceLinux>>;

class BlockingTaskHelper;
class BlockingTaskRunnerHelper;

void OnDeviceAdded(const std::string& device_path,
const UsbDeviceDescriptor& descriptor,
Expand All @@ -57,7 +57,7 @@ class UsbServiceLinux : public UsbService {
std::list<GetDevicesCallback> enumeration_callbacks_;

scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
std::unique_ptr<BlockingTaskHelper> helper_;
std::unique_ptr<BlockingTaskRunnerHelper> helper_;
DeviceMap devices_by_path_;

base::WeakPtrFactory<UsbServiceLinux> weak_factory_;
Expand Down
13 changes: 7 additions & 6 deletions device/usb/usb_service_win.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,12 @@ bool GetHubDevicePath(const std::string& instance_id,

} // namespace

class UsbServiceWin::BlockingTaskHelper {
class UsbServiceWin::BlockingTaskRunnerHelper {
public:
explicit BlockingTaskHelper(base::WeakPtr<UsbServiceWin> service)
explicit BlockingTaskRunnerHelper(base::WeakPtr<UsbServiceWin> service)
: service_task_runner_(base::ThreadTaskRunnerHandle::Get()),
service_(service) {}
~BlockingTaskHelper() {}
~BlockingTaskRunnerHelper() {}

void EnumerateDevices() {
ScopedDevInfo dev_info(
Expand Down Expand Up @@ -297,9 +297,10 @@ UsbServiceWin::UsbServiceWin()
if (device_monitor)
device_observer_.Add(device_monitor);

helper_ = std::make_unique<BlockingTaskHelper>(weak_factory_.GetWeakPtr());
helper_ =
std::make_unique<BlockingTaskRunnerHelper>(weak_factory_.GetWeakPtr());
blocking_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&BlockingTaskHelper::EnumerateDevices,
FROM_HERE, base::BindOnce(&BlockingTaskRunnerHelper::EnumerateDevices,
base::Unretained(helper_.get())));
}

Expand All @@ -318,7 +319,7 @@ void UsbServiceWin::GetDevices(const GetDevicesCallback& callback) {
void UsbServiceWin::OnDeviceAdded(const GUID& class_guid,
const std::string& device_path) {
blocking_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&BlockingTaskHelper::EnumerateDevicePath,
FROM_HERE, base::BindOnce(&BlockingTaskRunnerHelper::EnumerateDevicePath,
base::Unretained(helper_.get()), device_path));
}

Expand Down
4 changes: 2 additions & 2 deletions device/usb/usb_service_win.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class UsbServiceWin : public DeviceMonitorWin::Observer, public UsbService {
~UsbServiceWin() override;

private:
class BlockingTaskHelper;
class BlockingTaskRunnerHelper;

// device::UsbService implementation
void GetDevices(const GetDevicesCallback& callback) override;
Expand Down Expand Up @@ -54,7 +54,7 @@ class UsbServiceWin : public DeviceMonitorWin::Observer, public UsbService {
std::list<GetDevicesCallback> enumeration_callbacks_;

scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
std::unique_ptr<BlockingTaskHelper> helper_;
std::unique_ptr<BlockingTaskRunnerHelper> helper_;
std::unordered_map<std::string, scoped_refptr<UsbDeviceWin>> devices_by_path_;

ScopedObserver<DeviceMonitorWin, DeviceMonitorWin::Observer> device_observer_;
Expand Down
Loading

0 comments on commit 28c1179

Please sign in to comment.