diff options
author | reillyg <reillyg@chromium.org> | 2014-12-03 16:26:27 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-12-04 00:27:20 +0000 |
commit | 56868deb02c83ade521fa482e2d03b9319162ae9 (patch) | |
tree | 7de3aa7d1acb084f9902bbcd943733fe7df1329a | |
parent | 255cf5511b0932fd3e500089ca3d53bd7e5c13c9 (diff) | |
download | chromium_src-56868deb02c83ade521fa482e2d03b9319162ae9.zip chromium_src-56868deb02c83ade521fa482e2d03b9319162ae9.tar.gz chromium_src-56868deb02c83ade521fa482e2d03b9319162ae9.tar.bz2 |
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}
-rw-r--r-- | chrome/browser/chrome_device_client.cc | 4 | ||||
-rw-r--r-- | device/hid/hid_connection_linux.cc | 274 | ||||
-rw-r--r-- | device/hid/hid_connection_linux.h | 62 | ||||
-rw-r--r-- | device/hid/hid_connection_unittest.cc | 4 | ||||
-rw-r--r-- | device/hid/hid_device_info.h | 4 | ||||
-rw-r--r-- | device/hid/hid_service.cc | 9 | ||||
-rw-r--r-- | device/hid/hid_service.h | 3 | ||||
-rw-r--r-- | device/hid/hid_service_linux.cc | 379 | ||||
-rw-r--r-- | device/hid/hid_service_linux.h | 38 | ||||
-rw-r--r-- | device/hid/hid_service_unittest.cc | 1 | ||||
-rw-r--r-- | extensions/browser/api/hid/hid_api.cc | 9 | ||||
-rw-r--r-- | extensions/browser/api/hid/hid_apitest.cc | 8 | ||||
-rw-r--r-- | extensions/browser/api/hid/hid_connection_resource.h | 9 | ||||
-rw-r--r-- | extensions/browser/api/hid/hid_device_manager.cc | 9 | ||||
-rw-r--r-- | extensions/shell/browser/shell_device_client.cc | 4 |
15 files changed, 510 insertions, 307 deletions
diff --git a/chrome/browser/chrome_device_client.cc b/chrome/browser/chrome_device_client.cc index 2e23634..f93fb90 100644 --- a/chrome/browser/chrome_device_client.cc +++ b/chrome/browser/chrome_device_client.cc @@ -22,7 +22,5 @@ device::UsbService* ChromeDeviceClient::GetUsbService() { device::HidService* ChromeDeviceClient::GetHidService() { return device::HidService::GetInstance( content::BrowserThread::GetMessageLoopProxyForThread( - content::BrowserThread::FILE), - content::BrowserThread::GetMessageLoopProxyForThread( - content::BrowserThread::UI)); + content::BrowserThread::FILE)); } 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 <errno.h> -#include <fcntl.h> #include <linux/hidraw.h> #include <sys/ioctl.h> #include <string> +#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<HidConnectionLinux> connection, + scoped_refptr<base::SingleThreadTaskRunner> 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<net::IOBuffer> 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<HidConnectionLinux> connection_; + scoped_refptr<base::SingleThreadTaskRunner> task_runner_; + base::MessagePumpLibevent::FileDescriptorWatcher file_watcher_; +}; + +HidConnectionLinux::HidConnectionLinux( + HidDeviceInfo device_info, + base::File device_file, + scoped_refptr<base::SingleThreadTaskRunner> 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<net::IOBuffer> 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<size_t>(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<net::IOBuffer> 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<size_t>(result) != expected_size) { + LOG(WARNING) << "Incomplete HID write: " << result + << " != " << expected_size; + } + callback.Run(true); + } +} + +void HidConnectionLinux::FinishGetFeatureReport( + uint8_t report_id, + scoped_refptr<net::IOBuffer> 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<net::IOBuffer> 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<net::IOBuffer> 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<HidConnectionLinux> 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<net::IOBuffer> buffer, + size_t size, + const InternalWriteCallback& callback, + scoped_refptr<base::SingleThreadTaskRunner> 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<net::IOBuffer> buffer, + const IoctlCallback& callback, + scoped_refptr<base::SingleThreadTaskRunner> 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<net::IOBuffer> buffer, diff --git a/device/hid/hid_connection_linux.h b/device/hid/hid_connection_linux.h index 4850d6a..d6befe4 100644 --- a/device/hid/hid_connection_linux.h +++ b/device/hid/hid_connection_linux.h @@ -8,18 +8,30 @@ #include <queue> #include "base/files/file.h" -#include "base/message_loop/message_pump_libevent.h" +#include "base/memory/weak_ptr.h" #include "device/hid/hid_connection.h" +namespace base { +class SingleThreadTaskRunner; +} + namespace device { -class HidConnectionLinux : public HidConnection, - public base::MessagePumpLibevent::Watcher { +class HidConnectionLinux : public HidConnection { public: - HidConnectionLinux(HidDeviceInfo device_info, std::string dev_node); + HidConnectionLinux( + HidDeviceInfo device_info, + base::File device_file, + scoped_refptr<base::SingleThreadTaskRunner> file_thread_runner); private: + class Helper; + friend class Helper; friend class base::RefCountedThreadSafe<HidConnectionLinux>; + + typedef base::Callback<void(ssize_t)> InternalWriteCallback; + typedef base::Callback<void(int)> IoctlCallback; + ~HidConnectionLinux() override; // HidConnection implementation. @@ -34,22 +46,52 @@ class HidConnectionLinux : public HidConnection, size_t size, const WriteCallback& callback) override; - // base::MessagePumpLibevent::Watcher implementation. - void OnFileCanReadWithoutBlocking(int fd) override; - void OnFileCanWriteWithoutBlocking(int fd) override; + // Callbacks for blocking operations run on the FILE thread. + void FinishWrite(size_t expected_size, + const WriteCallback& callback, + ssize_t result); + void FinishGetFeatureReport(uint8_t report_id, + scoped_refptr<net::IOBuffer> buffer, + const ReadCallback& callback, + int result); + void FinishSendFeatureReport(const WriteCallback& callback, int result); + + // Starts the FileDescriptorWatcher that reads input events from the device. + // Must be called on a thread that has a base::MessageLoopForIO. + void StartHelper(base::WeakPtr<HidConnectionLinux> weak_ptr); - void Disconnect(); + // Writes to the device. This operation may block. + static void BlockingWrite( + base::PlatformFile platform_file, + scoped_refptr<net::IOBuffer> buffer, + size_t size, + const InternalWriteCallback& callback, + scoped_refptr<base::SingleThreadTaskRunner> task_runner); + // Performs an ioctl on the device. This operation may block. + static void BlockingIoctl( + base::PlatformFile platform_file, + int request, + scoped_refptr<net::IOBuffer> buffer, + const IoctlCallback& callback, + scoped_refptr<base::SingleThreadTaskRunner> task_runner); + + // Closes the device file descriptor. Must be called on the FILE thread. + static void CloseDevice(base::File device_file); - void Flush(); void ProcessInputReport(scoped_refptr<net::IOBuffer> buffer, size_t size); void ProcessReadQueue(); base::File device_file_; - base::MessagePumpLibevent::FileDescriptorWatcher device_file_watcher_; + scoped_ptr<Helper> helper_; + + scoped_refptr<base::SingleThreadTaskRunner> task_runner_; + scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_; std::queue<PendingHidReport> pending_reports_; std::queue<PendingHidRead> pending_reads_; + base::WeakPtrFactory<HidConnectionLinux> weak_factory_; + DISALLOW_COPY_AND_ASSIGN(HidConnectionLinux); }; diff --git a/device/hid/hid_connection_unittest.cc b/device/hid/hid_connection_unittest.cc index a8530ee..fbacb52 100644 --- a/device/hid/hid_connection_unittest.cc +++ b/device/hid/hid_connection_unittest.cc @@ -98,9 +98,7 @@ class HidConnectionTest : public testing::Test { if (!UsbTestGadget::IsTestEnabled()) return; message_loop_.reset(new base::MessageLoopForIO()); - service_ = HidService::GetInstance( - message_loop_->message_loop_proxy(), - message_loop_->message_loop_proxy()); + service_ = HidService::GetInstance(message_loop_->message_loop_proxy()); ASSERT_TRUE(service_); test_gadget_ = UsbTestGadget::Claim(); diff --git a/device/hid/hid_device_info.h b/device/hid/hid_device_info.h index bb1cc04..917a129 100644 --- a/device/hid/hid_device_info.h +++ b/device/hid/hid_device_info.h @@ -39,6 +39,10 @@ struct HidDeviceInfo { size_t max_input_report_size; size_t max_output_report_size; size_t max_feature_report_size; + +#if defined(OS_LINUX) + std::string device_node; +#endif }; } // namespace device diff --git a/device/hid/hid_service.cc b/device/hid/hid_service.cc index 071b396..78273ae6 100644 --- a/device/hid/hid_service.cc +++ b/device/hid/hid_service.cc @@ -4,9 +4,7 @@ #include "device/hid/hid_service.h" -#include "base/lazy_instance.h" #include "base/logging.h" -#include "base/memory/scoped_vector.h" #include "base/message_loop/message_loop.h" #include "base/stl_util.h" @@ -45,11 +43,10 @@ class HidService::Destroyer : public base::MessageLoop::DestructionObserver { }; HidService* HidService::GetInstance( - scoped_refptr<base::SingleThreadTaskRunner> file_task_runner, - scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) { + scoped_refptr<base::SingleThreadTaskRunner> file_task_runner) { if (g_service == NULL) { #if defined(OS_LINUX) && defined(USE_UDEV) - g_service = new HidServiceLinux(ui_task_runner); + g_service = new HidServiceLinux(file_task_runner); #elif defined(OS_MACOSX) g_service = new HidServiceMac(file_task_runner); #elif defined(OS_WIN) @@ -87,6 +84,7 @@ void HidService::GetDevices(std::vector<HidDeviceInfo>* devices) { // Fills in the device info struct of the given device_id. bool HidService::GetDeviceInfo(const HidDeviceId& device_id, HidDeviceInfo* info) const { + DCHECK(thread_checker_.CalledOnValidThread()); DeviceMap::const_iterator it = devices_.find(device_id); if (it == devices_.end()) return false; @@ -95,7 +93,6 @@ bool HidService::GetDeviceInfo(const HidDeviceId& device_id, } HidService::HidService() { - DCHECK(thread_checker_.CalledOnValidThread()); } void HidService::AddDevice(const HidDeviceInfo& info) { diff --git a/device/hid/hid_service.h b/device/hid/hid_service.h index c01fcbc..2df2864 100644 --- a/device/hid/hid_service.h +++ b/device/hid/hid_service.h @@ -24,8 +24,7 @@ class HidService { ConnectCallback; static HidService* GetInstance( - scoped_refptr<base::SingleThreadTaskRunner> file_task_runner, - scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner); + scoped_refptr<base::SingleThreadTaskRunner> file_task_runner); static void SetInstanceForTest(HidService* instance); diff --git a/device/hid/hid_service_linux.cc b/device/hid/hid_service_linux.cc index 45688f5..1319331 100644 --- a/device/hid/hid_service_linux.cc +++ b/device/hid/hid_service_linux.cc @@ -4,19 +4,21 @@ #include "device/hid/hid_service_linux.h" +#include <fcntl.h> #include <string> #include "base/bind.h" #include "base/files/file.h" #include "base/files/file_path.h" #include "base/files/file_util.h" +#include "base/location.h" #include "base/logging.h" -#include "base/stl_util.h" +#include "base/scoped_observer.h" #include "base/strings/string_number_conversions.h" -#include "base/strings/string_piece.h" #include "base/strings/string_split.h" #include "base/thread_task_runner_handle.h" #include "base/threading/thread_restrictions.h" +#include "device/hid/device_monitor_linux.h" #include "device/hid/hid_connection_linux.h" #include "device/hid/hid_device_info.h" #include "device/hid/hid_report_descriptor.h" @@ -38,194 +40,275 @@ const char kHIDName[] = "HID_NAME"; const char kHIDUnique[] = "HID_UNIQ"; const char kSysfsReportDescriptorKey[] = "report_descriptor"; -#if defined(OS_CHROMEOS) -void OnRequestAccessComplete( - scoped_refptr<base::SingleThreadTaskRunner> reply_task_runner, - const base::Callback<void(bool success)>& callback, - bool success) { - reply_task_runner->PostTask(FROM_HERE, base::Bind(callback, success)); -} +} // namespace -void RequestAccess( - const std::string& device_node, - scoped_refptr<base::SingleThreadTaskRunner> reply_task_runner, - const base::Callback<void(bool success)>& callback) { - bool success = false; +struct HidServiceLinux::ConnectParams { + ConnectParams(const HidDeviceInfo& device_info, + const ConnectCallback& callback, + scoped_refptr<base::SingleThreadTaskRunner> task_runner, + scoped_refptr<base::SingleThreadTaskRunner> file_task_runner) + : device_info(device_info), + callback(callback), + task_runner(task_runner), + file_task_runner(file_task_runner) {} + ~ConnectParams() {} - if (base::SysInfo::IsRunningOnChromeOS()) { - chromeos::PermissionBrokerClient* client = - chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient(); - DCHECK(client) << "Could not get permission broker client."; - if (client) { - client->RequestPathAccess( - device_node, - -1, - base::Bind(OnRequestAccessComplete, reply_task_runner, callback)); - return; - } - } else { - // Not really running on Chrome OS, declare success. - success = true; + HidDeviceInfo device_info; + ConnectCallback callback; + scoped_refptr<base::SingleThreadTaskRunner> task_runner; + scoped_refptr<base::SingleThreadTaskRunner> file_task_runner; + base::File device_file; +}; + +class HidServiceLinux::Helper : public DeviceMonitorLinux::Observer, + public base::MessageLoop::DestructionObserver { + public: + Helper(base::WeakPtr<HidServiceLinux> service, + scoped_refptr<base::SingleThreadTaskRunner> task_runner) + : observer_(this), service_(service), task_runner_(task_runner) { + DeviceMonitorLinux* monitor = DeviceMonitorLinux::GetInstance(); + observer_.Add(monitor); + monitor->Enumerate( + base::Bind(&Helper::OnDeviceAdded, base::Unretained(this))); } - reply_task_runner->PostTask(FROM_HERE, base::Bind(callback, success)); -} -#endif + virtual ~Helper() { + DCHECK(thread_checker_.CalledOnValidThread()); + } -} // namespace + private: + // DeviceMonitorLinux::Observer: + void OnDeviceAdded(udev_device* device) override { + DCHECK(thread_checker_.CalledOnValidThread()); + const char* device_path = udev_device_get_syspath(device); + if (!device_path) { + return; + } + const char* subsystem = udev_device_get_subsystem(device); + if (!subsystem || strcmp(subsystem, kHidrawSubsystem) != 0) { + return; + } -HidServiceLinux::HidServiceLinux( - scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) - : ui_task_runner_(ui_task_runner), - weak_factory_(this) { - base::ThreadRestrictions::AssertIOAllowed(); - task_runner_ = base::ThreadTaskRunnerHandle::Get(); - DeviceMonitorLinux* monitor = DeviceMonitorLinux::GetInstance(); - monitor->AddObserver(this); - monitor->Enumerate( - base::Bind(&HidServiceLinux::OnDeviceAdded, weak_factory_.GetWeakPtr())); -} + HidDeviceInfo device_info; + device_info.device_id = device_path; -void HidServiceLinux::Connect(const HidDeviceId& device_id, - const ConnectCallback& callback) { - DCHECK(thread_checker_.CalledOnValidThread()); + const char* str_property = udev_device_get_devnode(device); + if (!str_property) { + return; + } + device_info.device_node = str_property; - ScopedUdevDevicePtr device = - DeviceMonitorLinux::GetInstance()->GetDeviceFromPath( - device_id); - if (!device) { - task_runner_->PostTask(FROM_HERE, base::Bind(callback, nullptr)); - return; - } + udev_device* parent = udev_device_get_parent(device); + if (!parent) { + return; + } - const char* device_node = udev_device_get_devnode(device.get()); - if (!device_node) { - task_runner_->PostTask(FROM_HERE, base::Bind(callback, nullptr)); - return; - } + const char* hid_id = udev_device_get_property_value(parent, kHIDID); + if (!hid_id) { + return; + } - base::Callback<void(bool success)> finish_connect = - base::Bind(&HidServiceLinux::FinishConnect, - weak_factory_.GetWeakPtr(), - device_id, - std::string(device_node), - callback); + std::vector<std::string> parts; + base::SplitString(hid_id, ':', &parts); + if (parts.size() != 3) { + return; + } -#if defined(OS_CHROMEOS) - ui_task_runner_->PostTask(FROM_HERE, - base::Bind(RequestAccess, - std::string(device_node), - task_runner_, - finish_connect)); -#else - // Use the task runner to preserve the asynchronous behavior of this call on - // non-Chrome OS platforms. - task_runner_->PostTask(FROM_HERE, base::Bind(finish_connect, true)); -#endif -} + uint32_t int_property = 0; + if (HexStringToUInt(base::StringPiece(parts[1]), &int_property)) { + device_info.vendor_id = int_property; + } -HidServiceLinux::~HidServiceLinux() { - if (DeviceMonitorLinux::HasInstance()) - DeviceMonitorLinux::GetInstance()->RemoveObserver(this); -} + if (HexStringToUInt(base::StringPiece(parts[2]), &int_property)) { + device_info.product_id = int_property; + } -void HidServiceLinux::OnDeviceAdded(udev_device* device) { - if (!device) - return; + str_property = udev_device_get_property_value(parent, kHIDUnique); + if (str_property != NULL) { + device_info.serial_number = str_property; + } - const char* device_path = udev_device_get_syspath(device); - if (!device_path) - return; - const char* subsystem = udev_device_get_subsystem(device); - if (!subsystem || strcmp(subsystem, kHidrawSubsystem) != 0) - return; + str_property = udev_device_get_property_value(parent, kHIDName); + if (str_property != NULL) { + device_info.product_name = str_property; + } - HidDeviceInfo device_info; - device_info.device_id = device_path; + const char* parent_sysfs_path = udev_device_get_syspath(parent); + if (!parent_sysfs_path) { + return; + } + base::FilePath report_descriptor_path = + base::FilePath(parent_sysfs_path).Append(kSysfsReportDescriptorKey); + std::string report_descriptor_str; + if (!base::ReadFileToString(report_descriptor_path, + &report_descriptor_str)) { + return; + } - uint32_t int_property = 0; - const char* str_property = NULL; + HidReportDescriptor report_descriptor( + reinterpret_cast<uint8_t*>(&report_descriptor_str[0]), + report_descriptor_str.length()); + report_descriptor.GetDetails( + &device_info.collections, &device_info.has_report_id, + &device_info.max_input_report_size, &device_info.max_output_report_size, + &device_info.max_feature_report_size); - udev_device* parent = udev_device_get_parent(device); - if (!parent) { - return; + task_runner_->PostTask(FROM_HERE, base::Bind(&HidServiceLinux::AddDevice, + service_, device_info)); } - const char* hid_id = udev_device_get_property_value(parent, kHIDID); - if (!hid_id) { - return; + void OnDeviceRemoved(udev_device* device) override { + DCHECK(thread_checker_.CalledOnValidThread()); + const char* device_path = udev_device_get_syspath(device); + if (device_path) { + task_runner_->PostTask( + FROM_HERE, + base::Bind(&HidServiceLinux::RemoveDevice, service_, device_path)); + } } - std::vector<std::string> parts; - base::SplitString(hid_id, ':', &parts); - if (parts.size() != 3) { - return; + // base::MessageLoop::DestructionObserver: + void WillDestroyCurrentMessageLoop() override { + DCHECK(thread_checker_.CalledOnValidThread()); + base::MessageLoop::current()->RemoveDestructionObserver(this); + delete this; } - if (HexStringToUInt(base::StringPiece(parts[1]), &int_property)) { - device_info.vendor_id = int_property; - } + base::ThreadChecker thread_checker_; + ScopedObserver<DeviceMonitorLinux, DeviceMonitorLinux::Observer> observer_; - if (HexStringToUInt(base::StringPiece(parts[2]), &int_property)) { - device_info.product_id = int_property; - } + // This weak pointer is only valid when checked on this task runner. + base::WeakPtr<HidServiceLinux> service_; + scoped_refptr<base::SingleThreadTaskRunner> task_runner_; +}; - str_property = udev_device_get_property_value(parent, kHIDUnique); - if (str_property != NULL) { - device_info.serial_number = str_property; - } +HidServiceLinux::HidServiceLinux( + scoped_refptr<base::SingleThreadTaskRunner> file_task_runner) + : file_task_runner_(file_task_runner), weak_factory_(this) { + task_runner_ = base::ThreadTaskRunnerHandle::Get(); + // The device watcher is passed a weak pointer back to this service so that it + // can be cleaned up after the service is destroyed however this weak pointer + // must be constructed on the this thread where it will be checked. + file_task_runner_->PostTask( + FROM_HERE, base::Bind(&HidServiceLinux::StartHelper, + weak_factory_.GetWeakPtr(), task_runner_)); +} - str_property = udev_device_get_property_value(parent, kHIDName); - if (str_property != NULL) { - device_info.product_name = str_property; - } +// static +void HidServiceLinux::StartHelper( + base::WeakPtr<HidServiceLinux> weak_ptr, + scoped_refptr<base::SingleThreadTaskRunner> task_runner) { + // Helper is a message loop destruction observer and will delete itself when + // this thread's message loop is destroyed. + new Helper(weak_ptr, task_runner); +} - const char* parent_sysfs_path = udev_device_get_syspath(parent); - if (!parent_sysfs_path) { +void HidServiceLinux::Connect(const HidDeviceId& device_id, + const ConnectCallback& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + + const auto& map_entry = devices().find(device_id); + if (map_entry == devices().end()) { + task_runner_->PostTask(FROM_HERE, base::Bind(callback, nullptr)); return; } - base::FilePath report_descriptor_path = - base::FilePath(parent_sysfs_path).Append(kSysfsReportDescriptorKey); - std::string report_descriptor_str; - if (!base::ReadFileToString(report_descriptor_path, &report_descriptor_str)) { + const HidDeviceInfo& device_info = map_entry->second; + + scoped_ptr<ConnectParams> params(new ConnectParams( + device_info, callback, task_runner_, file_task_runner_)); + +#if defined(OS_CHROMEOS) + if (base::SysInfo::IsRunningOnChromeOS()) { + chromeos::PermissionBrokerClient* client = + chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient(); + DCHECK(client) << "Could not get permission broker client."; + if (client) { + client->RequestPathAccess( + device_info.device_node, -1, + base::Bind(&HidServiceLinux::OnRequestPathAccessComplete, + base::Passed(¶ms))); + } else { + task_runner_->PostTask(FROM_HERE, base::Bind(callback, nullptr)); + } return; } +#endif // defined(OS_CHROMEOS) - HidReportDescriptor report_descriptor( - reinterpret_cast<uint8_t*>(&report_descriptor_str[0]), - report_descriptor_str.length()); - report_descriptor.GetDetails(&device_info.collections, - &device_info.has_report_id, - &device_info.max_input_report_size, - &device_info.max_output_report_size, - &device_info.max_feature_report_size); + file_task_runner_->PostTask( + FROM_HERE, + base::Bind(&HidServiceLinux::OpenDevice, base::Passed(¶ms))); +} - AddDevice(device_info); +HidServiceLinux::~HidServiceLinux() { + file_task_runner_->DeleteSoon(FROM_HERE, helper_.release()); } -void HidServiceLinux::OnDeviceRemoved(udev_device* device) { - const char* device_path = udev_device_get_syspath(device);; - if (device_path) { - RemoveDevice(device_path); +#if defined(OS_CHROMEOS) +// static +void HidServiceLinux::OnRequestPathAccessComplete( + scoped_ptr<ConnectParams> params, + bool success) { + if (success) { + scoped_refptr<base::SingleThreadTaskRunner> file_task_runner = + params->file_task_runner; + file_task_runner->PostTask( + FROM_HERE, + base::Bind(&HidServiceLinux::OpenDevice, base::Passed(¶ms))); + } else { + params->callback.Run(nullptr); } } +#endif // defined(OS_CHROMEOS) -void HidServiceLinux::FinishConnect( - const HidDeviceId& device_id, - const std::string device_node, - const base::Callback<void(scoped_refptr<HidConnection>)>& callback, - bool success) { - DCHECK(thread_checker_.CalledOnValidThread()); - if (!success) { - callback.Run(nullptr); +// static +void HidServiceLinux::OpenDevice(scoped_ptr<ConnectParams> params) { + base::ThreadRestrictions::AssertIOAllowed(); + scoped_refptr<base::SingleThreadTaskRunner> task_runner = params->task_runner; + base::FilePath device_path(params->device_info.device_node); + base::File& device_file = params->device_file; + int flags = + base::File::FLAG_OPEN | base::File::FLAG_READ | base::File::FLAG_WRITE; + device_file.Initialize(device_path, flags); + if (!device_file.IsValid()) { + base::File::Error file_error = device_file.error_details(); + + if (file_error == base::File::FILE_ERROR_ACCESS_DENIED) { + VLOG(1) << "Access denied opening device read-write, trying read-only."; + flags = base::File::FLAG_OPEN | base::File::FLAG_READ; + device_file.Initialize(device_path, flags); + } + } + if (!device_file.IsValid()) { + LOG(ERROR) << "Failed to open '" << params->device_info.device_node << "': " + << base::File::ErrorToString(device_file.error_details()); + task_runner->PostTask(FROM_HERE, base::Bind(params->callback, nullptr)); + return; } - const auto& map_entry = devices().find(device_id); - if (map_entry == devices().end()) { - callback.Run(nullptr); + int result = fcntl(device_file.GetPlatformFile(), F_GETFL); + if (result == -1) { + PLOG(ERROR) << "Failed to get flags from the device file descriptor"; + task_runner->PostTask(FROM_HERE, base::Bind(params->callback, nullptr)); + return; + } + + result = fcntl(device_file.GetPlatformFile(), F_SETFL, result | O_NONBLOCK); + if (result == -1) { + PLOG(ERROR) << "Failed to set the non-blocking flag on the device fd"; + task_runner->PostTask(FROM_HERE, base::Bind(params->callback, nullptr)); + return; } - callback.Run(new HidConnectionLinux(map_entry->second, device_node)); + task_runner->PostTask(FROM_HERE, base::Bind(&HidServiceLinux::ConnectImpl, + base::Passed(¶ms))); +} + +// static +void HidServiceLinux::ConnectImpl(scoped_ptr<ConnectParams> params) { + DCHECK(params->device_file.IsValid()); + params->callback.Run(make_scoped_refptr( + new HidConnectionLinux(params->device_info, params->device_file.Pass(), + params->file_task_runner))); } } // namespace device diff --git a/device/hid/hid_service_linux.h b/device/hid/hid_service_linux.h index 80239c4..59b6036 100644 --- a/device/hid/hid_service_linux.h +++ b/device/hid/hid_service_linux.h @@ -8,40 +8,50 @@ #include "base/compiler_specific.h" #include "base/macros.h" #include "base/memory/weak_ptr.h" -#include "device/hid/device_monitor_linux.h" #include "device/hid/hid_device_info.h" #include "device/hid/hid_service.h" -struct udev_device; - namespace device { class HidConnection; -class HidServiceLinux : public HidService, - public DeviceMonitorLinux::Observer { +class HidServiceLinux : public HidService { public: HidServiceLinux(scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner); void Connect(const HidDeviceId& device_id, const ConnectCallback& callback) override; - // Implements DeviceMonitorLinux::Observer: - void OnDeviceAdded(udev_device* device) override; - void OnDeviceRemoved(udev_device* device) override; - private: + struct ConnectParams; + class Helper; + friend class Helper; + ~HidServiceLinux() override; - void FinishConnect(const HidDeviceId& device_id, - const std::string device_node, - const ConnectCallback& callback, - bool success); + // Constructs this services helper object that lives on the FILE thread. + static void StartHelper( + base::WeakPtr<HidServiceLinux> weak_ptr, + scoped_refptr<base::SingleThreadTaskRunner> task_runner); + + // These functions implement the process of locating, requesting access to and + // opening a device. Because this operation crosses multiple threads these + // functions are static and the necessary parameters are passed as a single + // struct. +#if defined(OS_CHROMEOS) + static void OnRequestPathAccessComplete(scoped_ptr<ConnectParams> params, + bool success); +#endif // defined(OS_CHROMEOS) + static void OpenDevice(scoped_ptr<ConnectParams> params); + static void ConnectImpl(scoped_ptr<ConnectParams> params); scoped_refptr<base::SingleThreadTaskRunner> task_runner_; - scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_; + scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_; + // The helper lives on the FILE thread and holds a weak reference back to the + // service that owns it. base::WeakPtrFactory<HidServiceLinux> weak_factory_; + scoped_ptr<Helper> helper_; DISALLOW_COPY_AND_ASSIGN(HidServiceLinux); }; diff --git a/device/hid/hid_service_unittest.cc b/device/hid/hid_service_unittest.cc index 6e82ef1e..2291300 100644 --- a/device/hid/hid_service_unittest.cc +++ b/device/hid/hid_service_unittest.cc @@ -14,7 +14,6 @@ namespace device { TEST(HidServiceTest, Create) { base::MessageLoopForIO message_loop; HidService* service = HidService::GetInstance( - message_loop.message_loop_proxy(), message_loop.message_loop_proxy()); ASSERT_TRUE(service); diff --git a/extensions/browser/api/hid/hid_api.cc b/extensions/browser/api/hid/hid_api.cc index fcb8abf..27d495d 100644 --- a/extensions/browser/api/hid/hid_api.cc +++ b/extensions/browser/api/hid/hid_api.cc @@ -64,12 +64,11 @@ HidAsyncApiFunction::HidAsyncApiFunction() HidAsyncApiFunction::~HidAsyncApiFunction() {} bool HidAsyncApiFunction::PrePrepare() { -#if defined(OS_MACOSX) - // Migration from FILE thread to UI thread. OS X gets it first. - set_work_thread_id(content::BrowserThread::UI); -#else - // TODO(reillyg): Migrate Linux/CrOS and Windows as well. +#if defined(OS_WIN) + // TODO(reillyg): Migrate Windows backend from FILE thread to UI thread. set_work_thread_id(content::BrowserThread::FILE); +#else + set_work_thread_id(content::BrowserThread::UI); #endif device_manager_ = HidDeviceManager::Get(browser_context()); if (!device_manager_) { diff --git a/extensions/browser/api/hid/hid_apitest.cc b/extensions/browser/api/hid/hid_apitest.cc index ad91648..0f70ff7 100644 --- a/extensions/browser/api/hid/hid_apitest.cc +++ b/extensions/browser/api/hid/hid_apitest.cc @@ -169,12 +169,16 @@ class HidApiTest : public ShellApiTest { public: void SetUpOnMainThread() override { ShellApiTest::SetUpOnMainThread(); +#if defined(OS_WIN) + // TODO(reillyg): Migrate Windows backend from FILE thread to UI thread. base::RunLoop run_loop; - BrowserThread::PostTaskAndReply(BrowserThread::FILE, - FROM_HERE, + BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE, base::Bind(&HidApiTest::SetUpService, this), run_loop.QuitClosure()); run_loop.Run(); +#else + SetUpService(); +#endif } void SetUpService() { HidService::SetInstanceForTest(new MockHidService()); } diff --git a/extensions/browser/api/hid/hid_connection_resource.h b/extensions/browser/api/hid/hid_connection_resource.h index d2248c4..b3d8b66 100644 --- a/extensions/browser/api/hid/hid_connection_resource.h +++ b/extensions/browser/api/hid/hid_connection_resource.h @@ -21,14 +21,13 @@ namespace extensions { class HidConnectionResource : public ApiResource { public: -#if defined(OS_MACOSX) - // Migration from FILE thread to UI thread. OS X gets it first. +#if defined(OS_WIN) + // TODO(reillyg): Migrate Windows backend from FILE thread to UI thread. static const content::BrowserThread::ID kThreadId = - content::BrowserThread::UI; + content::BrowserThread::FILE; #else - // TODO(reillyg): Migrate Linux/CrOS and Windows as well. static const content::BrowserThread::ID kThreadId = - content::BrowserThread::FILE; + content::BrowserThread::UI; #endif HidConnectionResource(const std::string& owner_extension_id, diff --git a/extensions/browser/api/hid/hid_device_manager.cc b/extensions/browser/api/hid/hid_device_manager.cc index 838099e..1490e87 100644 --- a/extensions/browser/api/hid/hid_device_manager.cc +++ b/extensions/browser/api/hid/hid_device_manager.cc @@ -148,12 +148,11 @@ bool HidDeviceManager::HasPermission(const Extension* extension, // static bool HidDeviceManager::IsCalledOnValidThread() { -#if defined(OS_MACOSX) - // Migration from FILE thread to UI thread. OS X gets it first. - return content::BrowserThread::CurrentlyOn(content::BrowserThread::UI); -#else - // TODO(reillyg): Migrate Linux/CrOS and Windows as well. +#if defined(OS_WIN) + // TODO(reillyg): Migrate Windows backend from FILE thread to UI thread. return content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE); +#else + return content::BrowserThread::CurrentlyOn(content::BrowserThread::UI); #endif } diff --git a/extensions/shell/browser/shell_device_client.cc b/extensions/shell/browser/shell_device_client.cc index 8055881..90e974b 100644 --- a/extensions/shell/browser/shell_device_client.cc +++ b/extensions/shell/browser/shell_device_client.cc @@ -24,9 +24,7 @@ device::UsbService* ShellDeviceClient::GetUsbService() { device::HidService* ShellDeviceClient::GetHidService() { return device::HidService::GetInstance( content::BrowserThread::GetMessageLoopProxyForThread( - content::BrowserThread::FILE), - content::BrowserThread::GetMessageLoopProxyForThread( - content::BrowserThread::UI)); + content::BrowserThread::FILE)); } } |