diff options
author | reillyg <reillyg@chromium.org> | 2015-04-23 17:19:46 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-04-24 00:19:54 +0000 |
commit | 03256e5f3b29e4c29c102a289757f773e5a624a7 (patch) | |
tree | af2ec25724767d8fda1719ba80c0986edd53ed7b /extensions | |
parent | e5499ddebc7499f765ef3688d8c4c293ee3138cf (diff) | |
download | chromium_src-03256e5f3b29e4c29c102a289757f773e5a624a7.zip chromium_src-03256e5f3b29e4c29c102a289757f773e5a624a7.tar.gz chromium_src-03256e5f3b29e4c29c102a289757f773e5a624a7.tar.bz2 |
Separate USB device permissions prompt logic into subclasses.
This patch moves the specific logic for handling USB devices and
device permissions out of the DevicePermissionsPrompt class and into
a set of subclasses that are hidden in device_permissions_prompt.cc.
This should allow a parallel set of subclasses for handling HID devices
to be added without any more modifications to the base classes.
BUG=457899
Review URL: https://codereview.chromium.org/1098823003
Cr-Commit-Position: refs/heads/master@{#326697}
Diffstat (limited to 'extensions')
-rw-r--r-- | extensions/browser/api/device_permissions_prompt.cc | 290 | ||||
-rw-r--r-- | extensions/browser/api/device_permissions_prompt.h | 102 | ||||
-rw-r--r-- | extensions/browser/api/usb/usb_api.cc | 7 | ||||
-rw-r--r-- | extensions/browser/api/usb/usb_api.h | 10 | ||||
-rw-r--r-- | extensions/extensions_strings.grd | 4 |
5 files changed, 220 insertions, 193 deletions
diff --git a/extensions/browser/api/device_permissions_prompt.cc b/extensions/browser/api/device_permissions_prompt.cc index 09751b3..6e63045 100644 --- a/extensions/browser/api/device_permissions_prompt.cc +++ b/extensions/browser/api/device_permissions_prompt.cc @@ -4,10 +4,9 @@ #include "extensions/browser/api/device_permissions_prompt.h" -#include "base/barrier_closure.h" +#include "base/scoped_observer.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" -#include "content/public/browser/browser_thread.h" #include "device/core/device_client.h" #include "device/usb/usb_device.h" #include "device/usb/usb_device_filter.h" @@ -18,49 +17,167 @@ #include "extensions/strings/grit/extensions_strings.h" #include "ui/base/l10n/l10n_util.h" -using content::BrowserThread; using device::UsbDevice; using device::UsbDeviceFilter; using device::UsbService; namespace extensions { -DevicePermissionsPrompt::Delegate::~Delegate() { -} +namespace { + +class UsbDeviceInfo : public DevicePermissionsPrompt::Prompt::DeviceInfo { + public: + UsbDeviceInfo(scoped_refptr<UsbDevice> device) : device_(device) { + base::string16 manufacturer_string = device->manufacturer_string(); + if (manufacturer_string.empty()) { + const char* vendor_name = + device::UsbIds::GetVendorName(device->vendor_id()); + if (vendor_name) { + manufacturer_string = base::UTF8ToUTF16(vendor_name); + } else { + base::string16 vendor_id = base::ASCIIToUTF16( + base::StringPrintf("0x%04x", device->vendor_id())); + manufacturer_string = + l10n_util::GetStringFUTF16(IDS_DEVICE_UNKNOWN_VENDOR, vendor_id); + } + } + + base::string16 product_string = device->product_string(); + if (product_string.empty()) { + const char* product_name = device::UsbIds::GetProductName( + device->vendor_id(), device->product_id()); + if (product_name) { + product_string = base::UTF8ToUTF16(product_name); + } else { + base::string16 product_id = base::ASCIIToUTF16( + base::StringPrintf("0x%04x", device->product_id())); + product_string = + l10n_util::GetStringFUTF16(IDS_DEVICE_UNKNOWN_PRODUCT, product_id); + } + } + + name_ = l10n_util::GetStringFUTF16(IDS_DEVICE_PERMISSIONS_DEVICE_NAME, + product_string, manufacturer_string); + serial_number_ = device->serial_number(); + } + + ~UsbDeviceInfo() override {} + + const scoped_refptr<UsbDevice>& device() const { return device_; } + + private: + // TODO(reillyg): Convert this to a weak reference when UsbDevice has a + // connected flag. + scoped_refptr<UsbDevice> device_; +}; + +class UsbDevicePermissionsPrompt : public DevicePermissionsPrompt::Prompt, + public device::UsbService::Observer { + public: + UsbDevicePermissionsPrompt( + const Extension* extension, + content::BrowserContext* context, + bool multiple, + const std::vector<device::UsbDeviceFilter>& filters, + const DevicePermissionsPrompt::UsbDevicesCallback& callback) + : Prompt(extension, context, multiple), + filters_(filters), + callback_(callback), + service_observer_(this) {} + + private: + ~UsbDevicePermissionsPrompt() override {} + + // DevicePermissionsPrompt::Prompt implementation: + void SetObserver( + DevicePermissionsPrompt::Prompt::Observer* observer) override { + DevicePermissionsPrompt::Prompt::SetObserver(observer); + + if (observer) { + UsbService* service = device::DeviceClient::Get()->GetUsbService(); + if (service && !service_observer_.IsObserving(service)) { + service->GetDevices( + base::Bind(&UsbDevicePermissionsPrompt::OnDevicesEnumerated, this)); + service_observer_.Add(service); + } + } + } + + base::string16 GetHeading() const override { + return l10n_util::GetStringUTF16( + multiple() ? IDS_USB_DEVICE_PERMISSIONS_PROMPT_TITLE_MULTIPLE + : IDS_USB_DEVICE_PERMISSIONS_PROMPT_TITLE_SINGLE); + } + + void Dismissed() override { + DevicePermissionsManager* permissions_manager = + DevicePermissionsManager::Get(browser_context()); + std::vector<scoped_refptr<UsbDevice>> devices; + for (const DeviceInfo* device : devices_) { + if (device->granted()) { + const UsbDeviceInfo* usb_device = + static_cast<const UsbDeviceInfo*>(device); + devices.push_back(usb_device->device()); + if (permissions_manager) { + permissions_manager->AllowUsbDevice(extension()->id(), + usb_device->device()); + } + } + } + DCHECK(multiple() || devices.size() <= 1); + callback_.Run(devices); + callback_.Reset(); + } + + // device::UsbService::Observer implementation: + void OnDeviceAdded(scoped_refptr<UsbDevice> device) override { + if (!(filters_.empty() || UsbDeviceFilter::MatchesAny(device, filters_))) { + return; + } + + device->CheckUsbAccess(base::Bind( + &UsbDevicePermissionsPrompt::AddCheckedDevice, this, device)); + } -DevicePermissionsPrompt::Prompt::DeviceInfo::DeviceInfo( - scoped_refptr<UsbDevice> device) - : device(device) { - base::string16 manufacturer_string = device->manufacturer_string(); - if (manufacturer_string.empty()) { - const char* vendor_name = - device::UsbIds::GetVendorName(device->vendor_id()); - if (vendor_name) { - manufacturer_string = base::UTF8ToUTF16(vendor_name); - } else { - base::string16 vendor_id = - base::ASCIIToUTF16(base::StringPrintf("0x%04x", device->vendor_id())); - manufacturer_string = - l10n_util::GetStringFUTF16(IDS_DEVICE_UNKNOWN_VENDOR, vendor_id); + void OnDeviceRemoved(scoped_refptr<UsbDevice> device) override { + for (auto it = devices_.begin(); it != devices_.end(); ++it) { + const UsbDeviceInfo* entry = static_cast<const UsbDeviceInfo*>(*it); + if (entry->device() == device) { + devices_.erase(it); + if (observer()) { + observer()->OnDevicesChanged(); + } + return; + } } } - base::string16 product_string = device->product_string(); - if (product_string.empty()) { - const char* product_name = device::UsbIds::GetProductName( - device->vendor_id(), device->product_id()); - if (product_name) { - product_string = base::UTF8ToUTF16(product_name); - } else { - base::string16 product_id = base::ASCIIToUTF16( - base::StringPrintf("0x%04x", device->product_id())); - product_string = - l10n_util::GetStringFUTF16(IDS_DEVICE_UNKNOWN_PRODUCT, product_id); + void OnDevicesEnumerated( + const std::vector<scoped_refptr<UsbDevice>>& devices) { + for (const auto& device : devices) { + OnDeviceAdded(device); } } - name = l10n_util::GetStringFUTF16(IDS_DEVICE_PERMISSIONS_DEVICE_NAME, - product_string, manufacturer_string); + void AddCheckedDevice(scoped_refptr<UsbDevice> device, bool allowed) { + if (allowed) { + // TODO(reillyg): This method could be called after OnDeviceRemoved. We + // should check that the device is still connected. + devices_.push_back(new UsbDeviceInfo(device)); + if (observer()) { + observer()->OnDevicesChanged(); + } + } + } + + std::vector<UsbDeviceFilter> filters_; + DevicePermissionsPrompt::UsbDevicesCallback callback_; + ScopedObserver<UsbService, UsbService::Observer> service_observer_; +}; + +} // namespace + +DevicePermissionsPrompt::Prompt::DeviceInfo::DeviceInfo() { } DevicePermissionsPrompt::Prompt::DeviceInfo::~DeviceInfo() { @@ -69,32 +186,14 @@ DevicePermissionsPrompt::Prompt::DeviceInfo::~DeviceInfo() { DevicePermissionsPrompt::Prompt::Observer::~Observer() { } -DevicePermissionsPrompt::Prompt::Prompt(Delegate* delegate, - const Extension* extension, - content::BrowserContext* context) - : extension_(extension), - browser_context_(context), - delegate_(delegate), - usb_service_observer_(this) { +DevicePermissionsPrompt::Prompt::Prompt(const Extension* extension, + content::BrowserContext* context, + bool multiple) + : extension_(extension), browser_context_(context), multiple_(multiple) { } void DevicePermissionsPrompt::Prompt::SetObserver(Observer* observer) { observer_ = observer; - - if (observer_) { - UsbService* service = device::DeviceClient::Get()->GetUsbService(); - if (service && !usb_service_observer_.IsObserving(service)) { - service->GetDevices(base::Bind( - &DevicePermissionsPrompt::Prompt::OnDevicesEnumerated, this)); - usb_service_observer_.Add(service); - } - } -} - -base::string16 DevicePermissionsPrompt::Prompt::GetHeading() const { - return l10n_util::GetStringUTF16( - multiple_ ? IDS_DEVICE_PERMISSIONS_PROMPT_TITLE_MULTIPLE - : IDS_DEVICE_PERMISSIONS_PROMPT_TITLE_SINGLE); } base::string16 DevicePermissionsPrompt::Prompt::GetPromptMessage() const { @@ -107,90 +206,23 @@ base::string16 DevicePermissionsPrompt::Prompt::GetPromptMessage() const { base::string16 DevicePermissionsPrompt::Prompt::GetDeviceName( size_t index) const { DCHECK_LT(index, devices_.size()); - return devices_[index].name; + return devices_[index]->name(); } base::string16 DevicePermissionsPrompt::Prompt::GetDeviceSerialNumber( size_t index) const { DCHECK_LT(index, devices_.size()); - return devices_[index].device->serial_number(); + return devices_[index]->serial_number(); } void DevicePermissionsPrompt::Prompt::GrantDevicePermission(size_t index) { DCHECK_LT(index, devices_.size()); - devices_[index].granted = true; -} - -void DevicePermissionsPrompt::Prompt::Dismissed() { - DevicePermissionsManager* permissions_manager = - DevicePermissionsManager::Get(browser_context_); - std::vector<scoped_refptr<UsbDevice>> devices; - for (const DeviceInfo& device : devices_) { - if (device.granted) { - devices.push_back(device.device); - if (permissions_manager) { - permissions_manager->AllowUsbDevice(extension_->id(), device.device); - } - } - } - delegate_->OnUsbDevicesChosen(devices); -} - -void DevicePermissionsPrompt::Prompt::set_filters( - const std::vector<UsbDeviceFilter>& filters) { - filters_ = filters; + devices_[index]->set_granted(); } DevicePermissionsPrompt::Prompt::~Prompt() { } -void DevicePermissionsPrompt::Prompt::OnDeviceAdded( - scoped_refptr<UsbDevice> device) { - if (!(filters_.empty() || UsbDeviceFilter::MatchesAny(device, filters_))) { - return; - } - - device->CheckUsbAccess(base::Bind( - &DevicePermissionsPrompt::Prompt::AddCheckedUsbDevice, this, device)); -} - -void DevicePermissionsPrompt::Prompt::OnDeviceRemoved( - scoped_refptr<UsbDevice> device) { - bool removed_entry = false; - for (std::vector<DeviceInfo>::iterator it = devices_.begin(); - it != devices_.end(); ++it) { - if (it->device == device) { - devices_.erase(it); - removed_entry = true; - break; - } - } - if (observer_ && removed_entry) { - observer_->OnDevicesChanged(); - } -} - -void DevicePermissionsPrompt::Prompt::OnDevicesEnumerated( - const std::vector<scoped_refptr<UsbDevice>>& devices) { - for (const auto& device : devices) { - if (filters_.empty() || UsbDeviceFilter::MatchesAny(device, filters_)) { - device->CheckUsbAccess(base::Bind( - &DevicePermissionsPrompt::Prompt::AddCheckedUsbDevice, this, device)); - } - } -} - -void DevicePermissionsPrompt::Prompt::AddCheckedUsbDevice( - scoped_refptr<UsbDevice> device, - bool allowed) { - if (allowed) { - devices_.push_back(DeviceInfo(device)); - if (observer_) { - observer_->OnDevicesChanged(); - } - } -} - DevicePermissionsPrompt::DevicePermissionsPrompt( content::WebContents* web_contents) : web_contents_(web_contents) { @@ -200,15 +232,13 @@ DevicePermissionsPrompt::~DevicePermissionsPrompt() { } void DevicePermissionsPrompt::AskForUsbDevices( - Delegate* delegate, const Extension* extension, content::BrowserContext* context, bool multiple, - const std::vector<UsbDeviceFilter>& filters) { - prompt_ = new Prompt(delegate, extension, context); - prompt_->set_multiple(multiple); - prompt_->set_filters(filters); - + const std::vector<UsbDeviceFilter>& filters, + const UsbDevicesCallback& callback) { + prompt_ = new UsbDevicePermissionsPrompt(extension, context, multiple, + filters, callback); ShowDialog(); } diff --git a/extensions/browser/api/device_permissions_prompt.h b/extensions/browser/api/device_permissions_prompt.h index 5e93155..6820855 100644 --- a/extensions/browser/api/device_permissions_prompt.h +++ b/extensions/browser/api/device_permissions_prompt.h @@ -10,11 +10,8 @@ #include "base/callback.h" #include "base/logging.h" #include "base/memory/ref_counted.h" -#include "base/scoped_observer.h" +#include "base/memory/scoped_vector.h" #include "base/strings/string16.h" -#include "content/public/browser/browser_thread.h" -#include "device/usb/usb_device.h" -#include "device/usb/usb_service.h" namespace content { class BrowserContext; @@ -22,6 +19,7 @@ class WebContents; } namespace device { +class UsbDevice; class UsbDeviceFilter; } @@ -33,28 +31,30 @@ class Extension; // (similar to choosing files). class DevicePermissionsPrompt { public: - class Delegate { - public: - // Called with the list of selected USB devices. - virtual void OnUsbDevicesChosen( - const std::vector<scoped_refptr<device::UsbDevice>>& devices) = 0; - - protected: - virtual ~Delegate(); - }; + using UsbDevicesCallback = base::Callback<void( + const std::vector<scoped_refptr<device::UsbDevice>>&)>; // Context information available to the UI implementation. - class Prompt : public base::RefCounted<Prompt>, - public device::UsbService::Observer { + class Prompt : public base::RefCounted<Prompt> { public: - // Displayed properties of a device. - struct DeviceInfo { - DeviceInfo(scoped_refptr<device::UsbDevice> device); - ~DeviceInfo(); - - scoped_refptr<device::UsbDevice> device; - base::string16 name; - bool granted = false; + // This class stores the device information displayed in the UI. It should + // be extended to support particular device types. + class DeviceInfo { + public: + DeviceInfo(); + virtual ~DeviceInfo(); + + const base::string16& name() const { return name_; } + const base::string16& serial_number() const { return serial_number_; } + bool granted() const { return granted_; } + void set_granted() { granted_ = true; } + + protected: + base::string16 name_; + base::string16 serial_number_; + + private: + bool granted_ = false; }; // Since the set of devices can change while the UI is visible an @@ -67,14 +67,14 @@ class DevicePermissionsPrompt { virtual ~Observer(); }; - Prompt(Delegate* delegate, - const Extension* extension, - content::BrowserContext* context); + Prompt(const Extension* extension, + content::BrowserContext* context, + bool multiple); // Only one observer may be registered at a time. - void SetObserver(Observer* observer); + virtual void SetObserver(Observer* observer); - base::string16 GetHeading() const; + virtual base::string16 GetHeading() const = 0; base::string16 GetPromptMessage() const; size_t GetDeviceCount() const { return devices_.size(); } base::string16 GetDeviceName(size_t index) const; @@ -83,46 +83,44 @@ class DevicePermissionsPrompt { // Notifies the DevicePermissionsManager for the current extension that // access to the device at the given index is now granted. void GrantDevicePermission(size_t index); - void Dismissed(); - - bool multiple() const { return multiple_; } - void set_multiple(bool multiple) { multiple_ = multiple; } - void set_filters(const std::vector<device::UsbDeviceFilter>& filters); + virtual void Dismissed() = 0; - private: - friend class base::RefCounted<Prompt>; + // Allow the user to select multiple devices. + bool multiple() const { return multiple_; } + protected: virtual ~Prompt(); - // device::UsbService::Observer implementation: - void OnDeviceAdded(scoped_refptr<device::UsbDevice> device) override; - void OnDeviceRemoved(scoped_refptr<device::UsbDevice> device) override; + const Extension* extension() const { return extension_; } + Observer* observer() const { return observer_; } + content::BrowserContext* browser_context() const { + return browser_context_; + } - void OnDevicesEnumerated( - const std::vector<scoped_refptr<device::UsbDevice>>& devices); - void AddCheckedUsbDevice(scoped_refptr<device::UsbDevice> device, - bool allowed); + // Subclasses may fill this with a particular subclass of DeviceInfo and may + // assume that only that instances of that type are stored here. + ScopedVector<DeviceInfo> devices_; + + private: + friend class base::RefCounted<Prompt>; const extensions::Extension* extension_ = nullptr; + Observer* observer_ = nullptr; content::BrowserContext* browser_context_ = nullptr; - Delegate* delegate_ = nullptr; bool multiple_ = false; - std::vector<device::UsbDeviceFilter> filters_; - std::vector<DeviceInfo> devices_; - Observer* observer_ = nullptr; - ScopedObserver<device::UsbService, device::UsbService::Observer> - usb_service_observer_; + + DISALLOW_COPY_AND_ASSIGN(Prompt); }; DevicePermissionsPrompt(content::WebContents* web_contents); virtual ~DevicePermissionsPrompt(); - void AskForUsbDevices(Delegate* delegate, - const Extension* extension, + void AskForUsbDevices(const Extension* extension, content::BrowserContext* context, bool multiple, - const std::vector<device::UsbDeviceFilter>& filters); + const std::vector<device::UsbDeviceFilter>& filters, + const UsbDevicesCallback& callback); protected: virtual void ShowDialog() = 0; @@ -136,6 +134,8 @@ class DevicePermissionsPrompt { // Parameters available to the UI implementation. scoped_refptr<Prompt> prompt_; + + DISALLOW_COPY_AND_ASSIGN(DevicePermissionsPrompt); }; } // namespace extensions diff --git a/extensions/browser/api/usb/usb_api.cc b/extensions/browser/api/usb/usb_api.cc index d023813..0e33ca5 100644 --- a/extensions/browser/api/usb/usb_api.cc +++ b/extensions/browser/api/usb/usb_api.cc @@ -641,13 +641,13 @@ ExtensionFunction::ResponseAction UsbGetUserSelectedDevicesFunction::Run() { return RespondNow(Error(kErrorNotSupported)); } - AddRef(); prompt_->AskForUsbDevices( - this, extension(), browser_context(), multiple, filters); + extension(), browser_context(), multiple, filters, + base::Bind(&UsbGetUserSelectedDevicesFunction::OnDevicesChosen, this)); return RespondLater(); } -void UsbGetUserSelectedDevicesFunction::OnUsbDevicesChosen( +void UsbGetUserSelectedDevicesFunction::OnDevicesChosen( const std::vector<scoped_refptr<UsbDevice>>& devices) { scoped_ptr<base::ListValue> result(new base::ListValue()); for (const auto& device : devices) { @@ -655,7 +655,6 @@ void UsbGetUserSelectedDevicesFunction::OnUsbDevicesChosen( } Respond(OneArgument(result.release())); - Release(); } UsbRequestAccessFunction::UsbRequestAccessFunction() { diff --git a/extensions/browser/api/usb/usb_api.h b/extensions/browser/api/usb/usb_api.h index c7ab6c3..c06c557 100644 --- a/extensions/browser/api/usb/usb_api.h +++ b/extensions/browser/api/usb/usb_api.h @@ -14,7 +14,6 @@ #include "device/usb/usb_device_filter.h" #include "device/usb/usb_device_handle.h" #include "extensions/browser/api/api_resource_manager.h" -#include "extensions/browser/api/device_permissions_prompt.h" #include "extensions/browser/extension_function.h" #include "extensions/common/api/usb.h" #include "net/base/io_buffer.h" @@ -23,6 +22,7 @@ namespace extensions { class DevicePermissionEntry; class DevicePermissions; +class DevicePermissionsPrompt; class DevicePermissionsManager; class UsbDeviceResource; @@ -108,9 +108,7 @@ class UsbGetDevicesFunction : public UsbPermissionCheckingFunction { DISALLOW_COPY_AND_ASSIGN(UsbGetDevicesFunction); }; -class UsbGetUserSelectedDevicesFunction - : public UIThreadExtensionFunction, - public DevicePermissionsPrompt::Delegate { +class UsbGetUserSelectedDevicesFunction : public UIThreadExtensionFunction { public: DECLARE_EXTENSION_FUNCTION("usb.getUserSelectedDevices", USB_GETUSERSELECTEDDEVICES) @@ -123,8 +121,8 @@ class UsbGetUserSelectedDevicesFunction // ExtensionFunction: ResponseAction Run() override; - void OnUsbDevicesChosen( - const std::vector<scoped_refptr<device::UsbDevice>>& devices) override; + void OnDevicesChosen( + const std::vector<scoped_refptr<device::UsbDevice>>& devices); scoped_ptr<DevicePermissionsPrompt> prompt_; diff --git a/extensions/extensions_strings.grd b/extensions/extensions_strings.grd index fec2b34..b91c5f6 100644 --- a/extensions/extensions_strings.grd +++ b/extensions/extensions_strings.grd @@ -341,10 +341,10 @@ <message name="IDS_DEVICE_PERMISSIONS_DEVICE_NAME" desc="String describing a device in a list of devices."> <ph name="PRODUCT_NAME">$1<ex>SoundKnob</ex></ph> from <ph name="VENDOR_NAME">$2<ex>Griffin Technology</ex></ph> </message> - <message name="IDS_DEVICE_PERMISSIONS_PROMPT_TITLE_SINGLE" desc="Title of the device selection dialog when the app wants only one device selected."> + <message name="IDS_USB_DEVICE_PERMISSIONS_PROMPT_TITLE_SINGLE" desc="Title of the USB device selection dialog when the app wants only one device selected."> Select a USB device </message> - <message name="IDS_DEVICE_PERMISSIONS_PROMPT_TITLE_MULTIPLE" desc="Title of the device selection dialog when the app will accept one or more selected devices."> + <message name="IDS_USB_DEVICE_PERMISSIONS_PROMPT_TITLE_MULTIPLE" desc="Title of the USB device selection dialog when the app will accept one or more selected devices."> Select USB devices </message> <message name="IDS_DEVICE_PERMISSIONS_PROMPT_SINGLE" desc="Instructions asking the user to select one device for use with an app."> |