From 45919813ba21243f14a326b593a9fae52ff4a2f8 Mon Sep 17 00:00:00 2001 From: reillyg Date: Wed, 21 Jan 2015 18:39:59 -0800 Subject: Add browser tests for USB device add/remove events. These tests are essentially copies of the HID device add and remove event tests. In order to reconcile issues with the MockUsbService lifetime it is now tracked by a MessageLoop::DestructionObserver at the generic UsbService level. This is similar to the pattern used by the HidService until it was moved to the UI thread and could use a LazyInstance. BUG=411715 Review URL: https://codereview.chromium.org/800963005 Cr-Commit-Position: refs/heads/master@{#312538} --- device/usb/BUILD.gn | 2 + device/usb/usb.gyp | 2 + device/usb/usb_service.cc | 90 ++++++++++++++++++++++ device/usb/usb_service.h | 5 +- device/usb/usb_service_impl.cc | 165 +++++------------------------------------ device/usb/usb_service_impl.h | 82 ++++++++++++++++++++ 6 files changed, 198 insertions(+), 148 deletions(-) create mode 100644 device/usb/usb_service.cc create mode 100644 device/usb/usb_service_impl.h (limited to 'device/usb') diff --git a/device/usb/BUILD.gn b/device/usb/BUILD.gn index ee793d3..d4be7cf 100644 --- a/device/usb/BUILD.gn +++ b/device/usb/BUILD.gn @@ -25,8 +25,10 @@ source_set("usb") { "usb_error.h", "usb_ids.cc", "usb_ids.h", + "usb_service.cc", "usb_service.h", "usb_service_impl.cc", + "usb_service_impl.h", generated_ids, ] diff --git a/device/usb/usb.gyp b/device/usb/usb.gyp index 301bea9..c0995a1 100644 --- a/device/usb/usb.gyp +++ b/device/usb/usb.gyp @@ -34,8 +34,10 @@ 'usb_error.h', 'usb_ids.cc', 'usb_ids.h', + 'usb_service.cc', 'usb_service.h', 'usb_service_impl.cc', + 'usb_service_impl.h', ], 'actions': [ { diff --git a/device/usb/usb_service.cc b/device/usb/usb_service.cc new file mode 100644 index 0000000..70f054d --- /dev/null +++ b/device/usb/usb_service.cc @@ -0,0 +1,90 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "device/usb/usb_service.h" + +#include "base/message_loop/message_loop.h" +#include "device/usb/usb_device.h" +#include "device/usb/usb_service_impl.h" + +namespace device { + +namespace { + +UsbService* g_service; + +} // namespace + +// This class manages the lifetime of the global UsbService instance so that +// it is destroyed when the current message loop is destroyed. A lazy instance +// cannot be used because this object does not live on the main thread. +class UsbService::Destroyer : private base::MessageLoop::DestructionObserver { + public: + explicit Destroyer(UsbService* usb_service) : usb_service_(usb_service) { + base::MessageLoop::current()->AddDestructionObserver(this); + } + ~Destroyer() override {} + + private: + // base::MessageLoop::DestructionObserver implementation. + void WillDestroyCurrentMessageLoop() override { + base::MessageLoop::current()->RemoveDestructionObserver(this); + delete usb_service_; + delete this; + g_service = nullptr; + } + + UsbService* usb_service_; +}; + +void UsbService::Observer::OnDeviceAdded(scoped_refptr device) { +} + +void UsbService::Observer::OnDeviceRemoved(scoped_refptr device) { +} + +// static +UsbService* UsbService::GetInstance( + scoped_refptr ui_task_runner) { + if (!g_service) { + g_service = UsbServiceImpl::Create(ui_task_runner); + // This object will clean itself up when the message loop is destroyed. + new Destroyer(g_service); + } + return g_service; +} + +// static +void UsbService::SetInstanceForTest(UsbService* instance) { + g_service = instance; + new Destroyer(instance); +} + +UsbService::UsbService() { +} + +UsbService::~UsbService() { +} + +void UsbService::AddObserver(Observer* observer) { + DCHECK(CalledOnValidThread()); + observer_list_.AddObserver(observer); +} + +void UsbService::RemoveObserver(Observer* observer) { + DCHECK(CalledOnValidThread()); + observer_list_.RemoveObserver(observer); +} + +void UsbService::NotifyDeviceAdded(scoped_refptr device) { + DCHECK(CalledOnValidThread()); + FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceAdded(device)); +} + +void UsbService::NotifyDeviceRemoved(scoped_refptr device) { + DCHECK(CalledOnValidThread()); + FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceRemoved(device)); +} + +} // namespace device diff --git a/device/usb/usb_service.h b/device/usb/usb_service.h index 076d363..4655bfd 100644 --- a/device/usb/usb_service.h +++ b/device/usb/usb_service.h @@ -55,8 +55,6 @@ class UsbService : public base::NonThreadSafe { void RemoveObserver(Observer* observer); protected: - friend struct base::DefaultDeleter; - UsbService(); virtual ~UsbService(); @@ -65,6 +63,9 @@ class UsbService : public base::NonThreadSafe { ObserverList observer_list_; + private: + class Destroyer; + DISALLOW_COPY_AND_ASSIGN(UsbService); }; diff --git a/device/usb/usb_service_impl.cc b/device/usb/usb_service_impl.cc index 89fbd66a..d789e70 100644 --- a/device/usb/usb_service_impl.cc +++ b/device/usb/usb_service_impl.cc @@ -2,22 +2,17 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "device/usb/usb_service.h" +#include "device/usb/usb_service_impl.h" -#include #include #include "base/bind.h" -#include "base/lazy_instance.h" +#include "base/location.h" #include "base/memory/weak_ptr.h" -#include "base/message_loop/message_loop.h" #include "base/single_thread_task_runner.h" #include "base/stl_util.h" #include "base/thread_task_runner_handle.h" -#include "device/usb/usb_context.h" -#include "device/usb/usb_device_impl.h" #include "device/usb/usb_error.h" -#include "third_party/libusb/src/libusb/libusb.h" #if defined(OS_WIN) #include @@ -28,79 +23,6 @@ namespace device { -namespace { - -base::LazyInstance >::Leaky g_usb_service_instance = - LAZY_INSTANCE_INITIALIZER; - -} // namespace - -typedef struct libusb_device* PlatformUsbDevice; -typedef struct libusb_context* PlatformUsbContext; - -class UsbServiceImpl : public UsbService, - private base::MessageLoop::DestructionObserver { - public: - explicit UsbServiceImpl( - PlatformUsbContext context, - scoped_refptr ui_task_runner); - ~UsbServiceImpl() override; - - private: - // device::UsbService implementation - scoped_refptr GetDeviceById(uint32 unique_id) override; - void GetDevices(std::vector>* devices) override; - - // base::MessageLoop::DestructionObserver implementation. - void WillDestroyCurrentMessageLoop() override; - - // Enumerate USB devices from OS and update devices_ map. - void RefreshDevices(); - - // Adds a new UsbDevice to the devices_ map based on the given libusb device. - scoped_refptr AddDevice(PlatformUsbDevice platform_device); - - // Handle hotplug events from libusb. - static int LIBUSB_CALL HotplugCallback(libusb_context* context, - PlatformUsbDevice device, - libusb_hotplug_event event, - void* user_data); - // These functions release a reference to the provided platform device. - void OnDeviceAdded(PlatformUsbDevice platform_device); - void OnDeviceRemoved(PlatformUsbDevice platform_device); - - scoped_refptr context_; - scoped_refptr task_runner_; - scoped_refptr ui_task_runner_; - -#if defined(OS_WIN) - class UIThreadHelper; - UIThreadHelper* ui_thread_helper_; -#endif // OS_WIN - - // TODO(reillyg): Figure out a better solution for device IDs. - uint32 next_unique_id_; - - // When available the device list will be updated when new devices are - // connected instead of only when a full enumeration is requested. - // TODO(reillyg): Support this on all platforms. crbug.com/411715 - bool hotplug_enabled_; - libusb_hotplug_callback_handle hotplug_handle_; - - // The map from unique IDs to UsbDevices. - typedef std::map > DeviceMap; - DeviceMap devices_; - - // The map from PlatformUsbDevices to UsbDevices. - typedef std::map > - PlatformDeviceMap; - PlatformDeviceMap platform_devices_; - - base::WeakPtrFactory weak_factory_; - - DISALLOW_COPY_AND_ASSIGN(UsbServiceImpl); -}; - #if defined(OS_WIN) // This class lives on the application main thread so that it can listen for // device change notification window messages. It registers for notifications @@ -141,6 +63,23 @@ class UsbServiceImpl::UIThreadHelper final }; #endif +// static +UsbService* UsbServiceImpl::Create( + scoped_refptr ui_task_runner) { + PlatformUsbContext context = NULL; + const int rv = libusb_init(&context); + if (rv != LIBUSB_SUCCESS) { + VLOG(1) << "Failed to initialize libusb: " + << ConvertPlatformUsbErrorToString(rv); + return nullptr; + } + if (!context) { + return nullptr; + } + + return new UsbServiceImpl(context, ui_task_runner); +} + scoped_refptr UsbServiceImpl::GetDeviceById(uint32 unique_id) { DCHECK(CalledOnValidThread()); RefreshDevices(); @@ -165,11 +104,6 @@ void UsbServiceImpl::GetDevices( } } -void UsbServiceImpl::WillDestroyCurrentMessageLoop() { - DCHECK(CalledOnValidThread()); - g_usb_service_instance.Get().reset(NULL); -} - UsbServiceImpl::UsbServiceImpl( PlatformUsbContext context, scoped_refptr ui_task_runner) @@ -178,7 +112,6 @@ UsbServiceImpl::UsbServiceImpl( next_unique_id_(0), hotplug_enabled_(false), weak_factory_(this) { - base::MessageLoop::current()->AddDestructionObserver(this); task_runner_ = base::ThreadTaskRunnerHandle::Get(); int rv = libusb_hotplug_register_callback( context_->context(), @@ -200,7 +133,6 @@ UsbServiceImpl::UsbServiceImpl( } UsbServiceImpl::~UsbServiceImpl() { - base::MessageLoop::current()->RemoveDestructionObserver(this); if (hotplug_enabled_) { libusb_hotplug_deregister_callback(context_->context(), hotplug_handle_); } @@ -356,63 +288,4 @@ void UsbServiceImpl::OnDeviceRemoved(PlatformUsbDevice platform_device) { libusb_unref_device(platform_device); } -void UsbService::Observer::OnDeviceAdded(scoped_refptr device) { -} - -void UsbService::Observer::OnDeviceRemoved(scoped_refptr device) { -} - -// static -UsbService* UsbService::GetInstance( - scoped_refptr ui_task_runner) { - UsbService* instance = g_usb_service_instance.Get().get(); - if (!instance) { - PlatformUsbContext context = NULL; - - const int rv = libusb_init(&context); - if (rv != LIBUSB_SUCCESS) { - VLOG(1) << "Failed to initialize libusb: " - << ConvertPlatformUsbErrorToString(rv); - return NULL; - } - if (!context) - return NULL; - - instance = new UsbServiceImpl(context, ui_task_runner); - g_usb_service_instance.Get().reset(instance); - } - return instance; -} - -// static -void UsbService::SetInstanceForTest(UsbService* instance) { - g_usb_service_instance.Get().reset(instance); -} - -UsbService::UsbService() { -} - -UsbService::~UsbService() { -} - -void UsbService::AddObserver(Observer* observer) { - DCHECK(CalledOnValidThread()); - observer_list_.AddObserver(observer); -} - -void UsbService::RemoveObserver(Observer* observer) { - DCHECK(CalledOnValidThread()); - observer_list_.RemoveObserver(observer); -} - -void UsbService::NotifyDeviceAdded(scoped_refptr device) { - DCHECK(CalledOnValidThread()); - FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceAdded(device)); -} - -void UsbService::NotifyDeviceRemoved(scoped_refptr device) { - DCHECK(CalledOnValidThread()); - FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceRemoved(device)); -} - } // namespace device diff --git a/device/usb/usb_service_impl.h b/device/usb/usb_service_impl.h new file mode 100644 index 0000000..25cfb30 --- /dev/null +++ b/device/usb/usb_service_impl.h @@ -0,0 +1,82 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "device/usb/usb_service.h" + +#include + +#include "base/memory/weak_ptr.h" +#include "base/single_thread_task_runner.h" +#include "device/usb/usb_context.h" +#include "device/usb/usb_device_impl.h" +#include "third_party/libusb/src/libusb/libusb.h" + +namespace device { + +typedef struct libusb_device* PlatformUsbDevice; +typedef struct libusb_context* PlatformUsbContext; + +class UsbServiceImpl : public UsbService { + public: + static UsbService* Create( + scoped_refptr ui_task_runner); + + private: + explicit UsbServiceImpl( + PlatformUsbContext context, + scoped_refptr ui_task_runner); + ~UsbServiceImpl() override; + + // device::UsbService implementation + scoped_refptr GetDeviceById(uint32 unique_id) override; + void GetDevices(std::vector>* devices) override; + + // Enumerate USB devices from OS and update devices_ map. + void RefreshDevices(); + + // Adds a new UsbDevice to the devices_ map based on the given libusb device. + scoped_refptr AddDevice(PlatformUsbDevice platform_device); + + // Handle hotplug events from libusb. + static int LIBUSB_CALL HotplugCallback(libusb_context* context, + PlatformUsbDevice device, + libusb_hotplug_event event, + void* user_data); + // These functions release a reference to the provided platform device. + void OnDeviceAdded(PlatformUsbDevice platform_device); + void OnDeviceRemoved(PlatformUsbDevice platform_device); + + scoped_refptr context_; + scoped_refptr task_runner_; + scoped_refptr ui_task_runner_; + +#if defined(OS_WIN) + class UIThreadHelper; + UIThreadHelper* ui_thread_helper_; +#endif // OS_WIN + + // TODO(reillyg): Figure out a better solution for device IDs. + uint32 next_unique_id_; + + // When available the device list will be updated when new devices are + // connected instead of only when a full enumeration is requested. + // TODO(reillyg): Support this on all platforms. crbug.com/411715 + bool hotplug_enabled_; + libusb_hotplug_callback_handle hotplug_handle_; + + // The map from unique IDs to UsbDevices. + typedef std::map> DeviceMap; + DeviceMap devices_; + + // The map from PlatformUsbDevices to UsbDevices. + typedef std::map> + PlatformDeviceMap; + PlatformDeviceMap platform_devices_; + + base::WeakPtrFactory weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(UsbServiceImpl); +}; + +} // namespace device -- cgit v1.1