From 56868deb02c83ade521fa482e2d03b9319162ae9 Mon Sep 17 00:00:00 2001 From: reillyg Date: Wed, 3 Dec 2014 16:26:27 -0800 Subject: Migrate HidServiceLinux and HidConnectionLinux to BrowserThread::UI. This patch is a follow-up to https://crrev.com/e8fa00efd0965a7eb5816a that moves the HidService and HidConnection implementations on Linux from the browser's FILE thread to the UI thread. This is being done because the HID APIs on platforms other than Linux are more natural to use on the UI thread. The users of these objects are also usually on the UI thread. (For example, the extension API bindings.) BUG=422540 Review URL: https://codereview.chromium.org/771393002 Cr-Commit-Position: refs/heads/master@{#306729} --- device/hid/hid_connection_linux.cc | 274 +++++++++++++++++++++++-------------- 1 file changed, 174 insertions(+), 100 deletions(-) (limited to 'device/hid/hid_connection_linux.cc') diff --git a/device/hid/hid_connection_linux.cc b/device/hid/hid_connection_linux.cc index bec4de7..29e5cce 100644 --- a/device/hid/hid_connection_linux.cc +++ b/device/hid/hid_connection_linux.cc @@ -5,15 +5,17 @@ #include "device/hid/hid_connection_linux.h" #include -#include #include #include #include +#include "base/bind.h" #include "base/files/file_path.h" #include "base/message_loop/message_loop.h" +#include "base/message_loop/message_pump_libevent.h" #include "base/posix/eintr_wrapper.h" +#include "base/thread_task_runner_handle.h" #include "base/threading/thread_restrictions.h" #include "base/tuple.h" #include "device/hid/hid_service.h" @@ -28,54 +30,110 @@ namespace device { -HidConnectionLinux::HidConnectionLinux(HidDeviceInfo device_info, - std::string dev_node) - : HidConnection(device_info) { - int flags = base::File::FLAG_OPEN | - base::File::FLAG_READ | - base::File::FLAG_WRITE; - - base::File device_file(base::FilePath(dev_node), flags); - if (!device_file.IsValid()) { - base::File::Error file_error = device_file.error_details(); +class HidConnectionLinux::Helper : public base::MessagePumpLibevent::Watcher { + public: + Helper(base::PlatformFile platform_file, + const HidDeviceInfo& device_info, + base::WeakPtr connection, + scoped_refptr task_runner) + : platform_file_(platform_file), + connection_(connection), + task_runner_(task_runner) { + if (!base::MessageLoopForIO::current()->WatchFileDescriptor( + platform_file_, true, base::MessageLoopForIO::WATCH_READ, + &file_watcher_, this)) { + LOG(ERROR) << "Failed to start watching device file."; + } - if (file_error == base::File::FILE_ERROR_ACCESS_DENIED) { - VLOG(1) << "Access denied opening device read-write, trying read-only."; + // Report buffers must always have room for the report ID. + report_buffer_size_ = device_info.max_input_report_size + 1; + has_report_id_ = device_info.has_report_id; + } - flags = base::File::FLAG_OPEN | base::File::FLAG_READ; + virtual ~Helper() { DCHECK(thread_checker_.CalledOnValidThread()); } + + private: + // base::MessagePumpLibevent::Watcher implementation. + void OnFileCanReadWithoutBlocking(int fd) override { + DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK_EQ(fd, platform_file_); + + scoped_refptr buffer(new net::IOBuffer(report_buffer_size_)); + char* data = buffer->data(); + size_t length = report_buffer_size_; + if (!has_report_id_) { + // Linux will not prefix the buffer with a report ID if report IDs are not + // used by the device. Prefix the buffer with 0. + *data++ = 0; + length--; + } - device_file = base::File(base::FilePath(dev_node), flags); + ssize_t bytes_read = HANDLE_EINTR(read(platform_file_, data, length)); + if (bytes_read < 0) { + if (errno == EAGAIN) { + return; + } + VPLOG(1) << "Read failed"; + return; } - } - if (!device_file.IsValid()) { - LOG(ERROR) << "Failed to open '" << dev_node << "': " - << base::File::ErrorToString(device_file.error_details()); - return; + if (!has_report_id_) { + // Behave as if the byte prefixed above as the the report ID was read. + bytes_read++; + } + + task_runner_->PostTask(FROM_HERE, + base::Bind(&HidConnectionLinux::ProcessInputReport, + connection_, buffer, bytes_read)); } - if (fcntl(device_file.GetPlatformFile(), F_SETFL, - fcntl(device_file.GetPlatformFile(), F_GETFL) | O_NONBLOCK)) { - PLOG(ERROR) << "Failed to set non-blocking flag to device file"; - return; + void OnFileCanWriteWithoutBlocking(int fd) override { + NOTREACHED(); // Only listening for reads. } + + base::ThreadChecker thread_checker_; + base::PlatformFile platform_file_; + size_t report_buffer_size_; + bool has_report_id_; + base::WeakPtr connection_; + scoped_refptr task_runner_; + base::MessagePumpLibevent::FileDescriptorWatcher file_watcher_; +}; + +HidConnectionLinux::HidConnectionLinux( + HidDeviceInfo device_info, + base::File device_file, + scoped_refptr file_task_runner) + : HidConnection(device_info), + file_task_runner_(file_task_runner), + weak_factory_(this) { + task_runner_ = base::ThreadTaskRunnerHandle::Get(); device_file_ = device_file.Pass(); - if (!base::MessageLoopForIO::current()->WatchFileDescriptor( - device_file_.GetPlatformFile(), - true, - base::MessageLoopForIO::WATCH_READ_WRITE, - &device_file_watcher_, - this)) { - LOG(ERROR) << "Failed to start watching device file."; - } + // The helper is passed a weak pointer to this connection so that it can be + // cleaned up after the connection is closed. The weak pointer must be + // constructed on this thread. + file_task_runner_->PostTask(FROM_HERE, + base::Bind(&HidConnectionLinux::StartHelper, this, + weak_factory_.GetWeakPtr())); } HidConnectionLinux::~HidConnectionLinux() { } void HidConnectionLinux::PlatformClose() { - Disconnect(); - Flush(); + // By closing the device file on the FILE thread (1) the requirement that + // base::File::Close is called on a thread where I/O is allowed is satisfied + // and (2) any tasks posted to this task runner that refer to this file will + // complete before it is closed. + file_task_runner_->DeleteSoon(FROM_HERE, helper_.release()); + file_task_runner_->PostTask(FROM_HERE, + base::Bind(&HidConnectionLinux::CloseDevice, + base::Passed(&device_file_))); + + while (!pending_reads_.empty()) { + pending_reads_.front().callback.Run(false, NULL, 0); + pending_reads_.pop(); + } } void HidConnectionLinux::PlatformRead(const ReadCallback& callback) { @@ -90,19 +148,13 @@ void HidConnectionLinux::PlatformWrite(scoped_refptr buffer, const WriteCallback& callback) { // Linux expects the first byte of the buffer to always be a report ID so the // buffer can be used directly. - const ssize_t bytes_written = - HANDLE_EINTR(write(device_file_.GetPlatformFile(), buffer->data(), size)); - if (bytes_written < 0) { - VPLOG(1) << "Write failed"; - Disconnect(); - callback.Run(false); - } else { - if (static_cast(bytes_written) != size) { - LOG(WARNING) << "Incomplete HID write: " << bytes_written - << " != " << size; - } - callback.Run(true); - } + file_task_runner_->PostTask( + FROM_HERE, + base::Bind(&HidConnectionLinux::BlockingWrite, + device_file_.GetPlatformFile(), buffer, size, + base::Bind(&HidConnectionLinux::FinishWrite, + weak_factory_.GetWeakPtr(), size, callback), + task_runner_)); } void HidConnectionLinux::PlatformGetFeatureReport( @@ -115,9 +167,51 @@ void HidConnectionLinux::PlatformGetFeatureReport( new net::IOBufferWithSize(device_info().max_feature_report_size + 1)); buffer->data()[0] = report_id; - int result = ioctl(device_file_.GetPlatformFile(), - HIDIOCGFEATURE(buffer->size()), - buffer->data()); + file_task_runner_->PostTask( + FROM_HERE, + base::Bind( + &HidConnectionLinux::BlockingIoctl, device_file_.GetPlatformFile(), + HIDIOCGFEATURE(buffer->size()), buffer, + base::Bind(&HidConnectionLinux::FinishGetFeatureReport, + weak_factory_.GetWeakPtr(), report_id, buffer, callback), + task_runner_)); +} + +void HidConnectionLinux::PlatformSendFeatureReport( + scoped_refptr buffer, + size_t size, + const WriteCallback& callback) { + // Linux expects the first byte of the buffer to always be a report ID so the + // buffer can be used directly. + file_task_runner_->PostTask( + FROM_HERE, + base::Bind(&HidConnectionLinux::BlockingIoctl, + device_file_.GetPlatformFile(), HIDIOCSFEATURE(size), buffer, + base::Bind(&HidConnectionLinux::FinishSendFeatureReport, + weak_factory_.GetWeakPtr(), callback), + task_runner_)); +} + +void HidConnectionLinux::FinishWrite(size_t expected_size, + const WriteCallback& callback, + ssize_t result) { + if (result < 0) { + VPLOG(1) << "Write failed"; + callback.Run(false); + } else { + if (static_cast(result) != expected_size) { + LOG(WARNING) << "Incomplete HID write: " << result + << " != " << expected_size; + } + callback.Run(true); + } +} + +void HidConnectionLinux::FinishGetFeatureReport( + uint8_t report_id, + scoped_refptr buffer, + const ReadCallback& callback, + int result) { if (result < 0) { VPLOG(1) << "Failed to get feature report"; callback.Run(false, NULL, 0); @@ -134,14 +228,8 @@ void HidConnectionLinux::PlatformGetFeatureReport( } } -void HidConnectionLinux::PlatformSendFeatureReport( - scoped_refptr buffer, - size_t size, - const WriteCallback& callback) { - // Linux expects the first byte of the buffer to always be a report ID so the - // buffer can be used directly. - int result = ioctl( - device_file_.GetPlatformFile(), HIDIOCSFEATURE(size), buffer->data()); +void HidConnectionLinux::FinishSendFeatureReport(const WriteCallback& callback, + int result) { if (result < 0) { VPLOG(1) << "Failed to send feature report"; callback.Run(false); @@ -150,55 +238,41 @@ void HidConnectionLinux::PlatformSendFeatureReport( } } -void HidConnectionLinux::OnFileCanReadWithoutBlocking(int fd) { - DCHECK(thread_checker().CalledOnValidThread()); - DCHECK_EQ(fd, device_file_.GetPlatformFile()); - - size_t expected_report_size = device_info().max_input_report_size + 1; - scoped_refptr buffer(new net::IOBuffer(expected_report_size)); - char* data = buffer->data(); - if (!device_info().has_report_id) { - // Linux will not prefix the buffer with a report ID if they are not used - // by the device. - data[0] = 0; - data++; - expected_report_size--; - } - - ssize_t bytes_read = HANDLE_EINTR( - read(device_file_.GetPlatformFile(), data, expected_report_size)); - if (bytes_read < 0) { - if (errno == EAGAIN) { - return; - } - VPLOG(1) << "Read failed"; - Disconnect(); - return; - } - if (!device_info().has_report_id) { - // Include the byte prepended earlier. - bytes_read++; - } +void HidConnectionLinux::StartHelper( + base::WeakPtr weak_ptr) { + base::ThreadRestrictions::AssertIOAllowed(); - ProcessInputReport(buffer, bytes_read); + helper_.reset(new Helper(device_file_.GetPlatformFile(), device_info(), + weak_ptr, task_runner_)); } -void HidConnectionLinux::OnFileCanWriteWithoutBlocking(int fd) { +// static +void HidConnectionLinux::BlockingWrite( + base::PlatformFile platform_file, + scoped_refptr buffer, + size_t size, + const InternalWriteCallback& callback, + scoped_refptr task_runner) { + base::ThreadRestrictions::AssertIOAllowed(); + ssize_t result = HANDLE_EINTR(write(platform_file, buffer->data(), size)); + task_runner->PostTask(FROM_HERE, base::Bind(callback, result)); } -void HidConnectionLinux::Disconnect() { - DCHECK(thread_checker().CalledOnValidThread()); - device_file_watcher_.StopWatchingFileDescriptor(); - device_file_.Close(); - - Flush(); +// static +void HidConnectionLinux::BlockingIoctl( + base::PlatformFile platform_file, + int request, + scoped_refptr buffer, + const IoctlCallback& callback, + scoped_refptr task_runner) { + base::ThreadRestrictions::AssertIOAllowed(); + int result = ioctl(platform_file, request, buffer->data()); + task_runner->PostTask(FROM_HERE, base::Bind(callback, result)); } -void HidConnectionLinux::Flush() { - while (!pending_reads_.empty()) { - pending_reads_.front().callback.Run(false, NULL, 0); - pending_reads_.pop(); - } +// static +void HidConnectionLinux::CloseDevice(base::File device_file) { + device_file.Close(); } void HidConnectionLinux::ProcessInputReport(scoped_refptr buffer, -- cgit v1.1