From d6d8ce13c1ab8141e85717f5832c4ad1af3128bb Mon Sep 17 00:00:00 2001 From: reillyg Date: Fri, 4 Mar 2016 15:29:41 -0800 Subject: Convert the PermissionProvider Mojo interface to plain C++. This interface allows a separate Mojo service to provide permissions management for a DeviceManager instance. This will be useful if USB device handling is ever moved into a separate process but until then it adds needless complexity to the implementation because access checks are made asynchronous. Make it a normal C++ interface for now. BUG=None Review URL: https://codereview.chromium.org/1742333003 Cr-Commit-Position: refs/heads/master@{#379393} --- device/usb/mojo/BUILD.gn | 2 + device/usb/mojo/device_impl.cc | 173 ++++++++----------- device/usb/mojo/device_impl.h | 19 ++- device/usb/mojo/device_impl_unittest.cc | 9 +- device/usb/mojo/device_manager_impl.cc | 188 ++++++--------------- device/usb/mojo/device_manager_impl.h | 39 ++--- device/usb/mojo/device_manager_impl_unittest.cc | 4 +- device/usb/mojo/fake_permission_provider.cc | 36 ++-- device/usb/mojo/fake_permission_provider.h | 28 ++- device/usb/mojo/permission_provider.cc | 15 ++ device/usb/mojo/permission_provider.h | 34 ++++ device/usb/public/interfaces/BUILD.gn | 1 - .../public/interfaces/permission_provider.mojom | 29 ---- device/usb/usb.gyp | 3 +- 14 files changed, 228 insertions(+), 352 deletions(-) create mode 100644 device/usb/mojo/permission_provider.cc create mode 100644 device/usb/mojo/permission_provider.h delete mode 100644 device/usb/public/interfaces/permission_provider.mojom (limited to 'device') diff --git a/device/usb/mojo/BUILD.gn b/device/usb/mojo/BUILD.gn index d48cf37..a2cc898 100644 --- a/device/usb/mojo/BUILD.gn +++ b/device/usb/mojo/BUILD.gn @@ -8,6 +8,8 @@ source_set("mojo") { "device_impl.h", "device_manager_impl.cc", "device_manager_impl.h", + "permission_provider.cc", + "permission_provider.h", "type_converters.cc", "type_converters.h", ] diff --git a/device/usb/mojo/device_impl.cc b/device/usb/mojo/device_impl.cc index ae5f4bb..9472571 100644 --- a/device/usb/mojo/device_impl.cc +++ b/device/usb/mojo/device_impl.cc @@ -50,16 +50,6 @@ base::Callback WrapMojoCallback( return base::Bind(&CallMojoCallback, base::Passed(&callback_ptr)); } -void OnPermissionCheckComplete( - const base::Callback& callback, - const base::Callback&)>& action, - bool allowed) { - if (allowed) - action.Run(callback); - else - callback.Run(false); -} - scoped_refptr CreateTransferBuffer(size_t size) { scoped_refptr buffer = new net::IOBuffer( std::max(static_cast(1u), static_cast(size))); @@ -82,27 +72,6 @@ void OnTransferIn(scoped_ptr callback, callback->Run(mojo::ConvertTo(status), std::move(data)); } -void OnControlTransferInPermissionCheckComplete( - scoped_refptr device_handle, - ControlTransferParamsPtr params, - int length, - int timeout, - scoped_ptr callback, - bool allowed) { - if (allowed) { - scoped_refptr buffer = CreateTransferBuffer(length); - device_handle->ControlTransfer( - USB_DIRECTION_INBOUND, - mojo::ConvertTo(params->type), - mojo::ConvertTo(params->recipient), - params->request, params->value, params->index, buffer, length, timeout, - base::Bind(&OnTransferIn, base::Passed(&callback))); - } else { - mojo::Array data; - callback->Run(TransferStatus::PERMISSION_DENIED, std::move(data)); - } -} - void OnTransferOut(scoped_ptr callback, UsbTransferStatus status, scoped_refptr buffer, @@ -110,28 +79,6 @@ void OnTransferOut(scoped_ptr callback, callback->Run(mojo::ConvertTo(status)); } -void OnControlTransferOutPermissionCheckComplete( - scoped_refptr device_handle, - ControlTransferParamsPtr params, - mojo::Array data, - int timeout, - scoped_ptr callback, - bool allowed) { - if (allowed) { - scoped_refptr buffer = CreateTransferBuffer(data.size()); - const std::vector& storage = data.storage(); - std::copy(storage.begin(), storage.end(), buffer->data()); - device_handle->ControlTransfer( - USB_DIRECTION_OUTBOUND, - mojo::ConvertTo(params->type), - mojo::ConvertTo(params->recipient), - params->request, params->value, params->index, buffer, data.size(), - timeout, base::Bind(&OnTransferOut, base::Passed(&callback))); - } else { - callback->Run(TransferStatus::PERMISSION_DENIED); - } -} - mojo::Array BuildIsochronousPacketArray( mojo::Array packet_lengths, TransferStatus status) { @@ -177,21 +124,21 @@ void OnIsochronousTransferOut( } // namespace DeviceImpl::DeviceImpl(scoped_refptr device, - PermissionProviderPtr permission_provider, + DeviceInfoPtr device_info, + base::WeakPtr permission_provider, mojo::InterfaceRequest request) : device_(device), + device_info_(std::move(device_info)), + permission_provider_(permission_provider), observer_(this), - permission_provider_(std::move(permission_provider)), binding_(this, std::move(request)), weak_factory_(this) { DCHECK(device_); // This object owns itself and will be destroyed if, - // * the device is disconnected, - // * the message pipe it is bound to is closed, or - // * the PermissionProvider it depends on is unavailable. + // * the device is disconnected or + // * the message pipe it is bound to is closed observer_.Add(device_.get()); binding_.set_connection_error_handler([this]() { delete this; }); - permission_provider_.set_connection_error_handler([this]() { delete this; }); } DeviceImpl::~DeviceImpl() { @@ -204,38 +151,35 @@ void DeviceImpl::CloseHandle() { device_handle_ = nullptr; } -void DeviceImpl::HasControlTransferPermission( +bool DeviceImpl::HasControlTransferPermission( ControlTransferRecipient recipient, - uint16_t index, - const base::Callback& callback) { + uint16_t index) { DCHECK(device_handle_); const UsbConfigDescriptor* config = device_->GetActiveConfiguration(); + if (!permission_provider_) + return false; + if (recipient == ControlTransferRecipient::INTERFACE || recipient == ControlTransferRecipient::ENDPOINT) { - if (!config) { - callback.Run(false); - return; - } + if (!config) + return false; uint8_t interface_number = index & 0xff; - if (recipient == ControlTransferRecipient::ENDPOINT) { - if (!device_handle_->FindInterfaceByEndpoint(index & 0xff, - &interface_number)) { - callback.Run(false); - return; - } + if (recipient == ControlTransferRecipient::ENDPOINT && + !device_handle_->FindInterfaceByEndpoint(index & 0xff, + &interface_number)) { + return false; } - permission_provider_->HasInterfacePermission( - interface_number, config->configuration_value, - DeviceInfo::From(*device_), callback); + return permission_provider_->HasInterfacePermission( + interface_number, config->configuration_value, *device_info_); } else if (config) { - permission_provider_->HasConfigurationPermission( - config->configuration_value, DeviceInfo::From(*device_), callback); + return permission_provider_->HasConfigurationPermission( + config->configuration_value, *device_info_); } else { // Client must already have device permission to have gotten this far. - callback.Run(true); + return true; } } @@ -246,7 +190,7 @@ void DeviceImpl::OnOpen(const OpenCallback& callback, } void DeviceImpl::GetDeviceInfo(const GetDeviceInfoCallback& callback) { - callback.Run(DeviceInfo::From(*device_)); + callback.Run(device_info_->Clone()); } void DeviceImpl::GetConfiguration(const GetConfigurationCallback& callback) { @@ -274,12 +218,12 @@ void DeviceImpl::SetConfiguration(uint8_t value, return; } - auto set_configuration = - base::Bind(&UsbDeviceHandle::SetConfiguration, device_handle_, value); - permission_provider_->HasConfigurationPermission( - value, DeviceInfo::From(*device_), - base::Bind(&OnPermissionCheckComplete, WrapMojoCallback(callback), - set_configuration)); + if (permission_provider_ && + permission_provider_->HasConfigurationPermission(value, *device_info_)) { + device_handle_->SetConfiguration(value, WrapMojoCallback(callback)); + } else { + callback.Run(false); + } } void DeviceImpl::ClaimInterface(uint8_t interface_number, @@ -295,12 +239,14 @@ void DeviceImpl::ClaimInterface(uint8_t interface_number, return; } - auto claim_interface = base::Bind(&UsbDeviceHandle::ClaimInterface, - device_handle_, interface_number); - permission_provider_->HasInterfacePermission( - interface_number, config->configuration_value, DeviceInfo::From(*device_), - base::Bind(&OnPermissionCheckComplete, WrapMojoCallback(callback), - claim_interface)); + if (permission_provider_ && + permission_provider_->HasInterfacePermission( + interface_number, config->configuration_value, *device_info_)) { + device_handle_->ClaimInterface(interface_number, + WrapMojoCallback(callback)); + } else { + callback.Run(false); + } } void DeviceImpl::ReleaseInterface(uint8_t interface_number, @@ -355,14 +301,20 @@ void DeviceImpl::ControlTransferIn(ControlTransferParamsPtr params, return; } - auto callback_ptr = make_scoped_ptr(new ControlTransferInCallback(callback)); - ControlTransferRecipient recipient = params->recipient; - uint16_t index = params->index; - HasControlTransferPermission( - recipient, index, - base::Bind(&OnControlTransferInPermissionCheckComplete, device_handle_, - base::Passed(¶ms), length, timeout, - base::Passed(&callback_ptr))); + if (HasControlTransferPermission(params->recipient, params->index)) { + scoped_refptr buffer = CreateTransferBuffer(length); + auto callback_ptr = + make_scoped_ptr(new ControlTransferInCallback(callback)); + device_handle_->ControlTransfer( + USB_DIRECTION_INBOUND, + mojo::ConvertTo(params->type), + mojo::ConvertTo(params->recipient), + params->request, params->value, params->index, buffer, length, timeout, + base::Bind(&OnTransferIn, base::Passed(&callback_ptr))); + } else { + mojo::Array data; + callback.Run(TransferStatus::PERMISSION_DENIED, std::move(data)); + } } void DeviceImpl::ControlTransferOut( @@ -375,14 +327,21 @@ void DeviceImpl::ControlTransferOut( return; } - auto callback_ptr = make_scoped_ptr(new ControlTransferOutCallback(callback)); - ControlTransferRecipient recipient = params->recipient; - uint16_t index = params->index; - HasControlTransferPermission( - recipient, index, - base::Bind(&OnControlTransferOutPermissionCheckComplete, device_handle_, - base::Passed(¶ms), base::Passed(&data), timeout, - base::Passed(&callback_ptr))); + if (HasControlTransferPermission(params->recipient, params->index)) { + scoped_refptr buffer = CreateTransferBuffer(data.size()); + const std::vector& storage = data.storage(); + std::copy(storage.begin(), storage.end(), buffer->data()); + auto callback_ptr = + make_scoped_ptr(new ControlTransferOutCallback(callback)); + device_handle_->ControlTransfer( + USB_DIRECTION_OUTBOUND, + mojo::ConvertTo(params->type), + mojo::ConvertTo(params->recipient), + params->request, params->value, params->index, buffer, data.size(), + timeout, base::Bind(&OnTransferOut, base::Passed(&callback_ptr))); + } else { + callback.Run(TransferStatus::PERMISSION_DENIED); + } } void DeviceImpl::GenericTransferIn(uint8_t endpoint_number, diff --git a/device/usb/mojo/device_impl.h b/device/usb/mojo/device_impl.h index 487a7d4..6c77d38 100644 --- a/device/usb/mojo/device_impl.h +++ b/device/usb/mojo/device_impl.h @@ -12,8 +12,8 @@ #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" #include "base/scoped_observer.h" +#include "device/usb/mojo/permission_provider.h" #include "device/usb/public/interfaces/device.mojom.h" -#include "device/usb/public/interfaces/permission_provider.mojom.h" #include "device/usb/usb_device.h" #include "device/usb/usb_device_handle.h" #include "mojo/public/cpp/bindings/binding.h" @@ -27,13 +27,16 @@ class IOBuffer; namespace device { namespace usb { +class PermissionProvider; + // Implementation of the public Device interface. Instances of this class are // constructed by DeviceManagerImpl and are strongly bound to their MessagePipe // lifetime. class DeviceImpl : public Device, public device::UsbDevice::Observer { public: DeviceImpl(scoped_refptr device, - PermissionProviderPtr permission_provider, + DeviceInfoPtr device_info, + base::WeakPtr permission_provider, mojo::InterfaceRequest request); ~DeviceImpl() override; @@ -43,9 +46,8 @@ class DeviceImpl : public Device, public device::UsbDevice::Observer { void CloseHandle(); // Checks interface permissions for control transfers. - void HasControlTransferPermission(ControlTransferRecipient recipient, - uint16_t index, - const base::Callback& callback); + bool HasControlTransferPermission(ControlTransferRecipient recipient, + uint16_t index); // Handles completion of an open request. void OnOpen(const OpenCallback& callback, @@ -97,15 +99,16 @@ class DeviceImpl : public Device, public device::UsbDevice::Observer { const IsochronousTransferOutCallback& callback) override; // device::UsbDevice::Observer implementation: - void OnDeviceRemoved(scoped_refptr) override; + void OnDeviceRemoved(scoped_refptr device) override; - scoped_refptr device_; + const scoped_refptr device_; + const DeviceInfoPtr device_info_; + base::WeakPtr permission_provider_; ScopedObserver observer_; // The device handle. Will be null before the device is opened and after it // has been closed. scoped_refptr device_handle_; - PermissionProviderPtr permission_provider_; mojo::Binding binding_; base::WeakPtrFactory weak_factory_; diff --git a/device/usb/mojo/device_impl_unittest.cc b/device/usb/mojo/device_impl_unittest.cc index 25cc048..78a1f85 100644 --- a/device/usb/mojo/device_impl_unittest.cc +++ b/device/usb/mojo/device_impl_unittest.cc @@ -23,6 +23,7 @@ #include "device/usb/mock_usb_device.h" #include "device/usb/mock_usb_device_handle.h" #include "device/usb/mojo/fake_permission_provider.h" +#include "device/usb/mojo/type_converters.h" #include "mojo/public/cpp/bindings/interface_request.h" #include "net/base/io_buffer.h" #include "testing/gtest/include/gtest/gtest.h" @@ -168,11 +169,11 @@ class USBDeviceImplTest : public testing::Test { new MockUsbDevice(vendor_id, product_id, manufacturer, product, serial); mock_handle_ = new MockUsbDeviceHandle(mock_device_.get()); - PermissionProviderPtr permission_provider; - permission_provider_.Bind(mojo::GetProxy(&permission_provider)); DevicePtr proxy; - new DeviceImpl(mock_device_, std::move(permission_provider), - mojo::GetProxy(&proxy)); + new DeviceImpl( + mock_device_, + DeviceInfo::From(static_cast(*mock_device_)), + permission_provider_.GetWeakPtr(), mojo::GetProxy(&proxy)); // Set up mock handle calls to respond based on mock device configs // established by the test. diff --git a/device/usb/mojo/device_manager_impl.cc b/device/usb/mojo/device_manager_impl.cc index 78ecaf9..69e238a 100644 --- a/device/usb/mojo/device_manager_impl.cc +++ b/device/usb/mojo/device_manager_impl.cc @@ -13,6 +13,7 @@ #include "base/stl_util.h" #include "device/core/device_client.h" #include "device/usb/mojo/device_impl.h" +#include "device/usb/mojo/permission_provider.h" #include "device/usb/mojo/type_converters.h" #include "device/usb/public/interfaces/device.mojom.h" #include "device/usb/usb_device.h" @@ -24,51 +25,30 @@ namespace device { namespace usb { -namespace { - -using DeviceList = DeviceManagerImpl::DeviceList; -using DeviceMap = DeviceManagerImpl::DeviceMap; - -void FilterAndConvertDevicesAndThen( - const DeviceMap& devices, - const DeviceManagerImpl::GetDevicesCallback& callback, - mojo::Array allowed_guids) { - mojo::Array allowed_devices(allowed_guids.size()); - for (size_t i = 0; i < allowed_guids.size(); ++i) { - const auto it = devices.find(allowed_guids[i]); - DCHECK(it != devices.end()); - allowed_devices[i] = DeviceInfo::From(*it->second); - } - - callback.Run(std::move(allowed_devices)); -} - -} // namespace - // static -void DeviceManagerImpl::Create(PermissionProviderPtr permission_provider, - mojo::InterfaceRequest request) { - // The created object is owned by its binding. - new DeviceManagerImpl(std::move(permission_provider), std::move(request)); +void DeviceManagerImpl::Create( + base::WeakPtr permission_provider, + mojo::InterfaceRequest request) { + DCHECK(DeviceClient::Get()); + UsbService* usb_service = DeviceClient::Get()->GetUsbService(); + if (usb_service) { + new DeviceManagerImpl(permission_provider, usb_service, std::move(request)); + } } DeviceManagerImpl::DeviceManagerImpl( - PermissionProviderPtr permission_provider, + base::WeakPtr permission_provider, + UsbService* usb_service, mojo::InterfaceRequest request) - : permission_provider_(std::move(permission_provider)), + : permission_provider_(permission_provider), + usb_service_(usb_service), observer_(this), binding_(this, std::move(request)), weak_factory_(this) { - // This object owns itself and will be destroyed if either the message pipe - // it is bound to is closed or the PermissionProvider it depends on is - // unavailable. + // This object owns itself and will be destroyed if the message pipe it is + // bound to is closed or the UsbService is shut down. binding_.set_connection_error_handler([this]() { delete this; }); - permission_provider_.set_connection_error_handler([this]() { delete this; }); - - DCHECK(DeviceClient::Get()); - usb_service_ = DeviceClient::Get()->GetUsbService(); - if (usb_service_) - observer_.Add(usb_service_); + observer_.Add(usb_service_); } DeviceManagerImpl::~DeviceManagerImpl() { @@ -77,12 +57,6 @@ DeviceManagerImpl::~DeviceManagerImpl() { void DeviceManagerImpl::GetDevices(EnumerationOptionsPtr options, const GetDevicesCallback& callback) { - if (!usb_service_) { - mojo::Array no_devices; - callback.Run(std::move(no_devices)); - return; - } - usb_service_->GetDevices(base::Bind(&DeviceManagerImpl::OnGetDevices, weak_factory_.GetWeakPtr(), base::Passed(&options), callback)); @@ -97,131 +71,79 @@ void DeviceManagerImpl::GetDeviceChanges( void DeviceManagerImpl::GetDevice( const mojo::String& guid, mojo::InterfaceRequest device_request) { - if (!usb_service_) - return; - scoped_refptr device = usb_service_->GetDevice(guid); if (!device) return; - mojo::Array requested_devices(1); - requested_devices[0] = DeviceInfo::From(*device); - permission_provider_->HasDevicePermission( - std::move(requested_devices), - base::Bind(&DeviceManagerImpl::OnGetDevicePermissionCheckComplete, - base::Unretained(this), device, - base::Passed(&device_request))); -} - -void DeviceManagerImpl::OnGetDevicePermissionCheckComplete( - scoped_refptr device, - mojo::InterfaceRequest device_request, - mojo::Array allowed_guids) { - if (allowed_guids.size() == 0) - return; - - DCHECK(allowed_guids.size() == 1); - PermissionProviderPtr permission_provider; - permission_provider_->Bind(mojo::GetProxy(&permission_provider)); - new DeviceImpl(device, std::move(permission_provider), - std::move(device_request)); + DeviceInfoPtr device_info = DeviceInfo::From(*device); + if (permission_provider_ && + permission_provider_->HasDevicePermission(*device_info)) { + new DeviceImpl(device, std::move(device_info), permission_provider_, + std::move(device_request)); + } } -void DeviceManagerImpl::OnGetDevices(EnumerationOptionsPtr options, - const GetDevicesCallback& callback, - const DeviceList& devices) { +void DeviceManagerImpl::OnGetDevices( + EnumerationOptionsPtr options, + const GetDevicesCallback& callback, + const std::vector>& devices) { std::vector filters; if (options) filters = options->filters.To>(); - std::map> device_map; - mojo::Array requested_devices; + mojo::Array device_infos; for (const auto& device : devices) { if (filters.empty() || UsbDeviceFilter::MatchesAny(device, filters)) { - device_map[device->guid()] = device; - requested_devices.push_back(DeviceInfo::From(*device)); + DeviceInfoPtr device_info = DeviceInfo::From(*device); + if (permission_provider_ && + permission_provider_->HasDevicePermission(*device_info)) { + device_infos.push_back(std::move(device_info)); + } } } - permission_provider_->HasDevicePermission( - std::move(requested_devices), - base::Bind(&FilterAndConvertDevicesAndThen, device_map, callback)); + callback.Run(std::move(device_infos)); } void DeviceManagerImpl::OnDeviceAdded(scoped_refptr device) { - DCHECK(!ContainsKey(devices_removed_, device->guid())); - devices_added_[device->guid()] = device; - MaybeRunDeviceChangesCallback(); + DeviceInfoPtr device_info = DeviceInfo::From(*device); + if (permission_provider_ && + permission_provider_->HasDevicePermission(*device_info)) { + devices_added_[device->guid()] = std::move(device_info); + MaybeRunDeviceChangesCallback(); + } } void DeviceManagerImpl::OnDeviceRemoved(scoped_refptr device) { - if (devices_added_.erase(device->guid()) == 0) - devices_removed_[device->guid()] = device; - MaybeRunDeviceChangesCallback(); + if (devices_added_.erase(device->guid()) == 0) { + DeviceInfoPtr device_info = DeviceInfo::From(*device); + if (permission_provider_ && + permission_provider_->HasDevicePermission(*device_info)) { + devices_removed_.push_back(std::move(device_info)); + MaybeRunDeviceChangesCallback(); + } + } } void DeviceManagerImpl::WillDestroyUsbService() { - observer_.RemoveAll(); - usb_service_ = nullptr; + delete this; } void DeviceManagerImpl::MaybeRunDeviceChangesCallback() { - if (!permission_request_pending_ && !device_change_callbacks_.empty()) { - DeviceMap devices_added; - devices_added.swap(devices_added_); - DeviceMap devices_removed; - devices_removed.swap(devices_removed_); - - mojo::Array requested_devices(devices_added.size() + - devices_removed.size()); - { - size_t i = 0; - for (const auto& map_entry : devices_added) - requested_devices[i++] = DeviceInfo::From(*map_entry.second); - for (const auto& map_entry : devices_removed) - requested_devices[i++] = DeviceInfo::From(*map_entry.second); - } - - permission_request_pending_ = true; - permission_provider_->HasDevicePermission( - std::move(requested_devices), - base::Bind(&DeviceManagerImpl::OnEnumerationPermissionCheckComplete, - base::Unretained(this), devices_added, devices_removed)); - } -} - -void DeviceManagerImpl::OnEnumerationPermissionCheckComplete( - const DeviceMap& devices_added, - const DeviceMap& devices_removed, - mojo::Array allowed_guids) { - permission_request_pending_ = false; - - if (allowed_guids.size() > 0) { + if (!device_change_callbacks_.empty()) { DeviceChangeNotificationPtr notification = DeviceChangeNotification::New(); - notification->devices_added.resize(0); - notification->devices_removed.resize(0); - - for (size_t i = 0; i < allowed_guids.size(); ++i) { - const mojo::String& guid = allowed_guids[i]; - auto it = devices_added.find(guid); - if (it != devices_added.end()) { - DCHECK(!ContainsKey(devices_removed, guid)); - notification->devices_added.push_back(DeviceInfo::From(*it->second)); - } else { - it = devices_removed.find(guid); - DCHECK(it != devices_removed.end()); - notification->devices_removed.push_back(DeviceInfo::From(*it->second)); - } + notification->devices_added.SetToEmpty(); + notification->devices_removed.SetToEmpty(); + for (auto& map_entry : devices_added_) { + notification->devices_added.push_back(std::move(map_entry.second)); } + devices_added_.clear(); + notification->devices_removed.Swap(&devices_removed_); - DCHECK(!device_change_callbacks_.empty()); const GetDeviceChangesCallback& callback = device_change_callbacks_.front(); callback.Run(std::move(notification)); device_change_callbacks_.pop(); } - - if (devices_added_.size() > 0 || !devices_removed_.empty()) - MaybeRunDeviceChangesCallback(); } } // namespace usb diff --git a/device/usb/mojo/device_manager_impl.h b/device/usb/mojo/device_manager_impl.h index 85e46ce..6ddf5d1 100644 --- a/device/usb/mojo/device_manager_impl.h +++ b/device/usb/mojo/device_manager_impl.h @@ -14,7 +14,6 @@ #include "base/memory/weak_ptr.h" #include "base/scoped_observer.h" #include "device/usb/public/interfaces/device_manager.mojom.h" -#include "device/usb/public/interfaces/permission_provider.mojom.h" #include "device/usb/usb_service.h" #include "mojo/public/cpp/bindings/array.h" #include "mojo/public/cpp/bindings/binding.h" @@ -32,20 +31,17 @@ class UsbDeviceHandle; namespace usb { -class DeviceManagerDelegate; +class PermissionProvider; // Implementation of the public DeviceManager interface. This interface can be // requested from the devices app located at "mojo:devices", if available. -class DeviceManagerImpl : public DeviceManager, - public device::UsbService::Observer { +class DeviceManagerImpl : public DeviceManager, public UsbService::Observer { public: - using DeviceList = std::vector>; - using DeviceMap = std::map>; - - static void Create(PermissionProviderPtr permission_provider, + static void Create(base::WeakPtr permission_provider, mojo::InterfaceRequest request); - DeviceManagerImpl(PermissionProviderPtr permission_provider, + DeviceManagerImpl(base::WeakPtr permission_provider, + UsbService* usb_service, mojo::InterfaceRequest request); ~DeviceManagerImpl() override; @@ -62,40 +58,29 @@ class DeviceManagerImpl : public DeviceManager, mojo::InterfaceRequest device_request) override; // Callbacks to handle the async responses from the underlying UsbService. - void OnGetDevicePermissionCheckComplete( - scoped_refptr device, - mojo::InterfaceRequest device_request, - mojo::Array allowed_guids); void OnGetDevices(EnumerationOptionsPtr options, const GetDevicesCallback& callback, - const DeviceList& devices); + const std::vector>& devices); // UsbService::Observer implementation: - void OnDeviceAdded(scoped_refptr device) override; - void OnDeviceRemoved(scoped_refptr device) override; + void OnDeviceAdded(scoped_refptr device) override; + void OnDeviceRemoved(scoped_refptr device) override; void WillDestroyUsbService() override; void MaybeRunDeviceChangesCallback(); - void OnEnumerationPermissionCheckComplete( - const DeviceMap& devices_added, - const DeviceMap& devices_removed, - mojo::Array allowed_guids); - PermissionProviderPtr permission_provider_; + base::WeakPtr permission_provider_; // If there are unfinished calls to GetDeviceChanges their callbacks // are stored in |device_change_callbacks_|. Otherwise device changes // are collected in |devices_added_| and |devices_removed_| until the // next call to GetDeviceChanges. std::queue device_change_callbacks_; - DeviceMap devices_added_; - DeviceMap devices_removed_; - // To ensure that GetDeviceChangesCallbacks are called in the correct order - // only perform a single request to |permission_provider_| at a time. - bool permission_request_pending_ = false; + std::map devices_added_; + std::vector devices_removed_; UsbService* usb_service_; - ScopedObserver observer_; + ScopedObserver observer_; mojo::Closure connection_error_handler_; diff --git a/device/usb/mojo/device_manager_impl_unittest.cc b/device/usb/mojo/device_manager_impl_unittest.cc index 139f5a03..9fc01df 100644 --- a/device/usb/mojo/device_manager_impl_unittest.cc +++ b/device/usb/mojo/device_manager_impl_unittest.cc @@ -38,10 +38,8 @@ class USBDeviceManagerImplTest : public testing::Test { protected: DeviceManagerPtr ConnectToDeviceManager() { - PermissionProviderPtr permission_provider; - permission_provider_.Bind(mojo::GetProxy(&permission_provider)); DeviceManagerPtr device_manager; - DeviceManagerImpl::Create(std::move(permission_provider), + DeviceManagerImpl::Create(permission_provider_.GetWeakPtr(), mojo::GetProxy(&device_manager)); return device_manager; } diff --git a/device/usb/mojo/fake_permission_provider.cc b/device/usb/mojo/fake_permission_provider.cc index 9dd7b4f..c100318 100644 --- a/device/usb/mojo/fake_permission_provider.cc +++ b/device/usb/mojo/fake_permission_provider.cc @@ -10,36 +10,30 @@ namespace device { namespace usb { -FakePermissionProvider::FakePermissionProvider() {} +FakePermissionProvider::FakePermissionProvider() : weak_factory_(this) {} FakePermissionProvider::~FakePermissionProvider() {} -void FakePermissionProvider::HasDevicePermission( - mojo::Array requested_devices, - const HasDevicePermissionCallback& callback) { - mojo::Array allowed_guids(requested_devices.size()); - for (size_t i = 0; i < requested_devices.size(); ++i) - allowed_guids[i] = requested_devices[i]->guid; - callback.Run(std::move(allowed_guids)); +base::WeakPtr FakePermissionProvider::GetWeakPtr() { + return weak_factory_.GetWeakPtr(); } -void FakePermissionProvider::HasConfigurationPermission( +bool FakePermissionProvider::HasDevicePermission( + const device::usb::DeviceInfo& device_info) const { + return true; +} + +bool FakePermissionProvider::HasConfigurationPermission( uint8_t requested_configuration, - device::usb::DeviceInfoPtr device, - const HasInterfacePermissionCallback& callback) { - callback.Run(true); + const device::usb::DeviceInfo& device_info) const { + return true; } -void FakePermissionProvider::HasInterfacePermission( + +bool FakePermissionProvider::HasInterfacePermission( uint8_t requested_interface, uint8_t configuration_value, - device::usb::DeviceInfoPtr device, - const HasInterfacePermissionCallback& callback) { - callback.Run(true); -} - -void FakePermissionProvider::Bind( - mojo::InterfaceRequest request) { - bindings_.AddBinding(this, std::move(request)); + const device::usb::DeviceInfo& device_info) const { + return true; } } // namespace usb diff --git a/device/usb/mojo/fake_permission_provider.h b/device/usb/mojo/fake_permission_provider.h index 5ca1610..8d03813 100644 --- a/device/usb/mojo/fake_permission_provider.h +++ b/device/usb/mojo/fake_permission_provider.h @@ -7,10 +7,8 @@ #include -#include "device/usb/public/interfaces/permission_provider.mojom.h" -#include "mojo/public/cpp/bindings/array.h" -#include "mojo/public/cpp/bindings/binding_set.h" -#include "mojo/public/cpp/bindings/interface_request.h" +#include "base/memory/weak_ptr.h" +#include "device/usb/mojo/permission_provider.h" namespace device { namespace usb { @@ -20,22 +18,16 @@ class FakePermissionProvider : public PermissionProvider { FakePermissionProvider(); ~FakePermissionProvider() override; - void HasDevicePermission( - mojo::Array requested_devices, - const HasDevicePermissionCallback& callback) override; - void HasConfigurationPermission( - uint8_t requested_configuration, - device::usb::DeviceInfoPtr device, - const HasInterfacePermissionCallback& callback) override; - void HasInterfacePermission( - uint8_t requested_interface, - uint8_t configuration_value, - device::usb::DeviceInfoPtr device, - const HasInterfacePermissionCallback& callback) override; - void Bind(mojo::InterfaceRequest request) override; + base::WeakPtr GetWeakPtr(); + bool HasDevicePermission(const DeviceInfo& device_info) const override; + bool HasConfigurationPermission(uint8_t requested_configuration, + const DeviceInfo& device_info) const override; + bool HasInterfacePermission(uint8_t requested_interface, + uint8_t configuration_value, + const DeviceInfo& device_info) const override; private: - mojo::BindingSet bindings_; + base::WeakPtrFactory weak_factory_; }; } // namespace usb diff --git a/device/usb/mojo/permission_provider.cc b/device/usb/mojo/permission_provider.cc new file mode 100644 index 0000000..3d6d956 --- /dev/null +++ b/device/usb/mojo/permission_provider.cc @@ -0,0 +1,15 @@ +// Copyright 2016 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/mojo/permission_provider.h" + +namespace device { +namespace usb { + +PermissionProvider::PermissionProvider() {} + +PermissionProvider::~PermissionProvider() {} + +} // namespace usb +} // namespace device diff --git a/device/usb/mojo/permission_provider.h b/device/usb/mojo/permission_provider.h new file mode 100644 index 0000000..8366047 --- /dev/null +++ b/device/usb/mojo/permission_provider.h @@ -0,0 +1,34 @@ +// Copyright 2016 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. + +#ifndef DEVICE_USB_MOJO_PERMISSION_PROVIDER_H_ +#define DEVICE_USB_MOJO_PERMISSION_PROVIDER_H_ + +#include + +namespace device { +namespace usb { + +class DeviceInfo; + +// An implementation of this interface must be provided to a DeviceManager in +// order to implement device permission checks. +class PermissionProvider { + public: + PermissionProvider(); + virtual ~PermissionProvider(); + + virtual bool HasDevicePermission(const DeviceInfo& device_info) const = 0; + virtual bool HasConfigurationPermission( + uint8_t requested_configuration, + const DeviceInfo& device_info) const = 0; + virtual bool HasInterfacePermission(uint8_t requested_interface, + uint8_t configuration_value, + const DeviceInfo& device_info) const = 0; +}; + +} // namespace usb +} // namespace device + +#endif // DEVICE_USB_MOJO_PERMISSION_PROVIDER_H_ diff --git a/device/usb/public/interfaces/BUILD.gn b/device/usb/public/interfaces/BUILD.gn index f9d6e0c..eca5418 100644 --- a/device/usb/public/interfaces/BUILD.gn +++ b/device/usb/public/interfaces/BUILD.gn @@ -8,6 +8,5 @@ mojom("interfaces") { sources = [ "device.mojom", "device_manager.mojom", - "permission_provider.mojom", ] } diff --git a/device/usb/public/interfaces/permission_provider.mojom b/device/usb/public/interfaces/permission_provider.mojom deleted file mode 100644 index f5d7111..0000000 --- a/device/usb/public/interfaces/permission_provider.mojom +++ /dev/null @@ -1,29 +0,0 @@ -// 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. - -module device.usb; - -import "device.mojom"; - -interface PermissionProvider { - // Filters a set of |requested_devices| down to the set of |allowed_guids| - // that should be accessible to clients of the DeviceManager instance. - HasDevicePermission(array requested_devices) - => (array allowed_guids); - - // Returns whether or not the client has permission to access - // |requested_configuration| on |device|. - HasConfigurationPermission(uint8 requested_configuration, - DeviceInfo device) => (bool allowed); - - // Returns whether or not the client has permission to access - // |requested_interface| on |device| when it is in configuration - // |configuration_value|. - HasInterfacePermission(uint8 requested_interface, - uint8 configuration_value, - DeviceInfo device) => (bool allowed); - - // Requests a new binding to this service. - Bind(PermissionProvider& request); -}; diff --git a/device/usb/usb.gyp b/device/usb/usb.gyp index ee6d34f..a553f57 100644 --- a/device/usb/usb.gyp +++ b/device/usb/usb.gyp @@ -29,6 +29,8 @@ 'mojo/device_manager_impl.h', 'mojo/type_converters.cc', 'mojo/type_converters.h', + 'mojo/permission_provider.cc', + 'mojo/permission_provider.h', 'usb_configuration_android.cc', 'usb_configuration_android.h', 'usb_context.cc', @@ -128,7 +130,6 @@ 'sources': [ 'public/interfaces/device.mojom', 'public/interfaces/device_manager.mojom', - 'public/interfaces/permission_provider.mojom', ], 'includes': [ '../../mojo/mojom_bindings_generator.gypi', -- cgit v1.1