summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorreillyg <reillyg@chromium.org>2015-09-09 13:58:55 -0700
committerCommit bot <commit-bot@chromium.org>2015-09-09 20:59:42 +0000
commit17b8a58c99537bab4d8944ec2f96023dac0a15e7 (patch)
tree681eeb93ae7f3adeea7e3c0309d5dcf26c5dbdc1
parent60b43d0fb14581c647034594f29e98f4ec2babf5 (diff)
downloadchromium_src-17b8a58c99537bab4d8944ec2f96023dac0a15e7.zip
chromium_src-17b8a58c99537bab4d8944ec2f96023dac0a15e7.tar.gz
chromium_src-17b8a58c99537bab4d8944ec2f96023dac0a15e7.tar.bz2
Convert DeviceManagerDelegate to PermissionProvider mojo interface.
This changes the device::usb::DeviceManager service so that instead of expecting a C++ implementation of the DeviceManagerDelegate interface it depends on the client connecting to the devices app to provide an implementation of the PermissionProvider mojo service interface. This will allow the permissions checker to be injected by the FrameMojoShell when a renderer requests a connection to the devices app. BUG=492204 Review URL: https://codereview.chromium.org/1316203006 Cr-Commit-Position: refs/heads/master@{#347998}
-rw-r--r--chrome/browser/chrome_content_browser_client.cc8
-rw-r--r--chrome/browser/usb/DEPS3
-rw-r--r--chrome/browser/usb/OWNERS2
-rw-r--r--chrome/browser/usb/web_usb_permission_provider.cc38
-rw-r--r--chrome/browser/usb/web_usb_permission_provider.h38
-rw-r--r--chrome/chrome_browser.gypi2
-rw-r--r--device/BUILD.gn1
-rw-r--r--device/devices_app/BUILD.gn1
-rw-r--r--device/devices_app/devices_app.cc35
-rw-r--r--device/devices_app/devices_app.gyp2
-rw-r--r--device/devices_app/usb/device_manager_impl.cc181
-rw-r--r--device/devices_app/usb/device_manager_impl.h32
-rw-r--r--device/devices_app/usb/device_manager_impl_unittest.cc40
-rw-r--r--device/devices_app/usb/public/cpp/BUILD.gn15
-rw-r--r--device/devices_app/usb/public/cpp/device_manager_delegate.h27
-rw-r--r--device/devices_app/usb/public/interfaces/BUILD.gn1
-rw-r--r--device/devices_app/usb/public/interfaces/device_manager.mojom9
-rw-r--r--device/devices_app/usb/public/interfaces/permission_provider.mojom14
18 files changed, 282 insertions, 167 deletions
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index c8d718f..0149527 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -259,6 +259,10 @@
#include "chrome/browser/media/router/presentation_service_delegate_impl.h"
#endif
+#if !defined(OS_ANDROID) && !defined(OS_IOS)
+#include "chrome/browser/usb/web_usb_permission_provider.h"
+#endif
+
using base::FileDescriptor;
using blink::WebWindowFeatures;
using content::AccessTokenStore;
@@ -2516,6 +2520,10 @@ void ChromeContentBrowserClient::RegisterFrameMojoShellServices(
base::Bind(&chromeos::attestation::PlatformVerificationImpl::Create,
render_frame_host));
#endif
+#if !defined(OS_ANDROID) && !defined(OS_IOS)
+ registry->AddService(
+ base::Bind(&WebUSBPermissionProvider::Create, render_frame_host));
+#endif
}
void ChromeContentBrowserClient::RegisterInProcessMojoApplications(
diff --git a/chrome/browser/usb/DEPS b/chrome/browser/usb/DEPS
new file mode 100644
index 0000000..722f640
--- /dev/null
+++ b/chrome/browser/usb/DEPS
@@ -0,0 +1,3 @@
+include_rules = [
+ "+device/devices_app/usb/public"
+]
diff --git a/chrome/browser/usb/OWNERS b/chrome/browser/usb/OWNERS
new file mode 100644
index 0000000..b16946a
--- /dev/null
+++ b/chrome/browser/usb/OWNERS
@@ -0,0 +1,2 @@
+reillyg@chromium.org
+rockot@chromium.org
diff --git a/chrome/browser/usb/web_usb_permission_provider.cc b/chrome/browser/usb/web_usb_permission_provider.cc
new file mode 100644
index 0000000..0c86908
--- /dev/null
+++ b/chrome/browser/usb/web_usb_permission_provider.cc
@@ -0,0 +1,38 @@
+// 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 "chrome/browser/usb/web_usb_permission_provider.h"
+
+#include "content/public/browser/browser_thread.h"
+
+// static
+void WebUSBPermissionProvider::Create(
+ content::RenderFrameHost* render_frame_host,
+ mojo::InterfaceRequest<PermissionProvider> request) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ DCHECK(render_frame_host);
+
+ // The created object is strongly bound to (and owned by) the pipe.
+ new WebUSBPermissionProvider(render_frame_host, request.Pass());
+}
+
+WebUSBPermissionProvider::~WebUSBPermissionProvider() {}
+
+WebUSBPermissionProvider::WebUSBPermissionProvider(
+ content::RenderFrameHost* render_frame_host,
+ mojo::InterfaceRequest<PermissionProvider> request)
+ : binding_(this, request.Pass()),
+ render_frame_host_(render_frame_host) {}
+
+void WebUSBPermissionProvider::HasDevicePermission(
+ mojo::Array<mojo::String> requested_guids,
+ const HasDevicePermissionCallback& callback) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ // TODO(reillyg): Look up permissions granted to the render frame host's
+ // current origin in the render frame process's browser context. Until then
+ // |render_frame_host_| is unused so this ignore_result() is needed.
+ ignore_result(render_frame_host_);
+ mojo::Array<mojo::String> allowed_guids(0);
+ callback.Run(allowed_guids.Pass());
+}
diff --git a/chrome/browser/usb/web_usb_permission_provider.h b/chrome/browser/usb/web_usb_permission_provider.h
new file mode 100644
index 0000000..05b1242
--- /dev/null
+++ b/chrome/browser/usb/web_usb_permission_provider.h
@@ -0,0 +1,38 @@
+// 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.
+
+#ifndef CHROME_BROWSER_USB_WEB_USB_PERMISSION_PROVIDER_H_
+#define CHROME_BROWSER_USB_WEB_USB_PERMISSION_PROVIDER_H_
+
+#include "device/devices_app/usb/public/interfaces/permission_provider.mojom.h"
+#include "third_party/mojo/src/mojo/public/cpp/bindings/array.h"
+#include "third_party/mojo/src/mojo/public/cpp/bindings/interface_request.h"
+#include "third_party/mojo/src/mojo/public/cpp/bindings/strong_binding.h"
+
+namespace content {
+class RenderFrameHost;
+}
+
+class WebUSBPermissionProvider : public device::usb::PermissionProvider {
+ public:
+ static void Create(
+ content::RenderFrameHost* render_frame_host,
+ mojo::InterfaceRequest<device::usb::PermissionProvider> request);
+
+ ~WebUSBPermissionProvider() override;
+
+ private:
+ WebUSBPermissionProvider(content::RenderFrameHost* render_frame_host,
+ mojo::InterfaceRequest<PermissionProvider> request);
+
+ // device::usb::PermissionProvider implementation.
+ void HasDevicePermission(
+ mojo::Array<mojo::String> requested_guids,
+ const HasDevicePermissionCallback& callback) override;
+
+ mojo::StrongBinding<PermissionProvider> binding_;
+ content::RenderFrameHost* const render_frame_host_;
+};
+
+#endif // CHROME_BROWSER_USB_WEB_USB_PERMISSION_PROVIDER_H_
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index a55c3dd..39d552c 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1277,6 +1277,8 @@
'browser/sync/glue/synced_tab_delegate_desktop.cc',
'browser/sync/sync_ui_util.cc',
'browser/sync/sync_ui_util.h',
+ 'browser/usb/web_usb_permission_provider.cc',
+ 'browser/usb/web_usb_permission_provider.h',
],
'chrome_browser_win_sources': [
'browser/browser_process_platform_part_aurawin.cc',
diff --git a/device/BUILD.gn b/device/BUILD.gn
index 8c5bf66f..4961292 100644
--- a/device/BUILD.gn
+++ b/device/BUILD.gn
@@ -126,7 +126,6 @@ test("device_unittests") {
deps += [
"//device/core",
"//device/devices_app:lib",
- "//device/devices_app/usb/public/cpp",
"//device/devices_app/usb/public/interfaces",
"//device/usb",
"//device/usb:mocks",
diff --git a/device/devices_app/BUILD.gn b/device/devices_app/BUILD.gn
index bb89b70..2450b63 100644
--- a/device/devices_app/BUILD.gn
+++ b/device/devices_app/BUILD.gn
@@ -18,7 +18,6 @@ source_set("lib") {
deps = [
"//device/core",
- "//device/devices_app/usb/public/cpp",
"//device/devices_app/usb/public/interfaces",
"//device/usb",
"//net",
diff --git a/device/devices_app/devices_app.cc b/device/devices_app/devices_app.cc
index 1531651..599c64f 100644
--- a/device/devices_app/devices_app.cc
+++ b/device/devices_app/devices_app.cc
@@ -13,7 +13,6 @@
#include "base/time/time.h"
#include "device/core/device_client.h"
#include "device/devices_app/usb/device_manager_impl.h"
-#include "device/devices_app/usb/public/cpp/device_manager_delegate.h"
#include "device/usb/usb_service.h"
#include "mojo/application/public/cpp/application_connection.h"
#include "mojo/application/public/cpp/application_impl.h"
@@ -28,32 +27,6 @@ namespace {
// exiting the app.
const int64 kIdleTimeoutInSeconds = 10;
-// A usb::DeviceManagerDelegate implementation which provides origin-based
-// device access control.
-//
-// TODO(rockot/reillyg): We should just get rid of DeviceManagerDelegate, or
-// at least repurpose it as test support to provide device mocks.
-class USBDeviceManagerDelegate : public usb::DeviceManagerDelegate {
- public:
- explicit USBDeviceManagerDelegate(const GURL& remote_url)
- : remote_url_(remote_url) {}
- ~USBDeviceManagerDelegate() override {}
-
- private:
- // usb::DeviceManagerDelegate:
- bool IsDeviceAllowed(const usb::DeviceInfo& device) override {
- // Also let browser apps and mojo apptests talk to all devices.
- if (remote_url_.SchemeIs("system") ||
- remote_url_ == GURL("mojo://devices_apptests/"))
- return true;
- return false;
- }
-
- GURL remote_url_;
-
- DISALLOW_COPY_AND_ASSIGN(USBDeviceManagerDelegate);
-};
-
// A DeviceClient implementation to be constructed iff the app is not running
// in an embedder that provides a DeviceClient (i.e. running as a standalone
// Mojo app, not in Chrome).
@@ -131,12 +104,14 @@ void DevicesApp::Quit() {
void DevicesApp::Create(mojo::ApplicationConnection* connection,
mojo::InterfaceRequest<usb::DeviceManager> request) {
- scoped_ptr<usb::DeviceManagerDelegate> delegate(new USBDeviceManagerDelegate(
- GURL(connection->GetRemoteApplicationURL())));
+ // Bind the new device manager to the connecting application's permission
+ // provider.
+ usb::PermissionProviderPtr permission_provider;
+ connection->ConnectToService(&permission_provider);
// Owned by its message pipe.
usb::DeviceManagerImpl* device_manager = new usb::DeviceManagerImpl(
- request.Pass(), delegate.Pass(), service_task_runner_);
+ request.Pass(), permission_provider.Pass(), service_task_runner_);
device_manager->set_connection_error_handler(
base::Bind(&DevicesApp::OnConnectionError, base::Unretained(this)));
diff --git a/device/devices_app/devices_app.gyp b/device/devices_app/devices_app.gyp
index 4d97aa9..11af914 100644
--- a/device/devices_app/devices_app.gyp
+++ b/device/devices_app/devices_app.gyp
@@ -20,7 +20,6 @@
'usb/device_impl.h',
'usb/device_manager_impl.cc',
'usb/device_manager_impl.h',
- 'usb/public/cpp/device_manager_delegate.h',
'usb/type_converters.cc',
'usb/type_converters.h',
],
@@ -45,6 +44,7 @@
'mojom_files': [
'usb/public/interfaces/device.mojom',
'usb/public/interfaces/device_manager.mojom',
+ 'usb/public/interfaces/permission_provider.mojom',
],
},
'includes': [
diff --git a/device/devices_app/usb/device_manager_impl.cc b/device/devices_app/usb/device_manager_impl.cc
index 559edf6..cfc45b6 100644
--- a/device/devices_app/usb/device_manager_impl.cc
+++ b/device/devices_app/usb/device_manager_impl.cc
@@ -14,7 +14,6 @@
#include "base/thread_task_runner_handle.h"
#include "device/core/device_client.h"
#include "device/devices_app/usb/device_impl.h"
-#include "device/devices_app/usb/public/cpp/device_manager_delegate.h"
#include "device/devices_app/usb/public/interfaces/device.mojom.h"
#include "device/devices_app/usb/type_converters.h"
#include "device/usb/usb_device.h"
@@ -108,6 +107,23 @@ void OpenDeviceOnServiceThread(
callback_task_runner));
}
+void FilterDeviceListAndThen(
+ const DeviceManagerImpl::GetDevicesCallback& callback,
+ mojo::Array<DeviceInfoPtr> devices,
+ mojo::Array<mojo::String> allowed_guids) {
+ std::set<std::string> allowed_guid_set;
+ for (size_t i = 0; i < allowed_guids.size(); ++i)
+ allowed_guid_set.insert(allowed_guids[i]);
+
+ mojo::Array<DeviceInfoPtr> allowed_devices(0);
+ for (size_t i = 0; i < devices.size(); ++i) {
+ if (ContainsKey(allowed_guid_set, devices[i]->guid))
+ allowed_devices.push_back(devices[i].Pass());
+ }
+
+ callback.Run(allowed_devices.Pass());
+}
+
} // namespace
class DeviceManagerImpl::ServiceThreadHelper
@@ -122,17 +138,10 @@ class DeviceManagerImpl::ServiceThreadHelper
base::MessageLoop::current()->RemoveDestructionObserver(this);
}
- static void Start(
- scoped_ptr<ServiceThreadHelper> self,
- const base::Callback<void(mojo::Array<DeviceInfoPtr>)>& callback) {
+ static void Start(scoped_ptr<ServiceThreadHelper> self) {
UsbService* usb_service = DeviceClient::Get()->GetUsbService();
- if (usb_service) {
+ if (usb_service)
self->observer_.Add(usb_service);
- std::vector<UsbDeviceFilter> no_filters;
- usb_service->GetDevices(base::Bind(&OnGetDevicesOnServiceThread,
- no_filters, callback,
- self->task_runner_));
- }
// |self| now owned by the current message loop.
base::MessageLoop::current()->AddDestructionObserver(self.release());
@@ -165,30 +174,38 @@ class DeviceManagerImpl::ServiceThreadHelper
DeviceManagerImpl::DeviceManagerImpl(
mojo::InterfaceRequest<DeviceManager> request,
- scoped_ptr<DeviceManagerDelegate> delegate,
+ PermissionProviderPtr permission_provider,
scoped_refptr<base::SequencedTaskRunner> service_task_runner)
- : binding_(this, request.Pass()),
- delegate_(delegate.Pass()),
+ : permission_provider_(permission_provider.Pass()),
service_task_runner_(service_task_runner),
+ binding_(this, request.Pass()),
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.
+ binding_.set_connection_error_handler([this]() { delete this; });
+ permission_provider_.set_connection_error_handler([this]() { delete this; });
+
+ scoped_ptr<ServiceThreadHelper> helper(new ServiceThreadHelper(
+ weak_factory_.GetWeakPtr(), base::ThreadTaskRunnerHandle::Get()));
+ helper_ = helper.get();
+ service_task_runner_->PostTask(
+ FROM_HERE,
+ base::Bind(&ServiceThreadHelper::Start, base::Passed(&helper)));
}
DeviceManagerImpl::~DeviceManagerImpl() {
- if (helper_) {
- // It is safe to call this if |helper_| was already destroyed when
- // |service_task_runner_| exited as the task will never execute.
- service_task_runner_->DeleteSoon(FROM_HERE, helper_);
- }
-}
-
-void DeviceManagerImpl::set_connection_error_handler(
- const mojo::Closure& error_handler) {
- binding_.set_connection_error_handler(error_handler);
+ // It is safe to call this if |helper_| was already destroyed when
+ // |service_task_runner_| exited as the task will never execute.
+ service_task_runner_->DeleteSoon(FROM_HERE, helper_);
+ connection_error_handler_.Run();
}
void DeviceManagerImpl::GetDevices(EnumerationOptionsPtr options,
const GetDevicesCallback& callback) {
- auto filters = options->filters.To<std::vector<UsbDeviceFilter>>();
+ std::vector<UsbDeviceFilter> filters;
+ if (options)
+ filters = options->filters.To<std::vector<UsbDeviceFilter>>();
auto get_devices_callback = base::Bind(&DeviceManagerImpl::OnGetDevices,
weak_factory_.GetWeakPtr(), callback);
service_task_runner_->PostTask(
@@ -199,53 +216,48 @@ void DeviceManagerImpl::GetDevices(EnumerationOptionsPtr options,
void DeviceManagerImpl::GetDeviceChanges(
const GetDeviceChangesCallback& callback) {
- if (helper_) {
- device_change_callbacks_.push(callback);
- MaybeRunDeviceChangesCallback();
- } else {
- scoped_ptr<ServiceThreadHelper> helper(new ServiceThreadHelper(
- weak_factory_.GetWeakPtr(), base::ThreadTaskRunnerHandle::Get()));
- helper_ = helper.get();
- auto get_devices_callback =
- base::Bind(&DeviceManagerImpl::OnGetInitialDevices,
- weak_factory_.GetWeakPtr(), callback);
- service_task_runner_->PostTask(
- FROM_HERE, base::Bind(&ServiceThreadHelper::Start,
- base::Passed(&helper), get_devices_callback));
- }
+ device_change_callbacks_.push(callback);
+ MaybeRunDeviceChangesCallback();
}
void DeviceManagerImpl::OpenDevice(
const mojo::String& guid,
mojo::InterfaceRequest<Device> device_request,
const OpenDeviceCallback& callback) {
+ mojo::Array<mojo::String> requested_guids(1);
+ requested_guids[0] = guid;
+ permission_provider_->HasDevicePermission(
+ requested_guids.Pass(),
+ base::Bind(&DeviceManagerImpl::OnOpenDevicePermissionCheckComplete,
+ base::Unretained(this), base::Passed(&device_request),
+ callback));
+}
+
+void DeviceManagerImpl::OnOpenDevicePermissionCheckComplete(
+ mojo::InterfaceRequest<Device> device_request,
+ const OpenDeviceCallback& callback,
+ mojo::Array<mojo::String> allowed_guids) {
+ if (allowed_guids.size() == 0) {
+ callback.Run(OPEN_DEVICE_ERROR_ACCESS_DENIED);
+ return;
+ }
+
+ DCHECK(allowed_guids.size() == 1);
service_task_runner_->PostTask(
- FROM_HERE, base::Bind(&OpenDeviceOnServiceThread, guid,
+ FROM_HERE, base::Bind(&OpenDeviceOnServiceThread, allowed_guids[0],
base::Passed(&device_request), callback,
base::ThreadTaskRunnerHandle::Get()));
}
void DeviceManagerImpl::OnGetDevices(const GetDevicesCallback& callback,
mojo::Array<DeviceInfoPtr> devices) {
- mojo::Array<DeviceInfoPtr> allowed_devices(0);
- for (size_t i = 0; i < devices.size(); ++i) {
- if (delegate_->IsDeviceAllowed(*devices[i]))
- allowed_devices.push_back(devices[i].Pass());
- }
- callback.Run(allowed_devices.Pass());
-}
+ mojo::Array<mojo::String> requested_guids(devices.size());
+ for (size_t i = 0; i < devices.size(); ++i)
+ requested_guids[i] = devices[i]->guid;
-void DeviceManagerImpl::OnGetInitialDevices(
- const GetDeviceChangesCallback& callback,
- mojo::Array<DeviceInfoPtr> devices) {
- DeviceChangeNotificationPtr notification = DeviceChangeNotification::New();
- notification->devices_added = mojo::Array<DeviceInfoPtr>::New(0);
- notification->devices_removed = mojo::Array<mojo::String>::New(0);
- for (size_t i = 0; i < devices.size(); ++i) {
- if (delegate_->IsDeviceAllowed(*devices[i]))
- notification->devices_added.push_back(devices[i].Pass());
- }
- callback.Run(notification.Pass());
+ permission_provider_->HasDevicePermission(
+ requested_guids.Pass(),
+ base::Bind(&FilterDeviceListAndThen, callback, base::Passed(&devices)));
}
void DeviceManagerImpl::OnDeviceAdded(DeviceInfoPtr device) {
@@ -270,18 +282,63 @@ void DeviceManagerImpl::OnDeviceRemoved(std::string device_guid) {
}
void DeviceManagerImpl::MaybeRunDeviceChangesCallback() {
- if (!device_change_callbacks_.empty()) {
+ if (!permission_request_pending_ && !device_change_callbacks_.empty()) {
+ mojo::Array<DeviceInfoPtr> devices_added;
+ devices_added.Swap(&devices_added_);
+ std::set<std::string> devices_removed;
+ devices_removed.swap(devices_removed_);
+
+ mojo::Array<mojo::String> requested_guids(devices_added.size() +
+ devices_removed.size());
+ {
+ size_t i;
+ for (i = 0; i < devices_added.size(); ++i)
+ requested_guids[i] = devices_added[i]->guid;
+ for (const std::string& guid : devices_removed)
+ requested_guids[i++] = guid;
+ }
+
+ permission_request_pending_ = true;
+ permission_provider_->HasDevicePermission(
+ requested_guids.Pass(),
+ base::Bind(&DeviceManagerImpl::OnEnumerationPermissionCheckComplete,
+ base::Unretained(this), base::Passed(&devices_added),
+ devices_removed));
+ }
+}
+
+void DeviceManagerImpl::OnEnumerationPermissionCheckComplete(
+ mojo::Array<DeviceInfoPtr> devices_added,
+ const std::set<std::string>& devices_removed,
+ mojo::Array<mojo::String> allowed_guids) {
+ permission_request_pending_ = false;
+
+ if (allowed_guids.size() > 0) {
+ std::set<std::string> allowed_guid_set;
+ for (size_t i = 0; i < allowed_guids.size(); ++i)
+ allowed_guid_set.insert(allowed_guids[i]);
+
DeviceChangeNotificationPtr notification = DeviceChangeNotification::New();
- notification->devices_added.Swap(&devices_added_);
+ notification->devices_added.resize(0);
+ for (size_t i = 0; i < devices_added.size(); ++i) {
+ if (ContainsKey(allowed_guid_set, devices_added[i]->guid))
+ notification->devices_added.push_back(devices_added[i].Pass());
+ }
+
notification->devices_removed.resize(0);
- for (const std::string& device : devices_removed_)
- notification->devices_removed.push_back(device);
- devices_removed_.clear();
+ for (const std::string& guid : devices_removed) {
+ if (ContainsKey(allowed_guid_set, guid))
+ notification->devices_removed.push_back(guid);
+ }
+ DCHECK(!device_change_callbacks_.empty());
const GetDeviceChangesCallback& callback = device_change_callbacks_.front();
callback.Run(notification.Pass());
device_change_callbacks_.pop();
}
+
+ if (devices_added_.size() > 0 || !devices_removed_.empty())
+ MaybeRunDeviceChangesCallback();
}
} // namespace usb
diff --git a/device/devices_app/usb/device_manager_impl.h b/device/devices_app/usb/device_manager_impl.h
index 69b991e..b84f254 100644
--- a/device/devices_app/usb/device_manager_impl.h
+++ b/device/devices_app/usb/device_manager_impl.h
@@ -14,9 +14,10 @@
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "device/devices_app/usb/public/interfaces/device_manager.mojom.h"
+#include "device/devices_app/usb/public/interfaces/permission_provider.mojom.h"
#include "third_party/mojo/src/mojo/public/cpp/bindings/array.h"
+#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h"
#include "third_party/mojo/src/mojo/public/cpp/bindings/interface_request.h"
-#include "third_party/mojo/src/mojo/public/cpp/bindings/strong_binding.h"
namespace base {
class SequencedTaskRunner;
@@ -38,11 +39,13 @@ class DeviceManagerImpl : public DeviceManager {
public:
DeviceManagerImpl(
mojo::InterfaceRequest<DeviceManager> request,
- scoped_ptr<DeviceManagerDelegate> delegate,
+ PermissionProviderPtr permission_provider,
scoped_refptr<base::SequencedTaskRunner> service_task_runner);
~DeviceManagerImpl() override;
- void set_connection_error_handler(const mojo::Closure& error_handler);
+ void set_connection_error_handler(const mojo::Closure& error_handler) {
+ connection_error_handler_ = error_handler;
+ }
private:
class ServiceThreadHelper;
@@ -55,20 +58,25 @@ class DeviceManagerImpl : public DeviceManager {
mojo::InterfaceRequest<Device> device_request,
const OpenDeviceCallback& callback) override;
+ void OnOpenDevicePermissionCheckComplete(
+ mojo::InterfaceRequest<Device> device_request,
+ const OpenDeviceCallback& callback,
+ mojo::Array<mojo::String> allowed_guids);
+
// Callbacks to handle the async responses from the underlying UsbService.
void OnGetDevices(const GetDevicesCallback& callback,
mojo::Array<DeviceInfoPtr> devices);
- void OnGetInitialDevices(const GetDeviceChangesCallback& callback,
- mojo::Array<DeviceInfoPtr> devices);
// Methods called by |helper_| when devices are added or removed.
void OnDeviceAdded(DeviceInfoPtr device);
void OnDeviceRemoved(std::string device_guid);
void MaybeRunDeviceChangesCallback();
+ void OnEnumerationPermissionCheckComplete(
+ mojo::Array<DeviceInfoPtr> devices_added,
+ const std::set<std::string>& devices_removed,
+ mojo::Array<mojo::String> allowed_guids);
- mojo::StrongBinding<DeviceManager> binding_;
-
- scoped_ptr<DeviceManagerDelegate> delegate_;
+ PermissionProviderPtr permission_provider_;
scoped_refptr<base::SequencedTaskRunner> service_task_runner_;
// If there are unfinished calls to GetDeviceChanges their callbacks
@@ -78,11 +86,17 @@ class DeviceManagerImpl : public DeviceManager {
std::queue<GetDeviceChangesCallback> device_change_callbacks_;
mojo::Array<DeviceInfoPtr> devices_added_;
std::set<std::string> 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;
// |helper_| is owned by the service thread and holds a weak reference
// back to the device manager that created it.
- ServiceThreadHelper* helper_ = nullptr;
+ ServiceThreadHelper* helper_;
+
+ mojo::Closure connection_error_handler_;
+ mojo::Binding<DeviceManager> binding_;
base::WeakPtrFactory<DeviceManagerImpl> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(DeviceManagerImpl);
diff --git a/device/devices_app/usb/device_manager_impl_unittest.cc b/device/devices_app/usb/device_manager_impl_unittest.cc
index e084430..b66d8fc 100644
--- a/device/devices_app/usb/device_manager_impl_unittest.cc
+++ b/device/devices_app/usb/device_manager_impl_unittest.cc
@@ -14,7 +14,6 @@
#include "device/core/device_client.h"
#include "device/devices_app/usb/device_impl.h"
#include "device/devices_app/usb/device_manager_impl.h"
-#include "device/devices_app/usb/public/cpp/device_manager_delegate.h"
#include "device/usb/mock_usb_device.h"
#include "device/usb/mock_usb_device_handle.h"
#include "device/usb/mock_usb_service.h"
@@ -29,14 +28,21 @@ namespace usb {
namespace {
-class TestDeviceManagerDelegate : public DeviceManagerDelegate {
+class TestPermissionProvider : public PermissionProvider {
public:
- TestDeviceManagerDelegate() {}
- ~TestDeviceManagerDelegate() override {}
+ TestPermissionProvider(mojo::InterfaceRequest<PermissionProvider> request)
+ : binding_(this, request.Pass()) {}
+ ~TestPermissionProvider() override {}
+
+ void HasDevicePermission(
+ mojo::Array<mojo::String> requested_guids,
+ const HasDevicePermissionCallback& callback) override {
+ // Permission to access all devices granted.
+ callback.Run(requested_guids.Pass());
+ }
private:
- // DeviceManagerDelegate implementation:
- bool IsDeviceAllowed(const DeviceInfo& device_info) override { return true; }
+ mojo::StrongBinding<PermissionProvider> binding_;
};
class TestDeviceClient : public DeviceClient {
@@ -66,11 +72,12 @@ class USBDeviceManagerImplTest : public testing::Test {
}
DeviceManagerPtr ConnectToDeviceManager() {
+ PermissionProviderPtr permission_provider;
+ new TestPermissionProvider(mojo::GetProxy(&permission_provider));
DeviceManagerPtr device_manager;
- new DeviceManagerImpl(
- mojo::GetProxy(&device_manager),
- scoped_ptr<DeviceManagerDelegate>(new TestDeviceManagerDelegate),
- base::ThreadTaskRunnerHandle::Get());
+ new DeviceManagerImpl(mojo::GetProxy(&device_manager),
+ permission_provider.Pass(),
+ base::ThreadTaskRunnerHandle::Get());
return device_manager.Pass();
}
@@ -236,13 +243,14 @@ TEST_F(USBDeviceManagerImplTest, GetDeviceChanges) {
DeviceManagerPtr device_manager = ConnectToDeviceManager();
{
- std::set<std::string> added_guids;
- std::set<std::string> removed_guids;
- added_guids.insert(device0->guid());
+ // Call GetDevices once to make sure the device manager is up and running
+ // or else we could end up waiting forever for device changes as the next
+ // block races with the ServiceThreadHelper startup.
+ std::set<std::string> guids;
+ guids.insert(device0->guid());
base::RunLoop loop;
- device_manager->GetDeviceChanges(base::Bind(&ExpectDeviceChangesAndThen,
- added_guids, removed_guids,
- loop.QuitClosure()));
+ device_manager->GetDevices(
+ nullptr, base::Bind(&ExpectDevicesAndThen, guids, loop.QuitClosure()));
loop.Run();
}
diff --git a/device/devices_app/usb/public/cpp/BUILD.gn b/device/devices_app/usb/public/cpp/BUILD.gn
deleted file mode 100644
index 9ca3cce..0000000
--- a/device/devices_app/usb/public/cpp/BUILD.gn
+++ /dev/null
@@ -1,15 +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.
-
-source_set("cpp") {
- sources = [
- "device_manager_delegate.h",
- ]
-
- public_deps = [
- "//base",
- "//device/devices_app/usb/public/interfaces",
- "//third_party/mojo/src/mojo/public/cpp/bindings",
- ]
-}
diff --git a/device/devices_app/usb/public/cpp/device_manager_delegate.h b/device/devices_app/usb/public/cpp/device_manager_delegate.h
deleted file mode 100644
index 6400591..0000000
--- a/device/devices_app/usb/public/cpp/device_manager_delegate.h
+++ /dev/null
@@ -1,27 +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.
-
-#ifndef DEVICE_DEVICES_APP_USB_PUBLIC_CPP_DEVICE_MANAGER_DELEGATE_H_
-#define DEVICE_DEVICES_APP_USB_PUBLIC_CPP_DEVICE_MANAGER_DELEGATE_H_
-
-#include "device/devices_app/usb/public/interfaces/device.mojom.h"
-
-namespace device {
-namespace usb {
-
-// Interface used by DeviceManager instances to delegate certain decisions and
-// behaviors out to their embedder.
-class DeviceManagerDelegate {
- public:
- virtual ~DeviceManagerDelegate() {}
-
- // Determines whether a given device should be accessible to clients of the
- // DeviceManager instance.
- virtual bool IsDeviceAllowed(const DeviceInfo& info) = 0;
-};
-
-} // namespace usb
-} // namespace device
-
-#endif // DEVICE_DEVICES_APP_USB_PUBLIC_CPP_DEVICE_MANAGER_DELEGATE_H_
diff --git a/device/devices_app/usb/public/interfaces/BUILD.gn b/device/devices_app/usb/public/interfaces/BUILD.gn
index 14b9362..0b3ad9a 100644
--- a/device/devices_app/usb/public/interfaces/BUILD.gn
+++ b/device/devices_app/usb/public/interfaces/BUILD.gn
@@ -8,5 +8,6 @@ mojom("interfaces") {
sources = [
"device.mojom",
"device_manager.mojom",
+ "permission_provider.mojom",
]
}
diff --git a/device/devices_app/usb/public/interfaces/device_manager.mojom b/device/devices_app/usb/public/interfaces/device_manager.mojom
index ffb05d5..a2d20ae 100644
--- a/device/devices_app/usb/public/interfaces/device_manager.mojom
+++ b/device/devices_app/usb/public/interfaces/device_manager.mojom
@@ -35,7 +35,7 @@ struct DeviceFilter {
};
struct EnumerationOptions {
- array<DeviceFilter> filters;
+ array<DeviceFilter>? filters;
};
struct DeviceChangeNotification {
@@ -46,12 +46,11 @@ struct DeviceChangeNotification {
interface DeviceManager {
// Retrieves information about all devices available to the DeviceManager
// implementation.
- GetDevices(EnumerationOptions options) => (array<DeviceInfo> results);
+ GetDevices(EnumerationOptions? options) => (array<DeviceInfo> results);
// Retrieves information about changes to the set of devices available to the
- // DeviceManager since the last call to this method. The first call will
- // return the set of all devices available. Device added and removed between
- // calls will not be included.
+ // DeviceManager since the last call to this method. A device that is added
+ // and removed between calls will not be included.
GetDeviceChanges() => (DeviceChangeNotification changes);
// Attempts to open a device given its GUID.
diff --git a/device/devices_app/usb/public/interfaces/permission_provider.mojom b/device/devices_app/usb/public/interfaces/permission_provider.mojom
new file mode 100644
index 0000000..90fd0ea
--- /dev/null
+++ b/device/devices_app/usb/public/interfaces/permission_provider.mojom
@@ -0,0 +1,14 @@
+// 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_guids| down to the set of |allowed_guids| that
+ // should be accessible to clients of the DeviceManager instance.
+ HasDevicePermission(array<string> requested_guids)
+ => (array<string> allowed_guids);
+};